Clean up ObjCPropertyDecl printing

Summary:
- `@property(attr, attr2)` instead of `@property ( attr,attr2 )`.
- Change priority of attributes (see code/comments inline).
- Support for printing weak and unsafe_unretained attributes.

Subscribers: arphaman, jfb, cfe-commits

Tags: #clang

Differential Revision: https://reviews.llvm.org/D57965

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@357937 91177308-0d34-0410-b5e6-96231b3b80d8

5 files changed, 80 insertions, 41 deletions

diff --git a/lib/AST/DeclPrinter.cpp b/lib/AST/DeclPrinter.cpp
index 2009673069..c98ec3b85d 100644
--- a/lib/AST/DeclPrinter.cpp
+++ b/lib/AST/DeclPrinter.cpp
@@ -1391,6 +1391,13 @@ void DeclPrinter::VisitObjCCompatibleAliasDecl(ObjCCompatibleAliasDecl *AID) {
 
 /// PrintObjCPropertyDecl - print a property declaration.
 ///
+/// Print attributes in the following order:
+/// - class
+/// - nonatomic | atomic
+/// - assign | retain | strong | copy | weak | unsafe_unretained
+/// - readwrite | readonly
+/// - getter & setter
+/// - nullability
 void DeclPrinter::VisitObjCPropertyDecl(ObjCPropertyDecl *PDecl) {
   if (PDecl->getPropertyImplementation() == ObjCPropertyDecl::Required)
     Out << "@required\n";
@@ -1402,58 +1409,69 @@ void DeclPrinter::VisitObjCPropertyDecl(ObjCPropertyDecl *PDecl) {
   Out << "@property";
   if (PDecl->getPropertyAttributes() != ObjCPropertyDecl::OBJC_PR_noattr) {
     bool first = true;
-    Out << " (";
-    if (PDecl->getPropertyAttributes() &
-        ObjCPropertyDecl::OBJC_PR_readonly) {
-      Out << (first ? ' ' : ',') << "readonly";
+    Out << "(";
+    if (PDecl->getPropertyAttributes() & ObjCPropertyDecl::OBJC_PR_class) {
+      Out << (first ? "" : ", ") << "class";
       first = false;
     }
 
-    if (PDecl->getPropertyAttributes() & ObjCPropertyDecl::OBJC_PR_getter) {
-      Out << (first ? ' ' : ',') << "getter = ";
-      PDecl->getGetterName().print(Out);
+    if (PDecl->getPropertyAttributes() &
+        ObjCPropertyDecl::OBJC_PR_nonatomic) {
+      Out << (first ? "" : ", ") << "nonatomic";
       first = false;
     }
-    if (PDecl->getPropertyAttributes() & ObjCPropertyDecl::OBJC_PR_setter) {
-      Out << (first ? ' ' : ',') << "setter = ";
-      PDecl->getSetterName().print(Out);
+    if (PDecl->getPropertyAttributes() &
+        ObjCPropertyDecl::OBJC_PR_atomic) {
+      Out << (first ? "" : ", ") << "atomic";
       first = false;
     }
 
     if (PDecl->getPropertyAttributes() & ObjCPropertyDecl::OBJC_PR_assign) {
-      Out << (first ? ' ' : ',') << "assign";
-      first = false;
-    }
-
-    if (PDecl->getPropertyAttributes() &
-        ObjCPropertyDecl::OBJC_PR_readwrite) {
-      Out << (first ? ' ' : ',') << "readwrite";
+      Out << (first ? "" : ", ") << "assign";
       first = false;
     }
-
     if (PDecl->getPropertyAttributes() & ObjCPropertyDecl::OBJC_PR_retain) {
-      Out << (first ? ' ' : ',') << "retain";
+      Out << (first ? "" : ", ") << "retain";
       first = false;
     }
 
     if (PDecl->getPropertyAttributes() & ObjCPropertyDecl::OBJC_PR_strong) {
-      Out << (first ? ' ' : ',') << "strong";
+      Out << (first ? "" : ", ") << "strong";
       first = false;
     }
-
     if (PDecl->getPropertyAttributes() & ObjCPropertyDecl::OBJC_PR_copy) {
-      Out << (first ? ' ' : ',') << "copy";
+      Out << (first ? "" : ", ") << "copy";
+      first = false;
+    }
+    if (PDecl->getPropertyAttributes() & ObjCPropertyDecl::OBJC_PR_weak) {
+      Out << (first ? "" : ", ") << "weak";
+      first = false;
+    }
+    if (PDecl->getPropertyAttributes()
+        & ObjCPropertyDecl::OBJC_PR_unsafe_unretained) {
+      Out << (first ? "" : ", ") << "unsafe_unretained";
       first = false;
     }
 
     if (PDecl->getPropertyAttributes() &
-        ObjCPropertyDecl::OBJC_PR_nonatomic) {
-      Out << (first ? ' ' : ',') << "nonatomic";
+        ObjCPropertyDecl::OBJC_PR_readwrite) {
+      Out << (first ? "" : ", ") << "readwrite";
       first = false;
     }
     if (PDecl->getPropertyAttributes() &
-        ObjCPropertyDecl::OBJC_PR_atomic) {
-      Out << (first ? ' ' : ',') << "atomic";
+        ObjCPropertyDecl::OBJC_PR_readonly) {
+      Out << (first ? "" : ", ") << "readonly";
+      first = false;
+    }
+
+    if (PDecl->getPropertyAttributes() & ObjCPropertyDecl::OBJC_PR_getter) {
+      Out << (first ? "" : ", ") << "getter = ";
+      PDecl->getGetterName().print(Out);
+      first = false;
+    }
+    if (PDecl->getPropertyAttributes() & ObjCPropertyDecl::OBJC_PR_setter) {
+      Out << (first ? "" : ", ") << "setter = ";
+      PDecl->getSetterName().print(Out);
       first = false;
     }
 
@@ -1463,25 +1481,24 @@ void DeclPrinter::VisitObjCPropertyDecl(ObjCPropertyDecl *PDecl) {
         if (*nullability == NullabilityKind::Unspecified &&
             (PDecl->getPropertyAttributes() &
                ObjCPropertyDecl::OBJC_PR_null_resettable)) {
-          Out << (first ? ' ' : ',') << "null_resettable";
+          Out << (first ? "" : ", ") << "null_resettable";
         } else {
-          Out << (first ? ' ' : ',')
+          Out << (first ? "" : ", ")
               << getNullabilitySpelling(*nullability, true);
         }
         first = false;
       }
     }
 
-    if (PDecl->getPropertyAttributes() & ObjCPropertyDecl::OBJC_PR_class) {
-      Out << (first ? ' ' : ',') << "class";
-      first = false;
-    }
-
     (void) first; // Silence dead store warning due to idiomatic code.
-    Out << " )";
+    Out << ")";
   }
-  Out << ' ' << PDecl->getASTContext().getUnqualifiedObjCPointerType(T).
-                  getAsString(Policy) << ' ' << *PDecl;
+  std::string TypeStr = PDecl->getASTContext().getUnqualifiedObjCPointerType(T).
+      getAsString(Policy);
+  Out << ' ' << TypeStr;
+  if (!StringRef(TypeStr).endswith("*"))
+    Out << ' ';
+  Out << *PDecl;
   if (Policy.PolishForDeclaration)
     Out << ';';
 }
diff --git a/test/AST/ast-print-objc-property.m b/test/AST/ast-print-objc-property.m
new file mode 100644
index 0000000000..5a2c8207bf
--- /dev/null
+++ b/test/AST/ast-print-objc-property.m
@@ -0,0 +1,22 @@
+// RUN: %clang_cc1 -ast-print %s -o - | FileCheck %s
+
+@interface NSObject
+@end
+
+@interface Properties : NSObject
+@property(class) int classFoo;
+@property(nonatomic) int atomicBar;
+@property(readonly) int readonlyConstant;
+@property(retain, nonatomic, setter=my_setter:, getter=my_getter) id                   __crazy_name;
+@property(nonatomic, strong, nullable) NSObject *                   objProperty;
+@property(nonatomic, weak, null_resettable) NSObject *   weakObj;
+@property(nonatomic, copy, nonnull) NSObject * copyObj;
+@end
+
+// CHECK: @property(class, atomic, assign, unsafe_unretained, readwrite) int classFoo;
+// CHECK: @property(nonatomic, assign, unsafe_unretained, readwrite) int atomicBar;
+// CHECK: @property(atomic, readonly) int readonlyConstant;
+// CHECK: @property(nonatomic, retain, readwrite, getter = my_getter, setter = my_setter:) id __crazy_name;
+// CHECK: @property(nonatomic, strong, readwrite, nullable) NSObject *objProperty;
+// CHECK: @property(nonatomic, weak, readwrite, null_resettable) NSObject *weakObj;
+// CHECK: @property(nonatomic, copy, readwrite, nonnull) NSObject *copyObj;
diff --git a/test/Index/comment-objc-decls.m b/test/Index/comment-objc-decls.m
index d53757cbc3..c93ad44a05 100644
--- a/test/Index/comment-objc-decls.m
+++ b/test/Index/comment-objc-decls.m
@@ -32,7 +32,7 @@
 @end
 // CHECK: <Declaration>@protocol MyProto\n@end</Declaration>
 // CHECK: <Declaration>- (unsigned int)MethodMyProto:(nullable id)anObject inRange:(unsigned int)range;</Declaration>
-// CHECK: <Declaration>@optional\n@property(readwrite, copy, atomic, nonnull) id PropertyMyProto;</Declaration>
+// CHECK: <Declaration>@optional\n@property(atomic, copy, readwrite, nonnull) id PropertyMyProto;</Declaration>
 // CHECK: <Declaration>+ (id)ClassMethodMyProto;</Declaration>
 
 /**
@@ -77,7 +77,7 @@
 // CHECK: <Declaration>id IvarMyClass</Declaration>
 // CHECK: <Declaration>- (id)MethodMyClass;</Declaration>
 // CHECK: <Declaration>+ (id)ClassMethodMyClass;</Declaration>
-// CHECK: <Declaration>@property(readwrite, copy, atomic) id PropertyMyClass;</Declaration
+// CHECK: <Declaration>@property(atomic, copy, readwrite) id PropertyMyClass;</Declaration
 
 /**
  * \brief - This is class extension of MyClass
@@ -110,7 +110,7 @@
 @end
 // CHECK: <Declaration>@interface MyClass (Category)\n@end</Declaration>
 // CHECK: <Declaration>- (void)MethodMyClassCategory;</Declaration>
-// CHECK: <Declaration>@property(readwrite, copy, atomic) id PropertyMyClassCategory;</Declaration>
+// CHECK: <Declaration>@property(atomic, copy, readwrite) id PropertyMyClassCategory;</Declaration>
 // CHECK: <Declaration>- (id)PropertyMyClassCategory;</Declaration>
 // CHECK: <Declaration>- (void)setPropertyMyClassCategory:(id)arg;</Declaration>
 
diff --git a/test/Index/comment-unqualified-objc-pointer.m b/test/Index/comment-unqualified-objc-pointer.m
index e9e1ceee23..cf297ef855 100644
--- a/test/Index/comment-unqualified-objc-pointer.m
+++ b/test/Index/comment-unqualified-objc-pointer.m
@@ -19,7 +19,7 @@
 
 //! This is a property to get the Name.
 @property (copy) NSString *Name;
-// CHECK: <Declaration>@property(readwrite, copy, atomic) NSString *Name;</Declaration>
+// CHECK: <Declaration>@property(atomic, copy, readwrite) NSString *Name;</Declaration>
 @end
 
 @implementation NSMutableArray
diff --git a/test/PCH/chain-remap-types.m b/test/PCH/chain-remap-types.m
index 13f2e39b14..e151a64750 100644
--- a/test/PCH/chain-remap-types.m
+++ b/test/PCH/chain-remap-types.m
@@ -6,7 +6,7 @@
 
 // CHECK: @class X;
 // CHECK: struct Y 
-// CHECK: @property ( assign,readwrite,atomic ) X * prop
+// CHECK: @property(atomic, assign, unsafe_unretained, readwrite) X *prop
 // CHECK: void h(X *);
 // CHECK: @interface X(Blah)
 // CHECK: void g(X *);