cameron314 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. ---------------- ilya-biryukov wrote: > 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. Alright, sold. There's already other places that use the size without checking the offset, it turns out. https://reviews.llvm.org/D37491 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
