[PATCH] D37491: [Preamble] Fixed preamble breaking with BOM presence (and particularly, fluctuating BOM presence)

Ilya Biryukov via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Sep 14 07:42:12 PDT 2017


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





More information about the cfe-commits mailing list