Slightly change the logic of dealing with each type of attributes and add 
more test cases :)

  For C++11 attributes, they could be printed anywhere except at end of 
declaration; for GNU attributes, we enforce that these attributes only appear 
at end of declaration to prevent them printing out multiple times; for special 
declaration location like empty declaration, we don't check the print 
constraints so all types of attributes are printed.

  Richard, what's your thoughts on this approach? I think I understand the 
principals you mentioned in last review, but I don't know how to apply it to 
cases like these so both can be printed out correctly if we only allow C++11 
attributes to print on a single location:
  [[...]] int x;
  struct [[...]] s;

  Thanks!

Hi rsmith,

http://llvm-reviews.chandlerc.com/D395

CHANGE SINCE LAST DIFF
  http://llvm-reviews.chandlerc.com/D395?vs=1315&id=1362#toc

Files:
  test/SemaCXX/cxx11-attr-print.cpp
  include/clang/AST/Attr.h
  utils/TableGen/ClangAttrEmitter.cpp
  lib/AST/DeclPrinter.cpp
  lib/AST/StmtPrinter.cpp
Index: test/SemaCXX/cxx11-attr-print.cpp
===================================================================
--- test/SemaCXX/cxx11-attr-print.cpp
+++ test/SemaCXX/cxx11-attr-print.cpp
@@ -7,14 +7,14 @@
 // CHECK: int y __declspec(align(4));
 __declspec(align(4)) int y;
 
-// CHECK: int z {{\[}}[gnu::aligned(4)]];
+// CHECK: {{\[}}[gnu::aligned(4)]] int z;
 int z [[gnu::aligned(4)]];
 
 // CHECK: __attribute__((deprecated("warning")));
 int a __attribute__((deprecated("warning")));
 
-// CHECK: int b {{\[}}[gnu::deprecated("warning")]];
-int b [[gnu::deprecated("warning")]];
+// CHECK: {{\[}}[gnu::deprecated("warning")]] int b __attribute__((unused));
+int b [[gnu::deprecated("warning")]] __attribute__((unused));
 
 // CHECK: int cxx11_alignas alignas(4);
 alignas(4) int cxx11_alignas;
@@ -31,34 +31,35 @@
 // CHECK: int f1() __attribute__((warn_unused_result));
 int f1() __attribute__((warn_unused_result));
 
-// CHECK: {{\[}}[clang::warn_unused_result]];
+// CHECK: {{\[}}[clang::warn_unused_result]] int f2();
 int f2 [[clang::warn_unused_result]] ();
 
-// CHECK: {{\[}}[gnu::warn_unused_result]];
+// CHECK: {{\[}}[gnu::warn_unused_result]] int f3();
 int f3 [[gnu::warn_unused_result]] ();
 
-// FIXME: ast-print need to print C++11
-// attribute after function declare-id.
-// CHECK: {{\[}}[noreturn]];
+// CHECK: {{\[}}[noreturn]] void f4();
 void f4 [[noreturn]] ();
 
-// CHECK: {{\[}}[std::noreturn]];
+// CHECK: {{\[}}[std::noreturn]] void f5();
 void f5 [[std::noreturn]] ();
 
 // CHECK: __attribute__((gnu_inline));
 inline void f6() __attribute__((gnu_inline));
 
-// CHECK: {{\[}}[gnu::gnu_inline]];
+// CHECK: {{\[}}[gnu::gnu_inline]] inline void f7();
 inline void f7 [[gnu::gnu_inline]] ();
 
 // arguments printing
 // CHECK: __attribute__((format("printf", 2, 3)));
 void f8 (void *, const char *, ...) __attribute__ ((format (printf, 2, 3)));
 
+// CHECK: _Noreturn void f9();
+_Noreturn void f9();
+
 // CHECK: int m __attribute__((aligned(4
 // CHECK: int n alignas(4
 // CHECK: static int f() __attribute__((pure))
-// CHECK: static int g() {{\[}}[gnu::pure]]
+// CHECK: {{\[}}[gnu::pure]] static int g()
 template <typename T> struct S {
   __attribute__((aligned(4))) int m;
   alignas(4) int n;
@@ -73,5 +74,18 @@
 // CHECK: int m __attribute__((aligned(4
 // CHECK: int n alignas(4
 // CHECK: static int f() __attribute__((pure))
-// CHECK: static int g() {{\[}}[gnu::pure]]
+// CHECK: {{\[}}[gnu::pure]] static int g()
 template struct S<int>;
+
+// CHECK: {{\[}}[gnu::deprecated("")]] {{\[}}[gnu::pure]] void combo() __attribute__((always_inline));
+void combo [[gnu::pure]] [[gnu::deprecated("")]] () __attribute__((always_inline));
+
+// CHECK: enum  {{\[}}[gnu::unused]] enum_attr {
+enum [[gnu::unused]] enum_attr {} __attribute__((deprecated));
+
+// CHECK: class  {{\[}}[gnu::unused]] class_attr {
+class [[gnu::unused]] class_attr {} __attribute__((deprecated));
+
+// CHECK: {{\[}}[gnu::unused]] int foooo __attribute__((deprecated(""))) = 0;
+int foooo [[gnu::unused]] __attribute__((deprecated)) = 0;
+
Index: include/clang/AST/Attr.h
===================================================================
--- include/clang/AST/Attr.h
+++ include/clang/AST/Attr.h
@@ -62,6 +62,22 @@
   }
 
 public:
+  /// \brief Describes the kind of a syntax location where the attribute
+  /// could appear. It is used to print out the attribute at right location.
+  enum LocationKind {
+    /// \brief Attribute appears at the start of a declaration.
+    LK_StartOfDecl,
+    /// \brief Attribute appears at the end of a declaration.
+    LK_EndOfDecl,
+    /// \brief Attribute appears at an empty declaration.
+    LK_EmptyDecl,
+    /// \brief Attribute appears between class-key and class-head-name.
+    LK_ClassHead,
+    /// \brief Attribute appears at the start of a statement.
+    LK_StartOfStmt
+  };
+
+public:
   // Forward so that the regular new and delete do not hide global ones.
   void* operator new(size_t Bytes, ASTContext &C,
                      size_t Alignment = 16) throw() {
@@ -101,7 +117,8 @@
 
   // Pretty print this attribute.
   virtual void printPretty(raw_ostream &OS,
-                           const PrintingPolicy &Policy) const = 0;
+                           const PrintingPolicy &Policy,
+                           LocationKind LK) const = 0;
 };
 
 class InheritableAttr : public Attr {
Index: utils/TableGen/ClangAttrEmitter.cpp
===================================================================
--- utils/TableGen/ClangAttrEmitter.cpp
+++ utils/TableGen/ClangAttrEmitter.cpp
@@ -755,7 +755,8 @@
   std::vector<Record*> Spellings = R.getValueAsListOfDefs("Spellings");
 
   OS << "void " << R.getName() << "Attr::printPretty("
-    << "raw_ostream &OS, const PrintingPolicy &Policy) const {\n";
+    << "raw_ostream &OS, const PrintingPolicy &Policy, "
+    << "LocationKind LK) const {\n";
 
   if (Spellings.size() == 0) {
     OS << "}\n\n";
@@ -800,10 +801,26 @@
 
     Spelling += Name;
 
-    OS <<
-      "  case " << I << " : {\n"
-      "    OS << \"" + Prefix.str() + Spelling.str();
+    OS << "  case " << I << " : {\n";
 
+    // C++11 attributes should not be printed at end of a declaration,
+    // because in case of a function declaration, attributes at end of
+    // declaration appertain to function types, which we don't support now.
+    if (Variety == "CXX11")
+      OS << "    if (LK != LK_EndOfDecl) {\n";
+
+    // C11 _Noreturn attribute should be printed only at the start of
+    // a declaration.
+    if (Variety == "Keyword" && Name == "_Noreturn")
+      OS << "    if (LK == LK_StartOfDecl) {\n";
+
+    // GNU attributes appear either at end of declaration, or at an empty
+    // declaration.
+    if (Variety == "GNU")
+      OS << "    if (LK == LK_EndOfDecl || LK == LK_EmptyDecl) {\n";
+
+    OS << "        OS << \"" + Prefix.str() + Spelling.str();
+
     if (Args.size()) OS << "(";
     if (Spelling == "availability") {
       writeAvailabilityValue(OS);
@@ -817,7 +834,14 @@
 
     if (Args.size()) OS << ")";
     OS << Suffix.str() + "\";\n";
+    // Emit a space after printing the attribute unless the attribute is
+    // at end of a declaration.
+    OS << "        if (LK != LK_EndOfDecl) OS << \" \";\n";
 
+    if (Variety == "CXX11" || Variety == "GNU" ||
+        (Variety == "Keyword" && Name == "_Noreturn"))
+      OS << "    }\n";
+
     OS <<
       "    break;\n"
       "  }\n";
@@ -946,7 +970,8 @@
 
     OS << "  virtual " << R.getName() << "Attr *clone (ASTContext &C) const;\n";
     OS << "  virtual void printPretty(raw_ostream &OS,\n"
-       << "                           const PrintingPolicy &Policy) const;\n";
+       << "                           const PrintingPolicy &Policy,\n"
+       << "                           LocationKind LC) const;\n";
 
     writeAttrAccessorDefinition(R, OS);
 
Index: lib/AST/DeclPrinter.cpp
===================================================================
--- lib/AST/DeclPrinter.cpp
+++ lib/AST/DeclPrinter.cpp
@@ -85,7 +85,7 @@
 
     void PrintTemplateParameters(const TemplateParameterList *Params,
                                  const TemplateArgumentList *Args = 0);
-    void prettyPrintAttributes(Decl *D);
+    void prettyPrintAttributes(Decl *D, Attr::LocationKind LC);
   };
 }
 
@@ -183,7 +183,7 @@
   return Out;
 }
 
-void DeclPrinter::prettyPrintAttributes(Decl *D) {
+void DeclPrinter::prettyPrintAttributes(Decl *D, Attr::LocationKind LK) {
   if (Policy.PolishForDeclaration)
     return;
   
@@ -191,7 +191,7 @@
     AttrVec &Attrs = D->getAttrs();
     for (AttrVec::const_iterator i=Attrs.begin(), e=Attrs.end(); i!=e; ++i) {
       Attr *A = *i;
-      A->printPretty(Out, Policy);
+      A->printPretty(Out, Policy, LK);
     }
   }
 }
@@ -328,14 +328,15 @@
 }
 
 void DeclPrinter::VisitTypedefDecl(TypedefDecl *D) {
+  prettyPrintAttributes(D, Attr::LK_StartOfDecl);
   if (!Policy.SuppressSpecifiers) {
     Out << "typedef ";
-    
+
     if (D->isModulePrivate())
       Out << "__module_private__ ";
   }
   D->getUnderlyingType().print(Out, Policy, D->getName());
-  prettyPrintAttributes(D);
+  prettyPrintAttributes(D, Attr::LK_EndOfDecl);
 }
 
 void DeclPrinter::VisitTypeAliasDecl(TypeAliasDecl *D) {
@@ -352,6 +353,9 @@
     else
       Out << "struct ";
   }
+
+  prettyPrintAttributes(D, Attr::LK_ClassHead);
+
   Out << *D;
 
   if (D->isFixed())
@@ -362,21 +366,27 @@
     VisitDeclContext(D);
     Indent() << "}";
   }
-  prettyPrintAttributes(D);
+
+  prettyPrintAttributes(D, Attr::LK_EndOfDecl);
 }
 
 void DeclPrinter::VisitRecordDecl(RecordDecl *D) {
   if (!Policy.SuppressSpecifiers && D->isModulePrivate())
     Out << "__module_private__ ";
   Out << D->getKindName();
-  if (D->getIdentifier())
-    Out << ' ' << *D;
+  if (D->getIdentifier()) {
+    Out << ' ';
+    prettyPrintAttributes(D, Attr::LK_ClassHead);
+    Out << *D;
+  }
 
   if (D->isCompleteDefinition()) {
     Out << " {\n";
     VisitDeclContext(D);
     Indent() << "}";
   }
+
+  prettyPrintAttributes(D, Attr::LK_EndOfDecl);
 }
 
 void DeclPrinter::VisitEnumConstantDecl(EnumConstantDecl *D) {
@@ -388,6 +398,7 @@
 }
 
 void DeclPrinter::VisitFunctionDecl(FunctionDecl *D) {
+  prettyPrintAttributes(D, Attr::LK_StartOfDecl);
   CXXConstructorDecl *CDecl = dyn_cast<CXXConstructorDecl>(D);
   if (!Policy.SuppressSpecifiers) {
     switch (D->getStorageClassAsWritten()) {
@@ -554,7 +565,7 @@
     Ty.print(Out, Policy, Proto);
   }
 
-  prettyPrintAttributes(D);
+  prettyPrintAttributes(D, Attr::LK_EndOfDecl);
 
   if (D->isPure())
     Out << " = 0";
@@ -609,6 +620,7 @@
 }
 
 void DeclPrinter::VisitFieldDecl(FieldDecl *D) {
+  prettyPrintAttributes(D, Attr::LK_StartOfDecl);
   if (!Policy.SuppressSpecifiers && D->isMutable())
     Out << "mutable ";
   if (!Policy.SuppressSpecifiers && D->isModulePrivate())
@@ -621,6 +633,8 @@
     D->getBitWidth()->printPretty(Out, 0, Policy, Indentation);
   }
 
+  prettyPrintAttributes(D, Attr::LK_EndOfDecl);
+
   Expr *Init = D->getInClassInitializer();
   if (!Policy.SuppressInitializers && Init) {
     if (D->getInClassInitStyle() == ICIS_ListInit)
@@ -629,7 +643,6 @@
       Out << " = ";
     Init->printPretty(Out, 0, Policy, Indentation);
   }
-  prettyPrintAttributes(D);
 }
 
 void DeclPrinter::VisitLabelDecl(LabelDecl *D) {
@@ -638,6 +651,7 @@
 
 
 void DeclPrinter::VisitVarDecl(VarDecl *D) {
+  prettyPrintAttributes(D, Attr::LK_StartOfDecl);
   StorageClass SCAsWritten = D->getStorageClassAsWritten();
   if (!Policy.SuppressSpecifiers && SCAsWritten != SC_None)
     Out << VarDecl::getStorageClassSpecifierString(SCAsWritten) << " ";
@@ -651,6 +665,9 @@
   if (ParmVarDecl *Parm = dyn_cast<ParmVarDecl>(D))
     T = Parm->getOriginalType();
   T.print(Out, Policy, D->getName());
+
+  prettyPrintAttributes(D, Attr::LK_EndOfDecl);
+
   Expr *Init = D->getInit();
   if (!Policy.SuppressInitializers && Init) {
     bool ImplicitInit = false;
@@ -673,7 +690,6 @@
         Out << ")";
     }
   }
-  prettyPrintAttributes(D);
 }
 
 void DeclPrinter::VisitParmVarDecl(ParmVarDecl *D) {
@@ -725,15 +741,18 @@
 }
 
 void DeclPrinter::VisitEmptyDecl(EmptyDecl *D) {
-  prettyPrintAttributes(D);
+  prettyPrintAttributes(D, Attr::LK_EmptyDecl);
 }
 
 void DeclPrinter::VisitCXXRecordDecl(CXXRecordDecl *D) {
   if (!Policy.SuppressSpecifiers && D->isModulePrivate())
     Out << "__module_private__ ";
   Out << D->getKindName();
-  if (D->getIdentifier())
-    Out << ' ' << *D;
+  if (D->getIdentifier()) {
+    Out << ' ';
+    prettyPrintAttributes(D, Attr::LK_ClassHead);
+    Out << *D;
+  }
 
   if (D->isCompleteDefinition()) {
     // Print the base classes
@@ -762,6 +781,8 @@
     Out << " {\n";
     VisitDeclContext(D);
     Indent() << "}";
+
+    prettyPrintAttributes(D, Attr::LK_EndOfDecl);
   }
 }
 
Index: lib/AST/StmtPrinter.cpp
===================================================================
--- lib/AST/StmtPrinter.cpp
+++ lib/AST/StmtPrinter.cpp
@@ -182,7 +182,7 @@
       first = false;
     }
     // TODO: check this
-    (*it)->printPretty(OS, Policy);
+    (*it)->printPretty(OS, Policy, Attr::LK_StartOfStmt);
   }
   OS << "]] ";
   PrintStmt(Node->getSubStmt(), 0);
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

Reply via email to