Patch for 1) Checking for duplicate methods decls in intterface and category.

2) Use of the new DenseSet<t> abstractions instead of DenseMap<t,char>.

llvm-svn: 42641

6 files changed, 134 insertions, 29 deletions

diff --git a/clang/Sema/Sema.h b/clang/Sema/Sema.h
index 04da4019d34..b497f5e0ffe 100644
--- a/clang/Sema/Sema.h
+++ b/clang/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 <vector>
 #include <string>
 
@@ -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<void *, char>& InsMap,
-                               const llvm::DenseMap<void *, char>& ClsMap);
+                               const llvm::DenseSet<void *>& InsMap,
+                               const llvm::DenseSet<void *>& 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/clang/Sema/SemaDecl.cpp b/clang/Sema/SemaDecl.cpp
index 25dd0c98209..f7d8dd8493b 100644
--- a/clang/Sema/SemaDecl.cpp
+++ b/clang/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<void *, char>& InsMap,
-             const llvm::DenseMap<void *, char>& ClsMap) {
+             const llvm::DenseSet<void *>& InsMap,
+             const llvm::DenseSet<void *>& 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<void *, char> InsMap;
+  llvm::DenseSet<void *> 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<void *, char> ClsMap;
+  llvm::DenseSet<void *> 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<void *, char> InsMap;
+  llvm::DenseSet<void *> 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<void *, char> ClsMap;
+  llvm::DenseSet<void *> 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<p1,p2> 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<ObjcMethodDecl*, 32> insMethods;
   llvm::SmallVector<ObjcMethodDecl*, 16> clsMethods;
-
+  
+  llvm::DenseMap<void *, const ObjcMethodDecl*> InsMap;
+  llvm::DenseMap<void *, const ObjcMethodDecl*> ClsMap;
+  
+  bool isClassDeclaration = 
+        (isa<ObjcInterfaceDecl>(static_cast<Decl *>(ClassDecl))
+         || isa<ObjcCategoryDecl>(static_cast<Decl *>(ClassDecl)));
+  
   for (unsigned i = 0; i < allNum; i++ ) {
     ObjcMethodDecl *Method =
       cast_or_null<ObjcMethodDecl>(static_cast<Decl*>(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<ObjcInterfaceDecl>(static_cast<Decl *>(ClassDecl))) {
     ObjcInterfaceDecl *Interface = cast<ObjcInterfaceDecl>(
diff --git a/clang/clang.xcodeproj/project.pbxproj b/clang/clang.xcodeproj/project.pbxproj
index a96d57f592b..eda2c6bf3e2 100644
--- a/clang/clang.xcodeproj/project.pbxproj
+++ b/clang/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 = "<group>"; };
 		DE67E70E0C020ECF00F66BC5 /* SemaExprCXX.cpp */ = {isa = PBXFileReference; fileEncoding = 30; lastKnownFileType = sourcecode.cpp.cpp; name = SemaExprCXX.cpp; path = Sema/SemaExprCXX.cpp; sourceTree = "<group>"; };
 		DE67E7100C020ED400F66BC5 /* SemaExpr.cpp */ = {isa = PBXFileReference; fileEncoding = 30; lastKnownFileType = sourcecode.cpp.cpp; name = SemaExpr.cpp; path = Sema/SemaExpr.cpp; sourceTree = "<group>"; };
-		DE67E7120C020ED900F66BC5 /* SemaDecl.cpp */ = {isa = PBXFileReference; fileEncoding = 30; lastKnownFileType = sourcecode.cpp.cpp; name = SemaDecl.cpp; path = Sema/SemaDecl.cpp; sourceTree = "<group>"; };
+		DE67E7120C020ED900F66BC5 /* SemaDecl.cpp */ = {isa = PBXFileReference; fileEncoding = 30; indentWidth = 2; lastKnownFileType = sourcecode.cpp.cpp; name = SemaDecl.cpp; path = Sema/SemaDecl.cpp; sourceTree = "<group>"; tabWidth = 8; usesTabs = 0; };
 		DE67E7140C020EDF00F66BC5 /* Sema.h */ = {isa = PBXFileReference; fileEncoding = 30; lastKnownFileType = sourcecode.c.h; name = Sema.h; path = Sema/Sema.h; sourceTree = "<group>"; };
 		DE67E7160C020EE400F66BC5 /* Sema.cpp */ = {isa = PBXFileReference; fileEncoding = 30; lastKnownFileType = sourcecode.cpp.cpp; name = Sema.cpp; path = Sema/Sema.cpp; sourceTree = "<group>"; };
 		DE67E7190C020F4F00F66BC5 /* ASTStreamer.cpp */ = {isa = PBXFileReference; fileEncoding = 30; lastKnownFileType = sourcecode.cpp.cpp; name = ASTStreamer.cpp; path = Sema/ASTStreamer.cpp; sourceTree = "<group>"; };
@@ -306,7 +306,7 @@
 		DEC8D9A30A94346E00353FCA /* AST.h */ = {isa = PBXFileReference; fileEncoding = 30; lastKnownFileType = sourcecode.c.h; name = AST.h; path = clang/AST/AST.h; sourceTree = "<group>"; };
 		DED626C80AE0C065001E80A4 /* TargetInfo.cpp */ = {isa = PBXFileReference; fileEncoding = 30; lastKnownFileType = sourcecode.cpp.cpp; path = TargetInfo.cpp; sourceTree = "<group>"; };
 		DED627020AE0C51D001E80A4 /* Targets.cpp */ = {isa = PBXFileReference; fileEncoding = 30; lastKnownFileType = sourcecode.cpp.cpp; name = Targets.cpp; path = Driver/Targets.cpp; sourceTree = "<group>"; };
-		DED62ABA0AE2EDF1001E80A4 /* Decl.cpp */ = {isa = PBXFileReference; fileEncoding = 30; lastKnownFileType = sourcecode.cpp.cpp; name = Decl.cpp; path = AST/Decl.cpp; sourceTree = "<group>"; usesTabs = 1; };
+		DED62ABA0AE2EDF1001E80A4 /* Decl.cpp */ = {isa = PBXFileReference; fileEncoding = 30; indentWidth = 2; lastKnownFileType = sourcecode.cpp.cpp; name = Decl.cpp; path = AST/Decl.cpp; sourceTree = "<group>"; tabWidth = 8; usesTabs = 0; };
 		DED676D00B6C786700AAD4A3 /* Builtins.def */ = {isa = PBXFileReference; fileEncoding = 30; lastKnownFileType = text; name = Builtins.def; path = clang/AST/Builtins.def; sourceTree = "<group>"; };
 		DED676F90B6C797B00AAD4A3 /* Builtins.h */ = {isa = PBXFileReference; fileEncoding = 30; lastKnownFileType = sourcecode.c.h; name = Builtins.h; path = clang/AST/Builtins.h; sourceTree = "<group>"; };
 		DED677C80B6C854100AAD4A3 /* Builtins.cpp */ = {isa = PBXFileReference; fileEncoding = 30; lastKnownFileType = sourcecode.cpp.cpp; name = Builtins.cpp; path = AST/Builtins.cpp; sourceTree = "<group>"; };
diff --git a/clang/include/clang/AST/DeclObjC.h b/clang/include/clang/AST/DeclObjC.h
index dfbb95c8e0a..ec132cc7e42 100644
--- a/clang/include/clang/AST/DeclObjC.h
+++ b/clang/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/clang/include/clang/Basic/DiagnosticKinds.def b/clang/include/clang/Basic/DiagnosticKinds.def
index 313af0da0e0..389238bf052 100644
--- a/clang/include/clang/Basic/DiagnosticKinds.def
+++ b/clang/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/clang/test/Sema/check-dup-decl-methods-1.m b/clang/test/Sema/check-dup-decl-methods-1.m
new file mode 100644
index 00000000000..1012900e999
--- /dev/null
+++ b/clang/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