Fix a few issues while skipping function bodies

[ogoffart]

Summary

Fix a few issues while skipping function bodies

• In functions with try { } catch { }, only the try block would be skipped, not the catch blocks
• The template functions would still be parsed.
• The initializers within a constructor would still be parsed.
• The inline functions within class would still be stored, only to be discared later.
• Invalid code with try would assert (as in "int foo() try assert_here")

This attempt to do even less while skipping function bodies.

[rsmith]

lib/Parse/ParseStmt.cpp

1989 ↗

(On Diff #59084)

Comments should start with a capital letter and end with a period.

1990 ↗

(On Diff #59084)

This is not correct. Skipping constructor initializer lists correctly is hard (which is probably why we used to parse the initializer list rather than skipping it). You can use Parser::ConsumeAndStoreFunctionPrologue to get this (approximately) correct.

[ogoffart]

Right, i forgot about the C++11 initializer list syntax. I hope I got it right now.

[ogoffart]

The idea is that when we see a ") {" or "} {" in the ctor-initializers, (optionally with "..."), it is necessarily the start of the body.

Unless there might be lambda expressions within a template aregument, as in:

  
A::A() : Base<[](){return 42; }()>(){}

But this is not valid because lambda expression cannot appear in this context as far as i know since lambda expression cannot appear in template argument

[rsmith]

Please call Parser::ConsumeAndStoreFunctionPrologue rather than trying to reinvent it. You're still getting a number of cases wrong that it handles properly.

You also need to handle the case where the code completion token appears within the constructor *mem-initializer*s.

[ogoffart]

Using Parser::ConsumeAndStoreFunctionPrologue this time

[rsmith]

LGTM

[chapuni]

cfe/trunk/unittests/Tooling/ToolingTest.cpp

315

It didn't pass for targeting *-msvc. Tweaked in r272985.

cfe/trunk/lib/Parse/ParseCXXInlineMethods.cpp

if (TryConsumeToken(tok::equal)) {
     } else if (ExpectAndConsume(tok::semi, diag::err_expected_after,
                                 Delete ? "delete" : "default")) {
       SkipUntil(tok::semi);
     }
 
     return FnD;
   }
 
+  if (SkipFunctionBodies && (!FnD || Actions.canSkipFunctionBody(FnD)) &&
+      trySkippingFunctionBody()) {
+    Actions.ActOnSkippedFunctionBody(FnD);
+    return FnD;
+  }
+
   // In delayed template parsing mode, if we are within a class template
   // or if we are about to parse function member template then consume
   // the tokens and store them for parsing at the end of the translation unit.
   if (getLangOpts().DelayedTemplateParsing &&
       D.getFunctionDefinitionKind() == FDK_Definition &&
       !D.getDeclSpec().isConstexprSpecified() &&
       !(FnD && FnD->getAsFunction() &&
         FnD->getAsFunction()->getReturnType()->getContainedAutoType()) &&

cfe/trunk/lib/Parse/ParseObjc.cpp

if (AutoreleasePoolBody.isInvalid())
     AutoreleasePoolBody = Actions.ActOnNullStmt(Tok.getLocation());
   return Actions.ActOnObjCAutoreleasePoolStmt(atLoc, 
                                                 AutoreleasePoolBody.get());
 }
 
 /// StashAwayMethodOrFunctionBodyTokens -  Consume the tokens and store them 
 /// for later parsing.
 void Parser::StashAwayMethodOrFunctionBodyTokens(Decl *MDecl) {
+  if (SkipFunctionBodies && (!MDecl || Actions.canSkipFunctionBody(MDecl)) &&
+      trySkippingFunctionBody()) {
+    Actions.ActOnSkippedFunctionBody(MDecl);
+    return;
+  }
+
   LexedMethod* LM = new LexedMethod(this, MDecl);
   CurParsedObjCImpl->LateParsedObjCMethods.push_back(LM);
   CachedTokens &Toks = LM->Toks;
   // Begin by storing the '{' or 'try' or ':' token.
   Toks.push_back(Tok);
   if (Tok.is(tok::kw_try)) {
     ConsumeToken();
     if (Tok.is(tok::colon)) {

cfe/trunk/lib/Parse/ParseStmt.cpp

StmtResult Parser::ParsePragmaLoopHint(StmtVector &Stmts,
   Attrs.takeAllFrom(TempAttrs);
   return S;
 }
 
 Decl *Parser::ParseFunctionStatementBody(Decl *Decl, ParseScope &BodyScope) {
   assert(Tok.is(tok::l_brace));
   SourceLocation LBraceLoc = Tok.getLocation();
 
-  if (SkipFunctionBodies && (!Decl || Actions.canSkipFunctionBody(Decl)) &&
-      trySkippingFunctionBody()) {
-    BodyScope.Exit();
-    return Actions.ActOnSkippedFunctionBody(Decl);
-  }
-
   PrettyDeclStackTraceEntry CrashInfo(Actions, Decl, LBraceLoc,
                                       "parsing function body");
 
   // Save and reset current vtordisp stack if we have entered a C++ method body.
   bool IsCXXMethod =
       getLangOpts().CPlusPlus && Decl && isa<CXXMethodDecl>(Decl);
   Sema::PragmaStackSentinelRAII
     PragmaStackSentinel(Actions, "InternalPragmaState", IsCXXMethod);
PrettyDeclStackTraceEntry CrashInfo(Actions, Decl, TryLoc,
                                       "parsing function try block");
 
   // Constructor initializer list?
   if (Tok.is(tok::colon))
     ParseConstructorInitializer(Decl);
   else
     Actions.ActOnDefaultCtorInitializers(Decl);
 
-  if (SkipFunctionBodies && Actions.canSkipFunctionBody(Decl) &&
-      trySkippingFunctionBody()) {
-    BodyScope.Exit();
-    return Actions.ActOnSkippedFunctionBody(Decl);
-  }
-
   // Save and reset current vtordisp stack if we have entered a C++ method body.
   bool IsCXXMethod =
       getLangOpts().CPlusPlus && Decl && isa<CXXMethodDecl>(Decl);
   Sema::PragmaStackSentinelRAII
     PragmaStackSentinel(Actions, "InternalPragmaState", IsCXXMethod);
 
   SourceLocation LBraceLoc = Tok.getLocation();
   StmtResult FnBody(ParseCXXTryBlockCommon(TryLoc, /*FnTry*/true));
   // If we failed to parse the try-catch, we just give the function an empty
   // compound statement as the body.
   if (FnBody.isInvalid()) {
     Sema::CompoundScopeRAII CompoundScope(Actions);
     FnBody = Actions.ActOnCompoundStmt(LBraceLoc, LBraceLoc, None, false);
   }
 
   BodyScope.Exit();
   return Actions.ActOnFinishFunctionBody(Decl, FnBody.get());
 }
 
 bool Parser::trySkippingFunctionBody() {
-  assert(Tok.is(tok::l_brace));
   assert(SkipFunctionBodies &&
          "Should only be called when SkipFunctionBodies is enabled");
-
   if (!PP.isCodeCompletionEnabled()) {
-    ConsumeBrace();
-    SkipUntil(tok::r_brace);
+    SkipFunctionBody();
     return true;
   }
 
   // We're in code-completion mode. Skip parsing for all function bodies unless
   // the body contains the code-completion point.
   TentativeParsingAction PA(*this);
-  ConsumeBrace();
-  if (SkipUntil(tok::r_brace, StopAtCodeCompletion)) {
+  bool IsTryCatch = Tok.is(tok::kw_try);
+  CachedTokens Toks;
+  bool ErrorInPrologue = ConsumeAndStoreFunctionPrologue(Toks);
+  if (llvm::any_of(Toks, [](const Token &Tok) {
+        return Tok.is(tok::code_completion);
+      })) {
+    PA.Revert();
+    return false;
+  }
+  if (ErrorInPrologue) {
     PA.Commit();
+    SkipMalformedDecl();
     return true;
   }
-
+  if (!SkipUntil(tok::r_brace, StopAtCodeCompletion)) {
     PA.Revert();
     return false;
   }
+  while (IsTryCatch && Tok.is(tok::kw_catch)) {
+    if (!SkipUntil(tok::l_brace, StopAtCodeCompletion) ||
+        !SkipUntil(tok::r_brace, StopAtCodeCompletion)) {
+      PA.Revert();
+      return false;
+    }
+  }
+  PA.Commit();
+  return true;
+}
 
 /// ParseCXXTryBlock - Parse a C++ try-block.
 ///
 ///       try-block:
 ///         'try' compound-statement handler-seq
 ///
 StmtResult Parser::ParseCXXTryBlock() {
   assert(Tok.is(tok::kw_try) && "Expected 'try'");

cfe/trunk/lib/Parse/Parser.cpp

if (getLangOpts().DelayedTemplateParsing && Tok.isNot(tok::equal) &&
     Scope *ParentScope = getCurScope()->getParent();
 
     D.setFunctionDefinitionKind(FDK_Definition);
     Decl *DP = Actions.HandleDeclarator(ParentScope, D,
                                         TemplateParameterLists);
     D.complete(DP);
     D.getMutableDeclSpec().abort();
 
+    if (SkipFunctionBodies && (!DP || Actions.canSkipFunctionBody(DP)) &&
+        trySkippingFunctionBody()) {
+      BodyScope.Exit();
+      return Actions.ActOnSkippedFunctionBody(DP);
+    }
+
     CachedTokens Toks;
     LexTemplateFunctionForLateParsing(Toks);
 
     if (DP) {
       FunctionDecl *FnD = DP->getAsFunction();
       Actions.CheckForFunctionRedefinition(FnD);
       Actions.MarkAsLateParsedTemplate(FnD, DP, Toks);
     }
if (Tok.is(tok::comma)) {
       SkipUntil(tok::semi);
     }
 
     Stmt *GeneratedBody = Res ? Res->getBody() : nullptr;
     Actions.ActOnFinishFunctionBody(Res, GeneratedBody, false);
     return Res;
   }
 
+  if (SkipFunctionBodies && (!Res || Actions.canSkipFunctionBody(Res)) &&
+      trySkippingFunctionBody()) {
+    BodyScope.Exit();
+    Actions.ActOnSkippedFunctionBody(Res);
+    return Actions.ActOnFinishFunctionBody(Res, nullptr, false);
+  }
+
   if (Tok.is(tok::kw_try))
     return ParseFunctionTryBlock(Res, BodyScope);
 
   // If we have a colon, then we're probably parsing a C++
   // ctor-initializer.
   if (Tok.is(tok::colon)) {
     ParseConstructorInitializer(Res);
 

cfe/trunk/lib/Sema/SemaDecl.cpp

bool Sema::canSkipFunctionBody(Decl *D) {
   return Consumer.shouldSkipFunctionBody(D);
 }
 
 Decl *Sema::ActOnSkippedFunctionBody(Decl *Decl) {
   if (FunctionDecl *FD = dyn_cast_or_null<FunctionDecl>(Decl))
     FD->setHasSkippedBody();
   else if (ObjCMethodDecl *MD = dyn_cast_or_null<ObjCMethodDecl>(Decl))
     MD->setHasSkippedBody();
-  return ActOnFinishFunctionBody(Decl, nullptr);
+  return Decl;
 }
 
 Decl *Sema::ActOnFinishFunctionBody(Decl *D, Stmt *BodyArg) {
   return ActOnFinishFunctionBody(D, BodyArg, false);
 }
 
 Decl *Sema::ActOnFinishFunctionBody(Decl *dcl, Stmt *Body,
                                     bool IsInstantiation) {

cfe/trunk/test/CodeCompletion/ctor-initializer.cpp

+struct Base1 {
+  Base1() : {}
+  // RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:2:12 %s -o - | FileCheck -check-prefix=CHECK-CC1 %s
+  // CHECK-CC1: COMPLETION: Pattern : member1(<#args#>)
+  // CHECK-CC1: COMPLETION: Pattern : member2(<#args#>
+
+  Base1(int) : member1(123), {}
+  // RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:7:30 %s -o - | FileCheck -check-prefix=CHECK-CC2 %s
+  // CHECK-CC2-NOT: COMPLETION: Pattern : member1(<#args#>)
+  // CHECK-CC2: COMPLETION: Pattern : member2(<#args#>
+
+  int member1;
+  float member2;
+};
+
+struct Derived : public Base1 {
+  Derived();
+  Derived(int);
+  Derived(float);
+  int deriv1;
+};
+
+Derived::Derived() : {}
+// RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:23:22 %s -o - | FileCheck -check-prefix=CHECK-CC3 %s
+// CHECK-CC3: COMPLETION: Pattern : Base1(<#args#>)
+// CHECK-CC3: COMPLETION: Pattern : deriv1(<#args#>)
+
+Derived::Derived(int) try : {
+} catch (...) {
+}
+// RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:28:29 %s -o - | FileCheck -check-prefix=CHECK-CC4 %s
+// CHECK-CC4: COMPLETION: Pattern : Base1(<#args#>)
+// CHECK-CC4: COMPLETION: Pattern : deriv1(<#args#>)
+
+Derived::Derived(float) try : Base1(),
+{
+} catch (...) {
+}
+// RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:35:39 %s -o - | FileCheck -check-prefix=CHECK-CC5 %s
+// CHECK-CC5-NOT: COMPLETION: Pattern : Base1(<#args#>)
+// CHECK-CC5: COMPLETION: Pattern : deriv1(<#args#>)

cfe/trunk/unittests/Tooling/ToolingTest.cpp

TEST(newFrontendActionFactory, InjectsSourceFileCallbacks) {
   EXPECT_EQ(2u, EndCallback.BeginCalled);
   EXPECT_EQ(2u, EndCallback.EndCalled);
 }
 #endif
 
 struct SkipBodyConsumer : public clang::ASTConsumer {
   /// Skip the 'skipMe' function.
   bool shouldSkipFunctionBody(Decl *D) override {
-    FunctionDecl *F = dyn_cast<FunctionDecl>(D);
+    NamedDecl *F = dyn_cast<NamedDecl>(D);
     return F && F->getNameAsString() == "skipMe";
   }
 };
 
 struct SkipBodyAction : public clang::ASTFrontendAction {
   std::unique_ptr<ASTConsumer> CreateASTConsumer(CompilerInstance &Compiler,
                                                  StringRef) override {
     Compiler.getFrontendOpts().SkipFunctionBodies = true;
     return llvm::make_unique<SkipBodyConsumer>();
   }
 };
 
 TEST(runToolOnCode, TestSkipFunctionBody) {
+  std::vector<std::string> Args = {"-std=c++11"};
+
   EXPECT_TRUE(runToolOnCode(new SkipBodyAction,
                             "int skipMe() { an_error_here }"));
   EXPECT_FALSE(runToolOnCode(new SkipBodyAction,
                              "int skipMeNot() { an_error_here }"));
+
+  // Test constructors with initializers
+  EXPECT_TRUE(runToolOnCodeWithArgs(
+      new SkipBodyAction,
+      "struct skipMe { skipMe() : an_error() { more error } };", Args));
+  EXPECT_TRUE(runToolOnCodeWithArgs(
+      new SkipBodyAction, "struct skipMe { skipMe(); };"
+                          "skipMe::skipMe() : an_error([](){;}) { more error }",
+      Args));
+  EXPECT_TRUE(runToolOnCodeWithArgs(
+      new SkipBodyAction, "struct skipMe { skipMe(); };"
+                          "skipMe::skipMe() : an_error{[](){;}} { more error }",
+      Args));
+  EXPECT_TRUE(runToolOnCodeWithArgs(
+      new SkipBodyAction,
+      "struct skipMe { skipMe(); };"
+      "skipMe::skipMe() : a<b<c>(e)>>(), f{}, g() { error }",
+      Args));
+  EXPECT_TRUE(runToolOnCodeWithArgs(
+      new SkipBodyAction, "struct skipMe { skipMe() : bases()... { error } };",
+      Args));
+
+  EXPECT_FALSE(runToolOnCodeWithArgs(
+      new SkipBodyAction, "struct skipMeNot { skipMeNot() : an_error() { } };",
+      Args));
+  EXPECT_FALSE(runToolOnCodeWithArgs(new SkipBodyAction,
+                                     "struct skipMeNot { skipMeNot(); };"
+                                     "skipMeNot::skipMeNot() : an_error() { }",
+                                     Args));
+
+  // Try/catch
+  EXPECT_TRUE(runToolOnCode(
+      new SkipBodyAction,
+      "void skipMe() try { an_error() } catch(error) { error };"));
+  EXPECT_TRUE(runToolOnCode(
+      new SkipBodyAction,
+      "struct S { void skipMe() try { an_error() } catch(error) { error } };"));
+  EXPECT_TRUE(
+      runToolOnCode(new SkipBodyAction,
+                    "void skipMe() try { an_error() } catch(error) { error; }"
+                    "catch(error) { error } catch (error) { }"));
+  EXPECT_FALSE(runToolOnCode(
+      new SkipBodyAction,
+      "void skipMe() try something;")); // don't crash while parsing
+
+  // Template
+  EXPECT_TRUE(runToolOnCode(
+      new SkipBodyAction, "template<typename T> int skipMe() { an_error_here }"
+                          "int x = skipMe<int>();"));
+  EXPECT_FALSE(
+      runToolOnCode(new SkipBodyAction,
+                    "template<typename T> int skipMeNot() { an_error_here }"));
 }
 
 TEST(runToolOnCodeWithArgs, TestNoDepFile) {
   llvm::SmallString<32> DepFilePath;
   ASSERT_FALSE(
       llvm::sys::fs::createTemporaryFile("depfile", "d", DepFilePath));
   std::vector<std::string> Args;
   Args.push_back("-MMD");