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

clang/docs/ReleaseNotes.rst

@@ -443,6 +443,8 @@ Improvements to Clang's diagnostics
 - A new warning ``-Wenum-compare-typo`` has been added to detect potential erroneous
   comparison operators when mixed with bitwise operators in enum value initializers.
   This can be locally disabled by explicitly casting the initializer value.
+- Clang now provides correct caret placement when attributes appear before
+  `enum class` (#GH163224).
 
 Improvements to Clang's time-trace
 ----------------------------------

clang/include/clang/AST/Decl.h

@@ -4040,6 +4040,11 @@ class EnumDecl : public TagDecl {
   /// and can be accessed with the provided accessors.
   unsigned ODRHash;
 
+  /// Source range covering the enum key:
+  ///  - 'enum'              (unscoped)
+  ///  - 'enum class|struct' (scoped)
+  SourceRange EnumKeyRange;
+
   EnumDecl(ASTContext &C, DeclContext *DC, SourceLocation StartLoc,
            SourceLocation IdLoc, IdentifierInfo *Id, EnumDecl *PrevDecl,
            bool Scoped, bool ScopedUsingClassTag, bool Fixed);
@@ -4077,6 +4082,10 @@ class EnumDecl : public TagDecl {
   /// Microsoft-style enumeration with a fixed underlying type.
   void setFixed(bool Fixed = true) { EnumDeclBits.IsFixed = Fixed; }
 
+  SourceRange getEnumKeyRange() const { return EnumKeyRange; }
+
+  void setEnumKeyRange(SourceRange Range) { EnumKeyRange = Range; }
+
 private:
   /// True if a valid hash is stored in ODRHash.
   bool hasODRHash() const { return EnumDeclBits.HasODRHash; }

clang/lib/Parse/Parser.cpp

@@ -1100,30 +1100,25 @@ 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");
+    // Suggest correct location to fix '[[attrib]] struct' to 'struct
+    // [[attrib]]'
+    SourceLocation CorrectLocationForAttributes{};
+    TypeSpecifierType TKind = DS.getTypeSpecType();
+    if (DeclSpec::isDeclRep(TKind)) {
+      if (TKind == DeclSpec::TST_enum) {
+        if (const auto *ED = dyn_cast_or_null<EnumDecl>(DS.getRepAsDecl())) {
+          CorrectLocationForAttributes =
+              PP.getLocForEndOfToken(ED->getEnumKeyRange().getEnd());
+        }
       }
-
-    };
-    // Suggest correct location to fix '[[attrib]] struct' to 'struct [[attrib]]'
-    SourceLocation CorrectLocationForAttributes =
-        DeclSpec::isDeclRep(DS.getTypeSpecType())
-            ? DS.getTypeSpecTypeLoc().getLocWithOffset(
-                  LengthOfTSTToken(DS.getTypeSpecType()))
-            : SourceLocation();
+      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;

clang/lib/Sema/SemaDecl.cpp

@@ -18467,17 +18467,21 @@ Sema::ActOnTag(Scope *S, unsigned TagSpec, TagUseKind TUK, SourceLocation KWLoc,
                            cast_or_null<EnumDecl>(PrevDecl), ScopedEnum,
                            ScopedEnumUsesClassTag, IsFixed);
 
+    EnumDecl *ED = cast<EnumDecl>(New);
+    ED->setEnumKeyRange(SourceRange(
+        KWLoc, ScopedEnumKWLoc.isValid() ? ScopedEnumKWLoc : KWLoc));
+
     if (isStdAlignValT && (!StdAlignValT || getStdAlignValT()->isImplicit()))
       StdAlignValT = cast<EnumDecl>(New);
 
     // If this is an undefined enum, warn.
     if (TUK != TagUseKind::Definition && !Invalid) {
       TagDecl *Def;
-      if (IsFixed && cast<EnumDecl>(New)->isFixed()) {
+      if (IsFixed && ED->isFixed()) {
         // C++0x: 7.2p2: opaque-enum-declaration.
         // Conflicts are diagnosed above. Do nothing.
-      }
-      else if (PrevDecl && (Def = cast<EnumDecl>(PrevDecl)->getDefinition())) {
+      } else if (PrevDecl &&
+                 (Def = cast<EnumDecl>(PrevDecl)->getDefinition())) {
         Diag(Loc, diag::ext_forward_ref_enum_def)
           << New;
         Diag(Def->getLocation(), diag::note_previous_definition);

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

@@ -0,0 +1,48 @@
+// 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]]"
+
+[[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]]:19-[[@LINE-5]]:19}:"{{\[\[}}nodiscard]]"
+
+namespace NS {
+  enum class E5;
+}
+
+[[nodiscard]] enum class NS::E5 { };
+// expected-error@-1 {{misplaced attributes; expected attributes here}}
+// CHECK: {{^}}{{\[\[}}nodiscard]] enum class NS::E5 { };
+// CHECK: {{^}}~~~~~~~~~~~~~           ^
+// CHECK: fix-it:"{{.*}}":{[[@LINE-4]]:1-[[@LINE-4]]:15}:""
+// CHECK: fix-it:"{{.*}}":{[[@LINE-5]]:25-[[@LINE-5]]:25}:"{{\[\[}}nodiscard]]"