tahonermann requested changes to this revision.
tahonermann added a comment.
This revision now requires changes to proceed.
This looks really good. I added a suggested edit for a comment that I had a
hard time understanding and noted an area of code that I'm not sure is working
as expected.
================
Comment at: clang/lib/Lex/Lexer.cpp:3378-3379
- if (LooseMatch)
+ // If no diagnostic has been emitted yet we do not want to recover here
+ // to make sure this function will be called again and a diagnostic emitted
then.
+ if (LooseMatch && Diagnose)
----------------
I'm having a hard time understanding what this comment is trying to convey. The
comment response to Aaron was much more helpful to me. How about this suggested
edit?
================
Comment at: clang/lib/Lex/Lexer.cpp:3386
+
+ if (!Result || CurPtr - StartPtr == (ptrdiff_t)(Buffer.size() + 4))
StartPtr = CurPtr;
----------------
This is a bit of a tangent, but under what circumstances would `CurPtr -
StartPtr` not be equal to `Buffer.size() + 4`? Actually, I'm not sure that +4
is correct. It looks like `StartPtr` is expected to point to `N` at the
beginning of the function, so the initial `\` is not included in the range. If
`N` isn't seen, the function returns early. Likewise, if either of the `{` or
`}` delimiters is not found, the function returns early.
I think the goal of this code is to skip the `getAndAdvanceChar()` machinery
when the buffer is being claimed (no need to parse UCNs or trigraphs in the
character name), but it looks like, particularly with this DR, that we might
always be able to do that.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D138861/new/
https://reviews.llvm.org/D138861
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
