[PATCH] D80383: Add AST_SIGNATURE record to unhashed control block of PCM files

Daniel Grumberg via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Jun 2 10:58:24 PDT 2020


dang marked an inline comment as done.
dang added inline comments.


================
Comment at: clang/include/clang/Serialization/ASTBitCodes.h:396-400
       /// Record code for the signature that identifiers this AST file.
       SIGNATURE = 1,
 
+      /// Record code for the signature of the AST block.
+      AST_SIGNATURE,
----------------
dang wrote:
> dexonsmith wrote:
> > These names and descriptions hard hard to differentiate. Is there another way of naming these that will be more clear?
> > 
> > (One idea I had is to create `CONTROL_BLOCK_HASH` and `AST_BLOCK_HASH` and then `SIGNATURE` could just be their hash-combine, but maybe you have another idea.)
> I kept the same hasher when computing both of these which mitigates the cost. I don't see the need for also emitting a hash for the control block, there are some optional records that are not in both the AST block and the control block anyway.
I also think that the `AST_BLOCK_HASH` and the `SIGNATURE` are enough information already. In most cases you can deduce if the control block was different by just checking if the signatures were different and the ASTs the same.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D80383/new/

https://reviews.llvm.org/D80383





More information about the cfe-commits mailing list