From: Argyrios Kyrtzidis Date: Tue, 7 Feb 2012 16:50:53 +0000 (+0000) Subject: Make parsing of objc @implementations more robust. X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=849639d8b548519cc5a00c0c9253f0c0d525060d;p=clang 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 7fcc37d41f..0c076210a2 100644 --- 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 PendingObjCImpDecl; - typedef SmallVector LateParsedObjCMethodContainer; - LateParsedObjCMethodContainer LateParsedObjCMethods; + struct ObjCImplParsingDataRAII { + Parser &P; + Decl *Dcl; + typedef SmallVector 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 b4023983af..7e087ef7ea 100644 --- 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::iterator diff --git a/lib/Parse/ParseDecl.cpp b/lib/Parse/ParseDecl.cpp index 8247b0b4d6..aa587e7196 100644 --- 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 3f80309b3a..3660b01816 100644 --- 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 3f4f4611fd..41dfeddaab 100644 --- 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 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 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 1526a44f21..4d8f474514 100644 --- 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 42c7897743..9c5bdbea6a 100644 --- 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 index 0000000000..0e6c8e664c --- /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 b50baa41a1..8378ed761c 100644 --- 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 c929c6d760..2c60482b35 100644 --- 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;