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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
