[Clang] Correctly handle $, @, and ` when represented as UCN

[cor3ntin]

Summary

This covers

• P2558R2 (C++, wg21.link/P2558)
• N2701 (C, https://www.open-std.org/jtc1/sc22/wg14/www/docs/n2701.htm)
• N3124 (C, https://www.open-std.org/jtc1/sc22/wg14/www/docs/n3124.pdf)

This patch

• Disallow representing $ as a UCN in all language mode, which did not properly work (see GH62133), and which in made ill-formed in C++ and C by P2558 and N3124 respectively
• Allow a UCN for any character in C2X, in string and character literals

Fixes #62133

[aaron.ballman]

I'm a bit confused as to whether we're planning to support this as an extension in older language modes or whether we expect this to be a new feature only enabled in newer language modes. Also, this should update the C and CXX status pages and have a release note.

clang/include/clang/Basic/DiagnosticLexKinds.td

200–207

Don't we need similar warnings for C++2c?

clang/lib/Lex/Lexer.cpp

3487–3489

You should update the comments here so they match the source.

3499–3500

Should this be looking for C2x?

clang/lib/Lex/LiteralSupport.cpp

630

Won't this give the C warning in C++98 mode? (We should add test coverage for C++98)

[cor3ntin]

clang/include/clang/Basic/DiagnosticLexKinds.td

200–207

The change in C++ dates back to C++11, the warning you are looking for are just above (unchanged text)

clang/lib/Lex/Lexer.cpp

3499–3500

See https://github.com/llvm/llvm-project/issues/62133
This never worked. Making it work in older C mode is something I looked into but it didn't seem worth it.

clang/lib/Lex/LiteralSupport.cpp

630

I'll be adding test and replace by Features.CPlusPlus. But to your question, no, in C++98, IsError is true so that branch is not taken

[aaron.ballman]

clang/include/clang/Basic/DiagnosticLexKinds.td

200–207

Ahhh, thanks!

clang/test/Preprocessor/ucn-allowed-chars.c

86

I'd like more context for this diagnostic message because I have no idea what it's trying to warn.

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

117

Ugh, that's unfortunate. I wonder if we should change this to use a regex so the line number doesn't matter?

[cor3ntin]

Address Aaron's comments

[cor3ntin]

Add regex in test file

[cor3ntin]

clang/test/Preprocessor/ucn-allowed-chars.c

86

The warning is either

whitespace required after macro name 
ISO C99 requires whitespace after the macro name

in C++ and C respectively.

Given it's not salient, I did that. Let me know if you want me to add a different verify tag

[cor3ntin]

Update comment to match C2x

[aaron.ballman]

Changes generally LGTM, but I'll leave it to @tahonermann to do the final sign-off given this is related to text.

clang/test/Preprocessor/ucn-allowed-chars.c

86

I'd say leave it as-is for now, but we really should unify these diagnostics (then we can at least use something more descriptive like whitespace required after macro name, leaving off the ISO C bits)

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

160

This is another spot where we really should be spelling out the diagnostic.

[cor3ntin]

Fix diagnostic test in ucn-pp-identifiers.c

[shafik]

clang/test/Lexer/utf8-char-literal.cpp

22

Why change this test line?

[cor3ntin]

clang/test/Lexer/utf8-char-literal.cpp

22

0x80 is too big to be represented in a char (and we have tests for that, I wanted a positive tests for control characters)

[cor3ntin]

In D153621#4449244, @aaron.ballman wrote:

Changes generally LGTM, but I'll leave it to @tahonermann to do the final sign-off given this is related to text.

@tahonermann Ping :)

[tahonermann]

Changes look good; I added a number of suggested edits for minor issues.

clang/docs/ReleaseNotes.rst

203–204 ↗

(On Diff #534602)

530–531 ↗

(On Diff #534602)

clang/include/clang/Basic/DiagnosticLexKinds.td

204–207

clang/lib/Lex/Lexer.cpp

3489–3490

clang/lib/Lex/LiteralSupport.cpp

617–618

630

clang/test/Preprocessor/ucn-allowed-chars.c

16

I assume this line was deleted to minimize the disruption to line numbers due to the additional RUN line?

[cor3ntin]

clang/test/Preprocessor/ucn-allowed-chars.c

16

Indeed!

[cor3ntin]

Address Tom's comments

[cor3ntin]

@tahonermann I'll probably merge that EoW unless you scream!

[tahonermann]

I'll probably merge that EoW unless you scream!

Why wait? :) Looks good!

clang/include/clang/Basic/DiagnosticLexKinds.td

def warn_c99_compat_unicode_id : Warning<
   InGroup<C99Compat>, DefaultIgnore;
 
 def warn_cxx98_compat_literal_ucn_escape_basic_scs : Warning<
   "specifying character '%0' with a universal character name "
   "is incompatible with C++98">, InGroup<CXX98Compat>, DefaultIgnore;
 def warn_cxx98_compat_literal_ucn_control_character : Warning<
   "universal character name referring to a control character "
   "is incompatible with C++98">, InGroup<CXX98Compat>, DefaultIgnore;
+def warn_c2x_compat_literal_ucn_escape_basic_scs : Warning<
+  "specifying character '%0' with a universal character name is "
+  "incompatible with C standards before C2x">,
+  InGroup<CPre2xCompat>, DefaultIgnore;
+def warn_c2x_compat_literal_ucn_control_character : Warning<
+  "universal character name referring to a control character "
+  "incompatible with C standards before C2x">,
+  InGroup<CPre2xCompat>, DefaultIgnore;
 def warn_ucn_not_valid_in_c89 : Warning<
   "universal character names are only valid in C99 or C++; "
   "treating as '\\' followed by identifier">, InGroup<Unicode>;
 def warn_ucn_not_valid_in_c89_literal : ExtWarn<
   "universal character names are only valid in C99 or C++">, InGroup<Unicode>;
 
 
 // Literal

clang/lib/Lex/Lexer.cpp

if (!CodePointOpt)
     return 0;
 
   uint32_t CodePoint = *CodePointOpt;
 
   // Don't apply C family restrictions to UCNs in assembly mode
   if (LangOpts.AsmPreprocessor)
     return CodePoint;
 
   // C99 6.4.3p2: A universal character name shall not specify a character whose
   //   short identifier is less than 00A0 other than 0024 ($), 0040 (@), or
   //   0060 (`), nor one in the range D800 through DFFF inclusive.)
   // C++11 [lex.charset]p2: If the hexadecimal value for a
   //   universal-character-name corresponds to a surrogate code point (in the
   //   range 0xD800-0xDFFF, inclusive), the program is ill-formed. Additionally,
   //   if the hexadecimal value for a universal-character-name outside the
   //   c-char-sequence, s-char-sequence, or r-char-sequence of a character or
   //   string literal corresponds to a control character (in either of the
   //   ranges 0x00-0x1F or 0x7F-0x9F, both inclusive) or to a character in the
   //   basic source character set, the program is ill-formed.
   if (CodePoint < 0xA0) {
-    if (CodePoint == 0x24 || CodePoint == 0x40 || CodePoint == 0x60)
-      return CodePoint;
-
     // We don't use isLexingRawMode() here because we need to warn about bad
     // UCNs even when skipping preprocessing tokens in a #if block.
     if (Result && PP) {
       if (CodePoint < 0x20 || CodePoint >= 0x7F)
         Diag(BufferPtr, diag::err_ucn_control_character);
       else {
         char C = static_cast<char>(CodePoint);
         Diag(BufferPtr, diag::err_ucn_escape_basic_scs) << StringRef(&C, 1);

clang/lib/Lex/LiteralSupport.cpp

static bool ProcessUCNEscape(const char *ThisTokBegin, const char *&ThisTokBuf,
   if ((0xD800 <= UcnVal && UcnVal <= 0xDFFF) || // surrogate codepoints
       UcnVal > 0x10FFFF) {                      // maximum legal UTF32 value
     if (Diags)
       Diag(Diags, Features, Loc, ThisTokBegin, UcnBegin, ThisTokBuf,
            diag::err_ucn_escape_invalid);
     return false;
   }
 
   // C++11 allows UCNs that refer to control characters and basic source
   // characters inside character and string literals
   if (UcnVal < 0xa0 &&
-      (UcnVal != 0x24 && UcnVal != 0x40 && UcnVal != 0x60)) {  // $, @, `
-    bool IsError = (!Features.CPlusPlus11 || !in_char_string_literal);
+      // $, @, ` are allowed in all language modes
+      (UcnVal != 0x24 && UcnVal != 0x40 && UcnVal != 0x60)) {
+    bool IsError = (!(Features.CPlusPlus11 || Features.C2x) || !in_char_string_literal);
     if (Diags) {
       char BasicSCSChar = UcnVal;
       if (UcnVal >= 0x20 && UcnVal < 0x7f)
         Diag(Diags, Features, Loc, ThisTokBegin, UcnBegin, ThisTokBuf,
              IsError ? diag::err_ucn_escape_basic_scs :
-                       diag::warn_cxx98_compat_literal_ucn_escape_basic_scs)
+                       Features.CPlusPlus11 ?
+                       diag::warn_cxx98_compat_literal_ucn_escape_basic_scs
+                     : diag::warn_c2x_compat_literal_ucn_escape_basic_scs )
             << StringRef(&BasicSCSChar, 1);
       else
         Diag(Diags, Features, Loc, ThisTokBegin, UcnBegin, ThisTokBuf,
              IsError ? diag::err_ucn_control_character :
-                       diag::warn_cxx98_compat_literal_ucn_control_character);
+                       Features.CPlusPlus11 ?
+                         diag::warn_cxx98_compat_literal_ucn_control_character
+                       : diag::warn_c2x_compat_literal_ucn_control_character);
     }
     if (IsError)
       return false;
   }
 
   if (!Features.CPlusPlus && !Features.C99 && Diags)
     Diag(Diags, Features, Loc, ThisTokBegin, UcnBegin, ThisTokBuf,
          diag::warn_ucn_not_valid_in_c89_literal);

clang/test/Lexer/char-literal.cpp

 // RUN: %clang_cc1 -triple x86_64-apple-darwin -std=c++11 -Wfour-char-constants -fsyntax-only -verify=cxx,expected %s
 // RUN: %clang_cc1 -triple x86_64-apple-darwin -std=c++17 -Wfour-char-constants -fsyntax-only -verify=cxx,expected %s
 // RUN: %clang_cc1 -triple x86_64-apple-darwin -std=c++20 -Wfour-char-constants -fsyntax-only -verify=cxx,expected %s
 // RUN: %clang_cc1 -triple x86_64-apple-darwin -std=c11 -x c -Wfour-char-constants -fsyntax-only -verify=c,expected %s
-// RUN: %clang_cc1 -triple x86_64-apple-darwin -std=c2x -x c -Wfour-char-constants -fsyntax-only -verify=c,expected %s
+// RUN: %clang_cc1 -triple x86_64-apple-darwin -std=c2x -x c -Wfour-char-constants -fsyntax-only -verify=c2x,expected %s
 
 #ifndef __cplusplus
 typedef __WCHAR_TYPE__ wchar_t;
 typedef __CHAR16_TYPE__ char16_t;
 typedef __CHAR32_TYPE__ char32_t;
 #endif
 
 int a = 'ab'; // expected-warning {{multi-character character constant}}

 #endif
 
 // UTF-8 character literal code point ranges.
 #if __cplusplus >= 201703L || __STDC_VERSION__ >= 201710L
 _Static_assert(u8'\U00000000' == 0x00, ""); // c-error {{universal character name refers to a control character}}
 _Static_assert(u8'\U0000007F' == 0x7F, ""); // c-error {{universal character name refers to a control character}}
 _Static_assert(u8'\U00000080', ""); // c-error {{universal character name refers to a control character}}
                                     // cxx-error@-1 {{character too large for enclosing character literal type}}
+                                    // c2x-error@-2 {{character too large for enclosing character literal type}}
 _Static_assert((unsigned char)u8'\xFF' == (unsigned char)0xFF, "");
 #endif
 
 // UTF-8 string literal code point ranges.
 _Static_assert(u8"\U00000000"[0] == 0x00, ""); // c-error {{universal character name refers to a control character}}
 _Static_assert(u8"\U0000007F"[0] == 0x7F, ""); // c-error {{universal character name refers to a control character}}
 _Static_assert((unsigned char)u8"\U00000080"[0] == (unsigned char)0xC2, ""); // c-error {{universal character name refers to a control character}}
 _Static_assert((unsigned char)u8"\U00000080"[1] == (unsigned char)0x80, ""); // c-error {{universal character name refers to a control character}}

 _Static_assert(U'\U00110000', ""); // expected-error {{invalid universal character}}
 
 // UTF-32 string literal code point ranges.
 _Static_assert(U"\U00000000"[0] == 0x00000000, ""); // c-error {{universal character name refers to a control character}}
 _Static_assert(U"\U0000D800"[0], ""); // expected-error {{invalid universal character}}
 _Static_assert(U"\U0000DFFF"[0], ""); // expected-error {{invalid universal character}}
 _Static_assert(U"\U0010FFFF"[0] == 0x0010FFFF, "");
 _Static_assert(U"\U00110000"[0], ""); // expected-error {{invalid universal character}}
+
+_Static_assert('\u0024' == '$', "");
+_Static_assert('\u0040' == '@', "");
+_Static_assert('\u0060' == '`', "");
+
+_Static_assert('\u0061' == 'a', ""); // c-error {{character 'a' cannot be specified by a universal character name}}
+_Static_assert('\u0000' == '\0', ""); // c-error {{universal character name refers to a control character}}

clang/test/Lexer/utf8-char-literal.cpp

 char b = u8'\x80'; // ok
 char c = u8'\u0080'; // expected-error {{character too large for enclosing character literal type}}
 char d = u8'\u1234'; // expected-error {{character too large for enclosing character literal type}}
 char e = u8'ሴ'; // expected-error {{character too large for enclosing character literal type}}
 char f = u8'ab'; // expected-error {{Unicode character literals may not contain multiple characters}}
 #elif __STDC_VERSION__ >= 202000L
 char a = u8'ñ';      // expected-error {{character too large for enclosing character literal type}}
 char b = u8'\x80';   // ok
-char c = u8'\u0080'; // expected-error {{universal character name refers to a control character}}
+char c = u8'\u0000'; // ok
 char d = u8'\u1234'; // expected-error {{character too large for enclosing character literal type}}
 char e = u8'ሴ';      // expected-error {{character too large for enclosing character literal type}}
 char f = u8'ab';     // expected-error {{Unicode character literals may not contain multiple characters}}
 _Static_assert(
     _Generic(u8'a',
              default : 0,
              unsigned char : 1),
     "Surprise!");

clang/test/Preprocessor/ucn-allowed-chars.c

 // RUN: %clang_cc1 %s -fsyntax-only -std=c99 -verify
+// RUN: %clang_cc1 %s -fsyntax-only -std=c2x -Wc99-compat -verify
 // RUN: %clang_cc1 %s -fsyntax-only -std=c11 -Wc99-compat -verify
 // RUN: %clang_cc1 %s -fsyntax-only -x c++ -std=c++03 -Wc++11-compat -verify
 // RUN: %clang_cc1 %s -fsyntax-only -x c++ -std=c++11 -Wc++98-compat -verify
 
 // Identifier characters
 extern char a\u01F6; // C11, C++11
 extern char a\u00AA; // C99, C11, C++11
 extern char a\u0384; // C++03, C11, C++11
 extern char a\u0E50; // C99, C++03, C11, C++11
 extern char a\uFFFF; // none
 
 
 
 
-
 // Identifier initial characters
 extern char \u0E50; // C++03, C11, C++11
 extern char \u0300; // disallowed in C99/C++03
 extern char \u0D61; // C99, C11, C++03, C++11
 
 
 
 
 
 
 
 // Disallowed everywhere
 #define A \u0000 // expected-error{{control character}}
 #define B \u001F // expected-error{{control character}}
 #define C \u007F // expected-error{{control character}}
 #define D \u009F // expected-error{{control character}}
 #define E \uD800 // C++03 allows UCNs representing surrogate characters!
 
 
 
 
 
 
 #if __cplusplus
-// expected-error@9 {{character <U+0384> not allowed in an identifier}}
-// expected-error@11 {{character <U+FFFF> not allowed in an identifier}}
+// expected-error@10 {{character <U+0384> not allowed in an identifier}}
+// expected-error@12 {{character <U+FFFF> not allowed in an identifier}}
 // expected-error@18 {{expected unqualified-id}}
 # if __cplusplus >= 201103L
 // C++11
 // expected-error@19 {{expected unqualified-id}}
 // expected-error@33 {{invalid universal character}}
 
 # else
 // C++03
 // expected-error@19 {{expected unqualified-id}}
 // expected-warning@33 {{universal character name refers to a surrogate character}}
 
 # endif
 #else
-# if __STDC_VERSION__ >= 201112L
+# if __STDC_VERSION__ >= 201800L
+// C2X
+// expected-warning@8 {{using this character in an identifier is incompatible with C99}}
+// expected-error@10 {{character <U+0384> not allowed in an identifier}}
+// expected-error@12 {{character <U+FFFF> not allowed in an identifier}}
+// expected-error@18 {{expected identifier}}
+// expected-error@19 {{expected identifier}}
+// expected-error@33 {{invalid universal character}}
+# elif __STDC_VERSION__ >= 201112L
 // C11
-// expected-warning@7 {{using this character in an identifier is incompatible with C99}}
-// expected-warning@9 {{using this character in an identifier is incompatible with C99}}
-// expected-error@11 {{character <U+FFFF> not allowed in an identifier}}
+// expected-warning@8 {{using this character in an identifier is incompatible with C99}}
+// expected-warning@10 {{using this character in an identifier is incompatible with C99}}
+// expected-error@12 {{character <U+FFFF> not allowed in an identifier}}
 // expected-warning@18 {{starting an identifier with this character is incompatible with C99}}
 // expected-error@19 {{expected identifier}}
 // expected-error@33 {{invalid universal character}}
 
 # else
 // C99
-// expected-error@7 {{not allowed in an identifier}}
-// expected-error@9 {{not allowed in an identifier}}
-// expected-error@11 {{not allowed in an identifier}}
+// expected-error@8 {{not allowed in an identifier}}
+// expected-error@10 {{not allowed in an identifier}}
+// expected-error@12 {{not allowed in an identifier}}
 // expected-error@18 {{expected identifier}}
 // expected-error@19 {{expected identifier}}
 // expected-error@33 {{invalid universal character}}
 
 # endif
 #endif
+
+#define AAA\u0024 // expected-error{{character '$' cannot be specified by a universal character name}} \
+                  // expected-warning{{whitespace}}
+#define AAB\u0040 // expected-error{{character '@' cannot be specified by a universal character name}} \
+                  // expected-warning{{whitespace}}
+#define AAC\u0060 // expected-error{{character '`' cannot be specified by a universal character name}} \
+                  // expected-warning{{whitespace}}
+
+#define ABA \u0024 // expected-error{{character '$' cannot be specified by a universal character name}}
+#define ABB \u0040 // expected-error{{character '@' cannot be specified by a universal character name}}
+#define ABC \u0060 // expected-error{{character '`' cannot be specified by a universal character name}}
+
+int GH62133_a\u0024; // expected-error {{character '$' cannot be specified by a universal character name}} \
+                     // expected-error {{}}
+int GH62133_b\u0040; // expected-error {{character '@' cannot be specified by a universal character name}} \
+                     // expected-error {{}}
+int GH62133_c\u0060; // expected-error {{character '`' cannot be specified by a universal character name}} \
+                     // expected-error {{}}

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

 // RUN: %clang_cc1 %s -fsyntax-only -std=c99 -pedantic -verify=expected,ext -Wundef -DTRIGRAPHS=1
+// RUN: %clang_cc1 %s -fsyntax-only -std=c2x -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++23 -pedantic -ftrigraphs -DTRIGRAPHS=1 -verify=expected,cxx23 -Wundef -Wpre-c++23-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

 #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}} cxx23-warning {{before C++23}}
 #define \N{ALERT}       // expected-error {{universal character name refers to a control character}} \
                    // expected-error {{macro name must be an identifier}} \
                    // ext-warning {{extension}} cxx23-warning {{before C++23}}
 #define \N{WASTEBASKET} // expected-error {{macro name must be an identifier}} \
                         // ext-warning {{extension}} cxx23-warning {{before C++23}}
-#define a\u0024
+#define a\u0024a  // expected-error {{character '$' cannot be specified by a universal character name}} \
+                  // expected-warning {{requires whitespace after the macro name}}
 
 #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}}
 #endif

 #else
 #error "Line splicing failed to produce UCNs"
 #endif
 
 
 #define capital_u_\U00FC
 // expected-warning@-1 {{incomplete universal character name}} expected-note@-1 {{did you mean to use '\u'?}} expected-warning@-1 {{whitespace}}
 // CHECK: note: did you mean to use '\u'?
-// CHECK-NEXT: {{^  112 | #define capital_u_\U00FC}}
+// CHECK-NEXT: {{^  114 | #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 \u1{123}       // expected-warning {{incomplete universal character name; treating as '\' followed by identifier}} 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}}

 
 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}}
+// expected-error@-1 {{expected}}
 #endif