This is the last 5% of the solution to teaching Sema::ActOnInstanceMessage() about private methods (r43989).

While the diff is large, the idea is very simple. When we parse method definitions (in an @implementation), we need to add them incrementally (rather than wait until the @end).

Other details...

- Renamed Sema::ActOnAddMethodsToObjcDecl() to Sema::ActOnAtEnd(). The methods are now optional arguments.
- Removed Parser::AllImplMethods (a nice cleanup).
- Added location info to ObjcImplementationDecl (since we will need it very soon:-)
- Modified message.m test to no longer allow the bogus diagnostic.



git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@43995 91177308-0d34-0410-b5e6-96231b3b80d8

AST/Decl.cpp

@@ -408,26 +408,6 @@ void ObjcCategoryImplDecl::addMethods(ObjcMethodDecl **insMethods,
   }
 }
 
-/// ObjcAddImplMethods - Insert instance and methods declarations into
-/// ObjcImplementationDecl's InsMethods and ClsMethods fields.
-///
-void ObjcImplementationDecl::addMethods(ObjcMethodDecl **insMethods, 
-                                        unsigned numInsMembers,
-                                        ObjcMethodDecl **clsMethods,
-                                        unsigned numClsMembers,
-                                        SourceLocation AtEndLoc) {
-  NumInstanceMethods = numInsMembers;
-  if (numInsMembers) {
-    InstanceMethods = new ObjcMethodDecl*[numInsMembers];
-    memcpy(InstanceMethods, insMethods, numInsMembers*sizeof(ObjcMethodDecl*));
-  }
-  NumClassMethods = numClsMembers;
-  if (numClsMembers) {
-    ClassMethods = new ObjcMethodDecl*[numClsMembers];
-    memcpy(ClassMethods, clsMethods, numClsMembers*sizeof(ObjcMethodDecl*));
-  }
-}
-
 // lookupInstanceMethod - This method returns an instance method by looking in
 // the class, it's categories, and it's super classes (using a linear search).
 ObjcMethodDecl *ObjcInterfaceDecl::lookupInstanceMethod(Selector &Sel) {
@@ -514,7 +494,7 @@ ObjcMethodDecl *ObjcInterfaceDecl::lookupClassMethod(Selector &Sel) {
 // the class implementation. Unlike interfaces, we don't look outside the
 // implementation.
 ObjcMethodDecl *ObjcImplementationDecl::lookupInstanceMethod(Selector &Sel) {
-  ObjcMethodDecl **methods = getInstanceMethods();
+  ObjcMethodDecl *const*methods = getInstanceMethods();
   int methodCount = getNumInstanceMethods();
   for (int i = 0; i < methodCount; ++i) {
     if (methods[i]->getSelector() == Sel) {
@@ -528,7 +508,7 @@ ObjcMethodDecl *ObjcImplementationDecl::lookupInstanceMethod(Selector &Sel) {
 // the class implementation. Unlike interfaces, we don't look outside the
 // implementation.
 ObjcMethodDecl *ObjcImplementationDecl::lookupClassMethod(Selector &Sel) {
-  ObjcMethodDecl **methods = getClassMethods();
+  ObjcMethodDecl *const*methods = getClassMethods();
   int methodCount = getNumClassMethods();
   for (int i = 0; i < methodCount; ++i) {
     if (methods[i]->getSelector() == Sel) {

Driver/RewriteTest.cpp

@@ -118,7 +118,7 @@ namespace {
     void RewriteObjcCategoryImplDecl(ObjcCategoryImplDecl *CDecl,
                                      std::string &Result);
     
-    void RewriteObjcMethodsMetaData(ObjcMethodDecl **Methods,
+    void RewriteObjcMethodsMetaData(ObjcMethodDecl *const*Methods,
                                     int NumMethods,
                                     bool IsInstanceMethod,
                                     const char *prefix,
@@ -1039,7 +1039,7 @@ void RewriteTest::SynthesizeObjcInternalStruct(ObjcInterfaceDecl *CDecl,
 
 // RewriteObjcMethodsMetaData - Rewrite methods metadata for instance or
 /// class methods.
-void RewriteTest::RewriteObjcMethodsMetaData(ObjcMethodDecl **Methods,
+void RewriteTest::RewriteObjcMethodsMetaData(ObjcMethodDecl *const*Methods,
                                              int NumMethods,
                                              bool IsInstanceMethod,
                                              const char *prefix,

Parse/ParseObjc.cpp

@@ -275,12 +275,8 @@ void Parser::ParseObjCInterfaceDeclList(DeclTy *interfaceDecl,
     }
   }
   /// Insert collected methods declarations into the @interface object.
-  /// This action is executed even if we don't have any methods (so the @end
-  /// can be recorded properly).
-  Actions.ActOnAddMethodsToObjcDecl(CurScope, interfaceDecl, &allMethods[0], 
-                                    allMethods.size(), 
-                                    &allProperties[0], allProperties.size(), 
-                                    AtEndLoc);
+  Actions.ActOnAtEnd(AtEndLoc, interfaceDecl, &allMethods[0], allMethods.size(), 
+                     &allProperties[0], allProperties.size());
 }
 
 ///   Parse property attribute declarations.
@@ -946,12 +942,7 @@ Parser::DeclTy *Parser::ParseObjCAtEndDeclaration(SourceLocation atLoc) {
     // Checking is not necessary except that a parse error might have caused
     // @implementation not to have been parsed to completion and ObjcImpDecl 
     // could be 0.
-    /// Insert collected methods declarations into the @interface object.
-    Actions.ActOnAddMethodsToObjcDecl(CurScope, ObjcImpDecl,
-                                      &AllImplMethods[0], AllImplMethods.size(),
-                                      (DeclTy **)0, 0,
-                                      atLoc);
-    AllImplMethods.clear();
+    Actions.ActOnAtEnd(atLoc, ObjcImpDecl);
   }
 
   return ObjcImpDecl;
@@ -1151,8 +1142,6 @@ Parser::StmtResult Parser::ParseObjCTryStmt(SourceLocation atLoc) {
 void Parser::ParseObjCInstanceMethodDefinition() {
   assert(Tok.is(tok::minus) && "Method definitions should start with '-'");
   DeclTy *MDecl = ParseObjCMethodPrototype(ObjcImpDecl);
-  // FIXME: @optional/@protocol??
-  AllImplMethods.push_back(MDecl);
   // parse optional ';'
   if (Tok.is(tok::semi))
     ConsumeToken();
@@ -1169,8 +1158,6 @@ void Parser::ParseObjCInstanceMethodDefinition() {
 void Parser::ParseObjCClassMethodDefinition() {
   assert(Tok.is(tok::plus) && "Class method definitions should start with '+'");
   DeclTy *MDecl = ParseObjCMethodPrototype(ObjcImpDecl);
-  // FIXME: @optional/@protocol??
-  AllImplMethods.push_back(MDecl);
   // parse optional ';'
   if (Tok.is(tok::semi))
     ConsumeToken();

Parse/Parser.cpp

@@ -23,7 +23,6 @@ Parser::Parser(Preprocessor &pp, Action &actions)
   NumCachedScopes = 0;
   ParenCount = BracketCount = BraceCount = 0;
   ObjcImpDecl = 0;
-  AllImplMethods.clear();
 }
 
 ///  Out-of-line virtual destructor to provide home for Action class.

Sema/Sema.h

@@ -541,10 +541,9 @@ public:
                                        llvm::SmallVector<DeclTy *, 8> & 
                                        Protocols);
 
-  virtual void ActOnAddMethodsToObjcDecl(Scope* S, DeclTy *ClassDecl, 
-				         DeclTy **allMethods, unsigned allNum,
-                                         DeclTy **allProperties, unsigned pNum,
-                                         SourceLocation AtEndLoc);
+  virtual void ActOnAtEnd(SourceLocation AtEndLoc, DeclTy *classDecl,
+                      DeclTy **allMethods = 0, unsigned allNum = 0,
+                      DeclTy **allProperties = 0, unsigned pNum = 0);
   
   virtual DeclTy *ActOnAddObjcProperties(SourceLocation AtLoc, 
                                          DeclTy **allProperties,

Sema/SemaDecl.cpp

@@ -1576,7 +1576,7 @@ void Sema::ImplMethodsVsClassMethods(ObjcImplementationDecl* IMPDecl,
   llvm::DenseSet<Selector> InsMap;
   // Check and see if instance methods in class interface have been
   // implemented in the implementation class.
-  ObjcMethodDecl **methods = IMPDecl->getInstanceMethods();
+  ObjcMethodDecl *const*methods = IMPDecl->getInstanceMethods();
   for (int i=0; i < IMPDecl->getNumInstanceMethods(); i++) 
     InsMap.insert(methods[i]->getSelector());
   
@@ -2079,10 +2079,9 @@ void Sema::AddFactoryMethodToGlobalPool(ObjcMethodDecl *Method) {
   }
 }
 
-void Sema::ActOnAddMethodsToObjcDecl(Scope* S, DeclTy *classDecl,
-                                     DeclTy **allMethods, unsigned allNum,
-                                     DeclTy **allProperties, unsigned pNum,
-                                     SourceLocation AtEndLoc) {
+void Sema::ActOnAtEnd(SourceLocation AtEndLoc, DeclTy *classDecl,
+                      DeclTy **allMethods, unsigned allNum,
+                      DeclTy **allProperties, unsigned pNum) {
   Decl *ClassDecl = static_cast<Decl *>(classDecl);
 
   // FIXME: If we don't have a ClassDecl, we have an error. I (snaroff) would
@@ -2164,8 +2163,7 @@ void Sema::ActOnAddMethodsToObjcDecl(Scope* S, DeclTy *classDecl,
   }
   else if (ObjcImplementationDecl *IC = 
                 dyn_cast<ObjcImplementationDecl>(ClassDecl)) {
-    IC->addMethods(&insMethods[0], insMethods.size(),
-                   &clsMethods[0], clsMethods.size(), AtEndLoc);
+    IC->setLocEnd(AtEndLoc);
     if (ObjcInterfaceDecl* IDecl = getObjCInterfaceDecl(IC->getIdentifier()))
       ImplMethodsVsClassMethods(IC, IDecl);
   } else {
@@ -2242,14 +2240,15 @@ Sema::DeclTy *Sema::ActOnMethodDeclaration(
   
   Decl *CDecl = static_cast<Decl*>(ClassDecl);
   ObjcInterfaceDecl *IDecl = 0;
+  ObjcImplementationDecl *ImpDecl = 0;
   if (isa<ObjcInterfaceDecl>(CDecl))
     IDecl = cast<ObjcInterfaceDecl>(CDecl);
   else if (isa<ObjcCategoryDecl>(CDecl))
-    IDecl = cast<ObjcCategoryDecl>(CDecl)->getClassInterface();
-  else if (isa<ObjcImplementationDecl>(CDecl))
-    IDecl = cast<ObjcImplementationDecl>(CDecl)->getClassInterface();
+    IDecl = cast<ObjcCategoryDecl>(CDecl)->getClassInterface(); // FIXME: what is this? (talk to fariborz)
+  else if ((ImpDecl = dyn_cast<ObjcImplementationDecl>(CDecl)))
+    IDecl = ImpDecl->getClassInterface(); // FIXME: what is this? (talk to fariborz)
   else if (isa<ObjcCategoryImplDecl>(CDecl))
-    IDecl = cast<ObjcCategoryImplDecl>(CDecl)->getClassInterface();
+    IDecl = cast<ObjcCategoryImplDecl>(CDecl)->getClassInterface(); // FIXME: what is this? (talk to fariborz)
   
   ObjcMethodDecl* ObjcMethod =  new ObjcMethodDecl(MethodLoc, EndLoc, Sel,
                                       resultDeclType,
@@ -2262,6 +2261,27 @@ Sema::DeclTy *Sema::ActOnMethodDeclaration(
   ObjcMethod->setMethodParams(&Params[0], Sel.getNumArgs());
   ObjcMethod->setObjcDeclQualifier(
     CvtQTToAstBitMask(ReturnQT.getObjcDeclQualifier()));
+  if (ImpDecl) {
+    // For implementations (which can be very "coarse grain"), we add the 
+    // method now. This allows the AST to implement lookup methods that work 
+    // incrementally (without waiting until we parse the @end). It also allows 
+    // us to flag multiple declaration errors as they occur.
+    // FIXME: still need to do this for ObjcCategoryImplDecl.
+    const ObjcMethodDecl *PrevMethod = 0;
+    if (MethodType == tok::minus) {
+      PrevMethod = ImpDecl->lookupInstanceMethod(Sel);
+      ImpDecl->addInstanceMethod(ObjcMethod);
+    } else {
+      PrevMethod = ImpDecl->lookupClassMethod(Sel);
+      ImpDecl->addClassMethod(ObjcMethod);
+    }
+    if (PrevMethod) {
+      // You can never have two method definitions with the same name.
+      Diag(ObjcMethod->getLocation(), diag::error_duplicate_method_decl,
+          ObjcMethod->getSelector().getName());
+      Diag(PrevMethod->getLocation(), diag::err_previous_declaration);
+    } 
+  }
   return ObjcMethod;
 }
 

include/clang/AST/DeclObjC.h

@@ -631,30 +631,33 @@ class ObjcImplementationDecl : public NamedDecl {
   int NumIvars;   // -1 if not defined.
 
   /// implemented instance methods
-  ObjcMethodDecl **InstanceMethods;  // Null if not defined
-  int NumInstanceMethods;  // -1 if not defined
+  llvm::SmallVector<ObjcMethodDecl*, 32> InstanceMethods;
   
   /// implemented class methods
-  ObjcMethodDecl **ClassMethods;  // Null if not defined
-  int NumClassMethods;  // -1 if not defined
+  llvm::SmallVector<ObjcMethodDecl*, 32> ClassMethods;
 
+  SourceLocation EndLoc;
 public:
   ObjcImplementationDecl(SourceLocation L, IdentifierInfo *Id,
                          ObjcInterfaceDecl *classInterface,
                          ObjcInterfaceDecl *superDecl)
     : NamedDecl(ObjcImplementation, L, Id),
-      ClassInterface(classInterface),
-      SuperClass(superDecl),
-      Ivars(0), NumIvars(-1),
-      InstanceMethods(0), NumInstanceMethods(-1), 
-      ClassMethods(0), NumClassMethods(-1) {}
+      ClassInterface(classInterface), SuperClass(superDecl),
+      Ivars(0), NumIvars(-1) {}
   
   void ObjcAddInstanceVariablesToClassImpl(ObjcIvarDecl **ivars, 
                                            unsigned numIvars);
     
-  void addMethods(ObjcMethodDecl **insMethods, unsigned numInsMembers,
-                  ObjcMethodDecl **clsMethods, unsigned numClsMembers,
-                  SourceLocation AtEndLoc);
+  void addInstanceMethod(ObjcMethodDecl *method) {
+    InstanceMethods.push_back(method);
+  }
+  void addClassMethod(ObjcMethodDecl *method) {
+    ClassMethods.push_back(method);
+  }    
+  // Location information, modeled after the Stmt API. 
+  SourceLocation getLocStart() const { return getLocation(); }
+  SourceLocation getLocEnd() const { return EndLoc; }
+  void setLocEnd(SourceLocation LE) { EndLoc = LE; };
   
   ObjcInterfaceDecl *getClassInterface() const { return ClassInterface; }
   ObjcInterfaceDecl *getSuperClass() const { return SuperClass; }
@@ -662,11 +665,17 @@ public:
   void setSuperClass(ObjcInterfaceDecl * superCls) 
          { SuperClass = superCls; }
   
-  ObjcMethodDecl **getInstanceMethods() const { return InstanceMethods; }
-  int getNumInstanceMethods() const { return NumInstanceMethods; }
+  // FIXME: Figure out how to remove the const pointer below.
+  ObjcMethodDecl *const*getInstanceMethods() const {
+    return &InstanceMethods[0];
+  }
+  int getNumInstanceMethods() const { return InstanceMethods.size(); }
   
-  ObjcMethodDecl **getClassMethods() const { return ClassMethods; }
-  int getNumClassMethods() const { return NumClassMethods; }
+  // FIXME: Figure out how to remove the const pointer below.
+  ObjcMethodDecl *const*getClassMethods() const { 
+    return &ClassMethods[0];
+  }
+  int getNumClassMethods() const { return ClassMethods.size(); }
 
   ObjcMethodDecl *lookupInstanceMethod(Selector &Sel);
   ObjcMethodDecl *lookupClassMethod(Selector &Sel);

include/clang/Parse/Action.h

@@ -576,19 +576,19 @@ public:
     tok::ObjCKeywordKind impKind) {
     return 0;
   }
-  // ActOnAddMethodsToObjcDecl - called to associate methods with an interface,
-  // protocol, category, or implementation. 
-  virtual void ActOnAddMethodsToObjcDecl(
-    Scope* S, 
-    DeclTy *ClassDecl,
-    DeclTy **allMethods, 
-    unsigned allNum,
-    DeclTy **allProperties,
-    unsigned NumProperties,
-    SourceLocation AtEndLoc) {
+  // ActOnAtEnd - called to mark the @end. For declarations (interfaces,
+  // protocols, categories), the parser passes all methods/properties. 
+  // For class implementations, these values default to 0. For implementations,
+  // methods are processed incrementally (by ActOnMethodDeclaration above).
+  virtual void ActOnAtEnd(
+    SourceLocation AtEndLoc, 
+    DeclTy *classDecl,
+    DeclTy **allMethods = 0, 
+    unsigned allNum = 0,
+    DeclTy **allProperties = 0, 
+    unsigned pNum = 0) {
     return;
   }
-  
   // ActOnAddObjcProperties - called to build one property AST
   virtual DeclTy *ActOnAddObjcProperties (SourceLocation AtLoc,
     DeclTy **allProperties, unsigned NumProperties, ObjcDeclSpec &DS) {

include/clang/Parse/Parser.h

@@ -269,8 +269,7 @@ private:
   DeclTy *ParseObjCAtProtocolDeclaration(SourceLocation atLoc);
   
   DeclTy *ObjcImpDecl;
-  /// Vector is used to collect method decls for each @implementation
-  llvm::SmallVector<DeclTy*, 32>  AllImplMethods;
+
   DeclTy *ParseObjCAtImplementationDeclaration(SourceLocation atLoc);
   DeclTy *ParseObjCAtEndDeclaration(SourceLocation atLoc);
   DeclTy *ParseObjCAtAliasDeclaration(SourceLocation atLoc);

test/Sema/message.m

@@ -6,6 +6,6 @@
 
 @implementation foo
 - (void) contents {}			// No declaration in @interface!
-- (void) meth { [self contents]; } // expected-warning {{method '-contents' not found (return type defaults to 'id')}}
+- (void) meth { [self contents]; } 
 @end