From 1235b0ebb16618bf41a9e7234d5e2690faf97d37 Mon Sep 17 00:00:00 2001 From: John McCall Date: Tue, 26 Oct 2010 00:53:53 +0000 Subject: [PATCH] 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 --- include/clang/Basic/DiagnosticSemaKinds.td | 6 +++ lib/Sema/SemaDeclObjC.cpp | 30 ++++++++++----- test/SemaObjC/method-conflict-1.m | 5 ++- test/SemaObjC/method-conflict-2.m | 44 ++++++++++++++++++++++ test/SemaObjC/method-typecheck-3.m | 8 ++-- 5 files changed, 77 insertions(+), 16 deletions(-) create mode 100644 test/SemaObjC/method-conflict-2.m 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>; 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>; 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:'}} -- 2.40.0