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

[cor3ntin]

Update DR web page

[erichkeane]

I dont know the lexer well enough to do better than this review, but the code itself seems fine.

clang/www/cxx_dr_status.html

15650

Shouldn't this be 'unreleased' and 'Clang 16'?

[cor3ntin]

Fix DR status

(I didn't know the script supported versions, neat)

[aaron.ballman]

Don't forget the release note! :-)

clang/lib/Lex/Lexer.cpp

3342

isVerticalWhitespace()?

clang/lib/Lex/LiteralSupport.cpp

552

isVerticalWhitespace()?

clang/test/CXX/drs/dr26xx.cpp

37

Should you add:

#define z(x) 0
#define a z(
int x = a\N{abc});

from the issue as a test case as well?

[cor3ntin]

• Add test
• Add release notes entry
• use isVerticalWhitespace

[aaron.ballman]

clang/test/CXX/drs/dr26xx.cpp

41

Why do we get two errors here?

Also, can you add this case:

int y = a\N{LOTUS});

to show that we now form an identifier for a\N{LOTUS} instead of trying to expand a as a macro; we should get a new diagnostic about the stray ) instead.

[cor3ntin]

Add test

[cor3ntin]

Slightly improve error recovery

[cor3ntin]

Avoid duplicating errors in macros.
Because of that we cannot alays recover nicely with a loose matching.

[cor3ntin]

clang/test/CXX/drs/dr26xx.cpp

41

I did manage to improve that a bit, took a while.
The tradeoff is slightly subpar recovery in some cases

[aaron.ballman]

clang/lib/Lex/Lexer.cpp

3316

Spurious whitespace

3382–3383

Why do we only want to do this if we're diagnosing?

3390–3402

clang/test/Preprocessor/ucn-pp-identifier.c

132–134

Why did this test get removed?

[cor3ntin]

Remove extraneous space and braces

[cor3ntin]

Restore a macro test

clang/lib/Lex/Lexer.cpp

3382–3383

The scenario we want to avoid:

There is a tentative parse of a token, no diagnostic is emitted.
The token gets cached and contains the loose match.
Then that cached token gets reused, and because it is well formed, we never get to emit a diagnostic for a loose match.
Only returning a Loose Match when a diagnostic ensure we always emit the diag once
Note that previously we would always emit a diag, but doing so caused the diag to be emitted twice for macros (once when parsing the directive and once more when substituting.

clang/test/Preprocessor/ucn-pp-identifier.c

132–134

It doesn't test anything anymore, as the comma is part of the escape sequence now
Thinking about it more, we can do a similar test over multiple lines

[aaron.ballman]

LGTM with a request for a comment, but please give @tahonermann a chance to look at the review before landing.

clang/lib/Lex/Lexer.cpp

3382–3383

Ahhh okay, that makes sense. I think it'd help to add a comment here to explain that because it's a bit subtle.

[cor3ntin]

Add comment

[cor3ntin]

@tahonermann Do you want to take a look at this?

[tahonermann]

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.

clang/lib/Lex/Lexer.cpp

3382–3388

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?

3401

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.

[cor3ntin]

Thanks Tom, I replied to your comments

clang/lib/Lex/Lexer.cpp

3382–3388

It's not that a diagnostic would be emitted twice, is that it would not be emitted at all.
By recovering - ie returning a loose match - we prevent the function to be called again on the same buffer.
So if during the first call Diagnose is false (because we were called from a tryXXXX function), then the compilation will continue assuming we were in fact able to parse an identifier and never informs us of an invalid name.

3401

afaict, 4 is correct here because we are one past-the-end.
I do however agree that this whole pattern which is used a few times is probably unnecessary, i do think it would be good to investigate. Not in this patch though, imo

[cor3ntin]

Thanks Tom, I replied to your comments

clang/lib/Lex/Lexer.cpp

3401

I looked into it, I'm sure we could improve but not easily, getAndAdvanceChar does set some flags on the token in the presence of trigraphs/line splicing and we need those flags to be set, this is the reason we do need to call that method.
It's not super efficient but it's such an edge case... I'd rather not touch that now

[tahonermann]

clang/lib/Lex/Lexer.cpp

3382–3388

More evidence of the difficulty I'm having appreciating this comment :)

Does this sound right?

// Only map a loose match to a code point when in a diagnosing state.
// If a mapped code point were to be returned in a non-diagnosing state,
// token caching would prevent the opportunity to issue a diagnostic when
// the token is later used.

Tangent: It might be worth renaming one of Res and Result. I keep getting confused due to the similar names. Perhaps rename Result to Token or ResultToken?

3401

My concern is that, as is, the code always takes the else branch (except when Result is non-null). Assuming a buffer containing "X", the pointers are arranged as follows (where $ is one past the end).

\ N { X } $
  |   |   `- CurPtr
  |   `- Buffer
  `- StartPtr

CurPtr - StartPtr is 4, but Buffer.size() + 4 is 5 (Buffer.size() is 1 in this case).

I think there might be an easy test to see if this is working as intended. If it isn't, I would expect a diagnostic to be issued if trigraphs are enabled and the buffer contains a trigraph sequence. Something like:

\N{LOTUS??>}

clang/test/Preprocessor/ucn-pp-identifier.c

144

How about adding a test for an escaped new line? I think this is currently UB according to the standard ((lex.phases)p2), but I believe you are trying to change that with P2621.

int \N{\
LATIN CAPITAL LETTER A WITH GRAVE};

[cor3ntin]

clang/lib/Lex/Lexer.cpp

3382–3388

I'm not sure caching is exactly right.
There are tentative parses, that should not give false positive answers. I'll try to message a better comment

3401

I can try to add tests

My concern is that, as is, the code always takes the else branch (except when Result is non-null).

Yes, the if branch sole purpose is to set some state in Result.

At the start of the function, StartPtr points to \
And I'll leave a comment, maybe that will clear up future confusions

There may be a potential refactor here, which is to have getAndAdvanceChar take a bool & ShouldCleanupToken parameter instead of a token so that we don't have to do that dance, but it's a bit outside of the scope of this patch...

clang/test/Preprocessor/ucn-pp-identifier.c

144

I can add a test, sure
P2621 is not super relevant (or rather, it standardize behaviors that clang already has)

[tahonermann]

clang/lib/Lex/Lexer.cpp

3401

At the start of the function, StartPtr points to \

It doesn't look like it does. The first use of StartPtr is at line 3314 and it expects to read N:

3314:   char C = getCharAndSize(StartPtr, CharSize);
3315:   assert(C == 'N' && "expected \\N{...}");

[tahonermann]

clang/lib/Lex/Lexer.cpp

3327

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
 ^

[cor3ntin]

• Rebase
• Add/Improve comments
• Add more trigrahs tests
• Fix crash when a trigraph appears at the end of a named escape sequence and trigraphs are disabled
• Fix when getAndAdvanceChar is called - alas there is no way to write tests for that but I did check in a debugger.
• Rename s/Res/Match

There are still some inefficiencies with getAndAdvanceChar, but it calls for a
much bigger refactor than what should be in scope for this patch.

[cor3ntin]

Format

[cor3ntin]

clang/lib/Lex/Lexer.cpp

3401

Yes, you are right. There was a bug in \u too, probably has been there for ages.
It's unfortunately not testable, any incorrect value would call getCharAndSize which is going to do the right thing.

[tahonermann]

This looks good. I'm accepting the change despite one remaining issue that was neither introduced nor addressed by this patch; the diagnostic caret will still be in the wrong location for many of the diagnostics issued by tryReadNamedUCN(). Most of them use StartPtr which we've established points to N instead of \. If you want to make an additional update to address that, I'll be happy to review again. I understand if you would rather that be fixed via a separate change.

clang/lib/Lex/Lexer.cpp

3327

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.

3401

Thank you; the added comment makes this more clear. Basically, we only want to skip the call to getAndAdvanceChar() when we're sure the token wasn't spelled with trigraphs or line spaces.

[tahonermann]

@cor3ntin, I suspect the answer is no, but do your changes perhaps address this assertion failure? https://godbolt.org/z/1bPcPcWzv

int \u1{234};

Stack dump:

clang++: /root/llvm-project/clang/lib/Lex/LiteralSupport.cpp:382: void clang::expandUCNs(llvm::SmallVectorImpl<char>&, llvm::StringRef): Assertion `Value != -1U' failed.
PLEASE submit a bug report to https://github.com/llvm/llvm-project/issues/ and include the crash backtrace, preprocessed source, and associated run script.
Stack dump:
0.	Program arguments: /opt/compiler-explorer/clang-assertions-trunk/bin/clang++ -gdwarf-4 -g -o /app/output.s -mllvm --x86-asm-syntax=intel -S --gcc-toolchain=/opt/compiler-explorer/gcc-snapshot -fcolor-diagnostics -fno-crash-diagnostics -std=c++20 <source>
1.	<source>:1:5: current parser token '\u1{234}'
 #0 0x0000564e3c70a2bf llvm::sys::PrintStackTrace(llvm::raw_ostream&, int) (/opt/compiler-explorer/clang-assertions-trunk/bin/clang+++0x3fe12bf)
 #1 0x0000564e3c70826c llvm::sys::CleanupOnSignal(unsigned long) (/opt/compiler-explorer/clang-assertions-trunk/bin/clang+++0x3fdf26c)
 #2 0x0000564e3c645468 CrashRecoverySignalHandler(int) CrashRecoveryContext.cpp:0:0
 #3 0x00007f19aebf0420 __restore_rt (/lib/x86_64-linux-gnu/libpthread.so.0+0x14420)
 #4 0x00007f19ae6bd00b raise (/lib/x86_64-linux-gnu/libc.so.6+0x4300b)
 #5 0x00007f19ae69c859 abort (/lib/x86_64-linux-gnu/libc.so.6+0x22859)
 #6 0x00007f19ae69c729 (/lib/x86_64-linux-gnu/libc.so.6+0x22729)
 #7 0x00007f19ae6adfd6 (/lib/x86_64-linux-gnu/libc.so.6+0x33fd6)
 #8 0x0000564e4055e8b3 clang::expandUCNs(llvm::SmallVectorImpl<char>&, llvm::StringRef) (/opt/compiler-explorer/clang-assertions-trunk/bin/clang+++0x7e358b3)
 #9 0x0000564e405dd531 clang::Preprocessor::LookUpIdentifierInfo(clang::Token&) const (/opt/compiler-explorer/clang-assertions-trunk/bin/clang+++0x7eb4531)
#10 0x0000564e4055646c clang::Lexer::LexIdentifierContinue(clang::Token&, char const*) (/opt/compiler-explorer/clang-assertions-trunk/bin/clang+++0x7e2d46c)
#11 0x0000564e4055993d clang::Lexer::LexTokenInternal(clang::Token&, bool) (/opt/compiler-explorer/clang-assertions-trunk/bin/clang+++0x7e3093d)
#12 0x0000564e4055b1c3 clang::Lexer::Lex(clang::Token&) (/opt/compiler-explorer/clang-assertions-trunk/bin/clang+++0x7e321c3)
#13 0x0000564e405e0297 clang::Preprocessor::Lex(clang::Token&) (/opt/compiler-explorer/clang-assertions-trunk/bin/clang+++0x7eb7297)
#14 0x0000564e3ed354d2 clang::Parser::ConsumeToken() (/opt/compiler-explorer/clang-assertions-trunk/bin/clang+++0x660c4d2)
#15 0x0000564e3ed673a3 clang::Parser::ParseDeclarationSpecifiers(clang::DeclSpec&, clang::Parser::ParsedTemplateInfo const&, clang::AccessSpecifier, clang::Parser::DeclSpecContext, clang::Parser::LateParsedAttrList*, clang::ImplicitTypenameContext) (/opt/compiler-explorer/clang-assertions-trunk/bin/clang+++0x663e3a3)
#16 0x0000564e3ed3814b clang::Parser::ParseDeclOrFunctionDefInternal(clang::ParsedAttributes&, clang::ParsedAttributes&, clang::ParsingDeclSpec&, clang::AccessSpecifier) (/opt/compiler-explorer/clang-assertions-trunk/bin/clang+++0x660f14b)
#17 0x0000564e3ed38a6f clang::Parser::ParseDeclarationOrFunctionDefinition(clang::ParsedAttributes&, clang::ParsedAttributes&, clang::ParsingDeclSpec*, clang::AccessSpecifier) (.part.0) Parser.cpp:0:0
#18 0x0000564e3ed3ed59 clang::Parser::ParseExternalDeclaration(clang::ParsedAttributes&, clang::ParsedAttributes&, clang::ParsingDeclSpec*) (/opt/compiler-explorer/clang-assertions-trunk/bin/clang+++0x6615d59)
#19 0x0000564e3ed3f6ad clang::Parser::ParseTopLevelDecl(clang::OpaquePtr<clang::DeclGroupRef>&, clang::Sema::ModuleImportState&) (/opt/compiler-explorer/clang-assertions-trunk/bin/clang+++0x66166ad)
#20 0x0000564e3ed3fb54 clang::Parser::ParseFirstTopLevelDecl(clang::OpaquePtr<clang::DeclGroupRef>&, clang::Sema::ModuleImportState&) (/opt/compiler-explorer/clang-assertions-trunk/bin/clang+++0x6616b54)
#21 0x0000564e3ed335ba clang::ParseAST(clang::Sema&, bool, bool) (/opt/compiler-explorer/clang-assertions-trunk/bin/clang+++0x660a5ba)
#22 0x0000564e3da51088 clang::CodeGenAction::ExecuteAction() (/opt/compiler-explorer/clang-assertions-trunk/bin/clang+++0x5328088)
#23 0x0000564e3d2d6599 clang::FrontendAction::Execute() (/opt/compiler-explorer/clang-assertions-trunk/bin/clang+++0x4bad599)
#24 0x0000564e3d25ca9e clang::CompilerInstance::ExecuteAction(clang::FrontendAction&) (/opt/compiler-explorer/clang-assertions-trunk/bin/clang+++0x4b33a9e)
#25 0x0000564e3d3bbf43 clang::ExecuteCompilerInvocation(clang::CompilerInstance*) (/opt/compiler-explorer/clang-assertions-trunk/bin/clang+++0x4c92f43)
#26 0x0000564e39bddf44 cc1_main(llvm::ArrayRef<char const*>, char const*, void*) (/opt/compiler-explorer/clang-assertions-trunk/bin/clang+++0x14b4f44)
#27 0x0000564e39bda027 ExecuteCC1Tool(llvm::SmallVectorImpl<char const*>&) driver.cpp:0:0
#28 0x0000564e3d0c6829 void llvm::function_ref<void ()>::callback_fn<clang::driver::CC1Command::Execute(llvm::ArrayRef<std::optional<llvm::StringRef>>, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>>*, bool*) const::'lambda'()>(long) Job.cpp:0:0
#29 0x0000564e3c645c0a llvm::CrashRecoveryContext::RunSafely(llvm::function_ref<void ()>) (/opt/compiler-explorer/clang-assertions-trunk/bin/clang+++0x3f1cc0a)
#30 0x0000564e3d0c70df clang::driver::CC1Command::Execute(llvm::ArrayRef<std::optional<llvm::StringRef>>, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>>*, bool*) const (.part.0) Job.cpp:0:0
#31 0x0000564e3d09039c clang::driver::Compilation::ExecuteCommand(clang::driver::Command const&, clang::driver::Command const*&, bool) const (/opt/compiler-explorer/clang-assertions-trunk/bin/clang+++0x496739c)
#32 0x0000564e3d090e1d clang::driver::Compilation::ExecuteJobs(clang::driver::JobList const&, llvm::SmallVectorImpl<std::pair<int, clang::driver::Command const*>>&, bool) const (/opt/compiler-explorer/clang-assertions-trunk/bin/clang+++0x4967e1d)
#33 0x0000564e3d09a24c clang::driver::Driver::ExecuteCompilation(clang::driver::Compilation&, llvm::SmallVectorImpl<std::pair<int, clang::driver::Command const*>>&) (/opt/compiler-explorer/clang-assertions-trunk/bin/clang+++0x497124c)
#34 0x0000564e39bdc6d2 clang_main(int, char**) (/opt/compiler-explorer/clang-assertions-trunk/bin/clang+++0x14b36d2)
#35 0x00007f19ae69e083 __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x24083)
#36 0x0000564e39bd52ce _start (/opt/compiler-explorer/clang-assertions-trunk/bin/clang+++0x14ac2ce)
clang-16: error: clang frontend command failed with exit code 134 (use -v to see invocation)
Compiler returned: 134

[cor3ntin]

Fix caret position
Fix tests (defining a macro named 'a' caused issues)

[cor3ntin]

In 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!

clang/lib/Lex/Lexer.cpp

3327

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.

[cor3ntin]

https://reviews.llvm.org/D139889

[tahonermann]

This has already been merged, but I'm commenting just to note (continued) approval of the most recent changes. Looks great, Corentin!

clang/docs/ReleaseNotes.rst

 C++2b Feature Support
 ^^^^^^^^^^^^^^^^^^^^^
 
 - Support label at end of compound statement (`P2324 <https://wg21.link/p2324r2>`_).
 - Implemented `P1169R4: static operator() <https://wg21.link/P1169R4>`_ and `P2589R1: static operator[] <https://wg21.link/P2589R1>`_.
 - Implemented "char8_t Compatibility and Portability Fix" (`P2513R3 <https://wg21.link/P2513R3>`_).
   This change was applied to C++20 as a Defect Report.
 - Implemented "Permitting static constexpr variables in constexpr functions" (`P2647R1 <https://wg21.link/P2647R1>_`).
+- Implemented `CWG2640 Allow more characters in an n-char sequence <https://wg21.link/CWG2640>_`.
 
 CUDA/HIP Language Changes in Clang
 ----------------------------------
 
  - Allow the use of ``__noinline__`` as a keyword (instead of ``__attribute__((noinline))``)
    in lambda declarations.
 
 Objective-C Language Changes in Clang

clang/include/clang/Lex/Lexer.h

private:
   bool isHexaLiteral(const char *Start, const LangOptions &LangOpts);
 
   void codeCompleteIncludedFile(const char *PathStart,
                                 const char *CompletionPoint, bool IsAngled);
 
   llvm::Optional<uint32_t>
   tryReadNumericUCN(const char *&StartPtr, const char *SlashLoc, Token *Result);
   llvm::Optional<uint32_t> tryReadNamedUCN(const char *&StartPtr,
-                                           Token *Result);
+                                           const char *SlashLoc, Token *Result);
 
   /// Read a universal character name.
   ///
   /// \param StartPtr The position in the source buffer after the initial '\'.
   ///                 If the UCN is syntactically well-formed (but not
   ///                 necessarily valid), this parameter will be updated to
   ///                 point to the character after the UCN.
   /// \param SlashLoc The position in the source buffer of the '\'.

clang/lib/Lex/Lexer.cpp

 }
 
 /// DecodeTrigraphChar - If the specified character is a legal trigraph when
 /// prefixed with ??, emit a trigraph warning.  If trigraphs are enabled,
 /// return the result character.  Finally, emit a warning about trigraph use
 /// whether trigraphs are enabled or not.
 static char DecodeTrigraphChar(const char *CP, Lexer *L, bool Trigraphs) {
   char Res = GetTrigraphCharForLetter(*CP);
-  if (!Res || !L) return Res;
+  if (!Res)
+    return Res;
 
   if (!Trigraphs) {
-    if (!L->isLexingRawMode())
+    if (L && !L->isLexingRawMode())
       L->Diag(CP-2, diag::trigraph_ignored);
     return 0;
   }
 
-  if (!L->isLexingRawMode())
+  if (L && !L->isLexingRawMode())
     L->Diag(CP-2, diag::trigraph_converted) << StringRef(&Res, 1);
   return Res;
 }
 
 /// getEscapedNewLineSize - Return the size of the specified escaped newline,
 /// or 0 if it is not an escaped newline. P[-1] is known to be a "\" or a
 /// trigraph equivalent on entry to this function.
 unsigned Lexer::getEscapedNewLineSize(const char *Ptr) {
if (Delimited && C == '}') {
       break;
     }
 
     unsigned Value = llvm::hexDigitValue(C);
     if (Value == -1U) {
       if (!Delimited)
         break;
       if (Diagnose)
-        Diag(BufferPtr, diag::warn_delimited_ucn_incomplete)
+        Diag(SlashLoc, diag::warn_delimited_ucn_incomplete)
             << StringRef(KindLoc, 1);
       return std::nullopt;
     }
 
     if (CodePoint & 0xF000'0000) {
       if (Diagnose)
         Diag(KindLoc, diag::err_escape_too_large) << 0;
       return std::nullopt;
     }
 
     CodePoint <<= 4;
     CodePoint |= Value;
     CurPtr += CharSize;
     Count++;
   }
 
   if (Count == 0) {
     if (Diagnose)
-      Diag(StartPtr, FoundEndDelimiter ? diag::warn_delimited_ucn_empty
+      Diag(SlashLoc, FoundEndDelimiter ? diag::warn_delimited_ucn_empty
                                        : diag::warn_ucn_escape_no_digits)
           << StringRef(KindLoc, 1);
     return std::nullopt;
   }
 
   if (Delimited && Kind == 'U') {
     if (Diagnose)
-      Diag(StartPtr, diag::err_hex_escape_no_digits) << StringRef(KindLoc, 1);
+      Diag(SlashLoc, diag::err_hex_escape_no_digits) << StringRef(KindLoc, 1);
     return std::nullopt;
   }
 
   if (!Delimited && Count != NumHexDigits) {
     if (Diagnose) {
-      Diag(BufferPtr, diag::warn_ucn_escape_incomplete);
+      Diag(SlashLoc, diag::warn_ucn_escape_incomplete);
       // If the user wrote \U1234, suggest a fixit to \u.
       if (Count == 4 && NumHexDigits == 8) {
         CharSourceRange URange = makeCharRange(*this, KindLoc, KindLoc + 1);
         Diag(KindLoc, diag::note_ucn_four_not_eight)
             << FixItHint::CreateReplacement(URange, "u");
       }
     }
     return std::nullopt;
   }
 
   if (Delimited && PP) {
-    Diag(BufferPtr, PP->getLangOpts().CPlusPlus2b
-                        ? diag::warn_cxx2b_delimited_escape_sequence
-                        : diag::ext_delimited_escape_sequence)
+    Diag(SlashLoc, PP->getLangOpts().CPlusPlus2b
         << /*delimited*/ 0 << (PP->getLangOpts().CPlusPlus ? 1 : 0);
   }
 
   if (Result) {
     Result->setFlag(Token::HasUCN);
-    if (CurPtr - StartPtr == (ptrdiff_t)(Count + 2 + (Delimited ? 2 : 0)))
+    // If the UCN contains either a trigraph or a line splicing,
+    // we need to call getAndAdvanceChar again to set the appropriate flags
+    // on Result.
+    if (CurPtr - StartPtr == (ptrdiff_t)(Count + 1 + (Delimited ? 2 : 0)))
       StartPtr = CurPtr;
     else
       while (StartPtr != CurPtr)
         (void)getAndAdvanceChar(StartPtr, *Result);
   } else {
     StartPtr = CurPtr;
   }
   return CodePoint;
 }
 
 llvm::Optional<uint32_t> Lexer::tryReadNamedUCN(const char *&StartPtr,
+                                                const char *SlashLoc,
                                                 Token *Result) {
   unsigned CharSize;
   bool Diagnose = Result && !isLexingRawMode();
 
   char C = getCharAndSize(StartPtr, CharSize);
   assert(C == 'N' && "expected \\N{...}");
 
   const char *CurPtr = StartPtr + CharSize;
   const char *KindLoc = &CurPtr[-1];
 
   C = getCharAndSize(CurPtr, CharSize);
   if (C != '{') {
     if (Diagnose)
-      Diag(StartPtr, diag::warn_ucn_escape_incomplete);
+      Diag(SlashLoc, diag::warn_ucn_escape_incomplete);
     return std::nullopt;
   }
   CurPtr += CharSize;
   const char *StartName = CurPtr;
   bool FoundEndDelimiter = false;
   llvm::SmallVector<char, 30> Buffer;
   while (C) {
     C = getCharAndSize(CurPtr, CharSize);
     CurPtr += CharSize;
     if (C == '}') {
       FoundEndDelimiter = true;
       break;
     }
 
-    if (!isAlphanumeric(C) && C != '_' && C != '-' && C != ' ')
+    if (isVerticalWhitespace(C))
       break;
     Buffer.push_back(C);
   }
 
   if (!FoundEndDelimiter || Buffer.empty()) {
     if (Diagnose)
-      Diag(StartPtr, FoundEndDelimiter ? diag::warn_delimited_ucn_empty
+      Diag(SlashLoc, FoundEndDelimiter ? diag::warn_delimited_ucn_empty
                                        : diag::warn_delimited_ucn_incomplete)
           << StringRef(KindLoc, 1);
     return std::nullopt;
   }
 
   StringRef Name(Buffer.data(), Buffer.size());
-  llvm::Optional<char32_t> Res =
+  llvm::Optional<char32_t> Match =
       llvm::sys::unicode::nameToCodepointStrict(Name);
   llvm::Optional<llvm::sys::unicode::LooseMatchingResult> LooseMatch;
-  if (!Res) {
-    if (!isLexingRawMode()) {
-      Diag(StartPtr, diag::err_invalid_ucn_name)
-          << StringRef(Buffer.data(), Buffer.size());
-      LooseMatch = llvm::sys::unicode::nameToCodepointLooseMatching(Name);
+  if (!Match) {
+    if (Diagnose) {
+      Diag(StartName, diag::err_invalid_ucn_name)
+          << StringRef(Buffer.data(), Buffer.size())
+          << makeCharRange(*this, StartName, CurPtr - CharSize);
       if (LooseMatch) {
         Diag(StartName, diag::note_invalid_ucn_name_loose_matching)
             << FixItHint::CreateReplacement(
                    makeCharRange(*this, StartName, CurPtr - CharSize),
                    LooseMatch->Name);
       }
     }
-    // When finding a match using Unicode loose matching rules
-    // recover after having emitted a diagnostic.
-    if (!LooseMatch)
-      return std::nullopt;
     // We do not offer misspelled character names suggestions here
     // as the set of what would be a valid suggestion depends on context,
     // and we should not make invalid suggestions.
   }
 
-  if (Diagnose && PP && !LooseMatch)
-    Diag(BufferPtr, PP->getLangOpts().CPlusPlus2b
-                        ? diag::warn_cxx2b_delimited_escape_sequence
-                        : diag::ext_delimited_escape_sequence)
+  if (Diagnose && Match)
+    Diag(SlashLoc, PP->getLangOpts().CPlusPlus2b
         << /*named*/ 1 << (PP->getLangOpts().CPlusPlus ? 1 : 0);
 
-  if (LooseMatch)
-    Res = LooseMatch->CodePoint;
+  // If no diagnostic has been emitted yet, likely because we are doing a
+  // tentative lexing, we do not want to recover here to make sure the token
+  // will not be incorrectly considered valid. This function will be called
+  // again and a diagnostic emitted then.
+  if (LooseMatch && Diagnose)
+    Match = LooseMatch->CodePoint;
 
   if (Result) {
     Result->setFlag(Token::HasUCN);
-    if (CurPtr - StartPtr == (ptrdiff_t)(Buffer.size() + 4))
+    // If the UCN contains either a trigraph or a line splicing,
+    // we need to call getAndAdvanceChar again to set the appropriate flags
+    // on Result.
+    if (CurPtr - StartPtr == (ptrdiff_t)(Buffer.size() + 3))
       StartPtr = CurPtr;
     else
       while (StartPtr != CurPtr)
         (void)getAndAdvanceChar(StartPtr, *Result);
   } else {
     StartPtr = CurPtr;
   }
-  return *Res;
+  return Match ? llvm::Optional<uint32_t>(*Match) : std::nullopt;
 }
 
 uint32_t Lexer::tryReadUCN(const char *&StartPtr, const char *SlashLoc,
                            Token *Result) {
 
   unsigned CharSize;
   llvm::Optional<uint32_t> CodePointOpt;
   char Kind = getCharAndSize(StartPtr, CharSize);
   if (Kind == 'u' || Kind == 'U')
     CodePointOpt = tryReadNumericUCN(StartPtr, SlashLoc, Result);
   else if (Kind == 'N')
-    CodePointOpt = tryReadNamedUCN(StartPtr, Result);
+    CodePointOpt = tryReadNamedUCN(StartPtr, SlashLoc, Result);
 
   if (!CodePointOpt)
     return 0;
 
   uint32_t CodePoint = *CodePointOpt;
 
   // Don't apply C family restrictions to UCNs in assembly mode
   if (LangOpts.AsmPreprocessor)

clang/lib/Lex/LiteralSupport.cpp

if (ThisTokBuf == ThisTokEnd || *ThisTokBuf != '{') {
     if (Diags) {
       Diag(Diags, Features, Loc, ThisTokBegin, UcnBegin, ThisTokBuf,
            diag::err_delimited_escape_missing_brace)
           << StringRef(&ThisTokBuf[-1], 1);
     }
     return false;
   }
   ThisTokBuf++;
-  const char *ClosingBrace =
-      std::find_if_not(ThisTokBuf, ThisTokEnd, [](char C) {
-        return llvm::isAlnum(C) || llvm::isSpace(C) || C == '_' || C == '-';
-      });
-  bool Incomplete = ClosingBrace == ThisTokEnd || *ClosingBrace != '}';
+  const char *ClosingBrace = std::find_if(ThisTokBuf, ThisTokEnd, [](char C) {
+    return C == '}' || isVerticalWhitespace(C);
+  bool Incomplete = ClosingBrace == ThisTokEnd;
   bool Empty = ClosingBrace == ThisTokBuf;
   if (Incomplete || Empty) {
     if (Diags) {
       Diag(Diags, Features, Loc, ThisTokBegin, UcnBegin, ThisTokBuf,
            Incomplete ? diag::err_ucn_escape_incomplete
                       : diag::err_delimited_escape_empty)
           << StringRef(&UcnBegin[1], 1);
     }

clang/test/CXX/drs/dr26xx.cpp

 
 }
 
 namespace dr2635 { // dr2635: 16
 template<typename T>
 concept UnaryC = true;
 template<typename T, typename U>
 concept BinaryC = true;
 
 struct S{ int i, j; };
 S get_S();
 
 template<typename T>
 T get_T();
 
 void use() {
   // expected-error@+1{{decomposition declaration cannot be declared with constrained 'auto'}}
   UnaryC auto [a, b] = get_S();
   // expected-error@+1{{decomposition declaration cannot be declared with constrained 'auto'}}
   BinaryC<int> auto [c, d] = get_S();
 }
 
 template<typename T>
 void TemplUse() {
   // expected-error@+1{{decomposition declaration cannot be declared with constrained 'auto'}}
   UnaryC auto [a, b] = get_T<T>();
   // expected-error@+1{{decomposition declaration cannot be declared with constrained 'auto'}}
   BinaryC<T> auto [c, d] = get_T<T>();
 }
 }
 
   // dr2636: na
 
+namespace dr2640 { // dr2640: 16
+
+int \N{Λ} = 0; //expected-error {{'Λ' is not a valid Unicode character name}} \
+               //expected-error {{expected unqualified-id}}
+const char* emoji = "\N{🤡}"; // expected-error {{'🤡' is not a valid Unicode character name}} \
+                              // expected-note 5{{did you mean}}
+
+#define z(x) 0
+#define dr2640_a z(
+int x = dr2640_a\N{abc}); // expected-error {{'abc' is not a valid Unicode character name}}
+int y = dr2640_a\N{LOTUS}); // expected-error {{character <U+1FAB7> not allowed in an identifier}} \
+                     // expected-error {{use of undeclared identifier 'dr2640_a🪷'}} \
+                     // expected-error {{extraneous ')' before ';'}}
+}
+
   // dr2642: na
 
 namespace dr2644 { // dr2644: yes
 
 auto z = [a = 42](int a) { // expected-error {{a lambda parameter cannot shadow an explicitly captured entity}} \
                            // expected-note {{variable 'a' is explicitly captured here}}
      return 1;
 };

clang/test/Lexer/char-escapes-delimited.c

void named(void) {
 
   unsigned g = u'\N{LOTUS}'; // expected-error {{character too large for enclosing character literal type}} \
                              // ext-warning {{extension}} cxx2b-warning {{C++2b}}
 
   unsigned h = U'\N{LOTUS}';                      // ext-warning {{extension}} cxx2b-warning {{C++2b}}
   unsigned i = u'\N{GREEK CAPITAL LETTER DELTA}'; // ext-warning {{extension}} cxx2b-warning {{C++2b}}
   char j = '\NN';                                 // expected-error {{expected '{' after '\N' escape sequence}} expected-warning {{multi-character character constant}}
   unsigned k = u'\N{LOTUS';                       // expected-error {{incomplete universal character name}}
+
+  const char* emoji = "\N{🤡}"; // expected-error {{'🤡' is not a valid Unicode character name}} \
+                                // expected-note 5{{did you mean}}
+  const char* nested = "\N{\N{SPARKLE}}"; // expected-error {{'\N{SPARKLE' is not a valid Unicode character name}} \
+                                          // expected-note 5{{did you mean}}
 }
 
 void separators(void) {
   (void)"\x{12'3}"; // expected-error {{invalid digit ''' in escape sequence}}
   (void)"\u{12'3}"; // expected-error {{invalid digit ''' in escape sequence}}
   (void)"\o{12'3}"; // expected-error {{invalid digit ''' in escape sequence}}
 
   '\x{12'3'}';   // expected-error {{expected '}'}}

clang/test/Lexer/unicode.c

 extern int 𑩐;
 extern int 𐠈;
 extern int ꙮ;
 extern int  \u1B4C;     // BALINESE LETTER ARCHAIC JNYA - Added in Unicode 14
 extern int  \U00016AA2; // TANGSA LETTER GA - Added in Unicode 14
 extern int  \U0001E4D0; // 𞓐 NAG MUNDARI LETTER O - Added in Unicode 15
 extern int _\N{TANGSA LETTER GA};
 extern int _\N{TANGSALETTERGA}; // expected-error {{'TANGSALETTERGA' is not a valid Unicode character name}} \
+                                // expected-error {{expected ';' after top level declarator}} \
                                 // expected-note {{characters names in Unicode escape sequences are sensitive to case and whitespace}}
 
 
 
 // This character doesn't have the XID_Start property
 extern int  \U00016AC0; // TANGSA DIGIT ZERO  // cxx-error {{expected unqualified-id}} \
                                               // c2x-error {{expected identifier or '('}}
 

clang/test/Preprocessor/ucn-pp-identifier.c

-// RUN: %clang_cc1 %s -fsyntax-only -std=c99 -pedantic -verify=expected,ext -Wundef
-// RUN: %clang_cc1 %s -fsyntax-only -x c++ -pedantic -verify=expected,ext -Wundef
-// RUN: %clang_cc1 %s -fsyntax-only -x c++ -std=c++2b -pedantic -ftrigraphs -verify=expected,cxx2b -Wundef -Wpre-c++2b-compat
+// RUN: %clang_cc1 %s -fsyntax-only -std=c99 -pedantic -verify=expected,ext -Wundef -DTRIGRAPHS=1
+// RUN: %clang_cc1 %s -fsyntax-only -x c++ -pedantic -verify=expected,ext -Wundef -fno-trigraphs
+// RUN: %clang_cc1 %s -fsyntax-only -x c++ -std=c++2b -pedantic -ftrigraphs -DTRIGRAPHS=1 -verify=expected,cxx2b -Wundef -Wpre-c++2b-compat
 // RUN: %clang_cc1 %s -fsyntax-only -x c++ -pedantic -verify=expected,ext -Wundef -ftrigraphs -DTRIGRAPHS=1
 // RUN: not %clang_cc1 %s -fsyntax-only -std=c99 -pedantic -Wundef 2>&1 | FileCheck -strict-whitespace %s
 
 #define \u00FC
 #define a\u00FD() 0
 #ifndef \u00FC
 #error "This should never happen"
 #endif

 #define \u0061          // expected-error {{character 'a' cannot be specified by a universal character name}} expected-error {{macro name must be an identifier}}
 #define \u{fffe}        // expected-error {{macro name must be an identifier}} \
                         // ext-warning {{extension}} cxx2b-warning {{before C++2b}}
 #define \N{ALERT}       // expected-error {{universal character name refers to a control character}} \
                    // expected-error {{macro name must be an identifier}} \
                    // ext-warning {{extension}} cxx2b-warning {{before C++2b}}
 #define \N{WASTEBASKET} // expected-error {{macro name must be an identifier}} \
                         // ext-warning {{extension}} cxx2b-warning {{before C++2b}}
-
 #define a\u0024
 
 #if \u0110 // expected-warning {{is not defined, evaluates to 0}}
 #endif
 
 
 #define \u0110 1 / 0
 #if \u0110 // expected-error {{division by zero in preprocessor expression}}

 // CHECK-NEXT:   #define capital_u_\U00FC
 // CHECK-NEXT: {{^                   \^}}
 // CHECK-NEXT: {{^                   u}}
 
 #define \u{}           // expected-warning {{empty delimited universal character name; treating as '\' 'u' '{' '}'}} expected-error {{macro name must be an identifier}}
 #define \u{123456789}  // expected-error {{hex escape sequence out of range}} expected-error {{macro name must be an identifier}}
 #define \u{            // expected-warning {{incomplete delimited universal character name; treating as '\' 'u' '{' identifier}} expected-error {{macro name must be an identifier}}
 #define \u{fgh}        // expected-warning {{incomplete delimited universal character name; treating as '\' 'u' '{' identifier}} expected-error {{macro name must be an identifier}}
-#define \N{            // expected-warning {{incomplete delimited universal character name; treating as '\' 'N' '{' identifier}} expected-error {{macro name must be an identifier}}
+#define \N{
+// expected-warning@-1 {{incomplete delimited universal character name; treating as '\' 'N' '{' identifier}}
+// expected-error@-2 {{macro name must be an identifier}}
 #define \N{}           // expected-warning {{empty delimited universal character name; treating as '\' 'N' '{' '}'}} expected-error {{macro name must be an identifier}}
 #define \N{NOTATHING}  // expected-error {{'NOTATHING' is not a valid Unicode character name}} \
                        // expected-error {{macro name must be an identifier}}
 #define \NN            // expected-warning {{incomplete universal character name; treating as '\' followed by identifier}} expected-error {{macro name must be an identifier}}
 #define \N{GREEK_SMALL-LETTERALPHA}  // expected-error {{'GREEK_SMALL-LETTERALPHA' is not a valid Unicode character name}} \
                                      // expected-note {{characters names in Unicode escape sequences are sensitive to case and whitespaces}}
+#define \N{🤡}  // expected-error {{'🤡' is not a valid Unicode character name}} \
+                // expected-error {{macro name must be an identifier}}
 
 #define CONCAT(A, B) A##B
-int CONCAT(\N{GREEK, CAPITALLETTERALPHA}); // expected-error{{expected}} \
-                                           // expected-warning {{incomplete delimited universal character name}}
+int CONCAT(\N{GREEK
+, CAPITALLETTERALPHA});
+// expected-error@-2 {{expected}} \
+// expected-warning@-2 {{incomplete delimited universal character name}}
+
+int \N{\
+LATIN CAPITAL LETTER A WITH GRAVE};
+//ext-warning@-2 {{extension}} cxx2b-warning@-2 {{before C++2b}}
 
 #ifdef TRIGRAPHS
-int \N??<GREEK CAPITAL LETTER ALPHA??> = 0; // expected-warning{{extension}} cxx2b-warning {{before C++2b}} \
+int \N??<GREEK CAPITAL LETTER ALPHA??> = 0; // cxx2b-warning {{before C++2b}} \
+                                            //ext-warning {{extension}}\
                                             // expected-warning 2{{trigraph converted}}
 
+int a\N{LATIN CAPITAL LETTER A WITH GRAVE??>; // expected-warning {{trigraph converted}}
+#endif
+
+#ifndef TRIGRAPHS
+int a\N{LATIN CAPITAL LETTER A WITH GRAVE??>;
+// expected-warning@-1 {{trigraph ignored}}\
+// expected-warning@-1 {{incomplete}}\
+// expected-error@-1 {{expected ';' after top level declarator}}
 #endif

clang/www/cxx_dr_status.html

<tr id="2639">
     <td>accepted</td>
     <td>new-lines after phase 1</td>
     <td class="none" align="center">Unknown</td>
   </tr>
   <tr id="2640">
     <td><a href="https://wg21.link/cwg2640">2640</a></td>
     <td>accepted</td>
     <td>Allow more characters in an n-char sequence</td>
-    <td class="none" align="center">Unknown</td>
+    <td class="unreleased" align="center">Clang 16</td>
   </tr>
   <tr id="2641">
     <td><a href="https://wg21.link/cwg2641">2641</a></td>
     <td>DR</td>
     <td>Redundant specification of value category of literals</td>
     <td class="none" align="center">Unknown</td>
   </tr>
   <tr id="2642">