[clang-doc] refactor BitcodeReader::readSubBlock

[evelez7]

Reduce boilerplate code in readSubBlock by creating a callable from a higher-order lambda based on the block's add need.

[evelez7]

• [clang-doc] refactor BitcodeReader::readSubBlock #145835 👈 (View in Graphite)
• [clang-doc] refactor JSONGenerator array usage #145595
• main

This stack of pull requests is managed by Graphite. Learn more about stacking.

[ilovepi]

LGTM, modulo the one Q.

[ilovepi]

Why not check the error here? wondering since you seem to do it in handlTypeSubBlock(), and I don't see why you need one check and not the other...

[evelez7]

The add functions passed here don't return any errors. They follow the pattern where an invalid Info will call the specialization that just exits. The add functions for types do return the errors.

I think we mentioned that in our last meeting. When the inconsistency is fixed we can just have the one handleSubBlock function.

[ilovepi]

oh, right. Thanks for reminding me. yeah, seems fine then. maybe leave a TODO so we don't forget?

[llvmbot]

@llvm/pr-subscribers-clang-tools-extra

Author: Erick Velez (evelez7)

Changes

Reduce boilerplate code in readSubBlock by creating a callable from a higher-order lambda based on the block's add need.

---

Full diff: https://github.com/llvm/llvm-project/pull/145835.diff

2 Files Affected:

• (modified) clang-tools-extra/clang-doc/BitcodeReader.cpp (+54-75)
• (modified) clang-tools-extra/clang-doc/BitcodeReader.h (+7)

diff --git a/clang-tools-extra/clang-doc/BitcodeReader.cpp b/clang-tools-extra/clang-doc/BitcodeReader.cpp
index cbdd5d245b8de..437599b879748 100644
--- a/clang-tools-extra/clang-doc/BitcodeReader.cpp
+++ b/clang-tools-extra/clang-doc/BitcodeReader.cpp
@@ -800,11 +800,37 @@ llvm::Error ClangDocBitcodeReader::readBlock(unsigned ID, T I) {
   }
 }
 
-// TODO: Create a helper that can receive a function to reduce repetition for
-// most blocks.
+template <typename InfoType, typename T, typename Callback>
+llvm::Error ClangDocBitcodeReader::handleSubBlock(unsigned ID, T Parent,
+                                                  Callback Function) {
+  InfoType Info;
+  if (auto Err = readBlock(ID, &Info))
+    return Err;
+  Function(Parent, std::move(Info));
+  return llvm::Error::success();
+}
+
+template <typename InfoType, typename T, typename Callback>
+llvm::Error ClangDocBitcodeReader::handleTypeSubBlock(unsigned ID, T Parent,
+                                                      Callback Function) {
+  InfoType Info;
+  if (auto Err = readBlock(ID, &Info))
+    return Err;
+  if (auto Err = Function(Parent, std::move(Info)))
+    return Err;
+  return llvm::Error::success();
+}
+
 template <typename T>
 llvm::Error ClangDocBitcodeReader::readSubBlock(unsigned ID, T I) {
   llvm::TimeTraceScope("Reducing infos", "readSubBlock");
+
+  static auto CreateAddFunc = [](auto AddFunc) {
+    return [AddFunc](auto Parent, auto Child) {
+      return AddFunc(Parent, std::move(Child));
+    };
+  };
+
   switch (ID) {
   // Blocks can only have certain types of sub blocks.
   case BI_COMMENT_BLOCK_ID: {
@@ -816,28 +842,16 @@ llvm::Error ClangDocBitcodeReader::readSubBlock(unsigned ID, T I) {
     return llvm::Error::success();
   }
   case BI_TYPE_BLOCK_ID: {
-    TypeInfo TI;
-    if (auto Err = readBlock(ID, &TI))
-      return Err;
-    if (auto Err = addTypeInfo(I, std::move(TI)))
-      return Err;
-    return llvm::Error::success();
+    return handleTypeSubBlock<TypeInfo>(
+        ID, I, CreateAddFunc(addTypeInfo<T, TypeInfo>));
   }
   case BI_FIELD_TYPE_BLOCK_ID: {
-    FieldTypeInfo TI;
-    if (auto Err = readBlock(ID, &TI))
-      return Err;
-    if (auto Err = addTypeInfo(I, std::move(TI)))
-      return Err;
-    return llvm::Error::success();
+    return handleTypeSubBlock<FieldTypeInfo>(
+        ID, I, CreateAddFunc(addTypeInfo<T, FieldTypeInfo>));
   }
   case BI_MEMBER_TYPE_BLOCK_ID: {
-    MemberTypeInfo TI;
-    if (auto Err = readBlock(ID, &TI))
-      return Err;
-    if (auto Err = addTypeInfo(I, std::move(TI)))
-      return Err;
-    return llvm::Error::success();
+    return handleTypeSubBlock<MemberTypeInfo>(
+        ID, I, CreateAddFunc(addTypeInfo<T, MemberTypeInfo>));
   }
   case BI_REFERENCE_BLOCK_ID: {
     Reference R;
@@ -848,81 +862,46 @@ llvm::Error ClangDocBitcodeReader::readSubBlock(unsigned ID, T I) {
     return llvm::Error::success();
   }
   case BI_FUNCTION_BLOCK_ID: {
-    FunctionInfo F;
-    if (auto Err = readBlock(ID, &F))
-      return Err;
-    addChild(I, std::move(F));
-    return llvm::Error::success();
+    return handleSubBlock<FunctionInfo>(
+        ID, I, CreateAddFunc(addChild<T, FunctionInfo>));
   }
   case BI_BASE_RECORD_BLOCK_ID: {
-    BaseRecordInfo BR;
-    if (auto Err = readBlock(ID, &BR))
-      return Err;
-    addChild(I, std::move(BR));
-    return llvm::Error::success();
+    return handleSubBlock<BaseRecordInfo>(
+        ID, I, CreateAddFunc(addChild<T, BaseRecordInfo>));
   }
   case BI_ENUM_BLOCK_ID: {
-    EnumInfo E;
-    if (auto Err = readBlock(ID, &E))
-      return Err;
-    addChild(I, std::move(E));
-    return llvm::Error::success();
+    return handleSubBlock<EnumInfo>(ID, I,
+                                    CreateAddFunc(addChild<T, EnumInfo>));
   }
   case BI_ENUM_VALUE_BLOCK_ID: {
-    EnumValueInfo EV;
-    if (auto Err = readBlock(ID, &EV))
-      return Err;
-    addChild(I, std::move(EV));
-    return llvm::Error::success();
+    return handleSubBlock<EnumValueInfo>(
+        ID, I, CreateAddFunc(addChild<T, EnumValueInfo>));
   }
   case BI_TEMPLATE_BLOCK_ID: {
-    TemplateInfo TI;
-    if (auto Err = readBlock(ID, &TI))
-      return Err;
-    addTemplate(I, std::move(TI));
-    return llvm::Error::success();
+    return handleSubBlock<TemplateInfo>(ID, I, CreateAddFunc(addTemplate<T>));
   }
   case BI_TEMPLATE_SPECIALIZATION_BLOCK_ID: {
-    TemplateSpecializationInfo TSI;
-    if (auto Err = readBlock(ID, &TSI))
-      return Err;
-    addTemplateSpecialization(I, std::move(TSI));
-    return llvm::Error::success();
+    return handleSubBlock<TemplateSpecializationInfo>(
+        ID, I, CreateAddFunc(addTemplateSpecialization<T>));
   }
   case BI_TEMPLATE_PARAM_BLOCK_ID: {
-    TemplateParamInfo TPI;
-    if (auto Err = readBlock(ID, &TPI))
-      return Err;
-    addTemplateParam(I, std::move(TPI));
-    return llvm::Error::success();
+    return handleSubBlock<TemplateParamInfo>(
+        ID, I, CreateAddFunc(addTemplateParam<T>));
   }
   case BI_TYPEDEF_BLOCK_ID: {
-    TypedefInfo TI;
-    if (auto Err = readBlock(ID, &TI))
-      return Err;
-    addChild(I, std::move(TI));
-    return llvm::Error::success();
+    return handleSubBlock<TypedefInfo>(ID, I,
+                                       CreateAddFunc(addChild<T, TypedefInfo>));
   }
   case BI_CONSTRAINT_BLOCK_ID: {
-    ConstraintInfo CI;
-    if (auto Err = readBlock(ID, &CI))
-      return Err;
-    addConstraint(I, std::move(CI));
-    return llvm::Error::success();
+    return handleSubBlock<ConstraintInfo>(ID, I,
+                                          CreateAddFunc(addConstraint<T>));
   }
   case BI_CONCEPT_BLOCK_ID: {
-    ConceptInfo CI;
-    if (auto Err = readBlock(ID, &CI))
-      return Err;
-    addChild(I, std::move(CI));
-    return llvm::Error::success();
+    return handleSubBlock<ConceptInfo>(ID, I,
+                                       CreateAddFunc(addChild<T, ConceptInfo>));
   }
   case BI_VAR_BLOCK_ID: {
-    VarInfo VI;
-    if (auto Err = readBlock(ID, &VI))
-      return Err;
-    addChild(I, std::move(VI));
-    return llvm::Error::success();
+    return handleSubBlock<VarInfo>(ID, I, CreateAddFunc(addChild<T, VarInfo>));
   }
   default:
     return llvm::createStringError(llvm::inconvertibleErrorCode(),
diff --git a/clang-tools-extra/clang-doc/BitcodeReader.h b/clang-tools-extra/clang-doc/BitcodeReader.h
index a046ee2f7a24a..4947721f0a06d 100644
--- a/clang-tools-extra/clang-doc/BitcodeReader.h
+++ b/clang-tools-extra/clang-doc/BitcodeReader.h
@@ -64,6 +64,13 @@ class ClangDocBitcodeReader {
   // Helper function to set up the appropriate type of Info.
   llvm::Expected<std::unique_ptr<Info>> readBlockToInfo(unsigned ID);
 
+  template <typename InfoType, typename T, typename CallbackFunction>
+  llvm::Error handleSubBlock(unsigned ID, T Parent, CallbackFunction Function);
+
+  template <typename InfoType, typename T, typename CallbackFunction>
+  llvm::Error handleTypeSubBlock(unsigned ID, T Parent,
+                                 CallbackFunction Function);
+
   llvm::BitstreamCursor &Stream;
   std::optional<llvm::BitstreamBlockInfo> BlockInfo;
   FieldId CurrentReferenceField;

[evelez7]

Merge activity

• Jun 27, 3:42 AM UTC: A user started a stack merge that includes this pull request via Graphite.
• Jun 27, 3:44 AM UTC: @evelez7 merged this pull request with Graphite.