]> granicus.if.org Git - clang/commitdiff
objective-c: warn if implementation of a method in category
authorFariborz Jahanian <fjahanian@apple.com>
Thu, 28 Jul 2011 23:19:50 +0000 (23:19 +0000)
committerFariborz Jahanian <fjahanian@apple.com>
Thu, 28 Jul 2011 23:19:50 +0000 (23:19 +0000)
masks an existing method in its primary class, class extensions,
and primary class's non-optional protocol methods; as primary
class, or one of its subclass's will implement this method.
This warning has potential of being noisy so it has its own
group.  // rdar://7020493

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

include/clang/Basic/DiagnosticGroups.td
include/clang/Basic/DiagnosticSemaKinds.td
include/clang/Sema/Sema.h
lib/Sema/SemaDeclObjC.cpp
test/SemaObjC/arc.m
test/SemaObjC/incomplete-implementation.m
test/SemaObjC/protocol-implementing-class-methods.m [new file with mode: 0644]
test/SemaObjC/related-result-type-inference.m

index df5bc1a043a22bd3048b478718907f7ff76ae8d4..af31baf6279b29ba4060a31ac70063c8ad81b9fb 100644 (file)
@@ -302,3 +302,4 @@ def Microsoft : DiagGroup<"microsoft">;
 
 def ObjCNonUnifiedException : DiagGroup<"objc-nonunified-exceptions">;
 
+def ObjCProtocolMethodImpl : DiagGroup<"objc-protocol-method-implementation">;
index 660528a2c14697c8dc95226b774018f371b0f607..7f660e1a279f96bf8588272c5841d5e17069662b 100644 (file)
@@ -409,6 +409,9 @@ def warn_non_contravariant_param_types : Warning<
 def warn_conflicting_variadic :Warning<
   "conflicting variadic declaration of method and its "
   "%select{implementation|declaration}0">;
+def warn_category_method_impl_match:Warning<
+  "category is implementing a method which will also be implemented"
+  " by its primary class">, InGroup<ObjCProtocolMethodImpl>;
 
 def warn_implements_nscopying : Warning<
 "default assign attribute on property %0 which implements "
index 81d93115e383cc98e1da6636b0950c23bddc5bb8..cbb096d9c5a99bd751a2fe97f2670a70ac8e11b4 100644 (file)
@@ -1775,6 +1775,12 @@ public:
                                    bool IsProtocolMethodDecl,
                                    bool IsDeclaration = false);
 
+  /// WarnExactTypedMethods - This routine issues a warning if method
+  /// implementation declaration matches exactly that of its declaration.
+  void WarnExactTypedMethods(ObjCMethodDecl *Method,
+                             ObjCMethodDecl *MethodDecl,
+                             bool IsProtocolMethodDecl);
+
   bool isPropertyReadonly(ObjCPropertyDecl *PropertyDecl,
                           ObjCInterfaceDecl *IDecl);
 
@@ -1894,13 +1900,19 @@ public:
                                   ObjCImplDecl* IMPDecl,
                                   ObjCContainerDecl* IDecl,
                                   bool &IncompleteImpl,
-                                  bool ImmediateClass);
+                                  bool ImmediateClass,
+                                  bool WarnExactMatch=false);
 
   /// MatchMethodsInClassAndItsProtocol - Check that any redeclaration of
   /// method in protocol in its qualified class match in their type and
   /// issue warnings otherwise.
   void MatchMethodsInClassAndItsProtocol(const ObjCInterfaceDecl *CDecl);
 
+  /// CheckCategoryVsClassMethodMatches - Checks that methods implemented in
+  /// category matches with those implemented in its primary class and
+  /// warns each time an exact match is found. 
+  void CheckCategoryVsClassMethodMatches(ObjCCategoryImplDecl *CatIMP);
+
 private:
   /// AddMethodToGlobalPool - Add an instance or factory method to the global
   /// pool. See descriptoin of AddInstanceMethodToGlobalPool.
index b91a81372ff7e43dd7ad27725e6d6dac43855143..0dc2279e37fd47c5607f2b1f71aba738e8c50578 100644 (file)
@@ -1071,25 +1071,32 @@ static SourceRange getTypeRange(TypeSourceInfo *TSI) {
   return (TSI ? TSI->getTypeLoc().getSourceRange() : SourceRange());
 }
 
-static void CheckMethodOverrideReturn(Sema &S,
+static bool CheckMethodOverrideReturn(Sema &S,
                                       ObjCMethodDecl *MethodImpl,
                                       ObjCMethodDecl *MethodDecl,
                                       bool IsProtocolMethodDecl,
-                                      bool IsDeclaration) {
+                                      bool IsDeclaration,
+                                      bool Warn) {
   if (IsProtocolMethodDecl &&
       (MethodDecl->getObjCDeclQualifier() !=
        MethodImpl->getObjCDeclQualifier())) {
-    S.Diag(MethodImpl->getLocation(), 
-           diag::warn_conflicting_ret_type_modifiers)
-        << MethodImpl->getDeclName() << IsDeclaration
-        << getTypeRange(MethodImpl->getResultTypeSourceInfo());
-    S.Diag(MethodDecl->getLocation(), diag::note_previous_declaration)
-        << getTypeRange(MethodDecl->getResultTypeSourceInfo());
+    if (Warn) {
+      S.Diag(MethodImpl->getLocation(), 
+             diag::warn_conflicting_ret_type_modifiers)
+          << MethodImpl->getDeclName() << IsDeclaration
+          << getTypeRange(MethodImpl->getResultTypeSourceInfo());
+        S.Diag(MethodDecl->getLocation(), diag::note_previous_declaration)
+          << getTypeRange(MethodDecl->getResultTypeSourceInfo());
+    }
+    else
+      return false;
   }
   
   if (S.Context.hasSameUnqualifiedType(MethodImpl->getResultType(),
                                        MethodDecl->getResultType()))
-    return;
+    return true;
+  if (!Warn)
+    return false;
 
   unsigned DiagID = diag::warn_conflicting_ret_types;
 
@@ -1104,7 +1111,7 @@ static void CheckMethodOverrideReturn(Sema &S,
       // return types that are subclasses of the declared return type,
       // or that are more-qualified versions of the declared type.
       if (isObjCTypeSubstitutable(S.Context, IfacePtrTy, ImplPtrTy, false))
-        return;
+        return false;
 
       DiagID = diag::warn_non_covariant_ret_types;
     }
@@ -1118,32 +1125,40 @@ static void CheckMethodOverrideReturn(Sema &S,
     << getTypeRange(MethodImpl->getResultTypeSourceInfo());
   S.Diag(MethodDecl->getLocation(), diag::note_previous_definition)
     << getTypeRange(MethodDecl->getResultTypeSourceInfo());
+  return false;
 }
 
-static void CheckMethodOverrideParam(Sema &S,
+static bool CheckMethodOverrideParam(Sema &S,
                                      ObjCMethodDecl *MethodImpl,
                                      ObjCMethodDecl *MethodDecl,
                                      ParmVarDecl *ImplVar,
                                      ParmVarDecl *IfaceVar,
                                      bool IsProtocolMethodDecl,
-                                     bool IsDeclaration) {
+                                     bool IsDeclaration,
+                                     bool Warn) {
   if (IsProtocolMethodDecl &&
       (ImplVar->getObjCDeclQualifier() !=
        IfaceVar->getObjCDeclQualifier())) {
-    S.Diag(ImplVar->getLocation(), 
-           diag::warn_conflicting_param_modifiers)
-        << getTypeRange(ImplVar->getTypeSourceInfo())
-        << MethodImpl->getDeclName() << IsDeclaration;
-    S.Diag(IfaceVar->getLocation(), diag::note_previous_declaration)
-        << getTypeRange(IfaceVar->getTypeSourceInfo());   
+    if (Warn) {
+      S.Diag(ImplVar->getLocation(), 
+             diag::warn_conflicting_param_modifiers)
+          << getTypeRange(ImplVar->getTypeSourceInfo())
+          << MethodImpl->getDeclName() << IsDeclaration;
+      S.Diag(IfaceVar->getLocation(), diag::note_previous_declaration)
+          << getTypeRange(IfaceVar->getTypeSourceInfo());   
+    }
+    else
+      return false;
   }
       
   QualType ImplTy = ImplVar->getType();
   QualType IfaceTy = IfaceVar->getType();
   
   if (S.Context.hasSameUnqualifiedType(ImplTy, IfaceTy))
-    return;
-
+    return true;
+  
+  if (!Warn)
+    return false;
   unsigned DiagID = diag::warn_conflicting_param_types;
 
   // Mismatches between ObjC pointers go into a different warning
@@ -1157,7 +1172,7 @@ static void CheckMethodOverrideParam(Sema &S,
       // implementation must accept any objects that the superclass
       // accepts, however it may also accept others.
       if (isObjCTypeSubstitutable(S.Context, ImplPtrTy, IfacePtrTy, true))
-        return;
+        return false;
 
       DiagID = diag::warn_non_contravariant_param_types;
     }
@@ -1169,6 +1184,7 @@ static void CheckMethodOverrideParam(Sema &S,
     << IsDeclaration;
   S.Diag(IfaceVar->getLocation(), diag::note_previous_definition)
     << getTypeRange(IfaceVar->getTypeSourceInfo());
+  return false;
 }
 
 /// In ARC, check whether the conventional meanings of the two methods
@@ -1250,13 +1266,13 @@ void Sema::WarnConflictingTypedMethods(ObjCMethodDecl *ImpMethodDecl,
     return;
 
   CheckMethodOverrideReturn(*this, ImpMethodDecl, MethodDecl, 
-                            IsProtocolMethodDecl, IsDeclaration);
+                            IsProtocolMethodDecl, IsDeclaration, true);
 
   for (ObjCMethodDecl::param_iterator IM = ImpMethodDecl->param_begin(),
        IF = MethodDecl->param_begin(), EM = ImpMethodDecl->param_end();
        IM != EM; ++IM, ++IF)
     CheckMethodOverrideParam(*this, ImpMethodDecl, MethodDecl, *IM, *IF,
-                             IsProtocolMethodDecl, IsDeclaration);
+                             IsProtocolMethodDecl, IsDeclaration, true);
 
   if (ImpMethodDecl->isVariadic() != MethodDecl->isVariadic()) {
     Diag(ImpMethodDecl->getLocation(), diag::warn_conflicting_variadic)
@@ -1265,6 +1281,44 @@ void Sema::WarnConflictingTypedMethods(ObjCMethodDecl *ImpMethodDecl,
   }
 }
 
+/// WarnExactTypedMethods - This routine issues a warning if method
+/// implementation declaration matches exactly that of its declaration.
+void Sema::WarnExactTypedMethods(ObjCMethodDecl *ImpMethodDecl,
+                                 ObjCMethodDecl *MethodDecl,
+                                 bool IsProtocolMethodDecl) {
+  // don't issue warning when protocol method is optional because primary
+  // class is not required to implement it and it is safe for protocol
+  // to implement it.
+  if (MethodDecl->getImplementationControl() == ObjCMethodDecl::Optional)
+    return;
+  // don't issue warning when primary class's method is 
+  // depecated/unavailable.
+  if (MethodDecl->hasAttr<UnavailableAttr>() ||
+      MethodDecl->hasAttr<DeprecatedAttr>())
+    return;
+  
+  bool match = CheckMethodOverrideReturn(*this, ImpMethodDecl, MethodDecl, 
+                                      IsProtocolMethodDecl, false, false);
+  if (match)
+    for (ObjCMethodDecl::param_iterator IM = ImpMethodDecl->param_begin(),
+         IF = MethodDecl->param_begin(), EM = ImpMethodDecl->param_end();
+         IM != EM; ++IM, ++IF) {
+      match = CheckMethodOverrideParam(*this, ImpMethodDecl, MethodDecl, 
+                                       *IM, *IF,
+                                       IsProtocolMethodDecl, false, false);
+      if (!match)
+        break;
+    }
+  if (match)
+    match = (ImpMethodDecl->isVariadic() == MethodDecl->isVariadic());
+  
+  if (match) {
+    Diag(ImpMethodDecl->getLocation(), 
+         diag::warn_category_method_impl_match);
+    Diag(MethodDecl->getLocation(), diag::note_method_declared_at);
+  }
+}
+
 /// FIXME: Type hierarchies in Objective-C can be deep. We could most likely
 /// improve the efficiency of selector lookups and type checking by associating
 /// with each protocol / interface / category the flattened instance tables. If
@@ -1368,7 +1422,8 @@ void Sema::MatchAllMethodDeclarations(const llvm::DenseSet<Selector> &InsMap,
                                       ObjCImplDecl* IMPDecl,
                                       ObjCContainerDecl* CDecl,
                                       bool &IncompleteImpl,
-                                      bool ImmediateClass) {
+                                      bool ImmediateClass,
+                                      bool WarnExactMatch) {
   // Check and see if instance methods in class interface have been
   // implemented in the implementation class. If so, their types match.
   for (ObjCInterfaceDecl::instmeth_iterator I = CDecl->instmeth_begin(),
@@ -1390,9 +1445,14 @@ void Sema::MatchAllMethodDeclarations(const llvm::DenseSet<Selector> &InsMap,
       assert(MethodDecl &&
              "MethodDecl is null in ImplMethodsVsClassMethods");
       // ImpMethodDecl may be null as in a @dynamic property.
-      if (ImpMethodDecl)
-        WarnConflictingTypedMethods(ImpMethodDecl, MethodDecl,
-                                    isa<ObjCProtocolDecl>(CDecl));
+      if (ImpMethodDecl) {
+        if (!WarnExactMatch)
+          WarnConflictingTypedMethods(ImpMethodDecl, MethodDecl,
+                                      isa<ObjCProtocolDecl>(CDecl));
+        else
+          WarnExactTypedMethods(ImpMethodDecl, MethodDecl,
+                               isa<ObjCProtocolDecl>(CDecl));
+      }
     }
   }
 
@@ -1412,8 +1472,12 @@ void Sema::MatchAllMethodDeclarations(const llvm::DenseSet<Selector> &InsMap,
         IMPDecl->getClassMethod((*I)->getSelector());
       ObjCMethodDecl *MethodDecl =
         CDecl->getClassMethod((*I)->getSelector());
-      WarnConflictingTypedMethods(ImpMethodDecl, MethodDecl, 
-                                  isa<ObjCProtocolDecl>(CDecl));
+      if (!WarnExactMatch)
+        WarnConflictingTypedMethods(ImpMethodDecl, MethodDecl, 
+                                    isa<ObjCProtocolDecl>(CDecl));
+      else
+        WarnExactTypedMethods(ImpMethodDecl, MethodDecl,
+                             isa<ObjCProtocolDecl>(CDecl));
     }
   }
   
@@ -1424,7 +1488,7 @@ void Sema::MatchAllMethodDeclarations(const llvm::DenseSet<Selector> &InsMap,
       MatchAllMethodDeclarations(InsMap, ClsMap, InsMapSeen, ClsMapSeen,
                                  IMPDecl,
                                  const_cast<ObjCCategoryDecl *>(ClsExtDecl), 
-                                 IncompleteImpl, false);
+                                 IncompleteImpl, false, WarnExactMatch);
     
     // Check for any implementation of a methods declared in protocol.
     for (ObjCInterfaceDecl::all_protocol_iterator
@@ -1432,13 +1496,17 @@ void Sema::MatchAllMethodDeclarations(const llvm::DenseSet<Selector> &InsMap,
           E = I->all_referenced_protocol_end(); PI != E; ++PI)
       MatchAllMethodDeclarations(InsMap, ClsMap, InsMapSeen, ClsMapSeen,
                                  IMPDecl,
-                                 (*PI), IncompleteImpl, false);
+                                 (*PI), IncompleteImpl, false, WarnExactMatch);
     
     // Check for any type mismtch of methods declared in class 
-    // and methods declared in protocol.
-    MatchMethodsInClassAndItsProtocol(I);
+    // and methods declared in protocol. Do this only when the class
+    // is being implementaed.
+    if (isa<ObjCImplementationDecl>(IMPDecl))
+      MatchMethodsInClassAndItsProtocol(I);
     
-    if (I->getSuperClass())
+    // FIXME. For now, we are not checking for extact match of methods 
+    // in category implementation and its primary class's super class. 
+    if (!WarnExactMatch && I->getSuperClass())
       MatchAllMethodDeclarations(InsMap, ClsMap, InsMapSeen, ClsMapSeen,
                                  IMPDecl,
                                  I->getSuperClass(), IncompleteImpl, false);
@@ -1490,6 +1558,9 @@ static void MatchMethodsInClassAndOneProtocol(Sema &S,
     MatchMethodsInClassAndOneProtocol(S, InsMap, ClsMap, IDecl, (*PI));
 }
 
+/// MatchMethodsInClassAndItsProtocol - Check that any redeclaration of
+/// method in protocol in its qualified class match in their type and
+/// issue warnings otherwise.
 void Sema::MatchMethodsInClassAndItsProtocol(const ObjCInterfaceDecl *CDecl) {
   if (CDecl->all_referenced_protocol_begin() ==
       CDecl->all_referenced_protocol_end())
@@ -1537,6 +1608,38 @@ void Sema::MatchMethodsInClassAndItsProtocol(const ObjCInterfaceDecl *CDecl) {
   }
 }
 
+/// CheckCategoryVsClassMethodMatches - Checks that methods implemented in
+/// category matches with those implemented in its primary class and
+/// warns each time an exact match is found. 
+void Sema::CheckCategoryVsClassMethodMatches(
+                                  ObjCCategoryImplDecl *CatIMPDecl) {
+  llvm::DenseSet<Selector> InsMap, ClsMap;
+  
+  for (ObjCImplementationDecl::instmeth_iterator
+       I = CatIMPDecl->instmeth_begin(), 
+       E = CatIMPDecl->instmeth_end(); I!=E; ++I)
+    InsMap.insert((*I)->getSelector());
+  
+  for (ObjCImplementationDecl::classmeth_iterator
+       I = CatIMPDecl->classmeth_begin(),
+       E = CatIMPDecl->classmeth_end(); I != E; ++I)
+    ClsMap.insert((*I)->getSelector());
+  if (InsMap.empty() && ClsMap.empty())
+    return;
+  
+  // Get category's primary class.
+  ObjCCategoryDecl *CatDecl = CatIMPDecl->getCategoryDecl();
+  if (!CatDecl)
+    return;
+  ObjCInterfaceDecl *IDecl = CatDecl->getClassInterface();
+  if (!IDecl)
+    return;
+  llvm::DenseSet<Selector> InsMapSeen, ClsMapSeen;
+  bool IncompleteImpl = false;
+  MatchAllMethodDeclarations(InsMap, ClsMap, InsMapSeen, ClsMapSeen,
+                             CatIMPDecl, IDecl,
+                             IncompleteImpl, false, true /*WarnExactMatch*/);
+}
 
 void Sema::ImplMethodsVsClassMethods(Scope *S, ObjCImplDecl* IMPDecl,
                                      ObjCContainerDecl* CDecl,
@@ -1567,6 +1670,11 @@ void Sema::ImplMethodsVsClassMethods(Scope *S, ObjCImplDecl* IMPDecl,
   MatchAllMethodDeclarations(InsMap, ClsMap, InsMapSeen, ClsMapSeen,
                              IMPDecl, CDecl,
                              IncompleteImpl, true);
+  // check all methods implemented in category against those declared
+  // in its primary class.
+  if (ObjCCategoryImplDecl *CatDecl = 
+        dyn_cast<ObjCCategoryImplDecl>(IMPDecl))
+    CheckCategoryVsClassMethodMatches(CatDecl);
 
   // Check the protocol list for unimplemented methods in the @implementation
   // class.
index 49dd8a69514c8ee961a3ad8badc03f8a2f910a2f..824fc08aec5c6c2cf0feee517f01c1f6930b7dbb 100644 (file)
@@ -41,10 +41,10 @@ __weak __strong id x; // expected-error {{the type '__strong id' already has ret
 // rdar://8843638
 
 @interface I
-- (id)retain;
-- (id)autorelease;
-- (oneway void)release;
-- (NSUInteger)retainCount;
+- (id)retain; // expected-note {{method declared here}}
+- (id)autorelease; // expected-note {{method declared here}}
+- (oneway void)release; // expected-note {{method declared here}}
+- (NSUInteger)retainCount; // expected-note {{method declared here}}
 @end
 
 @implementation I
@@ -55,10 +55,14 @@ __weak __strong id x; // expected-error {{the type '__strong id' already has ret
 @end
 
 @implementation I(CAT)
-- (id)retain{return 0;} // expected-error {{ARC forbids implementation of 'retain'}}
-- (id)autorelease{return 0;} // expected-error {{ARC forbids implementation of 'autorelease'}}
-- (oneway void)release{} // expected-error {{ARC forbids implementation of 'release'}}
-- (NSUInteger)retainCount{ return 0; } // expected-error {{ARC forbids implementation of 'retainCount'}}
+- (id)retain{return 0;} // expected-error {{ARC forbids implementation of 'retain'}} \
+                        // expected-warning {{category is implementing a method which will also be implemented by its primary class}}
+- (id)autorelease{return 0;} // expected-error {{ARC forbids implementation of 'autorelease'}} \
+                         // expected-warning {{category is implementing a method which will also be implemented by its primary class}}
+- (oneway void)release{} // expected-error {{ARC forbids implementation of 'release'}} \
+                          // expected-warning {{category is implementing a method which will also be implemented by its primary class}}
+- (NSUInteger)retainCount{ return 0; } // expected-error {{ARC forbids implementation of 'retainCount'}} \
+                          // expected-warning {{category is implementing a method which will also be implemented by its primary class}}
 @end
 
 // rdar://8861761
index 612c331ae8c2f187623ad5e8d2d790d2a1bafff7..f5c5a7cc1813b264e93d7433cc63006d6512d08a 100644 (file)
@@ -1,26 +1,27 @@
 // RUN: %clang_cc1 -fsyntax-only -verify %s
 
 @interface I
-- Meth; // expected-note{{method definition for 'Meth' not found}}
+- Meth; // expected-note{{method definition for 'Meth' not found}} \
+        // expected-note{{method declared here}}
 @end
 
 @implementation  I  // expected-warning{{incomplete implementation}}
 @end
 
 @implementation I(CAT)
-- Meth {return 0;}
+- Meth {return 0;} // expected-warning {{category is implementing a method which will also be implemented by its primary class}}
 @end
 
 #pragma GCC diagnostic ignored "-Wincomplete-implementation"
 @interface I2
-- Meth;
+- Meth; // expected-note{{method declared here}}
 @end
 
 @implementation  I2
 @end
 
 @implementation I2(CAT)
-- Meth {return 0;}
+- Meth {return 0;} // expected-warning {{category is implementing a method which will also be implemented by its primary class}}
 @end
 
 
diff --git a/test/SemaObjC/protocol-implementing-class-methods.m b/test/SemaObjC/protocol-implementing-class-methods.m
new file mode 100644 (file)
index 0000000..6d68e8c
--- /dev/null
@@ -0,0 +1,28 @@
+// RUN: %clang_cc1  -fsyntax-only -verify %s
+// rdar://7020493
+
+@protocol P1
+@optional
+- (int) PMeth;
+@required
+- (void) : (double) arg; // expected-note {{method declared here}}
+@end
+
+@interface NSImage <P1>
+- (void) initialize; // expected-note {{method declared here}}
+@end
+
+@interface NSImage (AirPortUI)
+- (void) initialize;
+@end
+
+@interface NSImage()
+- (void) CEMeth; // expected-note {{method declared here}}
+@end
+
+@implementation NSImage (AirPortUI)
+- (void) initialize {NSImage *p=0; [p initialize]; } // expected-warning {{category is implementing a method which will also be implemented by its primary class}}
+- (int) PMeth{ return 0; }
+- (void) : (double) arg{}; // expected-warning {{category is implementing a method which will also be implemented by its primary class}}
+- (void) CEMeth {}; // expected-warning {{category is implementing a method which will also be implemented by its primary class}}
+@end
index 094f19a6712994be89f525c06b3426d58d6c0a14..11b4b9602e141ee8c136af5001d41af42993d2d1 100644 (file)
@@ -149,8 +149,8 @@ void test_inference() {
 @end
 
 // <rdar://problem/9340699>
-@interface G
-- (id)_ABC_init __attribute__((objc_method_family(init)));
+@interface G 
+- (id)_ABC_init __attribute__((objc_method_family(init))); // expected-note {{method declared here}}
 @end
 
 @interface G (Additions)
@@ -158,7 +158,7 @@ void test_inference() {
 @end
 
 @implementation G (Additions)
-- (id)_ABC_init {
+- (id)_ABC_init { // expected-warning {{category is implementing a method which will also be implemented by its primary class}}
   return 0;
 }
 - (id)_ABC_init2 {