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