benlangmuir added inline comments.
================ Comment at: clang/lib/Serialization/ASTWriter.cpp:1169 + writeSignature(Sig, Out); + std::copy_n(Out.begin(), Out.size(), Buffer.begin() + Offset); + }; ---------------- 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. 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
