Fix the double space and double attribute printing of the final keyword. (#88600)

Fixes #56517.

Author: vgvassilev

clang/lib/AST/DeclPrinter.cpp

@@ -119,7 +119,7 @@ namespace {
     void printTemplateArguments(llvm::ArrayRef<TemplateArgumentLoc> Args,
                                 const TemplateParameterList *Params);
     enum class AttrPosAsWritten { Default = 0, Left, Right };
-    void
+    bool
     prettyPrintAttributes(const Decl *D,
                           AttrPosAsWritten Pos = AttrPosAsWritten::Default);
     void prettyPrintPragmas(Decl *D);
@@ -252,16 +252,19 @@ static DeclPrinter::AttrPosAsWritten getPosAsWritten(const Attr *A,
   return DeclPrinter::AttrPosAsWritten::Right;
 }
 
-void DeclPrinter::prettyPrintAttributes(const Decl *D,
+// returns true if an attribute was printed.
+bool DeclPrinter::prettyPrintAttributes(const Decl *D,
                                         AttrPosAsWritten Pos /*=Default*/) {
-  if (Policy.PolishForDeclaration)
-    return;
+  bool hasPrinted = false;
 
   if (D->hasAttrs()) {
     const AttrVec &Attrs = D->getAttrs();
     for (auto *A : Attrs) {
       if (A->isInherited() || A->isImplicit())
         continue;
+      // Print out the keyword attributes, they aren't regular attributes.
+      if (Policy.PolishForDeclaration && !A->isKeywordAttribute())
+        continue;
       switch (A->getKind()) {
 #define ATTR(X)
 #define PRAGMA_SPELLING_ATTR(X) case attr::X:
@@ -275,13 +278,15 @@ void DeclPrinter::prettyPrintAttributes(const Decl *D,
           if (Pos != AttrPosAsWritten::Left)
             Out << ' ';
           A->printPretty(Out, Policy);
+          hasPrinted = true;
           if (Pos == AttrPosAsWritten::Left)
             Out << ' ';
         }
         break;
       }
     }
   }
+  return hasPrinted;
 }
 
 void DeclPrinter::prettyPrintPragmas(Decl *D) {
@@ -1065,12 +1070,15 @@ void DeclPrinter::VisitCXXRecordDecl(CXXRecordDecl *D) {
   // FIXME: add printing of pragma attributes if required.
   if (!Policy.SuppressSpecifiers && D->isModulePrivate())
     Out << "__module_private__ ";
-  Out << D->getKindName();
 
-  prettyPrintAttributes(D);
+  Out << D->getKindName() << ' ';
 
-  if (D->getIdentifier()) {
+  // FIXME: Move before printing the decl kind to match the behavior of the
+  // attribute printing for variables and function where they are printed first.
+  if (prettyPrintAttributes(D, AttrPosAsWritten::Left))
     Out << ' ';
+
+  if (D->getIdentifier()) {
     if (auto *NNS = D->getQualifier())
       NNS->print(Out, Policy);
     Out << *D;
@@ -1087,16 +1095,13 @@ void DeclPrinter::VisitCXXRecordDecl(CXXRecordDecl *D) {
     }
   }
 
-  if (D->hasDefinition()) {
-    if (D->hasAttr<FinalAttr>()) {
-      Out << " final";
-    }
-  }
+  prettyPrintAttributes(D, AttrPosAsWritten::Right);
 
   if (D->isCompleteDefinition()) {
+    Out << ' ';
     // Print the base classes
     if (D->getNumBases()) {
-      Out << " : ";
+      Out << ": ";
       for (CXXRecordDecl::base_class_iterator Base = D->bases_begin(),
              BaseEnd = D->bases_end(); Base != BaseEnd; ++Base) {
         if (Base != D->bases_begin())
@@ -1115,14 +1120,15 @@ void DeclPrinter::VisitCXXRecordDecl(CXXRecordDecl *D) {
         if (Base->isPackExpansion())
           Out << "...";
       }
+      Out << ' ';
     }
 
     // Print the class definition
     // FIXME: Doesn't print access specifiers, e.g., "public:"
     if (Policy.TerseOutput) {
-      Out << " {}";
+      Out << "{}";
     } else {
-      Out << " {\n";
+      Out << "{\n";
       VisitDeclContext(D);
       Indent() << "}";
     }

clang/test/SemaCXX/cxx11-attr-print.cpp

@@ -87,3 +87,8 @@ template struct S<int>;
 
 // CHECK: using Small2 {{\[}}[gnu::mode(byte)]] = int;
 using Small2 [[gnu::mode(byte)]] = int;
+
+class FinalNonTemplate final {};
+// CHECK: class FinalNonTemplate final {
+template <typename T> class FinalTemplate final {};
+// CHECK: template <typename T> class FinalTemplate final {

clang/test/SemaHLSL/BuiltIns/RWBuffers.hlsl

@@ -6,11 +6,11 @@ typedef vector<float, 3> float3;
 RWBuffer<float3> Buffer;
 
 // expected-error@+2 {{class template 'RWBuffer' requires template arguments}}
-// expected-note@*:* {{template declaration from hidden source: template <class element_type> class RWBuffer final}}
+// expected-note@*:* {{template declaration from hidden source: template <class element_type> class RWBuffer}}
 RWBuffer BufferErr1;
 
 // expected-error@+2 {{too few template arguments for class template 'RWBuffer'}}
-// expected-note@*:* {{template declaration from hidden source: template <class element_type> class RWBuffer final}}
+// expected-note@*:* {{template declaration from hidden source: template <class element_type> class RWBuffer}}
 RWBuffer<> BufferErr2;
 
 [numthreads(1,1,1)]

clang/unittests/AST/DeclPrinterTest.cpp

@@ -86,16 +86,13 @@ ::testing::AssertionResult PrintedDeclCXX11Matches(StringRef Code,
                             ExpectedPrinted, "input.cc");
 }
 
-::testing::AssertionResult PrintedDeclCXX11Matches(
-                                  StringRef Code,
-                                  const DeclarationMatcher &NodeMatch,
-                                  StringRef ExpectedPrinted) {
+::testing::AssertionResult
+PrintedDeclCXX11Matches(StringRef Code, const DeclarationMatcher &NodeMatch,
+                        StringRef ExpectedPrinted,
+                        PrintingPolicyAdjuster PolicyModifier = nullptr) {
   std::vector<std::string> Args(1, "-std=c++11");
-  return PrintedDeclMatches(Code,
-                            Args,
-                            NodeMatch,
-                            ExpectedPrinted,
-                            "input.cc");
+  return PrintedDeclMatches(Code, Args, NodeMatch, ExpectedPrinted, "input.cc",
+                            PolicyModifier);
 }
 
 ::testing::AssertionResult PrintedDeclCXX11nonMSCMatches(
@@ -1555,3 +1552,25 @@ TEST(DeclPrinter, VarDeclWithInitializer) {
       PrintedDeclCXX17Matches("void foo() {int arr[42]; for(int a : arr);}",
                               namedDecl(hasName("a")).bind("id"), "int a"));
 }
+
+TEST(DeclPrinter, TestTemplateFinal) {
+  // By default we should print 'final' keyword whether class is implicitly or
+  // explicitly marked final.
+  ASSERT_TRUE(PrintedDeclCXX11Matches(
+      "template<typename T>\n"
+      "class FinalTemplate final {};",
+      classTemplateDecl(hasName("FinalTemplate")).bind("id"),
+      "template <typename T> class FinalTemplate final {}"));
+}
+
+TEST(DeclPrinter, TestTemplateFinalWithPolishForDecl) {
+  // clangd relies on the 'final' keyword being printed when
+  // PolishForDeclaration is enabled, so make sure it is even if implicit attrs
+  // are disabled.
+  ASSERT_TRUE(PrintedDeclCXX11Matches(
+      "template<typename T>\n"
+      "class FinalTemplate final {};",
+      classTemplateDecl(hasName("FinalTemplate")).bind("id"),
+      "template <typename T> class FinalTemplate final {}",
+      [](PrintingPolicy &Policy) { Policy.PolishForDeclaration = true; }));
+}

clang/utils/TableGen/ClangAttrEmitter.cpp

@@ -1608,7 +1608,7 @@ writePrettyPrintFunction(const Record &R,
       Prefix = "[";
       Suffix = "]";
     } else if (Variety == "Keyword") {
-      Prefix = " ";
+      Prefix = "";
       Suffix = "";
     } else if (Variety == "Pragma") {
       Prefix = "#pragma ";