[cfe-commits] r117346 - in /cfe/trunk: include/clang/Basic/DiagnosticSemaKinds.td lib/Sema/SemaDeclObjC.cpp test/SemaObjC/method-conflict-1.m test/SemaObjC/method-conflict-2.m test/SemaObjC/method-typecheck-3.m

[John McCall]

Author: rjmccall
Date: Mon Oct 25 19:53:53 2010
New Revision: 117346

URL: http://llvm.org/viewvc/llvm-project?rev=117346&view=rev
Log:
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.


Added:
    cfe/trunk/test/SemaObjC/method-conflict-2.m
Modified:
    cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
    cfe/trunk/lib/Sema/SemaDeclObjC.cpp
    cfe/trunk/test/SemaObjC/method-conflict-1.m
    cfe/trunk/test/SemaObjC/method-typecheck-3.m

Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=117346&r1=117345&r2=117346&view=diff
==============================================================================
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Mon Oct 25 19:53:53 2010
@@ -326,9 +326,15 @@
 
 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">;
 

Modified: cfe/trunk/lib/Sema/SemaDeclObjC.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclObjC.cpp?rev=117346&r1=117345&r2=117346&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaDeclObjC.cpp (original)
+++ cfe/trunk/lib/Sema/SemaDeclObjC.cpp Mon Oct 25 19:53:53 2010
@@ -832,13 +832,19 @@
     // 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 @@
     // 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);

Modified: cfe/trunk/test/SemaObjC/method-conflict-1.m
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaObjC/method-conflict-1.m?rev=117346&r1=117345&r2=117346&view=diff
==============================================================================
--- cfe/trunk/test/SemaObjC/method-conflict-1.m (original)
+++ cfe/trunk/test/SemaObjC/method-conflict-1.m Mon Oct 25 19:53:53 2010
@@ -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 *'}}
 }

Added: cfe/trunk/test/SemaObjC/method-conflict-2.m
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaObjC/method-conflict-2.m?rev=117346&view=auto
==============================================================================
--- cfe/trunk/test/SemaObjC/method-conflict-2.m (added)
+++ cfe/trunk/test/SemaObjC/method-conflict-2.m Mon Oct 25 19:53:53 2010
@@ -0,0 +1,44 @@
+// RUN: %clang_cc1 -Wno-objc-covariant-overrides -fsyntax-only -verify %s
+
+ at interface A @end
+ at interface B : A @end
+
+ at interface Test1 {}
+- (void) test1:(A*) object; // expected-note {{previous definition is here}} 
+- (void) test2:(B*) object;
+ at end
+
+ at implementation Test1
+- (void) test1:(B*) object {} // expected-warning {{conflicting parameter types in implementation of 'test1:': 'A *' vs 'B *'}}
+- (void) test2:(A*) object {}
+ at end
+
+ at interface Test2 {}
+- (void) test1:(id) object; // expected-note {{previous definition is here}} 
+- (void) test2:(A*) object;
+ at end
+
+ at implementation Test2
+- (void) test1:(A*) object {} // expected-warning {{conflicting parameter types in implementation of 'test1:': 'id' vs 'A *'}}
+- (void) test2:(id) object {}
+ at end
+
+ at interface Test3 {}
+- (A*) test1;
+- (B*) test2; // expected-note {{previous definition is here}} 
+ at end
+
+ at implementation Test3
+- (B*) test1 { return 0; }
+- (A*) test2 { return 0; } // expected-warning {{conflicting return type in implementation of 'test2': 'B *' vs 'A *'}}
+ at end
+
+// The particular case of overriding with an id return is white-listed.
+ at interface Test4 {}
+- (id) test1;
+- (A*) test2;
+ at end
+ at implementation Test4
+- (A*) test1 { return 0; }
+- (id) test2 { return 0; }
+ at end

Modified: cfe/trunk/test/SemaObjC/method-typecheck-3.m
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaObjC/method-typecheck-3.m?rev=117346&r1=117345&r2=117346&view=diff
==============================================================================
--- cfe/trunk/test/SemaObjC/method-typecheck-3.m (original)
+++ cfe/trunk/test/SemaObjC/method-typecheck-3.m Mon Oct 25 19:53:53 2010
@@ -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:'}}