[PATCH] D158573: [clang][modules] Move `UNHASHED_CONTROL_BLOCK` up in the AST file

Jan Svoboda via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Aug 23 14:54:06 PDT 2023


jansvoboda11 marked 5 inline comments as done.
jansvoboda11 added inline comments.


================
Comment at: clang/lib/Serialization/ASTWriter.cpp:1169
+      writeSignature(Sig, Out);
+      std::copy_n(Out.begin(), Out.size(), Buffer.begin() + Offset);
+    };
----------------
benlangmuir wrote:
> jansvoboda11 wrote:
> > I don't feel great about removing `const` from `Buffer` and writing into it directly, circumventing `Stream`. This currently works fine, because the `Stream` in `ASTWriter` is never backed by a file (and therefore never flushed). But if that ever changes, this code is problematic. Do you think this is worth spending more time on?
> > 
> > `Stream` already has the `BackpatchWord()` function, which makes sure the underlying file is updated as well in case we're backpatching already-flushed data.
> Interesting; what was the reason for not using BackpatchWord from the start? IIUC our signatures should be a multiple of 4 bytes already.
Not sure how I arrived at this. This is fixed in the latest revision.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158573



More information about the cfe-commits mailing list