aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
-rw-r--r--include/clang/Basic/DiagnosticSemaKinds.td8
-rw-r--r--lib/Sema/SemaDeclObjC.cpp149
-rw-r--r--test/SemaObjC/class-conforming-protocol-2.m2
-rw-r--r--test/SemaObjC/comptypes-a.m2
-rw-r--r--test/SemaObjC/method-conflict-1.m65
-rw-r--r--test/SemaObjC/method-conflict-2.m4
-rw-r--r--test/SemaObjC/method-conflict.m2
-rw-r--r--test/SemaObjC/method-def-1.m2
-rw-r--r--test/SemaObjC/method-typecheck-3.m11
9 files changed, 161 insertions, 84 deletions
diff --git a/include/clang/Basic/DiagnosticSemaKinds.td b/include/clang/Basic/DiagnosticSemaKinds.td
index ae177761cf..7efc7d5d41 100644
--- a/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/include/clang/Basic/DiagnosticSemaKinds.td
@@ -326,15 +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<
+def warn_non_covariant_ret_types : Warning<
"conflicting return type in implementation of %0: %1 vs %2">,
- InGroup<DiagGroup<"objc-covariant-overrides">>;
+ InGroup<DiagGroup<"method-signatures">>, DefaultIgnore;
def warn_conflicting_param_types : Warning<
"conflicting parameter types in implementation of %0: %1 vs %2">;
-def warn_contravariant_param_types : Warning<
+def warn_non_contravariant_param_types : Warning<
"conflicting parameter types in implementation of %0: %1 vs %2">,
- InGroup<DiagGroup<"objc-covariant-overrides">>;
+ InGroup<DiagGroup<"method-signatures">>, DefaultIgnore;
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 ed3f5a29b8..bd16fa3994 100644
--- a/lib/Sema/SemaDeclObjC.cpp
+++ b/lib/Sema/SemaDeclObjC.cpp
@@ -782,22 +782,12 @@ void Sema::WarnUndefinedMethod(SourceLocation ImpLoc, ObjCMethodDecl *method,
/// for explicit down-casting by callers.
///
/// Note: This is a stricter requirement than for assignment.
-static bool isObjCTypeSubstitutable(ASTContext &C, QualType A, QualType B, bool
- rejectId=false) {
- ObjCObjectPointerType *a = dyn_cast<ObjCObjectPointerType>(
- C.getCanonicalType(A));
- ObjCObjectPointerType *b = dyn_cast<ObjCObjectPointerType>(
- C.getCanonicalType(B));
- // Ignore non-ObjC types.
- if (!(a && b))
- {
- //a->dump(); b->dump();
- return false;
- }
- // A type is always substitutable for itself
- if (C.hasSameType(A, B)) return true;
-
- if (rejectId && C.isObjCIdType(B)) return false;
+static bool isObjCTypeSubstitutable(ASTContext &Context,
+ const ObjCObjectPointerType *A,
+ const ObjCObjectPointerType *B,
+ bool rejectId) {
+ // Reject a protocol-unqualified id.
+ if (rejectId && B->isObjCIdType()) return false;
// If B is a qualified id, then A must also be a qualified id and it must
// implement all of the protocols in B. It may not be a qualified class.
@@ -805,7 +795,9 @@ static bool isObjCTypeSubstitutable(ASTContext &C, QualType A, QualType B, bool
// stricter definition so it is not substitutable for id<A>.
if (B->isObjCQualifiedIdType()) {
return A->isObjCQualifiedIdType() &&
- C.ObjCQualifiedIdTypesAreCompatible(A, B, false);
+ Context.ObjCQualifiedIdTypesAreCompatible(QualType(A, 0),
+ QualType(B,0),
+ false);
}
/*
@@ -821,57 +813,94 @@ static bool isObjCTypeSubstitutable(ASTContext &C, QualType A, QualType B, bool
// Now we know that A and B are (potentially-qualified) class types. The
// normal rules for assignment apply.
- return C.canAssignObjCInterfaces(a, b);
+ return Context.canAssignObjCInterfaces(A, B);
}
-void Sema::WarnConflictingTypedMethods(ObjCMethodDecl *ImpMethodDecl,
- ObjCMethodDecl *IntfMethodDecl) {
- if (!Context.hasSameType(IntfMethodDecl->getResultType(),
- ImpMethodDecl->getResultType())) {
- // Allow non-matching return types as long as they don't violate the
- // 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.
-
- // 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);
+static SourceRange getTypeRange(TypeSourceInfo *TSI) {
+ return (TSI ? TSI->getTypeLoc().getSourceRange() : SourceRange());
+}
+
+static void CheckMethodOverrideReturn(Sema &S,
+ ObjCMethodDecl *MethodImpl,
+ ObjCMethodDecl *MethodIface) {
+ if (S.Context.hasSameUnqualifiedType(MethodImpl->getResultType(),
+ MethodIface->getResultType()))
+ return;
+
+ unsigned DiagID = diag::warn_conflicting_ret_types;
+
+ // Mismatches between ObjC pointers go into a different warning
+ // category, and sometimes they're even completely whitelisted.
+ if (const ObjCObjectPointerType *ImplPtrTy =
+ MethodImpl->getResultType()->getAs<ObjCObjectPointerType>()) {
+ if (const ObjCObjectPointerType *IfacePtrTy =
+ MethodIface->getResultType()->getAs<ObjCObjectPointerType>()) {
+ // Allow non-matching return types as long as they don't violate
+ // the 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(S.Context, IfacePtrTy, ImplPtrTy, false))
+ return;
+
+ DiagID = diag::warn_non_covariant_ret_types;
+ }
}
- for (ObjCMethodDecl::param_iterator IM = ImpMethodDecl->param_begin(),
- IF = IntfMethodDecl->param_begin(), EM = ImpMethodDecl->param_end();
- IM != EM; ++IM, ++IF) {
- QualType ParmDeclTy = (*IF)->getType().getUnqualifiedType();
- QualType ParmImpTy = (*IM)->getType().getUnqualifiedType();
- if (Context.hasSameType(ParmDeclTy, ParmImpTy))
- continue;
+ S.Diag(MethodImpl->getLocation(), DiagID)
+ << MethodImpl->getDeclName()
+ << MethodIface->getResultType()
+ << MethodImpl->getResultType()
+ << getTypeRange(MethodImpl->getResultTypeSourceInfo());
+ S.Diag(MethodIface->getLocation(), diag::note_previous_definition)
+ << getTypeRange(MethodIface->getResultTypeSourceInfo());
+}
- // 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.
+static void CheckMethodOverrideParam(Sema &S,
+ ObjCMethodDecl *MethodImpl,
+ ObjCMethodDecl *MethodIface,
+ ParmVarDecl *ImplVar,
+ ParmVarDecl *IfaceVar) {
+ QualType ImplTy = ImplVar->getType();
+ QualType IfaceTy = IfaceVar->getType();
+ if (S.Context.hasSameUnqualifiedType(ImplTy, IfaceTy))
+ return;
- // 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))
- DiagID = diag::warn_contravariant_param_types;
+ unsigned DiagID = diag::warn_conflicting_param_types;
+
+ // Mismatches between ObjC pointers go into a different warning
+ // category, and sometimes they're even completely whitelisted.
+ if (const ObjCObjectPointerType *ImplPtrTy =
+ ImplTy->getAs<ObjCObjectPointerType>()) {
+ if (const ObjCObjectPointerType *IfacePtrTy =
+ IfaceTy->getAs<ObjCObjectPointerType>()) {
+ // 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.
+ if (isObjCTypeSubstitutable(S.Context, ImplPtrTy, IfacePtrTy, true))
+ return;
- Diag((*IM)->getLocation(), DiagID)
- << ImpMethodDecl->getDeclName() << (*IF)->getType()
- << (*IM)->getType();
- Diag((*IF)->getLocation(), diag::note_previous_definition);
+ DiagID = diag::warn_non_contravariant_param_types;
+ }
}
+
+ S.Diag(ImplVar->getLocation(), DiagID)
+ << getTypeRange(ImplVar->getTypeSourceInfo())
+ << MethodImpl->getDeclName() << IfaceTy << ImplTy;
+ S.Diag(IfaceVar->getLocation(), diag::note_previous_definition)
+ << getTypeRange(IfaceVar->getTypeSourceInfo());
+}
+
+
+void Sema::WarnConflictingTypedMethods(ObjCMethodDecl *ImpMethodDecl,
+ ObjCMethodDecl *IntfMethodDecl) {
+ CheckMethodOverrideReturn(*this, ImpMethodDecl, IntfMethodDecl);
+
+ for (ObjCMethodDecl::param_iterator IM = ImpMethodDecl->param_begin(),
+ IF = IntfMethodDecl->param_begin(), EM = ImpMethodDecl->param_end();
+ IM != EM; ++IM, ++IF)
+ CheckMethodOverrideParam(*this, ImpMethodDecl, IntfMethodDecl, *IM, *IF);
+
if (ImpMethodDecl->isVariadic() != IntfMethodDecl->isVariadic()) {
Diag(ImpMethodDecl->getLocation(), diag::warn_conflicting_variadic);
Diag(IntfMethodDecl->getLocation(), diag::note_previous_declaration);
diff --git a/test/SemaObjC/class-conforming-protocol-2.m b/test/SemaObjC/class-conforming-protocol-2.m
index 29e5810338..a3bd0b1d23 100644
--- a/test/SemaObjC/class-conforming-protocol-2.m
+++ b/test/SemaObjC/class-conforming-protocol-2.m
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -fsyntax-only -verify %s
+// RUN: %clang_cc1 -Wmethod-signatures -fsyntax-only -verify %s
@protocol NSWindowDelegate @end
diff --git a/test/SemaObjC/comptypes-a.m b/test/SemaObjC/comptypes-a.m
index d7dddaad52..8480f524dc 100644
--- a/test/SemaObjC/comptypes-a.m
+++ b/test/SemaObjC/comptypes-a.m
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -fsyntax-only -verify -pedantic %s
+// RUN: %clang_cc1 -fsyntax-only -Wmethod-signatures -verify -pedantic %s
typedef signed char BOOL;
typedef int NSInteger;
diff --git a/test/SemaObjC/method-conflict-1.m b/test/SemaObjC/method-conflict-1.m
index fb62e2ef35..3cf2c6b5a9 100644
--- a/test/SemaObjC/method-conflict-1.m
+++ b/test/SemaObjC/method-conflict-1.m
@@ -1,4 +1,7 @@
// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+// This test case tests the default behavior.
+
// rdar://7933061
@interface NSObject @end
@@ -7,15 +10,14 @@
@interface MyClass : NSObject {
}
-- (void)myMethod:(NSArray *)object; // expected-note {{previous definition is here}}
-- (void)myMethod1:(NSObject *)object; // expected-note {{previous definition is here}}
+- (void)myMethod:(NSArray *)object;
+- (void)myMethod1:(NSObject *)object; // broken-note {{previous definition is here}}
@end
@implementation MyClass
-// Warn about this contravariant use for now:
-- (void)myMethod:(NSObject *)object { // expected-warning {{conflicting parameter types in implementation of 'myMethod:': 'NSArray *' vs 'NSObject *'}}
+- (void)myMethod:(NSObject *)object {
}
-- (void)myMethod1:(NSArray *)object { // expected-warning {{conflicting parameter types in implementation of 'myMethod1:': 'NSObject *' vs 'NSArray *'}}
+- (void)myMethod1:(NSArray *)object { // broken-warning {{conflicting parameter types in implementation of 'myMethod1:': 'NSObject *' vs 'NSArray *'}}
}
@end
@@ -24,13 +26,58 @@
@interface MyOtherClass : NSObject <MyProtocol> {
}
-- (void)myMethod:(id <MyProtocol>)object; // expected-note {{previous definition is here}}
-- (void)myMethod1:(id <MyProtocol>)object; // expected-note {{previous definition is here}}
+- (void)myMethod:(id <MyProtocol>)object; // broken-note {{previous definition is here}}
+- (void)myMethod1:(id <MyProtocol>)object; // broken-note {{previous definition is here}}
@end
@implementation MyOtherClass
-- (void)myMethod:(MyClass *)object { // expected-warning {{conflicting parameter types in implementation of 'myMethod:': 'id<MyProtocol>' vs 'MyClass *'}}
+- (void)myMethod:(MyClass *)object { // broken-warning {{conflicting parameter types in implementation of 'myMethod:': 'id<MyProtocol>' vs 'MyClass *'}}
}
-- (void)myMethod1:(MyClass<MyProtocol> *)object { // expected-warning {{conflicting parameter types in implementation of 'myMethod1:': 'id<MyProtocol>' vs 'MyClass<MyProtocol> *'}}
+- (void)myMethod1:(MyClass<MyProtocol> *)object { // broken-warning {{conflicting parameter types in implementation of 'myMethod1:': 'id<MyProtocol>' vs 'MyClass<MyProtocol> *'}}
}
@end
+
+
+
+@interface A @end
+@interface B : A @end
+
+@interface Test1 {}
+- (void) test1:(A*) object; // broken-note {{previous definition is here}}
+- (void) test2:(B*) object;
+@end
+
+@implementation Test1
+- (void) test1:(B*) object {} // broken-warning {{conflicting parameter types in implementation of 'test1:': 'A *' vs 'B *'}}
+- (void) test2:(A*) object {}
+@end
+
+// rdar://problem/8597621 wants id -> A* to be an exception
+@interface Test2 {}
+- (void) test1:(id) object; // broken-note {{previous definition is here}}
+- (void) test2:(A*) object;
+@end
+@implementation Test2
+- (void) test1:(A*) object {} // broken-warning {{conflicting parameter types in implementation of 'test1:': 'id' vs 'A *'}}
+- (void) test2:(id) object {}
+@end
+
+@interface Test3 {}
+- (A*) test1;
+- (B*) test2; // broken-note {{previous definition is here}}
+@end
+
+@implementation Test3
+- (B*) test1 { return 0; }
+- (A*) test2 { return 0; } // broken-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 -> A* is rdar://problem/8596987
+- (id) test2 { return 0; }
+@end
diff --git a/test/SemaObjC/method-conflict-2.m b/test/SemaObjC/method-conflict-2.m
index fd47282455..7b5a08ad9e 100644
--- a/test/SemaObjC/method-conflict-2.m
+++ b/test/SemaObjC/method-conflict-2.m
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -Wno-objc-covariant-overrides -fsyntax-only -verify %s
+// RUN: %clang_cc1 -Wmethod-signatures -fsyntax-only -verify %s
@interface A @end
@interface B : A @end
@@ -39,6 +39,6 @@
- (A*) test2;
@end
@implementation Test4
-- (A*) test1 { return 0; }
+- (A*) test1 { return 0; } // id -> A* is rdar://problem/8596987
- (id) test2 { return 0; }
@end
diff --git a/test/SemaObjC/method-conflict.m b/test/SemaObjC/method-conflict.m
index ecdf1d88ca..4eb7290612 100644
--- a/test/SemaObjC/method-conflict.m
+++ b/test/SemaObjC/method-conflict.m
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -fsyntax-only -verify %s
+// RUN: %clang_cc1 -Wmethod-signatures -fsyntax-only -verify %s
typedef signed char BOOL;
typedef unsigned int NSUInteger;
diff --git a/test/SemaObjC/method-def-1.m b/test/SemaObjC/method-def-1.m
index 7630ad0fbd..bc7ea7bc44 100644
--- a/test/SemaObjC/method-def-1.m
+++ b/test/SemaObjC/method-def-1.m
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -fsyntax-only -verify %s
+// RUN: %clang_cc1 -Wmethod-signatures -fsyntax-only -verify %s
@interface foo
- (int)meth;
diff --git a/test/SemaObjC/method-typecheck-3.m b/test/SemaObjC/method-typecheck-3.m
index 23036e65d5..1999b7d477 100644
--- a/test/SemaObjC/method-typecheck-3.m
+++ b/test/SemaObjC/method-typecheck-3.m
@@ -1,8 +1,9 @@
-// RUN: %clang_cc1 -fsyntax-only -verify %s
+// RUN: %clang_cc1 -Wmethod-signatures -fsyntax-only -verify %s
+@class B;
@interface A
-- (id)obj; // expected-note {{previous definition is here}}
-- (A*)a; // expected-note {{previous definition is here}}
+- (B*)obj;
+- (B*)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 +13,8 @@
@end
@implementation B
-- (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'}}
+- (id)obj {return self;} // 'id' overrides are white-listed?
+- (A*)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:'}}