ilya-biryukov added inline comments.

================
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.
----------------
cameron314 wrote:
> ilya-biryukov wrote:
> > 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?
> Yeah, I thought of that, but it's still nice to have the two separated.
> 
> ASTUnit.cpp, for example, checks if `Size != 0` to determine if there's a 
> preamble (without considering the offset). This isn't in the diff because it 
> was already like that.
Could we simply return `Size = 0` from `ComputePreambleBounds` if we simply 
skipped BOM and the preamble itself is empty?
My concern is that currently it's very easy to forget adding `StartOffset` and 
simply use `Size` when writing code that uses `PreambleBounds`. If we only have 
`Size`, probability of mistakes is much lower.


https://reviews.llvm.org/D37491



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to