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

[Corentin Jabot via Phabricator via cfe-commits]

cor3ntin added inline comments.


================
Comment at: clang/lib/Lex/Lexer.cpp:3379
 
-  if (LooseMatch)
+  if (LooseMatch && Diagnose)
     Res = LooseMatch->CodePoint;
----------------
aaron.ballman wrote:
> Why do we only want to do this if we're diagnosing?
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.


================
Comment at: clang/test/Preprocessor/ucn-pp-identifier.c:132-134
-#define CONCAT(A, B) A##B
-int CONCAT(\N{GREEK, CAPITALLETTERALPHA}); // expected-error{{expected}} \
-                                           // expected-warning {{incomplete delimited universal character name}}
----------------
aaron.ballman wrote:
> Why did this test get removed?
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


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138861