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

This revision was automatically updated to reflect the committed changes. Closed by commit rGbb8c7e756c51: Add AST_SIGNATURE record to unhashed control block of PCM files (authored by dang). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80383/new/ h

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

dang added inline comments. Comment at: clang/lib/Serialization/ASTReader.cpp:3706 unsigned LocalBaseMacroID = Record[1]; - F.MacroOffsetsBase = Record[2]; + F.MacroOffsetsBase = Record[2] + F.ASTBlockStartOffset; F.BaseMacroID = getTotalNumMacros(); -

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

dang updated this revision to Diff 270077. dang marked 3 inline comments as done. dang added a comment. Rebase on top of latest master that has needed NFC changes to calm down clang-format. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80383/new/

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

dexonsmith accepted this revision. dexonsmith added a comment. This revision is now accepted and ready to land. LGTM, with one more comment inline. Comment at: clang/lib/Serialization/ASTWriter.cpp:2345 FirstMacroID - NUM_PREDEF_MACRO_IDS

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

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, --

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

dang updated this revision to Diff 269854. dang marked an inline comment as done. dang added a comment. Address @dexonsmith feedback about making the relative offsets in the AST block relative to the nearest relevant subblock. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION h

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

dang marked an inline comment as done. dang added a comment. See D81347 for the parent revision that makes ASTFileSignature a `std::array`. Adding the abbreviation doesn't make that much sense at the moment because of how the `IMPORTS` record is structured as d

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

dang updated this revision to Diff 269155. dang marked an inline comment as done. dang added a comment. Address some code review feedback Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80383/new/ https://reviews.llvm.org/D80383 Files: clang/inclu

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

dexonsmith added a comment. In D80383#2068596 , @dang wrote: > I made the decision to make all relative offsets relative to the start of the > AST block rather than their own sub-block or a neighboring block, in order to > prevent adding complexity in `A

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

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

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

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

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

dang marked 4 inline comments 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

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

dang updated this revision to Diff 267914. dang added a comment. Address some of the code review comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80383/new/ https://reviews.llvm.org/D80383 Files: clang/include/clang/Basic/Module.h clang/

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

dang added a comment. I made the decision to make all relative offsets relative to the start of the AST block rather than their own sub-block or a neighboring block, in order to prevent adding complexity in `ASTReader`. My reasoning that it would be best if the reader didn't have to keep track

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

dexonsmith requested changes to this revision. dexonsmith added a comment. This revision now requires changes to proceed. Thanks for working on this! I have a number of suggestions inline. Comment at: clang/include/clang/Serialization/ASTBitCodes.h:44 /// AST files at this

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

dang updated this revision to Diff 265848. dang added a comment. This should address the Windows test failure as well as some offline feedback - Ensure that DeclOffset instances can only be read by providing an offset to the AST block - Simplify ASTSignature test by using -fdisable-module-hash

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

dang updated this revision to Diff 265535. dang added a comment. Accidently deleted a line in the previous update Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80383/new/ https://reviews.llvm.org/D80383 Files: clang/include/clang/Serialization/A

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

dang updated this revision to Diff 265530. dang added a comment. Herald added a reviewer: jdoerfert. Herald added subscribers: sstefan1, ormris. Formatting fixes Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80383/new/ https://reviews.llvm.org/D803

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

dang created this revision. dang added a reviewer: Bigcheese. Herald added subscribers: cfe-commits, dexonsmith. Herald added a project: clang. This record is constructed by hashing the bytes of the AST block in a similiar fashion to the SIGNATURE record. This new signature only means anything if