Pending further discussion, re-enable warnings for Objective C

covariant/contravariant overrides and implementations, but do so under
control of a new flag (-Wno-objc-covariant-overrides, which yes does cover
contravariance too).

*At least* the covariance cases will probably be enabled by default shortly,
but that's not totally uncontroversial.



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

5 files changed, 77 insertions, 16 deletions

diff --git a/include/clang/Basic/DiagnosticSemaKinds.td b/include/clang/Basic/DiagnosticSemaKinds.td
index ec730d1c79..ae177761cf 100644
--- a/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/include/clang/Basic/DiagnosticSemaKinds.td
@@ -326,9 +326,15 @@ def note_required_for_protocol_at :
 
 def warn_conflicting_ret_types : Warning<
   "conflicting return type in implementation of %0: %1 vs %2">;
+def warn_covariant_ret_types : Warning<
+  "conflicting return type in implementation of %0: %1 vs %2">,
+  InGroup<DiagGroup<"objc-covariant-overrides">>;
 
 def warn_conflicting_param_types : Warning<
   "conflicting parameter types in implementation of %0: %1 vs %2">;
+def warn_contravariant_param_types : Warning<
+  "conflicting parameter types in implementation of %0: %1 vs %2">,
+  InGroup<DiagGroup<"objc-covariant-overrides">>;
 def warn_conflicting_variadic :Warning<
   "conflicting variadic declaration of method and its implementation">;
 
diff --git a/lib/Sema/SemaDeclObjC.cpp b/lib/Sema/SemaDeclObjC.cpp
index c41de01f05..ed3f5a29b8 100644
--- a/lib/Sema/SemaDeclObjC.cpp
+++ b/lib/Sema/SemaDeclObjC.cpp
@@ -832,13 +832,19 @@ void Sema::WarnConflictingTypedMethods(ObjCMethodDecl *ImpMethodDecl,
     // principle of substitutability.  Specifically, we permit return types
     // that are subclasses of the declared return type, or that are
     // more-qualified versions of the declared type.
-    if (!isObjCTypeSubstitutable(Context, IntfMethodDecl->getResultType(),
-          ImpMethodDecl->getResultType())) {
-      Diag(ImpMethodDecl->getLocation(), diag::warn_conflicting_ret_types)
-        << ImpMethodDecl->getDeclName() << IntfMethodDecl->getResultType()
-        << ImpMethodDecl->getResultType();
-      Diag(IntfMethodDecl->getLocation(), diag::note_previous_definition);
-    }
+
+    // As a possibly-temporary adjustment, we still warn in the
+    // covariant case, just with a different diagnostic (mapped to
+    // nothing under certain circumstances).
+    unsigned DiagID = diag::warn_conflicting_ret_types;
+    if (isObjCTypeSubstitutable(Context, IntfMethodDecl->getResultType(),
+          ImpMethodDecl->getResultType()))
+      DiagID = diag::warn_covariant_ret_types;
+
+    Diag(ImpMethodDecl->getLocation(), DiagID)
+      << ImpMethodDecl->getDeclName() << IntfMethodDecl->getResultType()
+      << ImpMethodDecl->getResultType();
+    Diag(IntfMethodDecl->getLocation(), diag::note_previous_definition);
   }
 
   for (ObjCMethodDecl::param_iterator IM = ImpMethodDecl->param_begin(),
@@ -852,12 +858,16 @@ void Sema::WarnConflictingTypedMethods(ObjCMethodDecl *ImpMethodDecl,
     // Allow non-matching argument types as long as they don't violate the
     // principle of substitutability.  Specifically, the implementation must
     // accept any objects that the superclass accepts, however it may also
-    // accept others.  
+    // accept others.
 
+    // As a possibly-temporary adjustment, we still warn in the
+    // contravariant case, just with a different diagnostic (mapped to
+    // nothing under certain circumstances).
+    unsigned DiagID = diag::warn_conflicting_param_types;
     if (isObjCTypeSubstitutable(Context, ParmImpTy, ParmDeclTy, true))
-      continue;
+      DiagID = diag::warn_contravariant_param_types;
 
-    Diag((*IM)->getLocation(), diag::warn_conflicting_param_types)
+    Diag((*IM)->getLocation(), DiagID)
       << ImpMethodDecl->getDeclName() << (*IF)->getType()
       << (*IM)->getType();
     Diag((*IF)->getLocation(), diag::note_previous_definition);
diff --git a/test/SemaObjC/method-conflict-1.m b/test/SemaObjC/method-conflict-1.m
index b8789fa807..fb62e2ef35 100644
--- a/test/SemaObjC/method-conflict-1.m
+++ b/test/SemaObjC/method-conflict-1.m
@@ -7,12 +7,13 @@
 
 @interface MyClass : NSObject {
 }
-- (void)myMethod:(NSArray *)object; 
+- (void)myMethod:(NSArray *)object; // expected-note {{previous definition is here}}
 - (void)myMethod1:(NSObject *)object; // expected-note {{previous definition is here}}
 @end
 
 @implementation MyClass
-- (void)myMethod:(NSObject *)object { 
+// Warn about this contravariant use for now:
+- (void)myMethod:(NSObject *)object { // expected-warning {{conflicting parameter types in implementation of 'myMethod:': 'NSArray *' vs 'NSObject *'}}
 }
 - (void)myMethod1:(NSArray *)object { // expected-warning {{conflicting parameter types in implementation of 'myMethod1:': 'NSObject *' vs 'NSArray *'}}
 }
diff --git a/test/SemaObjC/method-conflict-2.m b/test/SemaObjC/method-conflict-2.m
new file mode 100644
index 0000000000..fd47282455
--- /dev/null
+++ b/test/SemaObjC/method-conflict-2.m
@@ -0,0 +1,44 @@
+// RUN: %clang_cc1 -Wno-objc-covariant-overrides -fsyntax-only -verify %s
+
+@interface A @end
+@interface B : A @end
+
+@interface Test1 {}
+- (void) test1:(A*) object; // expected-note {{previous definition is here}} 
+- (void) test2:(B*) object;
+@end
+
+@implementation Test1
+- (void) test1:(B*) object {} // expected-warning {{conflicting parameter types in implementation of 'test1:': 'A *' vs 'B *'}}
+- (void) test2:(A*) object {}
+@end
+
+@interface Test2 {}
+- (void) test1:(id) object; // expected-note {{previous definition is here}} 
+- (void) test2:(A*) object;
+@end
+
+@implementation Test2
+- (void) test1:(A*) object {} // expected-warning {{conflicting parameter types in implementation of 'test1:': 'id' vs 'A *'}}
+- (void) test2:(id) object {}
+@end
+
+@interface Test3 {}
+- (A*) test1;
+- (B*) test2; // expected-note {{previous definition is here}} 
+@end
+
+@implementation Test3
+- (B*) test1 { return 0; }
+- (A*) test2 { return 0; } // expected-warning {{conflicting return type in implementation of 'test2': 'B *' vs 'A *'}}
+@end
+
+// The particular case of overriding with an id return is white-listed.
+@interface Test4 {}
+- (id) test1;
+- (A*) test2;
+@end
+@implementation Test4
+- (A*) test1 { return 0; }
+- (id) test2 { return 0; }
+@end
diff --git a/test/SemaObjC/method-typecheck-3.m b/test/SemaObjC/method-typecheck-3.m
index 629889c3ad..23036e65d5 100644
--- a/test/SemaObjC/method-typecheck-3.m
+++ b/test/SemaObjC/method-typecheck-3.m
@@ -1,8 +1,8 @@
 // RUN: %clang_cc1 -fsyntax-only -verify %s
 
 @interface A
-- (id)obj;
-- (A*)a;
+- (id)obj; // expected-note {{previous definition is here}}
+- (A*)a; // expected-note {{previous definition is here}}
 - (void)takesA: (A*)a; // expected-note {{previous definition is here}}
 - (void)takesId: (id)a; // expected-note {{previous definition is here}}
 @end
@@ -12,8 +12,8 @@
 @end
 
 @implementation B
-- (B*)obj {return self;}
-- (B*)a { return self;} 
+- (B*)obj {return self;} // expected-warning {{conflicting return type in implementation of 'obj'}}
+- (B*)a { return self;}  // expected-warning {{conflicting return type in implementation of 'a'}}
 - (void)takesA: (B*)a  // expected-warning {{conflicting parameter types in implementation of 'takesA:'}}
 {}
 - (void)takesId: (B*)a // expected-warning {{conflicting parameter types in implementation of 'takesId:'}}