sammccall accepted this revision.
sammccall added inline comments.
This revision is now accepted and ready to land.
================
Comment at: clang/lib/Lex/TokenLexer.cpp:1038
+ for (Token& T : Partition) {
+ SourceLocation::IntTy RelativeOffset = T.getLocation().getRawEncoding() -
+ BeginLoc.getRawEncoding();
----------------
hokein wrote:
> sammccall wrote:
> > hmm, actually maybe just before this line would be the best place to assert
> > that T and BeginLoc are in the same FileID, as it justifies the subtraction
> I agree that this is the best place to put the check-file-id assertion.
>
> Simply adding `assert(getFileID(BeginLoc) == getFileID(T.getLocation()))` can
> work, but it creates N-1 unnecessary getFileID calls on BeginLoc for assert
> build, it is better to avoid it. The only way I can think of is
>
> ```
> #ifndef NDEBUG
> FileID BeginFID = SM.getFileID(BeginLoc);
> assert(BeginFID == getFileID(T.getLocation()));
> #endif
> ```
I assume you mean with the #if once above, but the assert outside the #if and
inside the loop?
That LGTM
There's also #ifdef EXPENSIVE_CHECKS, maybe not needed here though.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D134942/new/
https://reviews.llvm.org/D134942
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
