https://github.com/AaronBallman updated 
https://github.com/llvm/llvm-project/pull/135407

>From c7e0132617ab01c12b393876b39381171996b793 Mon Sep 17 00:00:00 2001
From: Aaron Ballman <aa...@aaronballman.com>
Date: Fri, 11 Apr 2025 13:03:07 -0400
Subject: [PATCH 1/5] Disable -fdollars-in-identifiers by default

Clang used to enable -fdollars-in-identifiers by default for
compatibility with GCC. However, this is no longer a conforming
extension after WG21 P2558R2 and WG14 N2701.

So this disables the dialect by default, which is a potentially
breaking change for users.

Note: some inline assembly constructs may use dollars in identifiers.
We cannot enable the dialect mode automatically based on the user
passing -fasm-blocks because that flag is implied by -fms-extensions
which is enabled by default on Windows, and thus would mean we'd be
enabling a non-conforming language extension by default on that
platform.

Users impacted by the change should explicitly add
-fdollars-in-identifiers to their build scripts.

Partially addresses #128939
---
 clang/docs/ReleaseNotes.rst                     |  5 +++++
 clang/include/clang/Basic/LangOptions.def       |  2 +-
 clang/include/clang/Driver/Options.td           |  2 +-
 clang/test/AST/ByteCode/codegen.m               |  2 +-
 clang/test/C/drs/dr0xx.c                        | 12 ++++++------
 clang/test/CodeGen/ms-inline-asm.c              |  2 +-
 clang/test/CodeGenObjC/extern-void-class-decl.m |  2 +-
 clang/test/Lexer/dollar-idents.c                |  2 +-
 clang/test/Lexer/gh128939.cpp                   | 17 +++++++++++++++++
 clang/test/Preprocessor/c90.c                   |  2 +-
 10 files changed, 35 insertions(+), 13 deletions(-)
 create mode 100644 clang/test/Lexer/gh128939.cpp

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 9c45965dc4d82..fe51de6fd5b7c 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -48,6 +48,11 @@ C/C++ Language Potentially Breaking Changes
   ensure they are not caught by these optimizations.  It is also possible to 
use
   ``-fwrapv-pointer`` or   ``-fno-delete-null-pointer-checks`` to make pointer 
arithmetic
   on null pointers well-defined. (#GH130734, #GH130742, #GH130952)
+- Use of the dollar sign (``$``) in an identifier is no longer a conforming
+  extension in either C or C++, so ``-fdollars-in-identifiers`` is no longer
+  enabled by default. Use of the dollar sign in an identifier will now be
+  diagnosed as an error unless ``-fdollars-in-identifiers`` is explicitly
+  enabled.
 
 C++ Specific Potentially Breaking Changes
 -----------------------------------------
diff --git a/clang/include/clang/Basic/LangOptions.def 
b/clang/include/clang/Basic/LangOptions.def
index 3879cc7942877..f08e179a38067 100644
--- a/clang/include/clang/Basic/LangOptions.def
+++ b/clang/include/clang/Basic/LangOptions.def
@@ -119,7 +119,7 @@ LANGOPT(WChar             , 1, 0, "wchar_t keyword")
 LANGOPT(Char8             , 1, 0, "char8_t keyword")
 LANGOPT(IEEE128           , 1, 0, "__ieee128 keyword")
 LANGOPT(DeclSpecKeyword   , 1, 0, "__declspec keyword")
-BENIGN_LANGOPT(DollarIdents   , 1, 1, "'$' in identifiers")
+BENIGN_LANGOPT(DollarIdents   , 1, 0, "'$' in identifiers")
 BENIGN_LANGOPT(AsmPreprocessor, 1, 0, "preprocessor in asm mode")
 LANGOPT(GNUMode           , 1, 1, "GNU extensions")
 LANGOPT(GNUKeywords       , 1, 1, "GNU keywords")
diff --git a/clang/include/clang/Driver/Options.td 
b/clang/include/clang/Driver/Options.td
index c1020b234b136..38eb332f40d27 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -2088,7 +2088,7 @@ def fno_discard_value_names : Flag<["-"], 
"fno-discard-value-names">,
   Group<f_clang_Group>, Visibility<[ClangOption, DXCOption]>,
   HelpText<"Do not discard value names in LLVM IR">;
 defm dollars_in_identifiers : BoolFOption<"dollars-in-identifiers",
-  LangOpts<"DollarIdents">, Default<!strconcat("!", asm_preprocessor.KeyPath)>,
+  LangOpts<"DollarIdents">, DefaultFalse,
   PosFlag<SetTrue, [], [ClangOption], "Allow">,
   NegFlag<SetFalse, [], [ClangOption], "Disallow">,
   BothFlags<[], [ClangOption, CC1Option], " '$' in identifiers">>;
diff --git a/clang/test/AST/ByteCode/codegen.m 
b/clang/test/AST/ByteCode/codegen.m
index 6139596c6337a..a7b3a100165eb 100644
--- a/clang/test/AST/ByteCode/codegen.m
+++ b/clang/test/AST/ByteCode/codegen.m
@@ -3,7 +3,7 @@
 /// See test/CodeGenObjC/constant-strings.m
 /// Test that we let the APValue we create for ObjCStringLiterals point to the 
right expression.
 
-// RUN: %clang_cc1 -triple x86_64-macho -emit-llvm -o %t %s 
-fexperimental-new-constant-interpreter
+// RUN: %clang_cc1 -triple x86_64-macho -fdollars-in-identifiers  -emit-llvm 
-o %t %s -fexperimental-new-constant-interpreter
 // RUN: FileCheck --check-prefix=CHECK-NEXT < %t %s
 
 // Check that we set alignment 1 on the string.
diff --git a/clang/test/C/drs/dr0xx.c b/clang/test/C/drs/dr0xx.c
index 5fe023deaece9..ffcd63b0cc9a7 100644
--- a/clang/test/C/drs/dr0xx.c
+++ b/clang/test/C/drs/dr0xx.c
@@ -1,9 +1,9 @@
-/* RUN: %clang_cc1 -std=c89 -fsyntax-only -verify=expected,c89only -pedantic 
-Wno-declaration-after-statement -Wno-c11-extensions %s
-   RUN: %clang_cc1 -std=c89 -fsyntax-only -verify=expected,c89only -pedantic 
-Wno-declaration-after-statement -Wno-c11-extensions -fno-signed-char %s
-   RUN: %clang_cc1 -std=c99 -fsyntax-only -verify=expected,c99untilc2x 
-pedantic -Wno-c11-extensions %s
-   RUN: %clang_cc1 -std=c11 -fsyntax-only -verify=expected,c99untilc2x 
-pedantic %s
-   RUN: %clang_cc1 -std=c17 -fsyntax-only -verify=expected,c99untilc2x 
-pedantic %s
-   RUN: %clang_cc1 -std=c2x -fsyntax-only -verify=expected,c2xandup -pedantic 
%s
+/* RUN: %clang_cc1 -std=c89 -fdollars-in-identifiers -fsyntax-only 
-verify=expected,c89only -pedantic -Wno-declaration-after-statement 
-Wno-c11-extensions %s
+   RUN: %clang_cc1 -std=c89 -fdollars-in-identifiers -fsyntax-only 
-verify=expected,c89only -pedantic -Wno-declaration-after-statement 
-Wno-c11-extensions -fno-signed-char %s
+   RUN: %clang_cc1 -std=c99 -fdollars-in-identifiers -fsyntax-only 
-verify=expected,c99untilc2x -pedantic -Wno-c11-extensions %s
+   RUN: %clang_cc1 -std=c11 -fdollars-in-identifiers -fsyntax-only 
-verify=expected,c99untilc2x -pedantic %s
+   RUN: %clang_cc1 -std=c17 -fdollars-in-identifiers -fsyntax-only 
-verify=expected,c99untilc2x -pedantic %s
+   RUN: %clang_cc1 -std=c2x -fdollars-in-identifiers -fsyntax-only 
-verify=expected,c2xandup -pedantic %s
  */
 
 /* The following are DRs which do not require tests to demonstrate
diff --git a/clang/test/CodeGen/ms-inline-asm.c 
b/clang/test/CodeGen/ms-inline-asm.c
index c3eef9a23e166..811e3ccd2a89a 100644
--- a/clang/test/CodeGen/ms-inline-asm.c
+++ b/clang/test/CodeGen/ms-inline-asm.c
@@ -1,5 +1,5 @@
 // REQUIRES: x86-registered-target
-// RUN: %clang_cc1 %s -triple i386-apple-darwin10 -fasm-blocks -emit-llvm -o - 
| FileCheck %s
+// RUN: %clang_cc1 %s -triple i386-apple-darwin10 -fdollars-in-identifiers 
-fasm-blocks -emit-llvm -o - | FileCheck %s
 
 void t1(void) {
 // CHECK: @t1
diff --git a/clang/test/CodeGenObjC/extern-void-class-decl.m 
b/clang/test/CodeGenObjC/extern-void-class-decl.m
index 826622b94c1bb..55b2f9565439a 100644
--- a/clang/test/CodeGenObjC/extern-void-class-decl.m
+++ b/clang/test/CodeGenObjC/extern-void-class-decl.m
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -triple x86_64-apple-macosx10.14.0 %s -emit-llvm -o - | 
FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-apple-macosx10.14.0 -fdollars-in-identifiers 
%s -emit-llvm -o - | FileCheck %s
 
 extern void OBJC_CLASS_$_f;
 Class c = (Class)&OBJC_CLASS_$_f;
diff --git a/clang/test/Lexer/dollar-idents.c b/clang/test/Lexer/dollar-idents.c
index a1263b4e572cc..6fd32a13bc591 100644
--- a/clang/test/Lexer/dollar-idents.c
+++ b/clang/test/Lexer/dollar-idents.c
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -dump-tokens %s 2>&1 | FileCheck %s
+// RUN: %clang_cc1 -dump-tokens -fdollars-in-identifiers %s 2>&1 | FileCheck %s
 // RUN: %clang_cc1 -dump-tokens -x assembler-with-cpp %s 2>&1 | FileCheck %s 
--check-prefix=CHECK-ASM
 // PR3808
 
diff --git a/clang/test/Lexer/gh128939.cpp b/clang/test/Lexer/gh128939.cpp
new file mode 100644
index 0000000000000..8abd8c8c6ec0a
--- /dev/null
+++ b/clang/test/Lexer/gh128939.cpp
@@ -0,0 +1,17 @@
+// RUN: %clang_cc1 -E -fdollars-in-identifiers %s 2>&1 | FileCheck %s 
--check-prefix=CHECK-DOLLARS
+// RUN: %clang_cc1 -E %s 2>&1 | FileCheck %s --check-prefix=CHECK-NO-DOLLARS
+// GH128939
+
+#define FOO$ 10
+#define STR(x) #x
+#define STR2(x) STR(x)
+const char *p = STR2(FOO$);
+
+// CHECK-NO-DOLLARS: const char *p = "$ 10$";
+// CHECK-DOLLARS: const char *p = "10";
+
+#define STR3 STR(
+const char *q = STR3$10);
+
+// CHECK-NO-DOLLARS: const char *q = "$10";
+// CHECK-DOLLARS: const char *q = STR3$10);
diff --git a/clang/test/Preprocessor/c90.c b/clang/test/Preprocessor/c90.c
index 3b9105fe6ee91..2d64c4be1234d 100644
--- a/clang/test/Preprocessor/c90.c
+++ b/clang/test/Preprocessor/c90.c
@@ -7,7 +7,7 @@
 #define foo`bar   /* expected-error {{whitespace required after macro name}} */
 #define foo2!bar  /* expected-warning {{whitespace recommended after macro 
name}} */
 
-#define foo3$bar  /* expected-error {{'$' in identifier}} */
+#define foo3$bar  /* expected-error {{whitespace required after macro name}} */
 
 /* CHECK-NOT: this comment should be missing
  * CHECK: {{^}}// this comment should be present{{$}}

>From 1e47954ed08d622a6750680ad57bbe8b09d4f06f Mon Sep 17 00:00:00 2001
From: Aaron Ballman <aa...@aaronballman.com>
Date: Fri, 11 Apr 2025 13:58:37 -0400
Subject: [PATCH 2/5] Enable dollar identifiers for unit tests

---
 clang/lib/Format/Format.cpp                            | 1 +
 clang/unittests/Analysis/MacroExpansionContextTest.cpp | 1 +
 2 files changed, 2 insertions(+)

diff --git a/clang/lib/Format/Format.cpp b/clang/lib/Format/Format.cpp
index b90bd8276e1e2..80d27e6939005 100644
--- a/clang/lib/Format/Format.cpp
+++ b/clang/lib/Format/Format.cpp
@@ -4036,6 +4036,7 @@ LangOptions getFormattingLangOpts(const FormatStyle 
&Style) {
   LangOpts.MicrosoftExt = 1;    // To get kw___try, kw___finally.
   LangOpts.DeclSpecKeyword = 1; // To get __declspec.
   LangOpts.C99 = 1; // To get kw_restrict for non-underscore-prefixed restrict.
+  LangOpts.DollarIdents = 1; // For $identifier$ testing.
 
   return LangOpts;
 }
diff --git a/clang/unittests/Analysis/MacroExpansionContextTest.cpp 
b/clang/unittests/Analysis/MacroExpansionContextTest.cpp
index 19074d7dcfdd4..e091459532420 100644
--- a/clang/unittests/Analysis/MacroExpansionContextTest.cpp
+++ b/clang/unittests/Analysis/MacroExpansionContextTest.cpp
@@ -41,6 +41,7 @@ class MacroExpansionContextTest : public ::testing::Test {
     TargetOpts->Triple = "x86_64-pc-linux-unknown";
     Target = TargetInfo::CreateTargetInfo(Diags, TargetOpts);
     LangOpts.CPlusPlus20 = 1; // For __VA_OPT__
+    LangOpts.DollarIdents = 1; // For $identifier$ testing.
   }
 
   IntrusiveRefCntPtr<llvm::vfs::InMemoryFileSystem> InMemoryFileSystem;

>From 72473ef6da6d5dcdd2ffd2f2a15444b57e27d09e Mon Sep 17 00:00:00 2001
From: Aaron Ballman <aa...@aaronballman.com>
Date: Mon, 14 Apr 2025 08:30:42 -0400
Subject: [PATCH 3/5] Switch to compatible langopt

---
 clang/include/clang/Basic/LangOptions.def | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/clang/include/clang/Basic/LangOptions.def 
b/clang/include/clang/Basic/LangOptions.def
index f08e179a38067..547b1441df9cf 100644
--- a/clang/include/clang/Basic/LangOptions.def
+++ b/clang/include/clang/Basic/LangOptions.def
@@ -119,7 +119,7 @@ LANGOPT(WChar             , 1, 0, "wchar_t keyword")
 LANGOPT(Char8             , 1, 0, "char8_t keyword")
 LANGOPT(IEEE128           , 1, 0, "__ieee128 keyword")
 LANGOPT(DeclSpecKeyword   , 1, 0, "__declspec keyword")
-BENIGN_LANGOPT(DollarIdents   , 1, 0, "'$' in identifiers")
+COMPATIBLE_LANGOPT(DollarIdents, 1, 0, "'$' in identifiers")
 BENIGN_LANGOPT(AsmPreprocessor, 1, 0, "preprocessor in asm mode")
 LANGOPT(GNUMode           , 1, 1, "GNU extensions")
 LANGOPT(GNUKeywords       , 1, 1, "GNU keywords")

>From fb0855fbea655ad470b63475919c51ed8f0a06b1 Mon Sep 17 00:00:00 2001
From: Aaron Ballman <aa...@aaronballman.com>
Date: Tue, 15 Apr 2025 07:29:54 -0400
Subject: [PATCH 4/5] Update comment based on review feedback; NFC

---
 clang/lib/Format/Format.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/clang/lib/Format/Format.cpp b/clang/lib/Format/Format.cpp
index 80d27e6939005..e0bf099765e5f 100644
--- a/clang/lib/Format/Format.cpp
+++ b/clang/lib/Format/Format.cpp
@@ -4036,7 +4036,7 @@ LangOptions getFormattingLangOpts(const FormatStyle 
&Style) {
   LangOpts.MicrosoftExt = 1;    // To get kw___try, kw___finally.
   LangOpts.DeclSpecKeyword = 1; // To get __declspec.
   LangOpts.C99 = 1; // To get kw_restrict for non-underscore-prefixed restrict.
-  LangOpts.DollarIdents = 1; // For $identifier$ testing.
+  LangOpts.DollarIdents = 1; // To allow $ in identifiers.
 
   return LangOpts;
 }

>From f9b9e817082a8745ac34db2939273f05270fdbf5 Mon Sep 17 00:00:00 2001
From: Aaron Ballman <aa...@aaronballman.com>
Date: Wed, 16 Apr 2025 08:28:51 -0400
Subject: [PATCH 5/5] Add new diagnostic based on review feedback

---
 clang/include/clang/Basic/DiagnosticLexKinds.td |  3 +++
 clang/lib/Lex/Lexer.cpp                         |  3 ++-
 clang/test/C/C99/n717.c                         |  2 +-
 clang/test/Preprocessor/c90.c                   | 15 +++++++++++++--
 clang/test/SemaCXX/scope-check.cpp              |  3 ++-
 5 files changed, 21 insertions(+), 5 deletions(-)

diff --git a/clang/include/clang/Basic/DiagnosticLexKinds.td 
b/clang/include/clang/Basic/DiagnosticLexKinds.td
index f29edfa835d42..41d3e29d1324e 100644
--- a/clang/include/clang/Basic/DiagnosticLexKinds.td
+++ b/clang/include/clang/Basic/DiagnosticLexKinds.td
@@ -57,6 +57,9 @@ def warn_no_newline_eof : Warning<"no newline at end of 
file">,
 
 def ext_dollar_in_identifier : Extension<"'$' in identifier">,
   InGroup<DiagGroup<"dollar-in-identifier-extension">>;
+def warn_dollar_in_identifier : Warning<
+  "'$' in identifier; did you mean to enable '-fdollars-in-identifiers'?">,
+  InGroup<DiagGroup<"dollar-in-identifier">>;
 def ext_charize_microsoft : Extension<
   "charizing operator #@ is a Microsoft extension">,
   InGroup<MicrosoftCharize>;
diff --git a/clang/lib/Lex/Lexer.cpp b/clang/lib/Lex/Lexer.cpp
index 93200458f04b4..81907174dea58 100644
--- a/clang/lib/Lex/Lexer.cpp
+++ b/clang/lib/Lex/Lexer.cpp
@@ -4036,8 +4036,9 @@ bool Lexer::LexTokenInternal(Token &Result, bool 
TokAtPhysicalStartOfLine) {
       MIOpt.ReadToken();
       return LexIdentifierContinue(Result, CurPtr);
     }
-
     Kind = tok::unknown;
+    if (!isLexingRawMode())
+      Diag(CurPtr - 1, diag::warn_dollar_in_identifier);
     break;
 
   // C99 6.4.4: Character Constants.
diff --git a/clang/test/C/C99/n717.c b/clang/test/C/C99/n717.c
index 25010b4137065..240ff2007c7e9 100644
--- a/clang/test/C/C99/n717.c
+++ b/clang/test/C/C99/n717.c
@@ -37,7 +37,7 @@ M(\U123456789) // Okay-ish, two tokens 
(valid-per-spec-but-actually-invalid UCN
 // GH87106.
 M(\u0024) // expected-error {{character '$' cannot be specified by a universal 
character name}}
 M(\U00000024) // expected-error {{character '$' cannot be specified by a 
universal character name}}
-M($)
+M($)      // expected-warning {{'$' in identifier; did you mean to enable 
'-fdollars-in-identifiers'?}}
 
 // These should always be rejected because they're not valid identifier
 // characters.
diff --git a/clang/test/Preprocessor/c90.c b/clang/test/Preprocessor/c90.c
index 2d64c4be1234d..cbf90b87b1e6d 100644
--- a/clang/test/Preprocessor/c90.c
+++ b/clang/test/Preprocessor/c90.c
@@ -1,4 +1,4 @@
-/* RUN: %clang_cc1 %s -std=c89 -Eonly -verify -pedantic-errors 
+/* RUN: %clang_cc1 %s -std=c89 -Eonly -DTEST_THIS_TOO -verify -pedantic-errors
  * RUN: %clang_cc1 %s -std=c89 -E | FileCheck %s
  */
 
@@ -7,7 +7,18 @@
 #define foo`bar   /* expected-error {{whitespace required after macro name}} */
 #define foo2!bar  /* expected-warning {{whitespace recommended after macro 
name}} */
 
-#define foo3$bar  /* expected-error {{whitespace required after macro name}} */
+#define foo3$bar  /* expected-error {{whitespace required after macro name}}
+                     expected-warning {{'$' in identifier; did you mean to 
enable '-fdollars-in-identifiers'?}}
+                   */
+
+#define test$     /* expected-error {{whitespace required after macro name}}
+                     expected-warning {{'$' in identifier; did you mean to 
enable '-fdollars-in-identifiers'?}}
+                   */
+#ifdef TEST_THIS_TOO
+#define $         /* expected-error {{macro name must be an identifier}}
+                     expected-warning {{'$' in identifier; did you mean to 
enable '-fdollars-in-identifiers'?}}
+                   */
+#endif
 
 /* CHECK-NOT: this comment should be missing
  * CHECK: {{^}}// this comment should be present{{$}}
diff --git a/clang/test/SemaCXX/scope-check.cpp 
b/clang/test/SemaCXX/scope-check.cpp
index 7eec58ca057a9..d23b042087a18 100644
--- a/clang/test/SemaCXX/scope-check.cpp
+++ b/clang/test/SemaCXX/scope-check.cpp
@@ -434,7 +434,8 @@ namespace test_recovery {
     a0:;
 
     switch (c) {
-    case $: // expected-error {{}}
+    case $: // expected-error {{}} \
+               expected-warning {{'$' in identifier; did you mean to enable 
'-fdollars-in-identifiers'?}}
     case 0:
       int x = 56; // expected-note {{jump bypasses variable initialization}}
     case 1: // expected-error {{cannot jump}}

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to