From: Fariborz Jahanian Date: Mon, 8 Aug 2011 18:03:17 +0000 (+0000) Subject: objective-c: diagnose protocol inconsistencies in following X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=2112190efa85f50af84a3c4efe03c5bf69247ba2;p=clang objective-c: diagnose protocol inconsistencies in following situation. When a class explicitly or implicitly (through inheritance) "conformsTo" two protocols which conflict (have methods which conflict). This patch fixes the previous patch where warnings were coming out in non-deterministic order. This is 2nd part of // rdar://6191214. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@137055 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/include/clang/Basic/IdentifierTable.h b/include/clang/Basic/IdentifierTable.h index 017af5caee..c9fa74ce42 100644 --- a/include/clang/Basic/IdentifierTable.h +++ b/include/clang/Basic/IdentifierTable.h @@ -557,6 +557,11 @@ public: bool operator!=(Selector RHS) const { return InfoPtr != RHS.InfoPtr; } + + bool operator < (Selector RHS) const { + return InfoPtr < RHS.InfoPtr; + } + void *getAsOpaquePtr() const { return reinterpret_cast(InfoPtr); } diff --git a/include/clang/Sema/Sema.h b/include/clang/Sema/Sema.h index d13edfa998..90d0195cff 100644 --- a/include/clang/Sema/Sema.h +++ b/include/clang/Sema/Sema.h @@ -493,6 +493,39 @@ public: /// a potentially evaluated expression. typedef SmallVector, 10> PotentiallyReferencedDecls; + + // FIXME. Improve on accessibility. + class PROTOCOL_METHODS { + public: + Selector Sel; + ObjCMethodDecl *Method; + PROTOCOL_METHODS(Selector S, ObjCMethodDecl *M) + : Sel(S), Method(M) {} + // Allow sorting based on selector's opaque pointer. + bool operator<(const PROTOCOL_METHODS &b) const { + return Sel < b.Sel; + } + }; + + /// \brief The set of protocols declared in protocols qualifying a + /// class. + typedef SmallVector MethodsInProtocols; + + class IDENTICAL_SELECTOR_METHODS { + public: + SourceLocation Loc; + ObjCMethodDecl *Method; + IDENTICAL_SELECTOR_METHODS(SourceLocation L, ObjCMethodDecl *M) + : Loc(L), Method(M) {} + // Allow sorting based on selector's source location. + bool operator<(const IDENTICAL_SELECTOR_METHODS &i) const { + return !(Loc < i.Loc); + } + }; + + /// \brief Methods with identical selectors to be type-matched against + /// one another. + typedef SmallVector IdenticalSelectorMethods; /// \brief A set of diagnostics that may be emitted. typedef SmallVector, 10> @@ -1780,7 +1813,12 @@ public: void WarnExactTypedMethods(ObjCMethodDecl *Method, ObjCMethodDecl *MethodDecl, bool IsProtocolMethodDecl); - + + /// WarnOnMismatchedProtocolMethods - Issues warning on type mismatched + /// protocols methods and then returns true(matched), or false(mismatched). + bool WarnOnMismatchedProtocolMethods(ObjCMethodDecl *Method, + ObjCMethodDecl *MethodDecl); + bool isPropertyReadonly(ObjCPropertyDecl *PropertyDecl, ObjCInterfaceDecl *IDecl); @@ -1904,10 +1942,16 @@ public: bool ImmediateClass, bool WarnExactMatch=false); + /// MatchIdenticalSelectorsInProtocols - Check that mathods with + /// identical selectors in all protocols of this class type match. + /// Issue warning if they don't. + void MatchIdenticalSelectorsInProtocols(const ObjCInterfaceDecl *CDecl); + /// MatchMethodsInClassAndItsProtocol - Check that any redeclaration of /// method in protocol in its qualified class match in their type and /// issue warnings otherwise. void MatchMethodsInClassAndItsProtocol(const ObjCInterfaceDecl *CDecl); + /// CheckCategoryVsClassMethodMatches - Checks that methods implemented in /// category matches with those implemented in its primary class and diff --git a/lib/Sema/SemaDeclObjC.cpp b/lib/Sema/SemaDeclObjC.cpp index 87948548a4..084ca0b77b 100644 --- a/lib/Sema/SemaDeclObjC.cpp +++ b/lib/Sema/SemaDeclObjC.cpp @@ -1281,6 +1281,35 @@ void Sema::WarnConflictingTypedMethods(ObjCMethodDecl *ImpMethodDecl, } } +/// WarnOnMismatchedProtocolMethods - Issues warning on type mismatched +/// protocols methods and then returns true(matched), or false(mismatched). +bool Sema::WarnOnMismatchedProtocolMethods(ObjCMethodDecl *ImpMethodDecl, + ObjCMethodDecl *MethodDecl) { + + bool match = CheckMethodOverrideReturn(*this, ImpMethodDecl, MethodDecl, + true, + true, true); + if (!match) + return false; + + for (ObjCMethodDecl::param_iterator IM = ImpMethodDecl->param_begin(), + IF = MethodDecl->param_begin(), EM = ImpMethodDecl->param_end(); + IM != EM; ++IM, ++IF) { + match = CheckMethodOverrideParam(*this, ImpMethodDecl, MethodDecl, *IM, *IF, + true, true, true); + if (!match) + return false; + } + + if (ImpMethodDecl->isVariadic() != MethodDecl->isVariadic()) { + Diag(ImpMethodDecl->getLocation(), diag::warn_conflicting_variadic) + << true; + Diag(MethodDecl->getLocation(), diag::note_previous_declaration); + return false; + } + return true; +} + /// WarnExactTypedMethods - This routine issues a warning if method /// implementation declaration matches exactly that of its declaration. void Sema::WarnExactTypedMethods(ObjCMethodDecl *ImpMethodDecl, @@ -1568,6 +1597,41 @@ static void CollectMethodsInOneProtocol(const ObjCProtocolDecl *PDecl, } } +/// CollectAllMethodsInProtocols - Helper routine to collect all methods +/// declared in given class's immediate and nested protocols. +static void CollectAllMethodsInProtocols(const ObjCContainerDecl *ContDecl, + Sema::MethodsInProtocols &InstMethodsInProtocols, + Sema::MethodsInProtocols & ClsMethodsInProtocols) { + if (const ObjCInterfaceDecl *CDecl = dyn_cast(ContDecl)) { + for (ObjCInterfaceDecl::all_protocol_iterator + PI = CDecl->all_referenced_protocol_begin(), + E = CDecl->all_referenced_protocol_end(); PI != E; ++PI) + CollectAllMethodsInProtocols(*PI, InstMethodsInProtocols, + ClsMethodsInProtocols); + } + + if (const ObjCProtocolDecl *PDecl = dyn_cast(ContDecl)) { + for (ObjCProtocolDecl::instmeth_iterator I = PDecl->instmeth_begin(), + E = PDecl->instmeth_end(); I != E; ++I) { + ObjCMethodDecl *method = *I; + InstMethodsInProtocols.push_back(Sema::PROTOCOL_METHODS(method->getSelector(), + method)); + } + for (ObjCProtocolDecl::classmeth_iterator I = PDecl->classmeth_begin(), + E = PDecl->classmeth_end(); I != E; ++I) { + ObjCMethodDecl *method = *I; + ClsMethodsInProtocols.push_back(Sema::PROTOCOL_METHODS(method->getSelector(), + method)); + } + + for (ObjCProtocolDecl::protocol_iterator + PI = PDecl->protocol_begin(), + E = PDecl->protocol_end(); PI != E; ++PI) + CollectAllMethodsInProtocols(*PI, InstMethodsInProtocols, + ClsMethodsInProtocols); + } +} + /// CollectMethodsInProtocols - This routine collects all methods declared /// in class's list and nested qualified protocols. Instance methods and /// class methods have separate containers as they have identical selectors. @@ -1612,6 +1676,77 @@ void Sema::MatchMethodsInClassAndItsProtocol(const ObjCInterfaceDecl *CDecl) { ClsMethodsInProtocols); } +/// MatchMethodsWithIdenticalSelectors - Helper routine to go through list +/// of identical selector lists and issue warning for any type mismatche +/// of these methods. +static bool MatchMethodsWithIdenticalSelectors(Sema &S, + const Sema::MethodsInProtocols Methods) { + bool res = true; + int size = Methods.size(); + int i = 0; + while (i < size) { + int upper = i; + while (upper < size && + (Methods[i].Sel == Methods[upper].Sel)) + upper++; + if (upper > i) { + int lo = i; + int hi = upper - 1; + + if (lo < hi) { + Sema::IdenticalSelectorMethods SelectedMethods; + for (int l = lo; l <= hi; l++) { + ObjCMethodDecl *method = Methods[l].Method; + Sema::IDENTICAL_SELECTOR_METHODS + SelectedMethod(method->getLocation(), method); + SelectedMethods.push_back(SelectedMethod); + } + llvm::array_pod_sort(SelectedMethods.begin(), SelectedMethods.end()); + lo = 0; hi = SelectedMethods.size()-1; + while (lo < hi) { + ObjCMethodDecl *targetMethod = SelectedMethods[lo].Method; + for (int j = lo+1; j <= hi; j++) { + // match two methods; + ObjCMethodDecl *otherMethod = SelectedMethods[j].Method; + if (!S.WarnOnMismatchedProtocolMethods(targetMethod, otherMethod)) + res = false; + } + ++lo; + } + } + } + i += upper; + } + return res; +} + +/// MatchIdenticalSelectorsInProtocols - Main routine to go through list of +/// class's protocols (and their protocols) and make sure that methods +/// type match across all protocols and issue warnings if they don't. +/// FIXME. This may move to static analyzer if performance is proven +/// prohibitive. +void Sema::MatchIdenticalSelectorsInProtocols(const ObjCInterfaceDecl *CDecl) { + Sema::MethodsInProtocols InsMethods; + Sema::MethodsInProtocols ClsMethods; + CollectAllMethodsInProtocols(CDecl, InsMethods, ClsMethods); + + bool match = true; + if (!InsMethods.empty()) { + llvm::array_pod_sort(InsMethods.begin(), InsMethods.end()); + if (!MatchMethodsWithIdenticalSelectors(*this, InsMethods)) + match = false; + } + + if (!ClsMethods.empty()) { + llvm::array_pod_sort(ClsMethods.begin(), ClsMethods.end()); + if (!MatchMethodsWithIdenticalSelectors(*this, ClsMethods)) + match = false; + } + if (!match) + Diag(CDecl->getLocation() ,diag::note_class_declared); +} + + /// CheckCategoryVsClassMethodMatches - Checks that methods implemented in /// category matches with those implemented in its primary class and /// warns each time an exact match is found. @@ -1678,8 +1813,10 @@ void Sema::ImplMethodsVsClassMethods(Scope *S, ObjCImplDecl* IMPDecl, // and methods declared in protocol. Do this only when the class // is being implementaed. if (isa(IMPDecl)) - if (const ObjCInterfaceDecl *I = dyn_cast(CDecl)) + if (const ObjCInterfaceDecl *I = dyn_cast(CDecl)) { + MatchIdenticalSelectorsInProtocols(I); MatchMethodsInClassAndItsProtocol(I); + } // check all methods implemented in category against those declared // in its primary class. diff --git a/test/SemaObjC/class-protocol-method-match.m b/test/SemaObjC/class-protocol-method-match.m index bffdb79e07..4022ce3aee 100644 --- a/test/SemaObjC/class-protocol-method-match.m +++ b/test/SemaObjC/class-protocol-method-match.m @@ -4,7 +4,7 @@ @protocol Bar @required - (unsigned char) baz; // expected-note {{previous definition is here}} -- (char) ok; +- (char) ok; // expected-note {{previous definition is here}} - (void) also_ok; @end @@ -17,11 +17,11 @@ @protocol Baz - (void) bar : (unsigned char)arg; // expected-note {{previous definition is here}} -- (void) ok; +- (void) ok; // expected-warning {{conflicting return type in declaration of 'ok': 'char' vs 'void'}} - (char) bak; // expected-note {{previous definition is here}} @end -@interface Foo +@interface Foo // expected-note {{class is declared here}} - (void) baz; // expected-warning {{conflicting return type in declaration of 'baz': 'unsigned char' vs 'void'}} - (void) bar : (unsigned char*)arg; // expected-warning {{conflicting parameter types in declaration of 'bar:': 'unsigned char' vs 'unsigned char *'}} - (void) ok; diff --git a/test/SemaObjC/qualified-protocol-method-conflicts.m b/test/SemaObjC/qualified-protocol-method-conflicts.m new file mode 100644 index 0000000000..da83c8338c --- /dev/null +++ b/test/SemaObjC/qualified-protocol-method-conflicts.m @@ -0,0 +1,39 @@ +// RUN: %clang_cc1 -fsyntax-only -verify %s +// rdar://6191214 + +@protocol Xint +-(void) setX: (int) arg0; // expected-note 3 {{previous definition is here}} ++(int) C; // expected-note 3 {{previous definition is here}} +@end + +@protocol Xfloat +-(void) setX: (float) arg0; // expected-warning 3 {{conflicting parameter types in declaration of 'setX:': 'int' vs 'float'}} ++(float) C; // expected-warning 3 {{conflicting return type in declaration of 'C': 'int' vs 'float'}} +@end + +@interface A // expected-note {{class is declared here}} +@end + +@implementation A +-(void) setX: (int) arg0 { } ++(int) C {return 0; } +@end + +@interface B // expected-note {{class is declared here}} +@end + +@implementation B +-(void) setX: (float) arg0 { } ++ (float) C {return 0.0; } +@end + +@protocol Xint_float +@end + +@interface C // expected-note {{class is declared here}} +@end + +@implementation C +-(void) setX: (int) arg0 { } ++ (int) C {return 0;} +@end