[cfe-commits] r117524 - in /cfe/trunk: include/clang/Basic/DiagnosticSemaKinds.td lib/Sema/SemaDeclObjC.cpp test/SemaObjC/class-conforming-protocol-2.m test/SemaObjC/comptypes-a.m test/SemaObjC/method-conflict-1.m test/SemaObjC/method-conflict-2.m test/SemaObjC/method-conflict.m test/SemaObjC/method-def-1.m test/SemaObjC/method-typecheck-3.m
John McCall
rjmccall at apple.com
Wed Oct 27 19:34:39 PDT 2010
Author: rjmccall
Date: Wed Oct 27 21:34:38 2010
New Revision: 117524
URL: http://llvm.org/viewvc/llvm-project?rev=117524&view=rev
Log:
Implement the newest status quo for method override checking. The idea now
is that we need more information to decide the exact conditions for whether
one ObjCObjectPointer is an acceptable return/parameter override for another,
so we're going to disable that entire class of warning for now. The
"forward developement" warning category, -Wmethod-signatures, can receive
unrestricted feature work, and when we're happy with how it acts, we'll
turn it on by default.
This is a pretty conservative change, and nobody's totally content with it.
Modified:
cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
cfe/trunk/lib/Sema/SemaDeclObjC.cpp
cfe/trunk/test/SemaObjC/class-conforming-protocol-2.m
cfe/trunk/test/SemaObjC/comptypes-a.m
cfe/trunk/test/SemaObjC/method-conflict-1.m
cfe/trunk/test/SemaObjC/method-conflict-2.m
cfe/trunk/test/SemaObjC/method-conflict.m
cfe/trunk/test/SemaObjC/method-def-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=117524&r1=117523&r2=117524&view=diff
==============================================================================
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Wed Oct 27 21:34:38 2010
@@ -326,15 +326,15 @@
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">;
Modified: cfe/trunk/lib/Sema/SemaDeclObjC.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclObjC.cpp?rev=117524&r1=117523&r2=117524&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaDeclObjC.cpp (original)
+++ cfe/trunk/lib/Sema/SemaDeclObjC.cpp Wed Oct 27 21:34:38 2010
@@ -782,22 +782,12 @@
/// 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 @@
// 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 @@
// 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);
+}
+
+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;
+ }
+ }
+
+ 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());
+}
+
+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;
+
+ 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;
+
+ 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) {
- 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);
- }
+ CheckMethodOverrideReturn(*this, ImpMethodDecl, IntfMethodDecl);
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;
+ IM != EM; ++IM, ++IF)
+ CheckMethodOverrideParam(*this, ImpMethodDecl, IntfMethodDecl, *IM, *IF);
- // 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.
-
- // 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;
-
- Diag((*IM)->getLocation(), DiagID)
- << ImpMethodDecl->getDeclName() << (*IF)->getType()
- << (*IM)->getType();
- Diag((*IF)->getLocation(), diag::note_previous_definition);
- }
if (ImpMethodDecl->isVariadic() != IntfMethodDecl->isVariadic()) {
Diag(ImpMethodDecl->getLocation(), diag::warn_conflicting_variadic);
Diag(IntfMethodDecl->getLocation(), diag::note_previous_declaration);
Modified: cfe/trunk/test/SemaObjC/class-conforming-protocol-2.m
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaObjC/class-conforming-protocol-2.m?rev=117524&r1=117523&r2=117524&view=diff
==============================================================================
--- cfe/trunk/test/SemaObjC/class-conforming-protocol-2.m (original)
+++ cfe/trunk/test/SemaObjC/class-conforming-protocol-2.m Wed Oct 27 21:34:38 2010
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -fsyntax-only -verify %s
+// RUN: %clang_cc1 -Wmethod-signatures -fsyntax-only -verify %s
@protocol NSWindowDelegate @end
Modified: cfe/trunk/test/SemaObjC/comptypes-a.m
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaObjC/comptypes-a.m?rev=117524&r1=117523&r2=117524&view=diff
==============================================================================
--- cfe/trunk/test/SemaObjC/comptypes-a.m (original)
+++ cfe/trunk/test/SemaObjC/comptypes-a.m Wed Oct 27 21:34:38 2010
@@ -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;
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=117524&r1=117523&r2=117524&view=diff
==============================================================================
--- cfe/trunk/test/SemaObjC/method-conflict-1.m (original)
+++ cfe/trunk/test/SemaObjC/method-conflict-1.m Wed Oct 27 21:34:38 2010
@@ -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
+
+
+
+ at interface A @end
+ at interface B : A @end
+
+ at interface Test1 {}
+- (void) test1:(A*) object; // broken-note {{previous definition is here}}
+- (void) test2:(B*) object;
+ at end
+
+ at implementation Test1
+- (void) test1:(B*) object {} // broken-warning {{conflicting parameter types in implementation of 'test1:': 'A *' vs 'B *'}}
+- (void) test2:(A*) object {}
+ at end
+
+// rdar://problem/8597621 wants id -> A* to be an exception
+ at interface Test2 {}
+- (void) test1:(id) object; // broken-note {{previous definition is here}}
+- (void) test2:(A*) object;
+ at end
+ at implementation Test2
+- (void) test1:(A*) object {} // broken-warning {{conflicting parameter types in implementation of 'test1:': 'id' vs 'A *'}}
+- (void) test2:(id) object {}
+ at end
+
+ at interface Test3 {}
+- (A*) test1;
+- (B*) test2; // broken-note {{previous definition is here}}
+ at end
+
+ at implementation Test3
+- (B*) test1 { return 0; }
+- (A*) test2 { return 0; } // broken-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 -> A* is rdar://problem/8596987
+- (id) test2 { return 0; }
+ at end
Modified: cfe/trunk/test/SemaObjC/method-conflict-2.m
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaObjC/method-conflict-2.m?rev=117524&r1=117523&r2=117524&view=diff
==============================================================================
--- cfe/trunk/test/SemaObjC/method-conflict-2.m (original)
+++ cfe/trunk/test/SemaObjC/method-conflict-2.m Wed Oct 27 21:34:38 2010
@@ -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
Modified: cfe/trunk/test/SemaObjC/method-conflict.m
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaObjC/method-conflict.m?rev=117524&r1=117523&r2=117524&view=diff
==============================================================================
--- cfe/trunk/test/SemaObjC/method-conflict.m (original)
+++ cfe/trunk/test/SemaObjC/method-conflict.m Wed Oct 27 21:34:38 2010
@@ -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;
Modified: cfe/trunk/test/SemaObjC/method-def-1.m
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaObjC/method-def-1.m?rev=117524&r1=117523&r2=117524&view=diff
==============================================================================
--- cfe/trunk/test/SemaObjC/method-def-1.m (original)
+++ cfe/trunk/test/SemaObjC/method-def-1.m Wed Oct 27 21:34:38 2010
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -fsyntax-only -verify %s
+// RUN: %clang_cc1 -Wmethod-signatures -fsyntax-only -verify %s
@interface foo
- (int)meth;
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=117524&r1=117523&r2=117524&view=diff
==============================================================================
--- cfe/trunk/test/SemaObjC/method-typecheck-3.m (original)
+++ cfe/trunk/test/SemaObjC/method-typecheck-3.m Wed Oct 27 21:34:38 2010
@@ -1,8 +1,9 @@
-// RUN: %clang_cc1 -fsyntax-only -verify %s
+// RUN: %clang_cc1 -Wmethod-signatures -fsyntax-only -verify %s
+ at 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:'}}
More information about the cfe-commits
mailing list