[Clang] adjust caret placement for the suggested attribute location for enum class #168092

a-tarasyuk · 2025-11-14T17:35:34Z

This patch addresses the issue by correcting the caret insertion location for attributes incorrectly positioned before an enum. The location is now derived from the associated EnumDecl: for named enums, the attribute is placed before the identifier, while for anonymous enum definitions, it is placed before the opening brace, with a fallback to the semicolon when no brace is present.

For example:

  [[nodiscard]] enum class E1 {};

is now suggested as:

  enum class [[nodiscard]] E1 {};

…or enum class

llvmbot · 2025-11-14T17:36:03Z

@llvm/pr-subscribers-clang

Author: Oleksandr T. (a-tarasyuk)

Changes

Fixes #163224

This patch addresses the issue by correcting the caret insertion location for attributes incorrectly positioned before enum class, computing the adjustment from the DeclSpec and using the Lexer to advance past enum and class / struct for scoped enums before selecting the suggested location.

For example:

  [[nodiscard]] enum class E1 {};

is now suggested as:

  enum class [[nodiscard]] E1 {};

Full diff: https://github.com/llvm/llvm-project/pull/168092.diff

3 Files Affected:

(modified) clang/docs/ReleaseNotes.rst (+2)
(modified) clang/lib/Parse/Parser.cpp (+22-20)
(added) clang/test/FixIt/fixit-cxx0x-attributes.cpp (+30)

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 88a05affebf9e..06195b5086a66 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -420,6 +420,8 @@ Improvements to Clang's diagnostics
   or continue (#GH166013)
 - Clang now emits a diagnostic in case `vector_size` or `ext_vector_type`
   attributes are used with a negative size (#GH165463).
+- Clang now provides correct caret placement when attributes appear before
+  `enum class` (#GH163224).
 
 Improvements to Clang's time-trace
 ----------------------------------
diff --git a/clang/lib/Parse/Parser.cpp b/clang/lib/Parse/Parser.cpp
index a6fc676f23a51..5bf366db7f509 100644
--- a/clang/lib/Parse/Parser.cpp
+++ b/clang/lib/Parse/Parser.cpp
@@ -1100,30 +1100,32 @@ Parser::DeclGroupPtrTy Parser::ParseDeclOrFunctionDefInternal(
   // C99 6.7.2.3p6: Handle "struct-or-union identifier;", "enum { X };"
   // declaration-specifiers init-declarator-list[opt] ';'
   if (Tok.is(tok::semi)) {
-    auto LengthOfTSTToken = [](DeclSpec::TST TKind) {
-      assert(DeclSpec::isDeclRep(TKind));
-      switch(TKind) {
-      case DeclSpec::TST_class:
-        return 5;
-      case DeclSpec::TST_struct:
-        return 6;
-      case DeclSpec::TST_union:
-        return 5;
-      case DeclSpec::TST_enum:
-        return 4;
-      case DeclSpec::TST_interface:
-        return 9;
-      default:
-        llvm_unreachable("we only expect to get the length of the class/struct/union/enum");
+    auto GetAdjustedAttrsLoc = [&]() {
+      auto TKind = DS.getTypeSpecType();
+      if (!DeclSpec::isDeclRep(TKind))
+        return SourceLocation();
+
+      if (TKind == DeclSpec::TST_enum) {
+        const auto *ED = dyn_cast_or_null<EnumDecl>(DS.getRepAsDecl());
+        if (ED && ED->isScoped()) {
+          const auto &SM = Actions.getSourceManager();
+          const auto &LangOpts = Actions.getLangOpts();
+          auto End = Lexer::getLocForEndOfToken(DS.getTypeSpecTypeLoc(),
+                                                /*Offset*/ 0, SM, LangOpts);
+          auto NextToken = Lexer::findNextToken(End, SM, LangOpts);
+          if (NextToken)
+            return NextToken->getEndLoc();
+        }
       }
 
+      const auto &Policy = Actions.getASTContext().getPrintingPolicy();
+      unsigned Offset =
+          StringRef(DeclSpec::getSpecifierName(TKind, Policy)).size();
+      return DS.getTypeSpecTypeLoc().getLocWithOffset(Offset);
     };
+
     // Suggest correct location to fix '[[attrib]] struct' to 'struct [[attrib]]'
-    SourceLocation CorrectLocationForAttributes =
-        DeclSpec::isDeclRep(DS.getTypeSpecType())
-            ? DS.getTypeSpecTypeLoc().getLocWithOffset(
-                  LengthOfTSTToken(DS.getTypeSpecType()))
-            : SourceLocation();
+    SourceLocation CorrectLocationForAttributes = GetAdjustedAttrsLoc();
     ProhibitAttributes(Attrs, CorrectLocationForAttributes);
     ConsumeToken();
     RecordDecl *AnonRecord = nullptr;
diff --git a/clang/test/FixIt/fixit-cxx0x-attributes.cpp b/clang/test/FixIt/fixit-cxx0x-attributes.cpp
new file mode 100644
index 0000000000000..f64b6530efa15
--- /dev/null
+++ b/clang/test/FixIt/fixit-cxx0x-attributes.cpp
@@ -0,0 +1,30 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+// RUN: not %clang_cc1 -fsyntax-only -fdiagnostics-parseable-fixits -fno-diagnostics-show-line-numbers %s 2>&1 | FileCheck %s -strict-whitespace
+
+[[nodiscard]] enum class E1 { };
+// expected-error@-1 {{misplaced attributes; expected attributes here}}
+// CHECK: {{^}}{{\[\[}}nodiscard]] enum class E1 { };
+// CHECK: {{^}}~~~~~~~~~~~~~           ^
+// CHECK: fix-it:"{{.*}}":{[[@LINE-4]]:1-[[@LINE-4]]:15}:""
+// CHECK: fix-it:"{{.*}}":{[[@LINE-5]]:25-[[@LINE-5]]:25}:"{{\[\[}}nodiscard]]"
+
+[[nodiscard]] enum struct E2 { };
+// expected-error@-1 {{misplaced attributes; expected attributes here}}
+// CHECK: {{^}}{{\[\[}}nodiscard]] enum struct E2 { };
+// CHECK: {{^}}~~~~~~~~~~~~~            ^
+// CHECK: fix-it:"{{.*}}":{[[@LINE-4]]:1-[[@LINE-4]]:15}:""
+// CHECK: fix-it:"{{.*}}":{[[@LINE-5]]:26-[[@LINE-5]]:26}:"{{\[\[}}nodiscard]]"
+
+[[nodiscard]] enum          class E3 { };
+// expected-error@-1 {{misplaced attributes; expected attributes here}}
+// CHECK: {{^}}{{\[\[}}nodiscard]] enum          class E3 { };
+// CHECK: {{^}}~~~~~~~~~~~~~                    ^
+// CHECK: fix-it:"{{.*}}":{[[@LINE-4]]:1-[[@LINE-4]]:15}:""
+// CHECK: fix-it:"{{.*}}":{[[@LINE-5]]:34-[[@LINE-5]]:34}:"{{\[\[}}nodiscard]]"
+
+[[nodiscard]] enum  /*comment*/ class E4 { };
+// expected-error@-1 {{misplaced attributes; expected attributes here}}
+// CHECK: {{^}}{{\[\[}}nodiscard]] enum  /*comment*/ class E4 { };
+// CHECK: {{^}}~~~~~~~~~~~~~                        ^
+// CHECK: fix-it:"{{.*}}":{[[@LINE-4]]:1-[[@LINE-4]]:15}:""
+// CHECK: fix-it:"{{.*}}":{[[@LINE-5]]:38-[[@LINE-5]]:38}:"{{\[\[}}nodiscard]]"

clang/lib/Parse/Parser.cpp

erichkeane · 2025-11-14T17:50:56Z

clang/lib/Parse/Parser.cpp

+      if (!DeclSpec::isDeclRep(TKind))
+        return SourceLocation();
+
+      if (TKind == DeclSpec::TST_enum) {


IMO, the 'get location after the TST loc' logic works for all of these, right? With the exception being this is an enum class/enum struct?

Which, incidentially, doesn't actually work for enum class/enum struct? We potentially have OTHER tokens between the 'enum' and the 'class/struct' keywords (even if ill-formed potentially?), so in reality, if this is a scoped enum, we actually have to track down the class/struct token instead.

I find myself wondering if we should just have EnumDecl store the location of the class instead?

Alternatively, and probably better than all of this:
Since we are at the semi-colon, we actually already KNOW the location for the enum insertion point, right? It is either 'the name' of the enum, or, if there is no identifier, it is the LBrace/semicolon location, right? All of which should be accessible from ED. AND this should work whether or not it is scoped, right?

I find myself wondering if we SHOULD be smarter about the OTHER cases as well, rather than a sizeof-keyword-offset funny business, since that also doesn't necessarily work if there are other tokens in the way.

Since we are at the semi-colon, we actually already KNOW the location for the enum insertion point, right? It is either 'the name' of the enum, or, if there is no identifier, it is the LBrace/semicolon location, right?

@erichkeane Thanks for the feedback. I had considered using the identifier or lbrace location, but I wasn’t sure whether placing the attribute directly before the name or the brace was acceptable.

I THINK it is appropriate (identifier, lbrace, or semicolon)? I'm curious what @AaronBallman has to say.

Regarding the rest of the DeclSpec kinds, we could probably leave those alone + a fixme to 'get good' here.

@AaronBallman, whenever you have time, could you please review these changes?

My reading of the grammar is that the attribute can go after the key (so either enum or enum class|struct), which I think is easier to reason about than what it can go before.

Because "before" means more logic; there could be an optional nested name specifier. e.g., https://godbolt.org/z/eMo5T8r53 or there could be no name but an enum base, e.g., https://godbolt.org/z/jbqjnrn3d and so on.

So I think what makes the most sense is for EnumDecl to track a source range for the enum key. So it'll either be a single token (enum) or it will be all of the tokens in the key (enum class). Then we can use the end of that range. Then the logic works regardless of whether the enum is scoped or not.

WDYT?

clang/lib/Parse/Parser.cpp

erichkeane · 2025-11-14T19:36:40Z

clang/lib/Parse/Parser.cpp

+      if (!DeclSpec::isDeclRep(TKind))
+        return SourceLocation();
+
+      if (TKind == DeclSpec::TST_enum) {


I THINK it is appropriate (identifier, lbrace, or semicolon)? I'm curious what @AaronBallman has to say.

Regarding the rest of the DeclSpec kinds, we could probably leave those alone + a fixme to 'get good' here.

clang/lib/Parse/Parser.cpp

erichkeane

A few nits, + want a few tests, else I think this is about right.

clang/lib/Parse/Parser.cpp

clang/test/FixIt/fixit-cxx0x-attributes.cpp

erichkeane

1 nit, else LGTM

clang/lib/Parse/Parser.cpp

… into fix/163224

github-actions · 2025-11-18T15:33:00Z

🐧 Linux x64 Test Results

111335 tests passed
4417 tests skipped

[Clang] adjust caret placement for the suggested attribute location f…

68dc4b9

…or enum class

llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Nov 14, 2025

a-tarasyuk requested review from AaronBallman and erichkeane November 14, 2025 17:37

erichkeane reviewed Nov 14, 2025

View reviewed changes

derive enum attribute locations from EnumDecl instead of Lexer

5152768

erichkeane reviewed Nov 14, 2025

View reviewed changes

remove lambda

9f93393

erichkeane reviewed Nov 14, 2025

View reviewed changes

clang/lib/Parse/Parser.cpp Outdated Show resolved Hide resolved

clang/lib/Parse/Parser.cpp Outdated Show resolved Hide resolved

clang/lib/Parse/Parser.cpp Outdated Show resolved Hide resolved

clang/test/FixIt/fixit-cxx0x-attributes.cpp Show resolved Hide resolved

cleanup, add additional comment

a495ee9

a-tarasyuk requested a review from erichkeane November 14, 2025 23:30

a-tarasyuk added 2 commits November 15, 2025 01:30

Merge branch 'main' into fix/163224

7bc4552

Merge branch 'main' into fix/163224

303b91b

erichkeane approved these changes Nov 17, 2025

View reviewed changes

clang/lib/Parse/Parser.cpp Outdated Show resolved Hide resolved

a-tarasyuk added 4 commits November 17, 2025 16:28

use explicit type

782923b

Merge branch 'fix/163224' of https://github.com/a-tarasyuk/llvm-project…

253af19

… into fix/163224

Merge branch 'main' into fix/163224

f0f5dfd

Merge branch 'main' into fix/163224

1f565db

Merge branch 'main' into fix/163224

a394503

[Clang] adjust caret placement for the suggested attribute location for enum class #168092

Are you sure you want to change the base?

[Clang] adjust caret placement for the suggested attribute location for enum class #168092

Conversation

a-tarasyuk commented Nov 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

llvmbot commented Nov 14, 2025

Uh oh!

Uh oh!

erichkeane Nov 14, 2025

Choose a reason for hiding this comment

Uh oh!

a-tarasyuk Nov 14, 2025

Choose a reason for hiding this comment

Uh oh!

erichkeane Nov 14, 2025

Choose a reason for hiding this comment

Uh oh!

a-tarasyuk Nov 18, 2025

Choose a reason for hiding this comment

Uh oh!

AaronBallman Nov 19, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

erichkeane Nov 14, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

erichkeane left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

erichkeane left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

github-actions bot commented Nov 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🐧 Linux x64 Test Results

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

a-tarasyuk commented Nov 14, 2025 •

edited

Loading

github-actions bot commented Nov 18, 2025 •

edited

Loading