Add -Wcustom-atomic-properties which warns if an atomic-by-default property has custom getter or setter.

The rationale is that it is highly likely that the user's getter/setter isn't atomically implemented. Off by default.
Addresses rdar://8782645.

-Wcustom-atomic-properties and -Wimplicit-atomic-properties are under the -Watomic-properties group.

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

4 files changed, 47 insertions, 4 deletions

diff --git a/include/clang/Basic/DiagnosticGroups.td b/include/clang/Basic/DiagnosticGroups.td
index a0e73639c1..f5fa924491 100644
--- a/include/clang/Basic/DiagnosticGroups.td
+++ b/include/clang/Basic/DiagnosticGroups.td
@@ -151,6 +151,9 @@ def ReadOnlySetterAttrs : DiagGroup<"readonly-setter-attrs">;
 def Reorder : DiagGroup<"reorder">;
 def UndeclaredSelector : DiagGroup<"undeclared-selector">;
 def ImplicitAtomic : DiagGroup<"implicit-atomic-properties">;
+def CustomAtomic : DiagGroup<"custom-atomic-properties">;
+def AtomicProperties : DiagGroup<"atomic-properties",
+                                 [ImplicitAtomic, CustomAtomic]>;
 def Selector : DiagGroup<"selector">;
 def NonfragileAbi2 : DiagGroup<"nonfragile-abi2">;
 def Protocol : DiagGroup<"protocol">;
diff --git a/include/clang/Basic/DiagnosticSemaKinds.td b/include/clang/Basic/DiagnosticSemaKinds.td
index f55120722c..fd2f69b549 100644
--- a/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/include/clang/Basic/DiagnosticSemaKinds.td
@@ -386,6 +386,10 @@ def warn_objc_property_copy_missing_on_block : Warning<
 def warn_atomic_property_rule : Warning<
   "writable atomic property %0 cannot pair a synthesized setter/getter "
   "with a user defined setter/getter">;
+def warn_default_atomic_custom_getter_setter : Warning<
+  "atomic by default property %0 has a user defined setter/getter "
+  "(property should be marked 'atomic' if this is intended)">,
+  InGroup<CustomAtomic>, DefaultIgnore;
 def err_use_continuation_class : Error<
   "illegal redeclaration of property in continuation class %0"
   " (attribute must be 'readwrite', while its primary must be 'readonly')">;
diff --git a/lib/Sema/SemaObjCProperty.cpp b/lib/Sema/SemaObjCProperty.cpp
index 88ad4d7527..2f5be47806 100644
--- a/lib/Sema/SemaObjCProperty.cpp
+++ b/lib/Sema/SemaObjCProperty.cpp
@@ -1081,7 +1081,32 @@ Sema::AtomicPropertySetterGetterRules (ObjCImplDecl* IMPDecl,
        E = IDecl->prop_end();
        I != E; ++I) {
     ObjCPropertyDecl *Property = (*I);
+    ObjCMethodDecl *GetterMethod = 0;
+    ObjCMethodDecl *SetterMethod = 0;
+    bool LookedUpGetterSetter = false;
+
     unsigned Attributes = Property->getPropertyAttributes();
+    unsigned AttributesAsWrittern = Property->getPropertyAttributesAsWritten();
+
+    if (!(AttributesAsWrittern & ObjCPropertyDecl::OBJC_PR_atomic) &&
+        !(AttributesAsWrittern & ObjCPropertyDecl::OBJC_PR_nonatomic)) {
+      GetterMethod = IMPDecl->getInstanceMethod(Property->getGetterName());
+      SetterMethod = IMPDecl->getInstanceMethod(Property->getSetterName());
+      LookedUpGetterSetter = true;
+      if (GetterMethod) {
+        Diag(GetterMethod->getLocation(),
+             diag::warn_default_atomic_custom_getter_setter)
+          << Property->getIdentifier();
+        Diag(Property->getLocation(), diag::note_property_declare);
+      }
+      if (SetterMethod) {
+        Diag(SetterMethod->getLocation(),
+             diag::warn_default_atomic_custom_getter_setter)
+          << Property->getIdentifier();
+        Diag(Property->getLocation(), diag::note_property_declare);
+      }
+    }
+
     // We only care about readwrite atomic property.
     if ((Attributes & ObjCPropertyDecl::OBJC_PR_nonatomic) ||
         !(Attributes & ObjCPropertyDecl::OBJC_PR_readwrite))
@@ -1090,10 +1115,11 @@ Sema::AtomicPropertySetterGetterRules (ObjCImplDecl* IMPDecl,
          = IMPDecl->FindPropertyImplDecl(Property->getIdentifier())) {
       if (PIDecl->getPropertyImplementation() == ObjCPropertyImplDecl::Dynamic)
         continue;
-      ObjCMethodDecl *GetterMethod =
-        IMPDecl->getInstanceMethod(Property->getGetterName());
-      ObjCMethodDecl *SetterMethod =
-        IMPDecl->getInstanceMethod(Property->getSetterName());
+      if (!LookedUpGetterSetter) {
+        GetterMethod = IMPDecl->getInstanceMethod(Property->getGetterName());
+        SetterMethod = IMPDecl->getInstanceMethod(Property->getSetterName());
+        LookedUpGetterSetter = true;
+      }
       if ((GetterMethod && !SetterMethod) || (!GetterMethod && SetterMethod)) {
         SourceLocation MethodLoc =
           (GetterMethod ? GetterMethod->getLocation()
diff --git a/test/SemaObjC/custom-atomic-property.m b/test/SemaObjC/custom-atomic-property.m
new file mode 100644
index 0000000000..cf3d473849
--- /dev/null
+++ b/test/SemaObjC/custom-atomic-property.m
@@ -0,0 +1,10 @@
+// RUN: %clang_cc1  -fsyntax-only -Wcustom-atomic-properties -verify %s
+
+@interface Foo
+@property (assign) Foo *myProp; // expected-note {{property declared here}} expected-note {{property declared here}}
+@end
+
+@implementation Foo
+ -(Foo*)myProp {return 0;} // expected-warning {{atomic by default property 'myProp' has a user defined setter/getter (property should be marked 'atomic' if this is intended)}}
+ -(void)setMyProp:(Foo*)e {} // expected-warning {{atomic by default property 'myProp' has a user defined setter/getter (property should be marked 'atomic' if this is intended)}}
+@end