[PATCH] D41102: Setup clang-doc frontend framework

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a comment.

Nice!
Some further notes based on the SHA1 nature.



================
Comment at: clang-doc/BitcodeWriter.cpp:74
+  AbbrevGen(Abbrev,
+            {// 0. Fixed-size integer (ref type)
+             llvm::BitCodeAbbrevOp(llvm::BitCodeAbbrevOp::Fixed,
----------------
Those are mixed up.
`USRLengthSize` is definitively supposed to be second.


================
Comment at: clang-doc/BitcodeWriter.cpp:81
+             // 2. The string blob
+             llvm::BitCodeAbbrevOp(llvm::BitCodeAbbrevOp::Blob)});
+}
----------------
The sha1 is all-printable, so how about using `BitCodeAbbrevOp::Encoding::Char6` ?
Char4 would work best, but it is not there.


================
Comment at: clang-doc/BitcodeWriter.cpp:149
+          {MEMBER_TYPE_ACCESS, {"Access", &IntAbbrev}},
+          {NAMESPACE_USR, {"USR", &StringAbbrev}},
+          {NAMESPACE_NAME, {"Name", &StringAbbrev}},
----------------
Ha, and all the `*_USR` are actually `StringAbbrev`'s, not confusing at all :)


================
Comment at: clang-doc/BitcodeWriter.cpp:309
+  assert(Ref.USR.size() < (1U << BitCodeConstants::USRLengthSize));
+  Record.push_back(Ref.USR.size());
+  Stream.EmitRecordWithBlob(Abbrevs.get(ID), Record, Ref.USR);
----------------
Now it would make sense to also assert that this sha1(usr).strlen() == 20


================
Comment at: clang-doc/BitcodeWriter.h:46
+  static constexpr unsigned ReferenceTypeSize = 8U;
+  static constexpr unsigned USRLengthSize = 16U;
+};
----------------
Can definitively lower this to `5U` (2^6 == 32, which is more than the 20 8-bit chars of sha1)


================
Comment at: clang-doc/Representation.h:59
+
+  SmallString<16> USR;
+  InfoType RefType = InfoType::IT_default;
----------------
Now that USR is sha1'd, this is **always** 20 8-bit characters long.


================
Comment at: clang-doc/Representation.h:107
+
+  SmallString<16> USR;
+  SmallString<16> Name;
----------------
`20`
Maybe place `using USRString = SmallString<20>; // SHA1 of USR` somewhere and use it everywhere?


https://reviews.llvm.org/D41102