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