From 85ff2646c1aeedd1105f867a5bba8a58febd1d2a Mon Sep 17 00:00:00 2001 From: Fariborz Jahanian Date: Fri, 5 Oct 2007 18:00:57 +0000 Subject: [PATCH] Patch for 1) Checking for duplicate methods decls in intterface and category. 2) Use of the new DenseSet abstractions instead of DenseMap. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@42641 91177308-0d34-0410-b5e6-96231b3b80d8 --- Sema/Sema.h | 10 ++- Sema/SemaDecl.cpp | 107 ++++++++++++++++++------ clang.xcodeproj/project.pbxproj | 4 +- include/clang/AST/DeclObjC.h | 6 ++ include/clang/Basic/DiagnosticKinds.def | 4 + test/Sema/check-dup-decl-methods-1.m | 32 +++++++ 6 files changed, 134 insertions(+), 29 deletions(-) create mode 100644 test/Sema/check-dup-decl-methods-1.m diff --git a/Sema/Sema.h b/Sema/Sema.h index 04da4019d3..b497f5e0ff 100644 --- a/Sema/Sema.h +++ b/Sema/Sema.h @@ -18,6 +18,7 @@ #include "clang/Parse/Action.h" #include "llvm/ADT/DenseMap.h" #include "llvm/ADT/SmallVector.h" +#include "llvm/ADT/DenseSet.h" #include #include @@ -52,6 +53,7 @@ namespace clang { class ObjcCategoryImplDecl; class ObjcCategoryDecl; class ObjcIvarDecl; + class ObjcMethodDecl; /// Sema - This implements semantic analysis and AST building for C. class Sema : public Action { @@ -214,8 +216,8 @@ private: /// Declared in protocol, and those referenced by it. void CheckProtocolMethodDefs(ObjcProtocolDecl *PDecl, bool& IncompleteImpl, - const llvm::DenseMap& InsMap, - const llvm::DenseMap& ClsMap); + const llvm::DenseSet& InsMap, + const llvm::DenseSet& ClsMap); /// CheckImplementationIvars - This routine checks if the instance variables /// listed in the implelementation match those listed in the interface. @@ -231,6 +233,10 @@ private: /// category interface is implemented in the category @implementation. void ImplCategoryMethodsVsIntfMethods(ObjcCategoryImplDecl *CatImplDecl, ObjcCategoryDecl *CatClassDecl); + /// MatchTwoMethodDeclarations - Checks if two methods' type match and returns + /// true, or false, accordingly. + bool MatchTwoMethodDeclarations(const ObjcMethodDecl *Method, + const ObjcMethodDecl *PrevMethod); //===--------------------------------------------------------------------===// // Statement Parsing Callbacks: SemaStmt.cpp. diff --git a/Sema/SemaDecl.cpp b/Sema/SemaDecl.cpp index 25dd0c9820..f7d8dd8493 100644 --- a/Sema/SemaDecl.cpp +++ b/Sema/SemaDecl.cpp @@ -25,6 +25,7 @@ #include "clang/Basic/TargetInfo.h" #include "llvm/ADT/SmallString.h" #include "llvm/ADT/SmallSet.h" +#include "llvm/ADT/DenseSet.h" using namespace clang; Sema::DeclTy *Sema::isTypeName(const IdentifierInfo &II, Scope *S) const { @@ -1219,17 +1220,19 @@ void Sema::CheckImplementationIvars(ObjcImplementationDecl *ImpDecl, /// Declared in protocol, and those referenced by it. void Sema::CheckProtocolMethodDefs(ObjcProtocolDecl *PDecl, bool& IncompleteImpl, - const llvm::DenseMap& InsMap, - const llvm::DenseMap& ClsMap) { + const llvm::DenseSet& InsMap, + const llvm::DenseSet& ClsMap) { // check unimplemented instance methods. ObjcMethodDecl** methods = PDecl->getInstanceMethods(); - for (int j = 0; j < PDecl->getNumInstanceMethods(); j++) - if (!InsMap.count(methods[j]->getSelector().getAsOpaquePtr())) { + for (int j = 0; j < PDecl->getNumInstanceMethods(); j++) { + void * cpv = methods[j]->getSelector().getAsOpaquePtr(); + if (!InsMap.count(cpv)) { llvm::SmallString<128> buf; Diag(methods[j]->getLocation(), diag::warn_undef_method_impl, methods[j]->getSelector().getName(buf)); IncompleteImpl = true; } + } // check unimplemented class methods methods = PDecl->getClassMethods(); for (int j = 0; j < PDecl->getNumClassMethods(); j++) @@ -1248,13 +1251,12 @@ void Sema::CheckProtocolMethodDefs(ObjcProtocolDecl *PDecl, void Sema::ImplMethodsVsClassMethods(ObjcImplementationDecl* IMPDecl, ObjcInterfaceDecl* IDecl) { - llvm::DenseMap InsMap; + llvm::DenseSet InsMap; // Check and see if instance methods in class interface have been // implemented in the implementation class. ObjcMethodDecl **methods = IMPDecl->getInstanceMethods(); - for (int i=0; i < IMPDecl->getNumInstanceMethods(); i++) { - InsMap[methods[i]->getSelector().getAsOpaquePtr()] = 'a'; - } + for (int i=0; i < IMPDecl->getNumInstanceMethods(); i++) + InsMap.insert(methods[i]->getSelector().getAsOpaquePtr()); bool IncompleteImpl = false; methods = IDecl->getInstanceMethods(); @@ -1265,13 +1267,12 @@ void Sema::ImplMethodsVsClassMethods(ObjcImplementationDecl* IMPDecl, methods[j]->getSelector().getName(buf)); IncompleteImpl = true; } - llvm::DenseMap ClsMap; + llvm::DenseSet ClsMap; // Check and see if class methods in class interface have been // implemented in the implementation class. methods = IMPDecl->getClassMethods(); - for (int i=0; i < IMPDecl->getNumClassMethods(); i++) { - ClsMap[methods[i]->getSelector().getAsOpaquePtr()] = 'a'; - } + for (int i=0; i < IMPDecl->getNumClassMethods(); i++) + ClsMap.insert(methods[i]->getSelector().getAsOpaquePtr()); methods = IDecl->getClassMethods(); for (int j = 0; j < IDecl->getNumClassMethods(); j++) @@ -1298,13 +1299,12 @@ void Sema::ImplMethodsVsClassMethods(ObjcImplementationDecl* IMPDecl, /// category interface is implemented in the category @implementation. void Sema::ImplCategoryMethodsVsIntfMethods(ObjcCategoryImplDecl *CatImplDecl, ObjcCategoryDecl *CatClassDecl) { - llvm::DenseMap InsMap; + llvm::DenseSet InsMap; // Check and see if instance methods in category interface have been // implemented in its implementation class. ObjcMethodDecl **methods = CatImplDecl->getInstanceMethods(); - for (int i=0; i < CatImplDecl->getNumInstanceMethods(); i++) { - InsMap[methods[i]->getSelector().getAsOpaquePtr()] = 'a'; - } + for (int i=0; i < CatImplDecl->getNumInstanceMethods(); i++) + InsMap.insert(methods[i]->getSelector().getAsOpaquePtr()); bool IncompleteImpl = false; methods = CatClassDecl->getInstanceMethods(); @@ -1315,13 +1315,12 @@ void Sema::ImplCategoryMethodsVsIntfMethods(ObjcCategoryImplDecl *CatImplDecl, methods[j]->getSelector().getName(buf)); IncompleteImpl = true; } - llvm::DenseMap ClsMap; + llvm::DenseSet ClsMap; // Check and see if class methods in category interface have been // implemented in its implementation class. methods = CatImplDecl->getClassMethods(); - for (int i=0; i < CatImplDecl->getNumClassMethods(); i++) { - ClsMap[methods[i]->getSelector().getAsOpaquePtr()] = 'a'; - } + for (int i=0; i < CatImplDecl->getNumClassMethods(); i++) + ClsMap.insert(methods[i]->getSelector().getAsOpaquePtr()); methods = CatClassDecl->getClassMethods(); for (int j = 0; j < CatClassDecl->getNumClassMethods(); j++) @@ -1684,6 +1683,23 @@ void Sema::ActOnFields(Scope* S, } } +/// MatchTwoMethodDeclarations - Checks that two methods have matching type and +/// returns true, or false, accordingly. +/// TODO: Handle protocol list; such as id in type comparisons +bool Sema:: MatchTwoMethodDeclarations(const ObjcMethodDecl *Method, + const ObjcMethodDecl *PrevMethod) { + if (Method->getMethodType().getCanonicalType() != + PrevMethod->getMethodType().getCanonicalType()) + return false; + for (int i = 0; i < Method->getNumParams(); i++) { + ParmVarDecl *ParamDecl = Method->getParamDecl(i); + ParmVarDecl *PrevParamDecl = PrevMethod->getParamDecl(i); + if (ParamDecl->getCanonicalType() != PrevParamDecl->getCanonicalType()) + return false; + } + return true; +} + void Sema::ActOnAddMethodsToObjcDecl(Scope* S, DeclTy *ClassDecl, DeclTy **allMethods, unsigned allNum) { // FIXME: Fix this when we can handle methods declared in protocols. @@ -1692,15 +1708,56 @@ void Sema::ActOnAddMethodsToObjcDecl(Scope* S, DeclTy *ClassDecl, return; llvm::SmallVector insMethods; llvm::SmallVector clsMethods; - + + llvm::DenseMap InsMap; + llvm::DenseMap ClsMap; + + bool isClassDeclaration = + (isa(static_cast(ClassDecl)) + || isa(static_cast(ClassDecl))); + for (unsigned i = 0; i < allNum; i++ ) { ObjcMethodDecl *Method = cast_or_null(static_cast(allMethods[i])); if (!Method) continue; // Already issued a diagnostic. - if (Method->isInstance()) - insMethods.push_back(Method); - else - clsMethods.push_back(Method); + if (Method->isInstance()) { + if (isClassDeclaration) { + /// Check for instance method of the same name with incompatible types + const ObjcMethodDecl *&PrevMethod = + InsMap[Method->getSelector().getAsOpaquePtr()]; + if (PrevMethod && !MatchTwoMethodDeclarations(Method, PrevMethod)) { + llvm::SmallString<128> buf; + Diag(Method->getLocation(), diag::error_duplicate_method_decl, + Method->getSelector().getName(buf)); + Diag(PrevMethod->getLocation(), diag::err_previous_declaration); + } + else { + insMethods.push_back(Method); + InsMap[Method->getSelector().getAsOpaquePtr()] = Method; + } + } + else + insMethods.push_back(Method); + } + else { + if (isClassDeclaration) { + /// Check for class method of the same name with incompatible types + const ObjcMethodDecl *&PrevMethod = + ClsMap[Method->getSelector().getAsOpaquePtr()]; + if (PrevMethod && !MatchTwoMethodDeclarations(Method, PrevMethod)) { + llvm::SmallString<128> buf; + Diag(Method->getLocation(), diag::error_duplicate_method_decl, + Method->getSelector().getName(buf)); + Diag(PrevMethod->getLocation(), diag::err_previous_declaration); + } + else { + clsMethods.push_back(Method); + ClsMap[Method->getSelector().getAsOpaquePtr()] = Method; + } + } + else + clsMethods.push_back(Method); + } } if (isa(static_cast(ClassDecl))) { ObjcInterfaceDecl *Interface = cast( diff --git a/clang.xcodeproj/project.pbxproj b/clang.xcodeproj/project.pbxproj index a96d57f592..eda2c6bf3e 100644 --- a/clang.xcodeproj/project.pbxproj +++ b/clang.xcodeproj/project.pbxproj @@ -280,7 +280,7 @@ DE67E70C0C020ECA00F66BC5 /* SemaStmt.cpp */ = {isa = PBXFileReference; fileEncoding = 30; lastKnownFileType = sourcecode.cpp.cpp; name = SemaStmt.cpp; path = Sema/SemaStmt.cpp; sourceTree = ""; }; DE67E70E0C020ECF00F66BC5 /* SemaExprCXX.cpp */ = {isa = PBXFileReference; fileEncoding = 30; lastKnownFileType = sourcecode.cpp.cpp; name = SemaExprCXX.cpp; path = Sema/SemaExprCXX.cpp; sourceTree = ""; }; DE67E7100C020ED400F66BC5 /* SemaExpr.cpp */ = {isa = PBXFileReference; fileEncoding = 30; lastKnownFileType = sourcecode.cpp.cpp; name = SemaExpr.cpp; path = Sema/SemaExpr.cpp; sourceTree = ""; }; - DE67E7120C020ED900F66BC5 /* SemaDecl.cpp */ = {isa = PBXFileReference; fileEncoding = 30; lastKnownFileType = sourcecode.cpp.cpp; name = SemaDecl.cpp; path = Sema/SemaDecl.cpp; sourceTree = ""; }; + DE67E7120C020ED900F66BC5 /* SemaDecl.cpp */ = {isa = PBXFileReference; fileEncoding = 30; indentWidth = 2; lastKnownFileType = sourcecode.cpp.cpp; name = SemaDecl.cpp; path = Sema/SemaDecl.cpp; sourceTree = ""; tabWidth = 8; usesTabs = 0; }; DE67E7140C020EDF00F66BC5 /* Sema.h */ = {isa = PBXFileReference; fileEncoding = 30; lastKnownFileType = sourcecode.c.h; name = Sema.h; path = Sema/Sema.h; sourceTree = ""; }; DE67E7160C020EE400F66BC5 /* Sema.cpp */ = {isa = PBXFileReference; fileEncoding = 30; lastKnownFileType = sourcecode.cpp.cpp; name = Sema.cpp; path = Sema/Sema.cpp; sourceTree = ""; }; DE67E7190C020F4F00F66BC5 /* ASTStreamer.cpp */ = {isa = PBXFileReference; fileEncoding = 30; lastKnownFileType = sourcecode.cpp.cpp; name = ASTStreamer.cpp; path = Sema/ASTStreamer.cpp; sourceTree = ""; }; @@ -306,7 +306,7 @@ DEC8D9A30A94346E00353FCA /* AST.h */ = {isa = PBXFileReference; fileEncoding = 30; lastKnownFileType = sourcecode.c.h; name = AST.h; path = clang/AST/AST.h; sourceTree = ""; }; DED626C80AE0C065001E80A4 /* TargetInfo.cpp */ = {isa = PBXFileReference; fileEncoding = 30; lastKnownFileType = sourcecode.cpp.cpp; path = TargetInfo.cpp; sourceTree = ""; }; DED627020AE0C51D001E80A4 /* Targets.cpp */ = {isa = PBXFileReference; fileEncoding = 30; lastKnownFileType = sourcecode.cpp.cpp; name = Targets.cpp; path = Driver/Targets.cpp; sourceTree = ""; }; - DED62ABA0AE2EDF1001E80A4 /* Decl.cpp */ = {isa = PBXFileReference; fileEncoding = 30; lastKnownFileType = sourcecode.cpp.cpp; name = Decl.cpp; path = AST/Decl.cpp; sourceTree = ""; usesTabs = 1; }; + DED62ABA0AE2EDF1001E80A4 /* Decl.cpp */ = {isa = PBXFileReference; fileEncoding = 30; indentWidth = 2; lastKnownFileType = sourcecode.cpp.cpp; name = Decl.cpp; path = AST/Decl.cpp; sourceTree = ""; tabWidth = 8; usesTabs = 0; }; DED676D00B6C786700AAD4A3 /* Builtins.def */ = {isa = PBXFileReference; fileEncoding = 30; lastKnownFileType = text; name = Builtins.def; path = clang/AST/Builtins.def; sourceTree = ""; }; DED676F90B6C797B00AAD4A3 /* Builtins.h */ = {isa = PBXFileReference; fileEncoding = 30; lastKnownFileType = sourcecode.c.h; name = Builtins.h; path = clang/AST/Builtins.h; sourceTree = ""; }; DED677C80B6C854100AAD4A3 /* Builtins.cpp */ = {isa = PBXFileReference; fileEncoding = 30; lastKnownFileType = sourcecode.cpp.cpp; name = Builtins.cpp; path = AST/Builtins.cpp; sourceTree = ""; }; diff --git a/include/clang/AST/DeclObjC.h b/include/clang/AST/DeclObjC.h index dfbb95c8e0..ec132cc7e4 100644 --- a/include/clang/AST/DeclObjC.h +++ b/include/clang/AST/DeclObjC.h @@ -242,6 +242,12 @@ public: assert(i < getNumMethodParams() && "Illegal param #"); return ParamInfo[i]; } + + int getNumParams() const { return NumMethodParams; } + ParmVarDecl *getParamDecl(int i) const { + assert(i < getNumParams() && "Illegal param #"); + return ParamInfo[i]; + } void setMethodParams(ParmVarDecl **NewParamInfo, unsigned NumParams); AttributeList *getMethodAttrs() const {return MethodAttrs;} diff --git a/include/clang/Basic/DiagnosticKinds.def b/include/clang/Basic/DiagnosticKinds.def index 313af0da0e..389238bf05 100644 --- a/include/clang/Basic/DiagnosticKinds.def +++ b/include/clang/Basic/DiagnosticKinds.def @@ -438,6 +438,10 @@ DIAG(warn_incomplete_impl_class, WARNING, "incomplete implementation of class '%0'") DIAG(warn_incomplete_impl_category, WARNING, "incomplete implementation of category '%0'") +DIAG(error_duplicate_method_decl, ERROR, + "duplicate declaration of method '%0'") +DIAG(err_previous_declaration, ERROR, + "previous declaration is here") //===----------------------------------------------------------------------===// diff --git a/test/Sema/check-dup-decl-methods-1.m b/test/Sema/check-dup-decl-methods-1.m new file mode 100644 index 0000000000..1012900e99 --- /dev/null +++ b/test/Sema/check-dup-decl-methods-1.m @@ -0,0 +1,32 @@ +// RUN: clang -fsyntax-only -verify %s + +@interface SUPER +- (int) meth; ++ (int) foobar; +@end + +@interface T @end + +@interface class1 : SUPER +- (int) meth; // expected-error {{previous declaration is here}} +- (int*) meth; // expected-error {{duplicate declaration of method 'meth'}} +- (T*) meth1; +- (T*) meth1; ++ (T*) meth1; +@end + +@interface class1(cat) +- (int) catm : (char)ch1; // expected-error {{previous declaration is here}} +- (int) catm1 : (char)ch : (int)i; +- (int) catm : (char*)ch1; // expected-error {{duplicate declaration of method 'catm:'}} ++ (int) catm1 : (char)ch : (int)i; ++ (T*) meth1; +@end + +@interface class1(cat1) ++ (int) catm1 : (char)ch : (int)i; // expected-error {{previous declaration is here}} ++ (T*) meth1; // expected-error {{previous declaration is here}} ++ (int) catm1 : (char)ch : (int*)i; // expected-error {{duplicate declaration of method 'catm1::'}} ++ (T**) meth1; // expected-error {{duplicate declaration of method 'meth1'}} ++ (int) foobar; +@end -- 2.40.0