[PATCH] D138861: [Clang] Implement CWG2640 Allow more characters in an n-char sequence

[Corentin Jabot via Phabricator via cfe-commits]

cor3ntin added a comment.

In D138861#3988954 <https://reviews.llvm.org/D138861#3988954>, @tahonermann wrote:

> @cor3ntin, I suspect the answer is no, but do your changes perhaps address this assertion failure? https://godbolt.org/z/1bPcPcWzv
>
>   int \u1{234};

Interesting bug. 
No, it doesn't. But i see what's going on here, it's pretty easy to fix!



================
Comment at: clang/lib/Lex/Lexer.cpp:3323
     if (Diagnose)
       Diag(StartPtr, diag::warn_ucn_escape_incomplete);
     return llvm::None;
----------------
tahonermann wrote:
> tahonermann wrote:
> > I was able to confirm that `StartPtr` does point to `N`. See the diagnostic generated at https://godbolt.org/z/qnajcMeso; the diagnostic caret points to `N` instead of `\`.
> >   <source>:1:2: warning: incomplete delimited universal character name; treating as '\' 'N' '{' identifier [-Wunicode]
> >   \N{abc
> >    ^
> I think this still needs to be addressed. `tryReadNumericUCN()` handles this case by requiring that callers pass the location of the `\` as `SlashLoc`. I guess this is technically required since the `\` character could be written as either `\` or the `??/` trigraph.
Done (by adding SlashLoc)!
There is still room for improvement here but if we wanted to clean that up further we'd need a different patch.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D138861/new/

https://reviews.llvm.org/D138861