Patch to warn if a property which is 'assign' by default

may not implement NSCopying protocol in -fobjc-gc[-only] mode.


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

include/clang/AST/ASTContext.h

@@ -557,6 +557,9 @@ public:
                                       const Decl *Container,
                                       std::string &S);
   
+  bool ProtocolCompatibleWithProtocol(ObjCProtocolDecl *lProto,
+                                      ObjCProtocolDecl *rProto);
+  
   /// getObjCEncodingTypeSize returns size of type for objective-c encoding
   /// purpose.
   int getObjCEncodingTypeSize(QualType t);

include/clang/AST/DeclObjC.h

@@ -530,6 +530,13 @@ public:
   bool isImplicitInterfaceDecl() const { return InternalInterface; }
   void setImplicitInterfaceDecl(bool val) { InternalInterface = val; }
   
+  /// ClassImplementsProtocol - Checks that 'lProto' protocol
+  /// has been implemented in IDecl class, its super class or categories (if
+  /// lookupCategory is true).
+  bool ClassImplementsProtocol(ObjCProtocolDecl *lProto,
+                               bool lookupCategory,
+                               bool RHSIsQualifiedID = false);
+  
   // Low-level accessor
   Type *getTypeForDecl() const { return TypeForDecl; }
   void setTypeForDecl(Type *TD) const { TypeForDecl = TD; }

include/clang/Basic/DiagnosticSemaKinds.td

@@ -221,6 +221,10 @@ def warn_conflicting_ret_types : Warning<
 def warn_conflicting_param_types : Warning<
   "conflicting parameter types in implementation of %0: %1 vs %2">;
 
+def warn_implements_nscopying : Warning<
+"default assign attribute on property %0 which implements "
+"NSCopying protocol is not appropriate with -fobjc-gc[-only]">;
+
 def warn_multiple_method_decl : Warning<"multiple methods named %0 found">;
 def warn_accessor_property_type_mismatch : Warning<
   "type of property %0 does not match type of accessor %1">;

lib/AST/ASTContext.cpp

@@ -3290,7 +3290,7 @@ static bool areCompatVectorTypes(const VectorType *LHS,
 
 /// ProtocolCompatibleWithProtocol - return 'true' if 'lProto' is in the
 /// inheritance hierarchy of 'rProto'.
-static bool ProtocolCompatibleWithProtocol(ObjCProtocolDecl *lProto,
+bool ASTContext::ProtocolCompatibleWithProtocol(ObjCProtocolDecl *lProto,
                                                 ObjCProtocolDecl *rProto) {
   if (lProto == rProto)
     return true;
@@ -3301,51 +3301,6 @@ static bool ProtocolCompatibleWithProtocol(ObjCProtocolDecl *lProto,
   return false;
 }
 
-/// ClassImplementsProtocol - Checks that 'lProto' protocol
-/// has been implemented in IDecl class, its super class or categories (if
-/// lookupCategory is true). 
-static bool ClassImplementsProtocol(ObjCProtocolDecl *lProto,
-                                    ObjCInterfaceDecl *IDecl, 
-                                    bool lookupCategory,
-                                    bool RHSIsQualifiedID = false) {
-  
-  // 1st, look up the class.
-  const ObjCList<ObjCProtocolDecl> &Protocols =
-    IDecl->getReferencedProtocols();
-
-  for (ObjCList<ObjCProtocolDecl>::iterator PI = Protocols.begin(),
-       E = Protocols.end(); PI != E; ++PI) {
-    if (ProtocolCompatibleWithProtocol(lProto, *PI))
-      return true;
-    // This is dubious and is added to be compatible with gcc.  In gcc, it is
-    // also allowed assigning a protocol-qualified 'id' type to a LHS object
-    // when protocol in qualified LHS is in list of protocols in the rhs 'id'
-    // object. This IMO, should be a bug.
-    // FIXME: Treat this as an extension, and flag this as an error when GCC
-    // extensions are not enabled.
-    if (RHSIsQualifiedID && ProtocolCompatibleWithProtocol(*PI, lProto))
-      return true;
-  }
-  
-  // 2nd, look up the category.
-  if (lookupCategory)
-    for (ObjCCategoryDecl *CDecl = IDecl->getCategoryList(); CDecl;
-         CDecl = CDecl->getNextClassCategory()) {
-      for (ObjCCategoryDecl::protocol_iterator PI = CDecl->protocol_begin(),
-           E = CDecl->protocol_end(); PI != E; ++PI)
-        if (ProtocolCompatibleWithProtocol(lProto, *PI))
-          return true;
-    }
-  
-  // 3rd, look up the super class(s)
-  if (IDecl->getSuperClass())
-    return 
-      ClassImplementsProtocol(lProto, IDecl->getSuperClass(), lookupCategory,
-                              RHSIsQualifiedID);
-  
-  return false;
-}
-
 /// QualifiedIdConformsQualifiedId - compare id<p,...> with id<p1,...>
 /// return true if lhs's protocols conform to rhs's protocol; false
 /// otherwise.
@@ -3381,7 +3336,7 @@ bool ASTContext::ObjCQualifiedIdTypesAreCompatible(QualType lhs, QualType rhs,
           // when comparing an id<P> on lhs with a static type on rhs,
           // see if static class implements all of id's protocols, directly or
           // through its super class and categories.
-          if (!ClassImplementsProtocol(*I, rhsID, true))
+          if (!rhsID->ClassImplementsProtocol(*I, true))
             return false;
         }
       }
@@ -3414,7 +3369,7 @@ bool ASTContext::ObjCQualifiedIdTypesAreCompatible(QualType lhs, QualType rhs,
           // when comparing an id<P> on lhs with a static type on rhs,
           // see if static class implements all of id's protocols, directly or
           // through its super class and categories.
-          if (ClassImplementsProtocol(*I, rhsID, true)) {
+          if (rhsID->ClassImplementsProtocol(*I, true)) {
             match = true;
             break;
           }
@@ -3440,7 +3395,7 @@ bool ASTContext::ObjCQualifiedIdTypesAreCompatible(QualType lhs, QualType rhs,
           // when comparing an id<P> on lhs with a static type on rhs,
           // see if static class implements all of id's protocols, directly or
           // through its super class and categories.
-          if (ClassImplementsProtocol(*I, lhsID, true)) {
+          if (lhsID->ClassImplementsProtocol(*I, true)) {
             match = true;
             break;
           }

lib/AST/DeclObjC.cpp

@@ -398,6 +398,51 @@ ObjCMethodDecl *ObjCInterfaceDecl::getCategoryClassMethod(Selector Sel) const {
   return 0;
 }
 
+/// ClassImplementsProtocol - Checks that 'lProto' protocol
+/// has been implemented in IDecl class, its super class or categories (if
+/// lookupCategory is true).
+bool ObjCInterfaceDecl::ClassImplementsProtocol(ObjCProtocolDecl *lProto,
+                                    bool lookupCategory,
+                                    bool RHSIsQualifiedID) {
+  ObjCInterfaceDecl *IDecl = this;
+  // 1st, look up the class.
+  const ObjCList<ObjCProtocolDecl> &Protocols =
+  IDecl->getReferencedProtocols();
+  
+  for (ObjCList<ObjCProtocolDecl>::iterator PI = Protocols.begin(),
+       E = Protocols.end(); PI != E; ++PI) {
+    if (getASTContext().ProtocolCompatibleWithProtocol(lProto, *PI))
+      return true;
+    // This is dubious and is added to be compatible with gcc.  In gcc, it is
+    // also allowed assigning a protocol-qualified 'id' type to a LHS object
+    // when protocol in qualified LHS is in list of protocols in the rhs 'id'
+    // object. This IMO, should be a bug.
+    // FIXME: Treat this as an extension, and flag this as an error when GCC
+    // extensions are not enabled.
+    if (RHSIsQualifiedID && 
+        getASTContext().ProtocolCompatibleWithProtocol(*PI, lProto))
+      return true;
+  }
+  
+  // 2nd, look up the category.
+  if (lookupCategory)
+    for (ObjCCategoryDecl *CDecl = IDecl->getCategoryList(); CDecl;
+         CDecl = CDecl->getNextClassCategory()) {
+      for (ObjCCategoryDecl::protocol_iterator PI = CDecl->protocol_begin(),
+           E = CDecl->protocol_end(); PI != E; ++PI)
+        if (getASTContext().ProtocolCompatibleWithProtocol(lProto, *PI))
+          return true;
+    }
+  
+  // 3rd, look up the super class(s)
+  if (IDecl->getSuperClass())
+    return
+  IDecl->getSuperClass()->ClassImplementsProtocol(lProto, lookupCategory,
+                                                  RHSIsQualifiedID);
+  
+  return false;
+}
+
 //===----------------------------------------------------------------------===//
 // ObjCIvarDecl
 //===----------------------------------------------------------------------===//

lib/Sema/SemaDeclObjC.cpp

@@ -1925,6 +1925,22 @@ Sema::DeclPtrTy Sema::ActOnProperty(Scope *S, SourceLocation AtLoc,
       } 
     }
   
+  // Issue a warning if property is 'assign' as default and its object, which is
+  // gc'able conforms to NSCopying protocol 
+  if (getLangOptions().getGCMode() != LangOptions::NonGC &&
+      isAssign && !(Attributes & ObjCDeclSpec::DQ_PR_assign))
+      if (T->isObjCObjectPointerType()) {
+        QualType InterfaceTy = T->getPointeeType();
+        ObjCInterfaceDecl *IDecl= 
+          InterfaceTy->getAsObjCInterfaceType()->getDecl();
+        if (IDecl)
+          if (ObjCProtocolDecl* PNSCopying = 
+                LookupProtocol(&Context.Idents.get("NSCopying")))
+            if (IDecl->ClassImplementsProtocol(PNSCopying, true))
+              Diag(AtLoc, diag::warn_implements_nscopying)  
+                << FD.D.getIdentifier();
+      }
+  
   DeclContext *DC = dyn_cast<DeclContext>(ClassDecl);
   assert(DC && "ClassDecl is not a DeclContext");
   ObjCPropertyDecl *PDecl = ObjCPropertyDecl::Create(Context, DC,

test/SemaObjC/warn-assign-property-nscopying.m

@@ -0,0 +1,14 @@
+// RUN: clang-cc  -fobjc-gc -fsyntax-only -verify %s
+
+@protocol NSCopying @end
+
+@interface NSObject <NSCopying>
+@end
+
+@interface NSDictionary : NSObject
+@end
+
+@interface INTF
+  @property NSDictionary* undoAction;  // expected-warning {{no 'assign', 'retain', or 'copy' attribute is specified - 'assign' is assumed}} // expected-warning {{default assign attribute on property 'undoAction' which implements NSCopying protocol is not appropriate with}}
+@end
+