ilya-biryukov accepted this revision. ilya-biryukov added a comment. This revision is now accepted and ready to land.
See my comments about removing `StartOffset` field, but other than that looks good. ================ Comment at: include/clang/Lex/Lexer.h:52 + /// a BOM is present at the start of the file. + unsigned StartOffset; + /// \brief Size of the preamble in bytes. ---------------- We could simplify it further by removing `StartOffset`, leaving only `Size`. If you look at the code, it always uses `StartOffset + Size` now, which is effectively size with BOM. What do you think? ================ Comment at: lib/Frontend/PrecompiledPreamble.cpp:227 + auto PreambleStart = MainFileBuffer->getBufferStart() + Bounds.StartOffset; + std::vector<char> PreambleBytes(PreambleStart, + PreambleStart + Bounds.Size); ---------------- Maybe store BOM bytes in `PreambleBytes` too? Would that allow to get rid of `StartOffset` field (see other comment)? https://reviews.llvm.org/D37491 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
