Revert "Rework the printing of attributes (#87281)"

This reverts commit a30662f due to bot failures.

Author: vgvassilev

clang/include/clang/Basic/Attr.td

@@ -324,10 +324,13 @@ class Spelling<string name, string variety, int version = 1> {
 }
 
 class GNU<string name> : Spelling<name, "GNU">;
-class Declspec<string name> : Spelling<name, "Declspec">;
+class Declspec<string name> : Spelling<name, "Declspec"> {
+  bit PrintOnLeft = 1;
+}
 class Microsoft<string name> : Spelling<name, "Microsoft">;
 class CXX11<string namespace, string name, int version = 1>
     : Spelling<name, "CXX11", version> {
+  bit CanPrintOnLeft = 0;
   string Namespace = namespace;
 }
 class C23<string namespace, string name, int version = 1>
@@ -593,6 +596,12 @@ class AttrSubjectMatcherAggregateRule<AttrSubject subject> {
 def SubjectMatcherForNamed : AttrSubjectMatcherAggregateRule<Named>;
 
 class Attr {
+  // Specifies that when printed, this attribute is meaningful on the
+  // 'left side' of the declaration.
+  bit CanPrintOnLeft = 1;
+  // Specifies that when printed, this attribute is required to be printed on
+  // the 'left side' of the declaration.
+  bit PrintOnLeft = 0;
   // The various ways in which an attribute can be spelled in source
   list<Spelling> Spellings;
   // The things to which an attribute can appertain
@@ -928,6 +937,7 @@ def AVRSignal : InheritableAttr, TargetSpecificAttr<TargetAVR> {
 }
 
 def AsmLabel : InheritableAttr {
+  let CanPrintOnLeft = 0;
   let Spellings = [CustomKeyword<"asm">, CustomKeyword<"__asm__">];
   let Args = [
     // Label specifies the mangled name for the decl.
@@ -1524,6 +1534,7 @@ def AllocSize : InheritableAttr {
 }
 
 def EnableIf : InheritableAttr {
+  let CanPrintOnLeft = 0;
   // Does not have a [[]] spelling because this attribute requires the ability
   // to parse function arguments but the attribute is not written in the type
   // position.
@@ -3160,6 +3171,7 @@ def Unavailable : InheritableAttr {
 }
 
 def DiagnoseIf : InheritableAttr {
+  let CanPrintOnLeft = 0;
   // Does not have a [[]] spelling because this attribute requires the ability
   // to parse function arguments but the attribute is not written in the type
   // position.

clang/include/clang/Basic/CMakeLists.txt

@@ -31,6 +31,16 @@ clang_tablegen(AttrList.inc -gen-clang-attr-list
   SOURCE Attr.td
   TARGET ClangAttrList)
 
+clang_tablegen(AttrLeftSideCanPrintList.inc -gen-clang-attr-can-print-left-list
+  -I ${CMAKE_CURRENT_SOURCE_DIR}/../../
+  SOURCE Attr.td
+  TARGET ClangAttrCanPrintLeftList)
+
+clang_tablegen(AttrLeftSideMustPrintList.inc -gen-clang-attr-must-print-left-list
+  -I ${CMAKE_CURRENT_SOURCE_DIR}/../../
+  SOURCE Attr.td
+  TARGET ClangAttrMustPrintLeftList)
+
 clang_tablegen(AttrSubMatchRulesList.inc -gen-clang-attr-subject-match-rule-list
   -I ${CMAKE_CURRENT_SOURCE_DIR}/../../
   SOURCE Attr.td

clang/lib/AST/DeclPrinter.cpp

@@ -21,7 +21,6 @@
 #include "clang/AST/ExprCXX.h"
 #include "clang/AST/PrettyPrinter.h"
 #include "clang/Basic/Module.h"
-#include "clang/Basic/SourceManager.h"
 #include "llvm/Support/raw_ostream.h"
 using namespace clang;
 
@@ -50,6 +49,18 @@ namespace {
 
     void PrintObjCTypeParams(ObjCTypeParamList *Params);
 
+    enum class AttrPrintLoc {
+      None = 0,
+      Left = 1,
+      Right = 2,
+      Any = Left | Right,
+
+      LLVM_MARK_AS_BITMASK_ENUM(/*DefaultValue=*/Any)
+    };
+
+    void prettyPrintAttributes(Decl *D, raw_ostream &out,
+                               AttrPrintLoc loc = AttrPrintLoc::Any);
+
   public:
     DeclPrinter(raw_ostream &Out, const PrintingPolicy &Policy,
                 const ASTContext &Context, unsigned Indentation = 0,
@@ -118,10 +129,11 @@ namespace {
                                 const TemplateParameterList *Params);
     void printTemplateArguments(llvm::ArrayRef<TemplateArgumentLoc> Args,
                                 const TemplateParameterList *Params);
-    enum class AttrPosAsWritten { Unknown = 0, Default, Left, Right };
-    void
-    prettyPrintAttributes(const Decl *D,
-                          AttrPosAsWritten Pos = AttrPosAsWritten::Default);
+
+    inline void prettyPrintAttributes(Decl *D) {
+      prettyPrintAttributes(D, Out);
+    }
+
     void prettyPrintPragmas(Decl *D);
     void printDeclType(QualType T, StringRef DeclName, bool Pack = false);
   };
@@ -238,53 +250,87 @@ raw_ostream& DeclPrinter::Indent(unsigned Indentation) {
   return Out;
 }
 
-static DeclPrinter::AttrPosAsWritten getPosAsWritten(const Attr *A,
-                                                     const Decl *D) {
-  SourceLocation ALoc = A->getLoc();
-  SourceLocation DLoc = D->getLocation();
-  const ASTContext &C = D->getASTContext();
-  if (ALoc.isInvalid() || DLoc.isInvalid())
-    return DeclPrinter::AttrPosAsWritten::Unknown;
+// For CLANG_ATTR_LIST_CanPrintOnLeft macro.
+#include "clang/Basic/AttrLeftSideCanPrintList.inc"
 
-  if (C.getSourceManager().isBeforeInTranslationUnit(ALoc, DLoc))
-    return DeclPrinter::AttrPosAsWritten::Left;
+// For CLANG_ATTR_LIST_PrintOnLeft macro.
+#include "clang/Basic/AttrLeftSideMustPrintList.inc"
 
-  return DeclPrinter::AttrPosAsWritten::Right;
+static bool canPrintOnLeftSide(attr::Kind kind) {
+#ifdef CLANG_ATTR_LIST_CanPrintOnLeft
+  switch (kind) {
+  CLANG_ATTR_LIST_CanPrintOnLeft
+    return true;
+  default:
+    return false;
+  }
+#else
+  return false;
+#endif
+}
+
+static bool canPrintOnLeftSide(const Attr *A) {
+  if (A->isStandardAttributeSyntax())
+    return false;
+
+  return canPrintOnLeftSide(A->getKind());
 }
 
-void DeclPrinter::prettyPrintAttributes(const Decl *D,
-                                        AttrPosAsWritten Pos /*=Default*/) {
+static bool mustPrintOnLeftSide(attr::Kind kind) {
+#ifdef CLANG_ATTR_LIST_PrintOnLeft
+  switch (kind) {
+  CLANG_ATTR_LIST_PrintOnLeft
+    return true;
+  default:
+    return false;
+  }
+#else
+  return false;
+#endif
+}
+
+static bool mustPrintOnLeftSide(const Attr *A) {
+  if (A->isDeclspecAttribute())
+    return true;
+
+  return mustPrintOnLeftSide(A->getKind());
+}
+
+void DeclPrinter::prettyPrintAttributes(Decl *D, llvm::raw_ostream &Out,
+                                        AttrPrintLoc Loc) {
   if (Policy.PolishForDeclaration)
     return;
 
   if (D->hasAttrs()) {
-    assert(Pos != AttrPosAsWritten::Unknown && "Use Default");
-    const AttrVec &Attrs = D->getAttrs();
+    AttrVec &Attrs = D->getAttrs();
     for (auto *A : Attrs) {
       if (A->isInherited() || A->isImplicit())
         continue;
-      switch (A->getKind()) {
-#define ATTR(X)
-#define PRAGMA_SPELLING_ATTR(X) case attr::X:
-#include "clang/Basic/AttrList.inc"
-        break;
-      default:
-        AttrPosAsWritten APos = getPosAsWritten(A, D);
-        // Might trigger on programatically created attributes or declarations
-        // with no source locations.
-        assert(APos != AttrPosAsWritten::Unknown &&
-               "Invalid source location for attribute or decl.");
-        assert(APos != AttrPosAsWritten::Default &&
-               "Default not a valid for an attribute location");
-        if (Pos == AttrPosAsWritten::Default || Pos == APos) {
-          if (Pos != AttrPosAsWritten::Left)
-            Out << ' ';
-          A->printPretty(Out, Policy);
-          if (Pos == AttrPosAsWritten::Left)
-            Out << ' ';
-        }
-        break;
+
+      AttrPrintLoc AttrLoc = AttrPrintLoc::Right;
+      if (mustPrintOnLeftSide(A)) {
+        // If we must always print on left side (e.g. declspec), then mark as
+        // so.
+        AttrLoc = AttrPrintLoc::Left;
+      } else if (canPrintOnLeftSide(A)) {
+        // For functions with body defined we print the attributes on the left
+        // side so that GCC accept our dumps as well.
+        if (const FunctionDecl *FD = dyn_cast<FunctionDecl>(D);
+            FD && FD->isThisDeclarationADefinition())
+          // In case Decl is a function with a body, then attrs should be print
+          // on the left side.
+          AttrLoc = AttrPrintLoc::Left;
+
+          // In case it is a variable declaration with a ctor, then allow
+          // printing on the left side for readbility.
+        else if (const VarDecl *VD = dyn_cast<VarDecl>(D);
+                   VD && VD->getInit() &&
+                   VD->getInitStyle() == VarDecl::CallInit)
+          AttrLoc = AttrPrintLoc::Left;
       }
+      // Only print the side matches the user requested.
+      if ((Loc & AttrLoc) != AttrPrintLoc::None)
+        A->printPretty(Out, Policy);
     }
   }
 }
@@ -645,10 +691,8 @@ static void MaybePrintTagKeywordIfSupressingScopes(PrintingPolicy &Policy,
 
 void DeclPrinter::VisitFunctionDecl(FunctionDecl *D) {
   if (!D->getDescribedFunctionTemplate() &&
-      !D->isFunctionTemplateSpecialization()) {
+      !D->isFunctionTemplateSpecialization())
     prettyPrintPragmas(D);
-    prettyPrintAttributes(D, AttrPosAsWritten::Left);
-  }
 
   if (D->isFunctionTemplateSpecialization())
     Out << "template<> ";
@@ -658,6 +702,22 @@ void DeclPrinter::VisitFunctionDecl(FunctionDecl *D) {
       printTemplateParameters(D->getTemplateParameterList(I));
   }
 
+  std::string LeftsideAttrs;
+  llvm::raw_string_ostream LSAS(LeftsideAttrs);
+
+  prettyPrintAttributes(D, LSAS, AttrPrintLoc::Left);
+
+  // prettyPrintAttributes print a space on left side of the attribute.
+  if (LeftsideAttrs[0] == ' ') {
+    // Skip the space prettyPrintAttributes generated.
+    LeftsideAttrs.erase(0, LeftsideAttrs.find_first_not_of(' '));
+
+    // Add a single space between the attribute and the Decl name.
+    LSAS << ' ';
+  }
+
+  Out << LeftsideAttrs;
+
   CXXConstructorDecl *CDecl = dyn_cast<CXXConstructorDecl>(D);
   CXXConversionDecl *ConversionDecl = dyn_cast<CXXConversionDecl>(D);
   CXXDeductionGuideDecl *GuideDecl = dyn_cast<CXXDeductionGuideDecl>(D);
@@ -823,7 +883,7 @@ void DeclPrinter::VisitFunctionDecl(FunctionDecl *D) {
     Ty.print(Out, Policy, Proto);
   }
 
-  prettyPrintAttributes(D, AttrPosAsWritten::Right);
+  prettyPrintAttributes(D, Out, AttrPrintLoc::Right);
 
   if (D->isPureVirtual())
     Out << " = 0";
@@ -916,12 +976,27 @@ void DeclPrinter::VisitLabelDecl(LabelDecl *D) {
 void DeclPrinter::VisitVarDecl(VarDecl *D) {
   prettyPrintPragmas(D);
 
-  prettyPrintAttributes(D, AttrPosAsWritten::Left);
-
   if (const auto *Param = dyn_cast<ParmVarDecl>(D);
       Param && Param->isExplicitObjectParameter())
     Out << "this ";
 
+  std::string LeftSide;
+  llvm::raw_string_ostream LeftSideStream(LeftSide);
+
+  // Print attributes that should be placed on the left, such as __declspec.
+  prettyPrintAttributes(D, LeftSideStream, AttrPrintLoc::Left);
+
+  // prettyPrintAttributes print a space on left side of the attribute.
+  if (LeftSide[0] == ' ') {
+    // Skip the space prettyPrintAttributes generated.
+    LeftSide.erase(0, LeftSide.find_first_not_of(' '));
+
+    // Add a single space between the attribute and the Decl name.
+    LeftSideStream << ' ';
+  }
+
+  Out << LeftSide;
+
   QualType T = D->getTypeSourceInfo()
     ? D->getTypeSourceInfo()->getType()
     : D->getASTContext().getUnqualifiedObjCPointerType(D->getType());
@@ -954,16 +1029,21 @@ void DeclPrinter::VisitVarDecl(VarDecl *D) {
     }
   }
 
+  StringRef Name;
+
+  Name = (isa<ParmVarDecl>(D) && Policy.CleanUglifiedParameters &&
+          D->getIdentifier())
+             ? D->getIdentifier()->deuglifiedName()
+             : D->getName();
+
   if (!Policy.SuppressTagKeyword && Policy.SuppressScope &&
       !Policy.SuppressUnwrittenScope)
     MaybePrintTagKeywordIfSupressingScopes(Policy, T, Out);
+  printDeclType(T, Name);
 
-  printDeclType(T, (isa<ParmVarDecl>(D) && Policy.CleanUglifiedParameters &&
-                    D->getIdentifier())
-                       ? D->getIdentifier()->deuglifiedName()
-                       : D->getName());
-
-  prettyPrintAttributes(D, AttrPosAsWritten::Right);
+  // Print the attributes that should be placed right before the end of the
+  // decl.
+  prettyPrintAttributes(D, Out, AttrPrintLoc::Right);
 
   Expr *Init = D->getInit();
   if (!Policy.SuppressInitializers && Init) {

clang/lib/AST/StmtPrinter.cpp

@@ -292,11 +292,8 @@ void StmtPrinter::VisitLabelStmt(LabelStmt *Node) {
 }
 
 void StmtPrinter::VisitAttributedStmt(AttributedStmt *Node) {
-  llvm::ArrayRef<const Attr *> Attrs = Node->getAttrs();
-  for (const auto *Attr : Attrs) {
+  for (const auto *Attr : Node->getAttrs()) {
     Attr->printPretty(OS, Policy);
-    if (Attr != Attrs.back())
-      OS << ' ';
   }
 
   PrintStmt(Node->getSubStmt(), 0);

clang/test/AST/ast-print-method-decl.cpp

@@ -94,7 +94,7 @@ struct DefMethodsWithoutBody {
   // CHECK-NEXT: DefMethodsWithoutBody() = default;
   ~DefMethodsWithoutBody() = default;
 
-  // CHECK-NEXT: void m1() __attribute__((alias("X")));
+  // CHECK-NEXT: __attribute__((alias("X"))) void m1();
   void m1() __attribute__((alias("X")));
 
   // CHECK-NEXT: };

clang/test/AST/ast-print-no-sanitize.cpp

@@ -4,4 +4,4 @@ void should_not_crash_1() __attribute__((no_sanitize_memory));
 [[clang::no_sanitize_memory]] void should_not_crash_2();
 
 // CHECK: void should_not_crash_1() __attribute__((no_sanitize("memory")));
-// CHECK: {{\[\[}}clang::no_sanitize("memory"){{\]\]}} void should_not_crash_2();
+// CHECK: void should_not_crash_2() {{\[\[}}clang::no_sanitize("memory"){{\]\]}};

clang/test/AST/attr-print-emit.cpp

@@ -73,18 +73,3 @@ class C {
   // CHECK: void pwtt(void *, int) __attribute__((pointer_with_type_tag(foo, 2, 3)));
   void pwtt(void *, int) __attribute__((pointer_with_type_tag(foo, 2, 3)));
 };
-
-#define ANNOTATE_ATTR __attribute__((annotate("Annotated")))
-ANNOTATE_ATTR int annotated_attr ANNOTATE_ATTR = 0;
-// CHECK: __attribute__((annotate("Annotated"))) int annotated_attr __attribute__((annotate("Annotated"))) = 0;
-
-// FIXME: We do not print the attribute as written after the type specifier.
-int ANNOTATE_ATTR annotated_attr_fixme = 0;
-// CHECK: __attribute__((annotate("Annotated"))) int annotated_attr_fixme = 0;
-
-#define NONNULL_ATTR  __attribute__((nonnull(1)))
-ANNOTATE_ATTR NONNULL_ATTR void fn_non_null_annotated_attr(int *) __attribute__((annotate("AnnotatedRHS")));
-// CHECK:__attribute__((annotate("Annotated"))) __attribute__((nonnull(1))) void fn_non_null_annotated_attr(int *) __attribute__((annotate("AnnotatedRHS")));
-
-[[gnu::nonnull(1)]] [[gnu::always_inline]] void cxx11_attr(int*) ANNOTATE_ATTR;
-// CHECK: {{\[\[}}gnu::nonnull(1)]] {{\[\[}}gnu::always_inline]] void cxx11_attr(int *) __attribute__((annotate("Annotated")));

clang/test/Analysis/scopes-cfg-output.cpp

@@ -1469,7 +1469,7 @@ void test_cleanup_functions2(int m) {
 // CHECK:       [B1]
 // CHECK-NEXT:    1: CFGScopeBegin(f)
 // CHECK-NEXT:    2:  (CXXConstructExpr, [B1.3], F)
-// CHECK-NEXT:    3: F f __attribute__((cleanup(cleanup_F)));
+// CHECK-NEXT:    3: __attribute__((cleanup(cleanup_F))) F f;
 // CHECK-NEXT:    4: CleanupFunction (cleanup_F)
 // CHECK-NEXT:    5: [B1.3].~F() (Implicit destructor)
 // CHECK-NEXT:    6: CFGScopeEnd(f)