Skip to content

Disable -fdollars-in-identifiers by default #135407

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
Next Next commit
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
  • Loading branch information
AaronBallman committed Apr 11, 2025
commit c7e0132617ab01c12b393876b39381171996b793
5 changes: 5 additions & 0 deletions clang/docs/ReleaseNotes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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
-----------------------------------------
Expand Down
2 changes: 1 addition & 1 deletion clang/include/clang/Basic/LangOptions.def
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down
2 changes: 1 addition & 1 deletion clang/include/clang/Driver/Options.td
Original file line number Diff line number Diff line change
Expand Up @@ -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">>;
Expand Down
2 changes: 1 addition & 1 deletion clang/test/AST/ByteCode/codegen.m
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
12 changes: 6 additions & 6 deletions clang/test/C/drs/dr0xx.c
Original file line number Diff line number Diff line change
@@ -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
Expand Down
2 changes: 1 addition & 1 deletion clang/test/CodeGen/ms-inline-asm.c
Original file line number Diff line number Diff line change
@@ -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
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just musing... if we wanted this to work automatically, we could implement detection of __asm in the preprocessor, and enable dollars-in-identifiers when lexing inside an __asm block. Detection of __asm at that phase is also necessary to properly handle ; comments, which right now we incorrectly discard after preprocessing. (I'm not sure if there are any other lexing differences for MS __asm that we don't properly handle, but there might be.) But it's probably not worth it for a feature that only works (in MSVC at least) when targeting 32-bit x86. :)

Perhaps a better idea would be to enable -fdollars-in-identifiers under -fms-compatibility, given that MSVC still enables support for dollars in identifiers even in its conforming mode. Though perhaps we can leave that option as a fallback for if someone actually complains.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps a better idea would be to enable -fdollars-in-identifiers under -fms-compatibility

I thought about this but explicitly decided against it because that means dollars in identifiers is enabled by default for anyone using Clang built by MSVC and targeting Windows. I think we may want to see what folks run into; maybe we want to enable it only for clang-cl and not clang? Maybe we want to be more clever than that? I dunno.

Copy link
Contributor

@R-Goc R-Goc Apr 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe we want to enable it only for clang-cl and not clang?

This sounds like an even less expected corner case that would be very annoying.


void t1(void) {
// CHECK: @t1
Expand Down
2 changes: 1 addition & 1 deletion clang/test/CodeGenObjC/extern-void-class-decl.m
Original file line number Diff line number Diff line change
@@ -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;
Expand Down
2 changes: 1 addition & 1 deletion clang/test/Lexer/dollar-idents.c
Original file line number Diff line number Diff line change
@@ -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

Expand Down
17 changes: 17 additions & 0 deletions clang/test/Lexer/gh128939.cpp
Original file line number Diff line number Diff line change
@@ -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);
2 changes: 1 addition & 1 deletion clang/test/Preprocessor/c90.c
Original file line number Diff line number Diff line change
Expand Up @@ -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}} */
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why does this diagnostic change?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In -pedentic-errors mode before the patch, dollar signs in identifiers are recognized and so we get # define <identifier> and issue the pedantic diagnostic. With the patch, dollar signs in identifiers are not recognized at all and so we get # define foo $ bar which has no whitespace between the macro name and its replacement list.


/* CHECK-NOT: this comment should be missing
* CHECK: {{^}}// this comment should be present{{$}}
Expand Down
Loading