https://github.com/a-tarasyuk updated https://github.com/llvm/llvm-project/pull/168092
>From 68dc4b9a562a5c17eb2c2f5be15d44b372f9cb2a Mon Sep 17 00:00:00 2001 From: Oleksandr Tarasiuk <[email protected]> Date: Fri, 14 Nov 2025 19:26:56 +0200 Subject: [PATCH 1/4] [Clang] adjust caret placement for the suggested attribute location for enum class --- clang/docs/ReleaseNotes.rst | 2 + clang/lib/Parse/Parser.cpp | 42 +++++++++++---------- clang/test/FixIt/fixit-cxx0x-attributes.cpp | 30 +++++++++++++++ 3 files changed, 54 insertions(+), 20 deletions(-) create mode 100644 clang/test/FixIt/fixit-cxx0x-attributes.cpp 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]]" >From 5152768fa7c65456ea9b6771933d419419a54c22 Mon Sep 17 00:00:00 2001 From: Oleksandr Tarasiuk <[email protected]> Date: Fri, 14 Nov 2025 21:04:12 +0200 Subject: [PATCH 2/4] derive enum attribute locations from EnumDecl instead of Lexer --- clang/lib/Parse/Parser.cpp | 15 +++++++------- clang/test/FixIt/fixit-cxx0x-attributes.cpp | 23 ++++++++++++++------- 2 files changed, 22 insertions(+), 16 deletions(-) diff --git a/clang/lib/Parse/Parser.cpp b/clang/lib/Parse/Parser.cpp index 5bf366db7f509..fe361268f24f9 100644 --- a/clang/lib/Parse/Parser.cpp +++ b/clang/lib/Parse/Parser.cpp @@ -1107,14 +1107,13 @@ Parser::DeclGroupPtrTy Parser::ParseDeclOrFunctionDefInternal( 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(); + if (ED) { + if (ED->isScoped() && ED->getIdentifier()) + return ED->getLocation(); + + const auto Begin = ED->getBraceRange().getBegin(); + if (Begin.isValid()) + return Begin; } } diff --git a/clang/test/FixIt/fixit-cxx0x-attributes.cpp b/clang/test/FixIt/fixit-cxx0x-attributes.cpp index f64b6530efa15..4dce66fea0d62 100644 --- a/clang/test/FixIt/fixit-cxx0x-attributes.cpp +++ b/clang/test/FixIt/fixit-cxx0x-attributes.cpp @@ -4,27 +4,34 @@ [[nodiscard]] enum class E1 { }; // expected-error@-1 {{misplaced attributes; expected attributes here}} // CHECK: {{^}}{{\[\[}}nodiscard]] enum class E1 { }; -// CHECK: {{^}}~~~~~~~~~~~~~ ^ +// CHECK: {{^}}~~~~~~~~~~~~~ ^ // CHECK: fix-it:"{{.*}}":{[[@LINE-4]]:1-[[@LINE-4]]:15}:"" -// CHECK: fix-it:"{{.*}}":{[[@LINE-5]]:25-[[@LINE-5]]:25}:"{{\[\[}}nodiscard]]" +// CHECK: fix-it:"{{.*}}":{[[@LINE-5]]:26-[[@LINE-5]]:26}:"{{\[\[}}nodiscard]]" [[nodiscard]] enum struct E2 { }; // expected-error@-1 {{misplaced attributes; expected attributes here}} // CHECK: {{^}}{{\[\[}}nodiscard]] enum struct E2 { }; -// CHECK: {{^}}~~~~~~~~~~~~~ ^ +// CHECK: {{^}}~~~~~~~~~~~~~ ^ // CHECK: fix-it:"{{.*}}":{[[@LINE-4]]:1-[[@LINE-4]]:15}:"" -// CHECK: fix-it:"{{.*}}":{[[@LINE-5]]:26-[[@LINE-5]]:26}:"{{\[\[}}nodiscard]]" +// CHECK: fix-it:"{{.*}}":{[[@LINE-5]]:27-[[@LINE-5]]:27}:"{{\[\[}}nodiscard]]" [[nodiscard]] enum class E3 { }; // expected-error@-1 {{misplaced attributes; expected attributes here}} // CHECK: {{^}}{{\[\[}}nodiscard]] enum class E3 { }; -// CHECK: {{^}}~~~~~~~~~~~~~ ^ +// CHECK: {{^}}~~~~~~~~~~~~~ ^ // CHECK: fix-it:"{{.*}}":{[[@LINE-4]]:1-[[@LINE-4]]:15}:"" -// CHECK: fix-it:"{{.*}}":{[[@LINE-5]]:34-[[@LINE-5]]:34}:"{{\[\[}}nodiscard]]" +// CHECK: fix-it:"{{.*}}":{[[@LINE-5]]:35-[[@LINE-5]]:35}:"{{\[\[}}nodiscard]]" [[nodiscard]] enum /*comment*/ class E4 { }; // expected-error@-1 {{misplaced attributes; expected attributes here}} // CHECK: {{^}}{{\[\[}}nodiscard]] enum /*comment*/ class E4 { }; -// CHECK: {{^}}~~~~~~~~~~~~~ ^ +// CHECK: {{^}}~~~~~~~~~~~~~ ^ +// CHECK: fix-it:"{{.*}}":{[[@LINE-4]]:1-[[@LINE-4]]:15}:"" +// CHECK: fix-it:"{{.*}}":{[[@LINE-5]]:39-[[@LINE-5]]:39}:"{{\[\[}}nodiscard]]" + +[[nodiscard]] enum { A = 0 }; +// expected-error@-1 {{misplaced attributes; expected attributes here}} +// CHECK: {{^}}{{\[\[}}nodiscard]] enum { A = 0 }; +// CHECK: {{^}}~~~~~~~~~~~~~ ^ // CHECK: fix-it:"{{.*}}":{[[@LINE-4]]:1-[[@LINE-4]]:15}:"" -// CHECK: fix-it:"{{.*}}":{[[@LINE-5]]:38-[[@LINE-5]]:38}:"{{\[\[}}nodiscard]]" +// CHECK: fix-it:"{{.*}}":{[[@LINE-5]]:20-[[@LINE-5]]:20}:"{{\[\[}}nodiscard]]" >From 9f93393da00e9f4df5f691f6c9fdace5748eff65 Mon Sep 17 00:00:00 2001 From: Oleksandr Tarasiuk <[email protected]> Date: Fri, 14 Nov 2025 21:55:34 +0200 Subject: [PATCH 3/4] remove lambda --- clang/lib/Parse/Parser.cpp | 40 +++++++++++++++++++------------------- 1 file changed, 20 insertions(+), 20 deletions(-) diff --git a/clang/lib/Parse/Parser.cpp b/clang/lib/Parse/Parser.cpp index fe361268f24f9..411b40ac29a3c 100644 --- a/clang/lib/Parse/Parser.cpp +++ b/clang/lib/Parse/Parser.cpp @@ -1100,31 +1100,31 @@ 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 GetAdjustedAttrsLoc = [&]() { - auto TKind = DS.getTypeSpecType(); - if (!DeclSpec::isDeclRep(TKind)) - return SourceLocation(); - + // Suggest correct location to fix '[[attrib]] struct' to 'struct + // [[attrib]]' + SourceLocation CorrectLocationForAttributes{}; + auto TKind = DS.getTypeSpecType(); + if (DeclSpec::isDeclRep(TKind)) { if (TKind == DeclSpec::TST_enum) { const auto *ED = dyn_cast_or_null<EnumDecl>(DS.getRepAsDecl()); if (ED) { - if (ED->isScoped() && ED->getIdentifier()) - return ED->getLocation(); - - const auto Begin = ED->getBraceRange().getBegin(); - if (Begin.isValid()) - return Begin; + if (ED->getIdentifier()) { + CorrectLocationForAttributes = ED->getLocation(); + } else { + const auto Begin = ED->getBraceRange().getBegin(); + CorrectLocationForAttributes = + Begin.isValid() ? Begin : ED->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 = GetAdjustedAttrsLoc(); + if (CorrectLocationForAttributes.isInvalid()) { + const auto &Policy = Actions.getASTContext().getPrintingPolicy(); + unsigned Offset = + StringRef(DeclSpec::getSpecifierName(TKind, Policy)).size(); + CorrectLocationForAttributes = + DS.getTypeSpecTypeLoc().getLocWithOffset(Offset); + } + } ProhibitAttributes(Attrs, CorrectLocationForAttributes); ConsumeToken(); RecordDecl *AnonRecord = nullptr; >From a495ee9194ed478743a6bfdb960479689d69a07d Mon Sep 17 00:00:00 2001 From: Oleksandr Tarasiuk <[email protected]> Date: Fri, 14 Nov 2025 22:16:12 +0200 Subject: [PATCH 4/4] cleanup, add additional comment --- clang/lib/Parse/Parser.cpp | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/clang/lib/Parse/Parser.cpp b/clang/lib/Parse/Parser.cpp index 411b40ac29a3c..b8818c6f9fe87 100644 --- a/clang/lib/Parse/Parser.cpp +++ b/clang/lib/Parse/Parser.cpp @@ -1103,17 +1103,19 @@ Parser::DeclGroupPtrTy Parser::ParseDeclOrFunctionDefInternal( // Suggest correct location to fix '[[attrib]] struct' to 'struct // [[attrib]]' SourceLocation CorrectLocationForAttributes{}; - auto TKind = DS.getTypeSpecType(); + TypeSpecifierType TKind = DS.getTypeSpecType(); if (DeclSpec::isDeclRep(TKind)) { if (TKind == DeclSpec::TST_enum) { - const auto *ED = dyn_cast_or_null<EnumDecl>(DS.getRepAsDecl()); - if (ED) { + if (const auto *ED = dyn_cast_or_null<EnumDecl>(DS.getRepAsDecl())) { if (ED->getIdentifier()) { CorrectLocationForAttributes = ED->getLocation(); } else { const auto Begin = ED->getBraceRange().getBegin(); CorrectLocationForAttributes = - Begin.isValid() ? Begin : ED->getEndLoc(); + Begin.isValid() ? Begin : + // If there is no brace, fall back to the end + // location, which corresponds to the semicolon. + ED->getEndLoc(); } } } _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
