dang marked an inline comment as done.
dang added inline comments.
================
Comment at: clang/lib/Serialization/ASTWriter.cpp:1084
PP.getHeaderSearchInfo().getHeaderSearchOpts().ModulesHashContent) {
- Signature = createSignature(StringRef(Buffer.begin(),
StartOfUnhashedControl));
+ Record.append(ASTSignature.begin(), ASTSignature.end());
+ Stream.EmitRecord(AST_SIGNATURE, Record);
----------------
dang wrote:
> dexonsmith wrote:
> > The default abbreviation (7-VBR) isn't great for hashes. Given that we're
> > going to have two of these records, I suggest specifying a custom
> > abbreviation.
> >
> > If you were to split out a prep commit as suggested above to change
> > `SIGNATURE` to be an array of 8-bit values (instead of the current array of
> > 32-bit values), that would be a good opportunity to add an abbreviation.
> I think encoding the signature as a blob makes the most sense if we make it
> an array of 20 8-bit values, which is what the hasher gives us. However it is
> not quite so simple, as it would need a big change in the way direct imports
> are serialized. The way it is currently done embeds multiple strings and
> signatures within a single unabbreviated record. I can replace the
> unabbreviated record with a sub-block that keeps track of a string table and
> a record for each import containing offsets inside the string table and blobs
> for the signature.
But I do think it is outside the scope of this change to do this.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D80383/new/
https://reviews.llvm.org/D80383
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
