Make parsing of objc @implementations more robust.

Parsing of @implementations was based on modifying global state from
the parser; the logic for late parsing of methods was spread in multiple places
making it difficult to have a robust error recovery.

  -it was difficult to ensure that we don't neglect parsing the lexed methods.
  -it was difficult to setup the original objc container context for parsing the lexed methods
   after completing ParseObjCAtImplementationDeclaration and returning to top level context.

Enhance parsing of @implementations by centralizing it in Parser::ParseObjCAtImplementationDeclaration().
ParseObjCAtImplementationDeclaration now returns only after an @implementation is fully parsed;
all the data and logic for late parsing of methods is now in one place.

This allows us to provide code-completion for late parsed methods with mis-matched braces.
rdar://10775381

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

diff --git a/include/clang/Parse/Parser.h b/include/clang/Parse/Parser.h
index 7fcc37d41f29ce5b71104417a3b91e5954cc5640..0c076210a25b9387321c71d3a13eaee34af6fea2 100644 (file)
--- a/include/clang/Parse/Parser.h
+++ b/include/clang/Parse/Parser.h
@@ -258,8 +258,6 @@ public:
   /// the EOF was encountered.
   bool ParseTopLevelDecl(DeclGroupPtrTy &Result);
 
-  DeclGroupPtrTy FinishPendingObjCActions();
-
 private:
   //===--------------------------------------------------------------------===//
   // Low-Level token peeking and consumption methods.
@@ -404,9 +402,6 @@ private:
     Tok.setKind(tok::eof);
   }
 
-  /// \brief Clear and free the cached objc methods.
-  void clearLateParsedObjCMethods();
-
   /// \brief Handle the annotation token produced for #pragma unused(...)
   void HandlePragmaUnused();
 
@@ -1227,12 +1222,28 @@ private:
   DeclGroupPtrTy ParseObjCAtProtocolDeclaration(SourceLocation atLoc,
                                                 ParsedAttributes &prefixAttrs);
 
-  Decl *ObjCImpDecl;
-  SmallVector<Decl *, 4> PendingObjCImpDecl;
-  typedef SmallVector<LexedMethod*, 2> LateParsedObjCMethodContainer;
-  LateParsedObjCMethodContainer LateParsedObjCMethods;
+  struct ObjCImplParsingDataRAII {
+    Parser &P;
+    Decl *Dcl;
+    typedef SmallVector<LexedMethod*, 8> LateParsedObjCMethodContainer;
+    LateParsedObjCMethodContainer LateParsedObjCMethods;
+
+    ObjCImplParsingDataRAII(Parser &parser, Decl *D)
+      : P(parser), Dcl(D) {
+      P.CurParsedObjCImpl = this;
+      Finished = false;
+    }
+    ~ObjCImplParsingDataRAII();
+
+    void finish(SourceRange AtEnd);
+    bool isFinished() const { return Finished; }
+
+  private:
+    bool Finished;
+  };
+  ObjCImplParsingDataRAII *CurParsedObjCImpl;
 
-  Decl *ParseObjCAtImplementationDeclaration(SourceLocation AtLoc);
+  DeclGroupPtrTy ParseObjCAtImplementationDeclaration(SourceLocation AtLoc);
   DeclGroupPtrTy ParseObjCAtEndDeclaration(SourceRange atEnd);
   Decl *ParseObjCAtAliasDeclaration(SourceLocation atLoc);
   Decl *ParseObjCPropertySynthesize(SourceLocation atLoc);

diff --git a/lib/Parse/ParseAST.cpp b/lib/Parse/ParseAST.cpp
index b4023983afc9d37751c13d3e4d402dd2a7274842..7e087ef7ea477b9ebe55cb552b1ba71664c35c11 100644 (file)
--- a/lib/Parse/ParseAST.cpp
+++ b/lib/Parse/ParseAST.cpp
@@ -96,10 +96,6 @@ void clang::ParseAST(Sema &S, bool PrintStats) {
 
   if (Abort)
     return;
-
-  // Check for any pending objective-c implementation decl.
-  while ((ADecl = P.FinishPendingObjCActions()))
-    Consumer->HandleTopLevelDecl(ADecl.get());
   
   // Process any TopLevelDecls generated by #pragma weak.
   for (SmallVector<Decl*,2>::iterator

diff --git a/lib/Parse/ParseDecl.cpp b/lib/Parse/ParseDecl.cpp
index 8247b0b4d6ad05b61821d6005631f4d2b2e8406a..aa587e7196b005eb17d7dc1106af86469d37a1a1 100644 (file)
--- a/lib/Parse/ParseDecl.cpp
+++ b/lib/Parse/ParseDecl.cpp
@@ -1723,7 +1723,7 @@ void Parser::ParseDeclarationSpecifiers(DeclSpec &DS,
                                     : Sema::PCC_Template;
       else if (DSContext == DSC_class)
         CCC = Sema::PCC_Class;
-      else if (ObjCImpDecl)
+      else if (CurParsedObjCImpl)
         CCC = Sema::PCC_ObjCImplementation;
       
       Actions.CodeCompleteOrdinaryName(getCurScope(), CCC);

diff --git a/lib/Parse/ParseExpr.cpp b/lib/Parse/ParseExpr.cpp
index 3f80309b3aa4a39562f01c604acb6fe9016b5f99..3660b0181651c40088d50aac0d609b0415a157b6 100644 (file)
--- a/lib/Parse/ParseExpr.cpp
+++ b/lib/Parse/ParseExpr.cpp
@@ -1416,7 +1416,8 @@ Parser::ParsePostfixExpressionSuffix(ExprResult LHS) {
       if (!LHS.isInvalid())
         LHS = Actions.ActOnMemberAccessExpr(getCurScope(), LHS.take(), OpLoc, 
                                             OpKind, SS, TemplateKWLoc, Name,
-                                            ObjCImpDecl, Tok.is(tok::l_paren));
+                                 CurParsedObjCImpl ? CurParsedObjCImpl->Dcl : 0,
+                                            Tok.is(tok::l_paren));
       break;
     }
     case tok::plusplus:    // postfix-expression: postfix-expression '++'

diff --git a/lib/Parse/ParseObjc.cpp b/lib/Parse/ParseObjc.cpp
index 3f4f4611fd3c89b77e07b6bab56eda685be89058..41dfeddaab9011d1b07e24ae89ef214f9291c022 100644 (file)
--- a/lib/Parse/ParseObjc.cpp
+++ b/lib/Parse/ParseObjc.cpp
@@ -52,8 +52,7 @@ Parser::DeclGroupPtrTy Parser::ParseObjCAtDirectives() {
     return ParseObjCAtProtocolDeclaration(AtLoc, attrs);
   }
   case tok::objc_implementation:
-    SingleDecl = ParseObjCAtImplementationDeclaration(AtLoc);
-    break;
+    return ParseObjCAtImplementationDeclaration(AtLoc);
   case tok::objc_end:
     return ParseObjCAtEndDeclaration(AtLoc);
   case tok::objc_compatibility_alias:
@@ -122,15 +121,17 @@ void Parser::CheckNestedObjCContexts(SourceLocation AtLoc)
   if (ock == Sema::OCK_None)
     return;
 
-  Decl *Decl = Actions.ActOnAtEnd(getCurScope(), AtLoc);
+  Decl *Decl = Actions.getObjCDeclContext();
+  if (CurParsedObjCImpl) {
+    CurParsedObjCImpl->finish(AtLoc);
+  } else {
+    Actions.ActOnAtEnd(getCurScope(), AtLoc);
+  }
   Diag(AtLoc, diag::err_objc_missing_end)
       << FixItHint::CreateInsertion(AtLoc, "@end\n");
   if (Decl)
     Diag(Decl->getLocStart(), diag::note_objc_container_start)
         << (int) ock;
-  if (!PendingObjCImpDecl.empty())
-    PendingObjCImpDecl.pop_back();
-  ObjCImpDecl = 0;
 }
 
 ///
@@ -403,7 +404,7 @@ void Parser::ParseObjCInterfaceDeclList(tok::ObjCKeywordKind contextKey,
     // Code completion within an Objective-C interface.
     if (Tok.is(tok::code_completion)) {
       Actions.CodeCompleteOrdinaryName(getCurScope(), 
-                                  ObjCImpDecl? Sema::PCC_ObjCImplementation
+                            CurParsedObjCImpl? Sema::PCC_ObjCImplementation
                                              : Sema::PCC_ObjCInterface);
       return cutOffParsing();
     }
@@ -1441,7 +1442,8 @@ Parser::ParseObjCAtProtocolDeclaration(SourceLocation AtLoc,
 ///
 ///   objc-category-implementation-prologue:
 ///     @implementation identifier ( identifier )
-Decl *Parser::ParseObjCAtImplementationDeclaration(SourceLocation AtLoc) {
+Parser::DeclGroupPtrTy
+Parser::ParseObjCAtImplementationDeclaration(SourceLocation AtLoc) {
   assert(Tok.isObjCAtKeyword(tok::objc_implementation) &&
          "ParseObjCAtImplementationDeclaration(): Expected @implementation");
   CheckNestedObjCContexts(AtLoc);
@@ -1451,16 +1453,17 @@ Decl *Parser::ParseObjCAtImplementationDeclaration(SourceLocation AtLoc) {
   if (Tok.is(tok::code_completion)) {
     Actions.CodeCompleteObjCImplementationDecl(getCurScope());
     cutOffParsing();
-    return 0;
+    return DeclGroupPtrTy();
   }
 
   if (Tok.isNot(tok::identifier)) {
     Diag(Tok, diag::err_expected_ident); // missing class or category name.
-    return 0;
+    return DeclGroupPtrTy();
   }
   // We have a class or category name - consume it.
   IdentifierInfo *nameId = Tok.getIdentifierInfo();
   SourceLocation nameLoc = ConsumeToken(); // consume class or category name
+  Decl *ObjCImpDecl = 0;
 
   if (Tok.is(tok::l_paren)) {
     // we have a category implementation.
@@ -1471,7 +1474,7 @@ Decl *Parser::ParseObjCAtImplementationDeclaration(SourceLocation AtLoc) {
     if (Tok.is(tok::code_completion)) {
       Actions.CodeCompleteObjCImplementationCategory(getCurScope(), nameId, nameLoc);
       cutOffParsing();
-      return 0;
+      return DeclGroupPtrTy();
     }
     
     if (Tok.is(tok::identifier)) {
@@ -1479,45 +1482,59 @@ Decl *Parser::ParseObjCAtImplementationDeclaration(SourceLocation AtLoc) {
       categoryLoc = ConsumeToken();
     } else {
       Diag(Tok, diag::err_expected_ident); // missing category name.
-      return 0;
+      return DeclGroupPtrTy();
     }
     if (Tok.isNot(tok::r_paren)) {
       Diag(Tok, diag::err_expected_rparen);
       SkipUntil(tok::r_paren, false); // don't stop at ';'
-      return 0;
+      return DeclGroupPtrTy();
     }
     rparenLoc = ConsumeParen();
-    Decl *ImplCatType = Actions.ActOnStartCategoryImplementation(
+    ObjCImpDecl = Actions.ActOnStartCategoryImplementation(
                                     AtLoc, nameId, nameLoc, categoryId,
                                     categoryLoc);
 
-    ObjCImpDecl = ImplCatType;
-    PendingObjCImpDecl.push_back(ObjCImpDecl);
-    return 0;
-  }
-  // We have a class implementation
-  SourceLocation superClassLoc;
-  IdentifierInfo *superClassId = 0;
-  if (Tok.is(tok::colon)) {
-    // We have a super class
-    ConsumeToken();
-    if (Tok.isNot(tok::identifier)) {
-      Diag(Tok, diag::err_expected_ident); // missing super class name.
-      return 0;
+  } else {
+    // We have a class implementation
+    SourceLocation superClassLoc;
+    IdentifierInfo *superClassId = 0;
+    if (Tok.is(tok::colon)) {
+      // We have a super class
+      ConsumeToken();
+      if (Tok.isNot(tok::identifier)) {
+        Diag(Tok, diag::err_expected_ident); // missing super class name.
+        return DeclGroupPtrTy();
+      }
+      superClassId = Tok.getIdentifierInfo();
+      superClassLoc = ConsumeToken(); // Consume super class name
     }
-    superClassId = Tok.getIdentifierInfo();
-    superClassLoc = ConsumeToken(); // Consume super class name
+    ObjCImpDecl = Actions.ActOnStartClassImplementation(
+                                    AtLoc, nameId, nameLoc,
+                                    superClassId, superClassLoc);
+  
+    if (Tok.is(tok::l_brace)) // we have ivars
+      ParseObjCClassInstanceVariables(ObjCImpDecl, tok::objc_private, AtLoc);
   }
-  Decl *ImplClsType = Actions.ActOnStartClassImplementation(
-                                  AtLoc, nameId, nameLoc,
-                                  superClassId, superClassLoc);
+  assert(ObjCImpDecl);
+
+  SmallVector<Decl *, 8> DeclsInGroup;
 
-  if (Tok.is(tok::l_brace)) // we have ivars
-    ParseObjCClassInstanceVariables(ImplClsType, tok::objc_private, AtLoc);
+  {
+    ObjCImplParsingDataRAII ObjCImplParsing(*this, ObjCImpDecl);
+    while (!ObjCImplParsing.isFinished() && Tok.isNot(tok::eof)) {
+      ParsedAttributesWithRange attrs(AttrFactory);
+      MaybeParseCXX0XAttributes(attrs);
+      MaybeParseMicrosoftAttributes(attrs);
+      if (DeclGroupPtrTy DGP = ParseExternalDeclaration(attrs)) {
+        DeclGroupRef DG = DGP.get();
+        DeclsInGroup.append(DG.begin(), DG.end());
+      }
+    }
+  }
 
-  ObjCImpDecl = ImplClsType;
-  PendingObjCImpDecl.push_back(ObjCImpDecl);
-  return 0;
+  DeclsInGroup.push_back(ObjCImpDecl);
+  return Actions.BuildDeclaratorGroup(
+           DeclsInGroup.data(), DeclsInGroup.size(), false);
 }
 
 Parser::DeclGroupPtrTy
@@ -1525,51 +1542,44 @@ Parser::ParseObjCAtEndDeclaration(SourceRange atEnd) {
   assert(Tok.isObjCAtKeyword(tok::objc_end) &&
          "ParseObjCAtEndDeclaration(): Expected @end");
   ConsumeToken(); // the "end" identifier
-  SmallVector<Decl *, 8> DeclsInGroup;
-  Actions.DefaultSynthesizeProperties(getCurScope(), ObjCImpDecl);
-  for (size_t i = 0; i < LateParsedObjCMethods.size(); ++i) {
-    Decl *D = ParseLexedObjCMethodDefs(*LateParsedObjCMethods[i]);
-    if (D)
-      DeclsInGroup.push_back(D);
-  }
-  DeclsInGroup.push_back(ObjCImpDecl);
-  
-  if (ObjCImpDecl) {
-    Actions.ActOnAtEnd(getCurScope(), atEnd);
-    PendingObjCImpDecl.pop_back();
-  }
+  if (CurParsedObjCImpl)
+    CurParsedObjCImpl->finish(atEnd);
   else
     // missing @implementation
     Diag(atEnd.getBegin(), diag::err_expected_objc_container);
-    
-  clearLateParsedObjCMethods();
-  ObjCImpDecl = 0;
-  return Actions.BuildDeclaratorGroup(
-           DeclsInGroup.data(), DeclsInGroup.size(), false);
+  return DeclGroupPtrTy();
 }
 
-Parser::DeclGroupPtrTy Parser::FinishPendingObjCActions() {
-  Actions.DiagnoseUseOfUnimplementedSelectors();
-  if (PendingObjCImpDecl.empty())
-    return Actions.ConvertDeclToDeclGroup(0);
+Parser::ObjCImplParsingDataRAII::~ObjCImplParsingDataRAII() {
+  if (!Finished) {
+    finish(P.Tok.getLocation());
+    if (P.Tok.is(tok::eof)) {
+      P.Diag(P.Tok, diag::err_objc_missing_end)
+          << FixItHint::CreateInsertion(P.Tok.getLocation(), "\n@end\n");
+      P.Diag(Dcl->getLocStart(), diag::note_objc_container_start)
+          << Sema::OCK_Implementation;
+    }
+  }
+  P.CurParsedObjCImpl = 0;
+  assert(LateParsedObjCMethods.empty());
+}
 
-  Decl *ImpDecl = PendingObjCImpDecl.pop_back_val();
-  Actions.ActOnAtEnd(getCurScope(), SourceRange(Tok.getLocation()));
-  Diag(Tok, diag::err_objc_missing_end)
-      << FixItHint::CreateInsertion(Tok.getLocation(), "\n@end\n");
-  if (ImpDecl)
-    Diag(ImpDecl->getLocStart(), diag::note_objc_container_start)
-        << Sema::OCK_Implementation;
+void Parser::ObjCImplParsingDataRAII::finish(SourceRange AtEnd) {
+  assert(!Finished);
+  P.Actions.DefaultSynthesizeProperties(P.getCurScope(), Dcl);
+  for (size_t i = 0; i < LateParsedObjCMethods.size(); ++i)
+    P.ParseLexedObjCMethodDefs(*LateParsedObjCMethods[i]);
 
-  return Actions.ConvertDeclToDeclGroup(ImpDecl);
-}
+  P.Actions.ActOnAtEnd(P.getCurScope(), AtEnd);
 
-void Parser::clearLateParsedObjCMethods() {
+  /// \brief Clear and free the cached objc methods.
   for (LateParsedObjCMethodContainer::iterator
          I = LateParsedObjCMethods.begin(),
          E = LateParsedObjCMethods.end(); I != E; ++I)
     delete *I;
   LateParsedObjCMethods.clear();
+
+  Finished = true;
 }
 
 ///   compatibility-alias-decl:
@@ -1908,7 +1918,7 @@ Decl *Parser::ParseObjCMethodDefinition() {
 
   // parse optional ';'
   if (Tok.is(tok::semi)) {
-    if (ObjCImpDecl) {
+    if (CurParsedObjCImpl) {
       Diag(Tok, diag::warn_semicolon_before_method_body)
         << FixItHint::CreateRemoval(Tok.getLocation());
     }
@@ -1926,19 +1936,32 @@ Decl *Parser::ParseObjCMethodDefinition() {
     if (Tok.isNot(tok::l_brace))
       return 0;
   }
+
+  if (!MDecl) {
+    ConsumeBrace();
+    SkipUntil(tok::r_brace, /*StopAtSemi=*/false);
+    return 0;
+  }
+
   // Allow the rest of sema to find private method decl implementations.
-  if (MDecl)
-    Actions.AddAnyMethodToGlobalPool(MDecl);
-    
-  // Consume the tokens and store them for later parsing.
-  LexedMethod* LM = new LexedMethod(this, MDecl);
-  LateParsedObjCMethods.push_back(LM);
-  CachedTokens &Toks = LM->Toks;
-  // Begin by storing the '{' token.
-  Toks.push_back(Tok);
-  ConsumeBrace();
-  // Consume everything up to (and including) the matching right brace.
-  ConsumeAndStoreUntil(tok::r_brace, Toks, /*StopAtSemi=*/false);
+  Actions.AddAnyMethodToGlobalPool(MDecl);
+
+  if (CurParsedObjCImpl) {
+    // Consume the tokens and store them for later parsing.
+    LexedMethod* LM = new LexedMethod(this, MDecl);
+    CurParsedObjCImpl->LateParsedObjCMethods.push_back(LM);
+    CachedTokens &Toks = LM->Toks;
+    // Begin by storing the '{' token.
+    Toks.push_back(Tok);
+    ConsumeBrace();
+    // Consume everything up to (and including) the matching right brace.
+    ConsumeAndStoreUntil(tok::r_brace, Toks, /*StopAtSemi=*/false);
+
+  } else {
+    ConsumeBrace();
+    SkipUntil(tok::r_brace, /*StopAtSemi=*/false);
+  }
+
   return MDecl;
 }
 

diff --git a/lib/Parse/Parser.cpp b/lib/Parse/Parser.cpp
index 1526a44f21fe5f2d1a5f02a5dec4ab7aedda6aaf..4d8f474514db0a2e7673386ef6b4f75fa588ea6c 100644 (file)
--- a/lib/Parse/Parser.cpp
+++ b/lib/Parse/Parser.cpp
@@ -39,7 +39,7 @@ Parser::Parser(Preprocessor &pp, Sema &actions)
   Actions.CurScope = 0;
   NumCachedScopes = 0;
   ParenCount = BracketCount = BraceCount = 0;
-  ObjCImpDecl = 0;
+  CurParsedObjCImpl = 0;
 
   // Add #pragma handlers. These are removed and destroyed in the
   // destructor.
@@ -367,8 +367,6 @@ Parser::~Parser() {
       it != LateParsedTemplateMap.end(); ++it)
     delete it->second;
 
-  clearLateParsedObjCMethods();
-
   // Remove the pragma handlers we installed.
   PP.RemovePragmaHandler(AlignHandler.get());
   AlignHandler.reset();
@@ -595,7 +593,7 @@ Parser::ParseExternalDeclaration(ParsedAttributesWithRange &attrs,
     break;
   case tok::code_completion:
       Actions.CodeCompleteOrdinaryName(getCurScope(), 
-                                   ObjCImpDecl? Sema::PCC_ObjCImplementation
+                             CurParsedObjCImpl? Sema::PCC_ObjCImplementation
                                               : Sema::PCC_Namespace);
     cutOffParsing();
     return DeclGroupPtrTy();

diff --git a/lib/Sema/Sema.cpp b/lib/Sema/Sema.cpp
index 42c7897743d14d2e03260e90bceb9b2bead0cef9..9c5bdbea6ad99816111f9fa75ed247ad0d28c568 100644 (file)
--- a/lib/Sema/Sema.cpp
+++ b/lib/Sema/Sema.cpp
@@ -418,6 +418,8 @@ void Sema::ActOnEndOfTranslationUnit() {
   // Only complete translation units define vtables and perform implicit
   // instantiations.
   if (TUKind == TU_Complete) {
+    DiagnoseUseOfUnimplementedSelectors();
+
     // If any dynamic classes have their key function defined within
     // this translation unit, then those vtables are considered "used" and must
     // be emitted.

diff --git a/test/Index/complete-in-invalid-method.m b/test/Index/complete-in-invalid-method.m
new file mode 100644 (file)
index 0000000..0e6c8e6
--- /dev/null
+++ b/test/Index/complete-in-invalid-method.m
@@ -0,0 +1,19 @@
+@interface I
+-(void)foo;
+@end
+
+struct S {
+  int x,y;
+};
+
+@implementation I
+-(void) foo {
+  struct S s;
+  if (1) {
+    s.
+}
+@end
+
+// RUN: c-index-test -code-completion-at=%s:13:7 -fobjc-nonfragile-abi %s | FileCheck %s
+// CHECK: FieldDecl:{ResultType int}{TypedText x}
+// CHECK: FieldDecl:{ResultType int}{TypedText y}

diff --git a/test/SemaObjC/invalid-code.m b/test/SemaObjC/invalid-code.m
index b50baa41a16e3e395eb38fe80b582c38f0fd61f9..8378ed761c416a60b8fd98ffdb037f429ebc524e 100644 (file)
--- a/test/SemaObjC/invalid-code.m
+++ b/test/SemaObjC/invalid-code.m
@@ -27,3 +27,17 @@ void foo() {
     return self;
 }
 @end
+
+@interface I
+@end
+@interface I2
+@end
+
+@implementation I // expected-note {{started here}}
+-(void) foo {}
+
+@implementation I2 // expected-error {{missing '@end'}}
+-(void) foo2 {}
+@end
+
+@end // expected-error {{'@end' must appear in an Objective-C context}}

diff --git a/test/SemaObjC/missing-atend-metadata.m b/test/SemaObjC/missing-atend-metadata.m
index c929c6d7603a634b5b587577d6a006b2b6f70a12..2c60482b357729313134abe4034a148606132409 100644 (file)
--- a/test/SemaObjC/missing-atend-metadata.m
+++ b/test/SemaObjC/missing-atend-metadata.m
@@ -10,7 +10,7 @@
 @end
 
 @implementation I1 // expected-note {{implementation started here}}
--(void) im0 { self = [super init]; }
+-(void) im0 { self = [super init]; } // expected-warning {{not found}}
 
 @interface I2 : I0 // expected-error {{missing '@end'}}
 - I2meth;