[PATCH] D41102: Setup clang-doc frontend framework

Roman Lebedev via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Mar 7 11:58:36 PST 2018


lebedev.ri added a comment.

In https://reviews.llvm.org/D41102#1028995, @lebedev.ri wrote:

> Some further notes based on the SHA1 nature.


I'm sorry, brainfreeze, i meant `40` chars, not `20`.
Updated comments...



================
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);
----------------
lebedev.ri wrote:
> Now it would make sense to also assert that this sha1(usr).strlen() == 20
40 that is


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


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


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


https://reviews.llvm.org/D41102





More information about the cfe-commits mailing list