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