objc: clang should warn if redeclaration of methods

declared in protocol in the class qualified by the
protocol have type conflicts. To reduce amount of
noise, this is done when class is implemented.
// rdar://9352731

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

diff --git a/include/clang/Basic/DiagnosticSemaKinds.td b/include/clang/Basic/DiagnosticSemaKinds.td
index a4ea1c5ff03170473f1c4b66e4c6fa641ebdfe28..cdfa11203eea653e95ba433f24e440a6de145562 100644 (file)
--- a/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/include/clang/Basic/DiagnosticSemaKinds.td
@@ -384,26 +384,31 @@ def note_required_for_protocol_at :
   Note<"required for direct or indirect protocol %0">;
 
 def warn_conflicting_ret_types : Warning<
-  "conflicting return type in implementation of %0: %1 vs %2">;
+  "conflicting return type in "
+  "%select{implementation|declaration}3 of %0: %1 vs %2">;
 def warn_conflicting_ret_type_modifiers : Warning<
   "conflicting distributed object modifiers on return type "
-  "in implementation of %0">,
+  "in %select{implementation|declaration}1 of %0">,
   InGroup<DiagGroup<"distributed-object-modifiers">>;
 def warn_non_covariant_ret_types : Warning<
-  "conflicting return type in implementation of %0: %1 vs %2">,
+  "conflicting return type in "
+  "%select{implementation|declaration}3 of %0: %1 vs %2">,
   InGroup<DiagGroup<"method-signatures">>, DefaultIgnore;
 
 def warn_conflicting_param_types : Warning<
-  "conflicting parameter types in implementation of %0: %1 vs %2">;
+  "conflicting parameter types in "
+  "%select{implementation|declaration}3 of %0: %1 vs %2">;
 def warn_conflicting_param_modifiers : Warning<
   "conflicting distributed object modifiers on parameter type "
-  "in implementation of %0">,
+  "in %select{implementation|declaration}1 of %0">,
   InGroup<DiagGroup<"distributed-object-modifiers">>;
 def warn_non_contravariant_param_types : Warning<
-  "conflicting parameter types in implementation of %0: %1 vs %2">,
+  "conflicting parameter types in "
+  "%select{implementation|declaration}3 of %0: %1 vs %2">,
   InGroup<DiagGroup<"method-signatures">>, DefaultIgnore;
 def warn_conflicting_variadic :Warning<
-  "conflicting variadic declaration of method and its implementation">;
+  "conflicting variadic declaration of method and its "
+  "%select{implementation|declaration}0">;
 
 def warn_implements_nscopying : Warning<
 "default assign attribute on property %0 which implements "

diff --git a/include/clang/Sema/Sema.h b/include/clang/Sema/Sema.h
index 35bcf074ff9261ad8fece3f9d9c4c9eeea371588..41d0e8a17673e19c6f7476f8e0205ff6e101cb37 100644 (file)
--- a/include/clang/Sema/Sema.h
+++ b/include/clang/Sema/Sema.h
@@ -1762,9 +1762,10 @@ public:
 
   void WarnUndefinedMethod(SourceLocation ImpLoc, ObjCMethodDecl *method,
                            bool &IncompleteImpl, unsigned DiagID);
-  void WarnConflictingTypedMethods(ObjCMethodDecl *ImpMethod,
+  void WarnConflictingTypedMethods(ObjCMethodDecl *Method,
                                    ObjCMethodDecl *MethodDecl,
-                                   bool IsProtocolMethodDecl);
+                                   bool IsProtocolMethodDecl,
+                                   bool IsDeclaration = false);
 
   bool isPropertyReadonly(ObjCPropertyDecl *PropertyDecl,
                           ObjCInterfaceDecl *IDecl);
@@ -1887,6 +1888,11 @@ public:
                                   bool &IncompleteImpl,
                                   bool ImmediateClass);
 
+  /// 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);
+
 private:
   /// AddMethodToGlobalPool - Add an instance or factory method to the global
   /// pool. See descriptoin of AddInstanceMethodToGlobalPool.

diff --git a/lib/Sema/SemaDeclObjC.cpp b/lib/Sema/SemaDeclObjC.cpp
index da3bedd12a350760749dffe8df49276e93dce930..e9a39b614e85014f2b1f5bfc091fda90089ae449 100644 (file)
--- a/lib/Sema/SemaDeclObjC.cpp
+++ b/lib/Sema/SemaDeclObjC.cpp
@@ -1074,13 +1074,14 @@ static SourceRange getTypeRange(TypeSourceInfo *TSI) {
 static void CheckMethodOverrideReturn(Sema &S,
                                       ObjCMethodDecl *MethodImpl,
                                       ObjCMethodDecl *MethodDecl,
-                                      bool IsProtocolMethodDecl) {
+                                      bool IsProtocolMethodDecl,
+                                      bool IsDeclaration) {
   if (IsProtocolMethodDecl &&
       (MethodDecl->getObjCDeclQualifier() !=
        MethodImpl->getObjCDeclQualifier())) {
     S.Diag(MethodImpl->getLocation(), 
            diag::warn_conflicting_ret_type_modifiers)
-        << MethodImpl->getDeclName()
+        << MethodImpl->getDeclName() << IsDeclaration
         << getTypeRange(MethodImpl->getResultTypeSourceInfo());
     S.Diag(MethodDecl->getLocation(), diag::note_previous_declaration)
         << getTypeRange(MethodDecl->getResultTypeSourceInfo());
@@ -1113,6 +1114,7 @@ static void CheckMethodOverrideReturn(Sema &S,
     << MethodImpl->getDeclName()
     << MethodDecl->getResultType()
     << MethodImpl->getResultType()
+    << IsDeclaration
     << getTypeRange(MethodImpl->getResultTypeSourceInfo());
   S.Diag(MethodDecl->getLocation(), diag::note_previous_definition)
     << getTypeRange(MethodDecl->getResultTypeSourceInfo());
@@ -1123,14 +1125,15 @@ static void CheckMethodOverrideParam(Sema &S,
                                      ObjCMethodDecl *MethodDecl,
                                      ParmVarDecl *ImplVar,
                                      ParmVarDecl *IfaceVar,
-                                     bool IsProtocolMethodDecl) {
+                                     bool IsProtocolMethodDecl,
+                                     bool IsDeclaration) {
   if (IsProtocolMethodDecl &&
       (ImplVar->getObjCDeclQualifier() !=
        IfaceVar->getObjCDeclQualifier())) {
     S.Diag(ImplVar->getLocation(), 
            diag::warn_conflicting_param_modifiers)
         << getTypeRange(ImplVar->getTypeSourceInfo())
-        << MethodImpl->getDeclName();
+        << MethodImpl->getDeclName() << IsDeclaration;
     S.Diag(IfaceVar->getLocation(), diag::note_previous_declaration)
         << getTypeRange(IfaceVar->getTypeSourceInfo());   
   }
@@ -1162,7 +1165,8 @@ static void CheckMethodOverrideParam(Sema &S,
 
   S.Diag(ImplVar->getLocation(), DiagID)
     << getTypeRange(ImplVar->getTypeSourceInfo())
-    << MethodImpl->getDeclName() << IfaceTy << ImplTy;
+    << MethodImpl->getDeclName() << IfaceTy << ImplTy 
+    << IsDeclaration;
   S.Diag(IfaceVar->getLocation(), diag::note_previous_definition)
     << getTypeRange(IfaceVar->getTypeSourceInfo());
 }
@@ -1239,22 +1243,24 @@ static bool checkMethodFamilyMismatch(Sema &S, ObjCMethodDecl *impl,
 
 void Sema::WarnConflictingTypedMethods(ObjCMethodDecl *ImpMethodDecl,
                                        ObjCMethodDecl *MethodDecl,
-                                       bool IsProtocolMethodDecl) {
+                                       bool IsProtocolMethodDecl,
+                                       bool IsDeclaration) {
   if (getLangOptions().ObjCAutoRefCount &&
       checkMethodFamilyMismatch(*this, ImpMethodDecl, MethodDecl))
     return;
 
   CheckMethodOverrideReturn(*this, ImpMethodDecl, MethodDecl, 
-                            IsProtocolMethodDecl);
+                            IsProtocolMethodDecl, IsDeclaration);
 
   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);
+                             IsProtocolMethodDecl, IsDeclaration);
 
   if (ImpMethodDecl->isVariadic() != MethodDecl->isVariadic()) {
-    Diag(ImpMethodDecl->getLocation(), diag::warn_conflicting_variadic);
+    Diag(ImpMethodDecl->getLocation(), diag::warn_conflicting_variadic)
+      << IsDeclaration;
     Diag(MethodDecl->getLocation(), diag::note_previous_declaration);
   }
 }
@@ -1427,6 +1433,11 @@ void Sema::MatchAllMethodDeclarations(const llvm::DenseSet<Selector> &InsMap,
       MatchAllMethodDeclarations(InsMap, ClsMap, InsMapSeen, ClsMapSeen,
                                  IMPDecl,
                                  (*PI), IncompleteImpl, false);
+    
+    // Check for any type mismtch of methods declared in class 
+    // and methods declared in protocol.
+    MatchMethodsInClassAndItsProtocol(I);
+    
     if (I->getSuperClass())
       MatchAllMethodDeclarations(InsMap, ClsMap, InsMapSeen, ClsMapSeen,
                                  IMPDecl,
@@ -1434,6 +1445,99 @@ void Sema::MatchAllMethodDeclarations(const llvm::DenseSet<Selector> &InsMap,
   }
 }
 
+static void MatchMethodsInClassAndOneProtocol(Sema &S, 
+                                              Sema::SelectorSet &InsMap,
+                                              Sema::SelectorSet &ClsMap,
+                                              const ObjCContainerDecl *IDecl,
+                                              const ObjCProtocolDecl *PDecl) {
+  if (!InsMap.empty())
+    for (ObjCInterfaceDecl::instmeth_iterator IM = PDecl->instmeth_begin(),
+         E = PDecl->instmeth_end(); IM != E; ++IM) {
+      Selector Sel = (*IM)->getSelector();
+      if (InsMap.count(Sel)) {
+        ObjCMethodDecl *ProtoMethodDecl = PDecl->getInstanceMethod(Sel);
+        ObjCMethodDecl *ClsMethodDecl = IDecl->getInstanceMethod(Sel);
+        if (ProtoMethodDecl && ClsMethodDecl)
+          S.WarnConflictingTypedMethods(
+                                      ClsMethodDecl, 
+                                      ProtoMethodDecl, true, true);
+        InsMap.erase(Sel);
+      }
+      if (InsMap.empty())
+        break;
+    }
+  if (!ClsMap.empty())
+    for (ObjCInterfaceDecl::classmeth_iterator IM = PDecl->classmeth_begin(),
+         E = PDecl->classmeth_end(); IM != E; ++IM) {
+      Selector Sel = (*IM)->getSelector();
+      if (ClsMap.count(Sel)) {
+        ObjCMethodDecl *ProtoMethodDecl = PDecl->getClassMethod(Sel);
+        ObjCMethodDecl *ClsMethodDecl = IDecl->getClassMethod(Sel);
+        if (ProtoMethodDecl && ClsMethodDecl)
+          S.WarnConflictingTypedMethods(
+                                        ClsMethodDecl, 
+                                        ProtoMethodDecl, true, true);
+        ClsMap.erase(Sel);
+      }
+      if (ClsMap.empty())
+        break;
+    }
+  if (InsMap.empty() && ClsMap.empty())
+    return;
+  
+  for (ObjCProtocolDecl::protocol_iterator PI = PDecl->protocol_begin(),
+       PE = PDecl->protocol_end(); PI != PE; ++PI)
+    MatchMethodsInClassAndOneProtocol(S, InsMap, ClsMap, IDecl, (*PI));
+}
+
+void Sema::MatchMethodsInClassAndItsProtocol(const ObjCInterfaceDecl *CDecl) {
+  if (CDecl->all_referenced_protocol_begin() ==
+      CDecl->all_referenced_protocol_end())
+    return;
+  
+  SelectorSet InsMap, ClsMap;
+  for (ObjCInterfaceDecl::instmeth_iterator I = CDecl->instmeth_begin(),
+       E = CDecl->instmeth_end(); I != E; ++I)
+    if (!InsMap.count((*I)->getSelector()))
+      InsMap.insert((*I)->getSelector());
+  
+  for (ObjCInterfaceDecl::classmeth_iterator
+       I = CDecl->classmeth_begin(), E = CDecl->classmeth_end(); I != E; ++I)
+    if (!ClsMap.count((*I)->getSelector()))
+      ClsMap.insert((*I)->getSelector());
+  
+  if (!InsMap.empty() || !ClsMap.empty())
+    for (ObjCInterfaceDecl::all_protocol_iterator
+         PI = CDecl->all_referenced_protocol_begin(),
+         E = CDecl->all_referenced_protocol_end(); PI != E; ++PI)
+      MatchMethodsInClassAndOneProtocol(*this, InsMap, ClsMap, CDecl, (*PI));
+  
+  // Also for class extensions
+  if (!CDecl->getFirstClassExtension())
+    return;
+  
+  for (const ObjCCategoryDecl *ClsExtDecl = CDecl->getFirstClassExtension();
+       ClsExtDecl; ClsExtDecl = ClsExtDecl->getNextClassExtension()) {
+    InsMap.clear();
+    ClsMap.clear();
+    for (ObjCCategoryDecl::instmeth_iterator I = ClsExtDecl->instmeth_begin(),
+         E = ClsExtDecl->instmeth_end(); I != E; ++I)
+      if (!InsMap.count((*I)->getSelector()))
+        InsMap.insert((*I)->getSelector());
+    for (ObjCCategoryDecl::classmeth_iterator I = ClsExtDecl->classmeth_begin(),
+         E = ClsExtDecl->classmeth_end(); I != E; ++I)
+      if (!ClsMap.count((*I)->getSelector()))
+        ClsMap.insert((*I)->getSelector());
+    if (InsMap.empty() && ClsMap.empty())
+      continue;
+    for (ObjCInterfaceDecl::all_protocol_iterator
+         PI = CDecl->all_referenced_protocol_begin(),
+         E = CDecl->all_referenced_protocol_end(); PI != E; ++PI)
+      MatchMethodsInClassAndOneProtocol(*this, InsMap, ClsMap, ClsExtDecl, (*PI));
+  }
+}
+
+
 void Sema::ImplMethodsVsClassMethods(Scope *S, ObjCImplDecl* IMPDecl,
                                      ObjCContainerDecl* CDecl,
                                      bool IncompleteImpl) {

diff --git a/test/Analysis/retain-release-gc-only.m b/test/Analysis/retain-release-gc-only.m
index ee1a6b4b782e0149c12cafdc3e15f93ce72241dd..e835bebfbf4407eaf39c9d262a31756c239e31e7 100644 (file)
--- a/test/Analysis/retain-release-gc-only.m
+++ b/test/Analysis/retain-release-gc-only.m
@@ -92,7 +92,7 @@ typedef struct _NSZone NSZone;
 + (id)allocWithZone:(NSZone *)zone;
 + (id)alloc;
 - (void)dealloc;
-- (void)release;
+- (oneway void)release;
 - (id)copy;
 @end
 @interface NSObject (NSCoderMethods)

diff --git a/test/SemaObjC/class-protocol-method-match.m b/test/SemaObjC/class-protocol-method-match.m
new file mode 100644 (file)
index 0000000..be1c1e0
--- /dev/null
+++ b/test/SemaObjC/class-protocol-method-match.m
@@ -0,0 +1,46 @@
+// RUN: %clang_cc1  -fsyntax-only -verify %s
+// rdar://9352731
+
+@protocol Bar 
+@required
+- (unsigned char) baz; // expected-note {{previous definition is here}}
+- (char) ok;
+- (void) also_ok;
+@end
+
+@protocol Bar1 
+@required
+- (unsigned char) baz;
+- (unsigned char) also_ok;
+- (void) ban : (int) arg, ...; // expected-note {{previous declaration is here}}
+@end
+
+@protocol Baz <Bar, Bar1>
+- (void) bar : (unsigned char)arg; // expected-note {{previous definition is here}}
+- (void) ok;
+- (char) bak; // expected-note {{previous definition is here}}
+@end
+
+@interface Foo <Baz>
+- (void) baz;  // expected-warning {{conflicting return type in declaration of 'baz': 'unsigned char' vs 'void'}}
+- (void) bar : (unsigned char*)arg; // expected-warning {{conflicting parameter types in declaration of 'bar:': 'unsigned char' vs 'unsigned char *'}}
+- (void) ok;
+- (void) also_ok;
+- (void) still_ok;
+- (void) ban : (int) arg; // expected-warning {{conflicting variadic declaration of method and its declaration}}
+@end
+
+@interface Foo()
+- (void) bak; // expected-warning {{conflicting return type in declaration of 'bak': 'char' vs 'void'}}
+@end
+
+@implementation Foo
+- (void) baz {}
+- (void) bar : (unsigned char*)arg {}
+- (void) ok {}
+- (void) also_ok {}
+- (void) still_ok {}
+- (void) ban : (int) arg {}
+- (void) bak {}
+@end
+

diff --git a/test/SemaObjC/dist-object-modifiers.m b/test/SemaObjC/dist-object-modifiers.m
index 98a9ce6cdc52742b6a620f8e2c5b0157aed0f034..77e9938177a1a9d398fca63bd3139610e6f76165 100644 (file)
--- a/test/SemaObjC/dist-object-modifiers.m
+++ b/test/SemaObjC/dist-object-modifiers.m
@@ -4,12 +4,12 @@
 @protocol P
 - (bycopy id)serverPID; // expected-note {{previous declaration is here}}
 - (void)doStuff:(bycopy id)clientId; // expected-note {{previous declaration is here}}
-- (bycopy id)Ok;
+- (bycopy id)Ok; // expected-note {{previous declaration is here}}
 + (oneway id) stillMore : (byref id)Arg : (bycopy oneway id)Arg1;  // expected-note 3 {{previous declaration is here}}
 @end
 
 @interface I <P>
-- (id)Ok;
+- (id)Ok; // expected-warning {{conflicting distributed object modifiers on return type in declaration of 'Ok'}}
 @end
 
 @implementation I