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

6 files changed, 182 insertions, 21 deletions

diff --git a/include/clang/Basic/DiagnosticSemaKinds.td b/include/clang/Basic/DiagnosticSemaKinds.td
index a4ea1c5ff0..cdfa11203e 100644
--- 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 35bcf074ff..41d0e8a176 100644
--- 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 da3bedd12a..e9a39b614e 100644
--- 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 ee1a6b4b78..e835bebfbf 100644
--- 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
index 0000000000..be1c1e03f3
--- /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 98a9ce6cdc..77e9938177 100644
--- 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