[PATCH] D41102: Setup clang-doc frontend framework

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a comment.

An idea on how to further generalize/cleanup `emitBlockInfoBlock()`.

While *i think* will help, i'm not sure how to further consolidate the `BlockIdNameMap`/`RecordIdNameMap` and the actual `emitBlock(*)`...



================
Comment at: clang-doc/BitcodeWriter.cpp:55
+    }();
+
+static const IndexedMap<StringRef, RecordIdToIndexFunctor> RecordIdNameMap =
----------------
After thinking, i think the solution is to turn the lambdas in `emit{String,...}Abbrev()` into static functions, and store not a stringref in RecordIdNameMap, but a struct with stringref + pointer to one of the functions.
Since `RecordIdNameMap` is only used in `emitRecordID()`, which is only used in `emitBlockInfoBlock()`, i think it should not be too intrusive..

So
```
// Or, decltype(&StringAbbrev), or std::function?
using AbbrevDsc = void (*)(std::shared_ptr<BitCodeAbbrev> &Abbrev);

static void StringAbbrev(std::shared_ptr<BitCodeAbbrev> &Abbrev) {
    Abbrev->Add(
        BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed,
                        BitCodeConstants::LineNumFixedSize));  // String size
    Abbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Blob));       // String
};
static void LocationAbbrev(std::shared_ptr<BitCodeAbbrev> &Abbrev) {
    Abbrev->Add(
        BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed,
                        BitCodeConstants::LineNumFixedSize));  // Line number
    Abbrev->Add(
        BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed,
                        BitCodeConstants::LineNumFixedSize));  // Filename size
    Abbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Blob));       // Filename
}
static void IntAbbrev = [](std::shared_ptr<BitCodeAbbrev> &Abbrev) {
    Abbrev->Add(
        BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed,
                        BitCodeConstants::LineNumFixedSize));  // Integer
};

struct RecordIdDsc {
  StringRef Name;
  AbbrevDsc Abbrev;

  RecordIdDsc() = default;

  RecordIdDsc(StringRef Name_, AbbrevDsc Abbrev_) : Name(Name_), Abbrev(Abbrev_) {}
}

static const IndexedMap<RecordIdDsc, RecordIdToIndexFunctor> RecordIdNameMap =
    []() {
      IndexedMap<RecordIdDsc, RecordIdToIndexFunctor> RecordIdNameMap;
      static constexpr unsigned ExpectedSize = RI_LAST - RI_FIRST + 1;
      RecordIdNameMap.resize(ExpectedSize);

      // There is no init-list constructor for the IndexedMap, so have to
      // improvise
      static constexpr std::initializer_list<
          std::pair<RecordId, RecordIdDsc>>
          inits = {{VERSION, {"Version", &IntAbbrev}},
....
```


================
Comment at: clang-doc/BitcodeWriter.cpp:156
+  prepRecordData(ID);
+  for (const char C : RecordIdNameMap[ID]) Record.push_back(C);
+  Stream.EmitRecord(llvm::bitc::BLOCKINFO_CODE_SETRECORDNAME, Record);
----------------
```
for (const char C : RecordIdNameMap[ID].Name) Record.push_back(C);
```


================
Comment at: clang-doc/BitcodeWriter.cpp:162
+
+template <typename Lambda>
+void ClangDocBitcodeWriter::emitAbbrev(RecordId ID, BlockId Block, Lambda &&L) {
----------------
```
void ClangDocBitcodeWriter::emitAbbrev(RecordId ID, BlockId Block) {
  auto Abbrev = std::make_shared<BitCodeAbbrev>();
  Abbrev->Add(BitCodeAbbrevOp(ID));
  RecordIdNameMap[ID].Abbrev(Abbrev);
  Abbrevs.add(ID, Stream.EmitBlockInfoAbbrev(Block, std::move(Abbrev)));
}
```


================
Comment at: clang-doc/BitcodeWriter.cpp:170
+
+void ClangDocBitcodeWriter::emitStringAbbrev(RecordId ID, BlockId Block) {
+  auto EmitString = [](std::shared_ptr<BitCodeAbbrev> &Abbrev) {
----------------
Those three will be gone


================
Comment at: clang-doc/BitcodeWriter.cpp:253
+        COMMENT_ARG, COMMENT_POSITION})
+    emitStringAbbrev(RID, BI_COMMENT_BLOCK_ID);
+  emitIntAbbrev(COMMENT_SELFCLOSING, BI_COMMENT_BLOCK_ID);
----------------
So now this is
```
  for (RecordId RID :
       {COMMENT_KIND, COMMENT_TEXT, COMMENT_NAME, COMMENT_DIRECTION,
        COMMENT_PARAMNAME, COMMENT_CLOSENAME, COMMENT_ATTRKEY, COMMENT_ATTRVAL,
        COMMENT_ARG, COMMENT_POSITION, COMMENT_SELFCLOSING, COMMENT_EXPLICIT})
    emitAbbrev(RID, BI_COMMENT_BLOCK_ID);
```


================
Comment at: clang-doc/BitcodeWriter.cpp:273
+    emitRecordID(RID);
+  emitStringAbbrev(MEMBER_TYPE_TYPE, BI_MEMBER_TYPE_BLOCK_ID);
+  emitStringAbbrev(MEMBER_TYPE_NAME, BI_MEMBER_TYPE_BLOCK_ID);
----------------
```
  for (RecordId RID : {MEMBER_TYPE_TYPE, MEMBER_TYPE_NAME, MEMBER_TYPE_ACCESS})
    emitAbbrev(RID, BI_MEMBER_TYPE_BLOCK_ID);
```
and so on


================
Comment at: clang-doc/BitcodeWriter.cpp:277
+
+  // Namespace Block
+  emitBlockID(BI_NAMESPACE_BLOCK_ID);
----------------
And The Next Pattern:
```
emitBlockID(BI_{STUFF}_BLOCK_ID); // <- same BI_{STUFF}_BLOCK_ID
for (RecordId RID : {STUFF_FOO, STUFF_BAR, STUFF_BAZ, ...}) // <- same init-list
  emitRecordID(RID);
for (RecordId RID : {STUFF_FOO, STUFF_BAR, STUFF_BAZ, ...}) // <- same init-list
  emitAbbrev(RID, BI_{STUFF}_BLOCK_ID); // <- same BI_{STUFF}_BLOCK_ID
```

I think it can be generalized as:
```
std::initializer_list<std::pair<BlockId, std::initializer_list<RecordId>>> TheBlocks {
...
  // Namespace Block
  {BI_NAMESPACE_BLOCK_ID, {NAMESPACE_NAME, NAMESPACE_NAMESPACE}},
...
};

for(const auto& Block : TheBlocks) {
  emitBlockID(Block.first);

  for(const auto& Record : Block.second)
    emitRecordID(Record);

  for(const auto& Record : Block.second)
    emitAbbrev(Record, Block.first);
}
```


================
Comment at: clang-doc/BitcodeWriter.h:209
+
+  void emitStringAbbrev(RecordId ID, BlockId Block);
+  void emitLocationAbbrev(RecordId ID, BlockId Block);
----------------
One more piece of the puzzle.
It seems, there is only one abbrev type (i.e. loc/string/int) for a given `RecordId`, yes?
If so, i believe one more mapping can be added somehow, to go from a given `RecordId` to the appropriate abbrev type.


https://reviews.llvm.org/D41102