[PATCH] D43341: [clang-doc] Implement reducer portion of the frontend framework

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a comment.

It's surprisingly difficult to review this :/
Just by itself the logic is simple, but the amount of blocks/records/record types is kinda demoralizing.
I really hope it will be possible to somehow tablegen this later.



================
Comment at: clang-doc/BitcodeReader.cpp:363
+CommentInfo &
+ClangDocBitcodeReader::getCommentInfo(std::unique_ptr<CommentInfo> &I) {
+  I->Children.emplace_back(llvm::make_unique<CommentInfo>());
----------------
Hmm, why does this function exist?
The only difference from the previous one is the `std::unique_ptr<>`.


================
Comment at: clang-doc/BitcodeReader.cpp:488
+  // Sniff for the signature.
+  if (Stream.Read(8) != 'D' || Stream.Read(8) != 'O' || Stream.Read(8) != 'C' ||
+      Stream.Read(8) != 'S')
----------------
This signature should be in one place (header?), and then just used here and in `ClangDocBitcodeWriter::emitHeader()`


================
Comment at: clang-doc/BitcodeWriter.cpp:537
 
+void ClangDocBitcodeWriter::emitInfoSet(InfoSet &ISet) {
+  for (const auto &I : ISet.getNamespaceInfos()) emitBlock(I);
----------------
It should be `void ClangDocBitcodeWriter::emitInfoSet(const InfoSet &ISet) {` then.


================
Comment at: clang-doc/BitcodeWriter.h:129
     // Check that the static size is large-enough.
     assert(Record.capacity() > BitCodeConstants::RecordSize);
   }
----------------
Isn't that the opposite of what was that assert supposed to do?
It would have been better to just `// disable` it, and add a `FIXME` note.


================
Comment at: clang-doc/tool/ClangDocMain.cpp:138
+    llvm::outs() << "Writing intermediate results...\n";
+    SmallString<4098> Buffer;
+    llvm::BitstreamWriter Stream(Buffer);
----------------
`4096`?


https://reviews.llvm.org/D43341