[clang-tools-extra] r365607 - [clangd] Stop recording tokens before running clang-tidy

[Ilya Biryukov via cfe-commits]

Author: ibiryukov
Date: Wed Jul 10 02:28:35 2019
New Revision: 365607

URL: http://llvm.org/viewvc/llvm-project?rev=365607&view=rev
Log:
[clangd] Stop recording tokens before running clang-tidy

modernize-trailing-return-type runs the preprocessor, breaking the token
collection logic.

This lead to a crash before, see the new test for a repro.

Modified:
    clang-tools-extra/trunk/clangd/ClangdUnit.cpp
    clang-tools-extra/trunk/clangd/unittests/ClangdUnitTests.cpp

Modified: clang-tools-extra/trunk/clangd/ClangdUnit.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdUnit.cpp?rev=365607&r1=365606&r2=365607&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/ClangdUnit.cpp (original)
+++ clang-tools-extra/trunk/clangd/ClangdUnit.cpp Wed Jul 10 02:28:35 2019
@@ -421,12 +421,16 @@ ParsedAST::build(std::unique_ptr<Compile
   Clang->getPreprocessor().addCommentHandler(IWYUHandler.get());
 
   // Collect tokens of the main file.
-  syntax::TokenCollector Tokens(Clang->getPreprocessor());
+  syntax::TokenCollector CollectTokens(Clang->getPreprocessor());
 
   if (llvm::Error Err = Action->Execute())
     log("Execute() failed when building AST for {0}: {1}", MainInput.getFile(),
         toString(std::move(Err)));
 
+  // We have to consume the tokens before running clang-tidy to avoid collecting
+  // tokens from running the preprocessor inside the checks (only
+  // modernize-use-trailing-return-type does that today).
+  syntax::TokenBuffer Tokens = std::move(CollectTokens).consume();
   std::vector<Decl *> ParsedDecls = Action->takeTopLevelDecls();
   // AST traversals should exclude the preamble, to avoid performance cliffs.
   Clang->getASTContext().setTraversalScope(ParsedDecls);
@@ -452,9 +456,8 @@ ParsedAST::build(std::unique_ptr<Compile
   if (Preamble)
     Diags.insert(Diags.begin(), Preamble->Diags.begin(), Preamble->Diags.end());
   return ParsedAST(std::move(Preamble), std::move(Clang), std::move(Action),
-                   std::move(Tokens).consume(), std::move(ParsedDecls),
-                   std::move(Diags), std::move(Includes),
-                   std::move(CanonIncludes));
+                   std::move(Tokens), std::move(ParsedDecls), std::move(Diags),
+                   std::move(Includes), std::move(CanonIncludes));
 }
 
 ParsedAST::ParsedAST(ParsedAST &&Other) = default;

Modified: clang-tools-extra/trunk/clangd/unittests/ClangdUnitTests.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/unittests/ClangdUnitTests.cpp?rev=365607&r1=365606&r2=365607&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/unittests/ClangdUnitTests.cpp (original)
+++ clang-tools-extra/trunk/clangd/unittests/ClangdUnitTests.cpp Wed Jul 10 02:28:35 2019
@@ -134,6 +134,27 @@ TEST(ClangdUnitTest, TokensAfterPreamble
   EXPECT_EQ(Spelled.back().text(SM), "last_token");
 }
 
+
+TEST(ClangdUnitTest, NoCrashOnTokensWithTidyCheck) {
+  TestTU TU;
+  // this check runs the preprocessor, we need to make sure it does not break
+  // our recording logic.
+  TU.ClangTidyChecks = "modernize-use-trailing-return-type";
+  TU.Code = "inline int foo() {}";
+
+  auto AST = TU.build();
+  const syntax::TokenBuffer &T = AST.getTokens();
+  const auto &SM = AST.getSourceManager();
+
+  ASSERT_GT(T.expandedTokens().size(), 7u);
+  // Check first token after the preamble.
+  EXPECT_EQ(T.expandedTokens().front().text(SM), "inline");
+  // Last token is always 'eof'.
+  EXPECT_EQ(T.expandedTokens().back().kind(), tok::eof);
+  // Check the token before 'eof'.
+  EXPECT_EQ(T.expandedTokens().drop_back().back().text(SM), "}");
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang