[clang-tools-extra] ea79b77 - [clangd] Dont work on diags if we are not going to emit
Kadir Cetinkaya via cfe-commits
cfe-commits at lists.llvm.org
Thu Sep 16 07:55:06 PDT 2021
Author: Kadir Cetinkaya
Date: 2021-09-16T16:50:42+02:00
New Revision: ea79b77da3eeba926e16c3dd8a4f6626c139e185
URL: https://github.com/llvm/llvm-project/commit/ea79b77da3eeba926e16c3dd8a4f6626c139e185
DIFF: https://github.com/llvm/llvm-project/commit/ea79b77da3eeba926e16c3dd8a4f6626c139e185.diff
LOG: [clangd] Dont work on diags if we are not going to emit
Don't install clang-tidy checks and IncludeFixer or process clang diags
when they're going to be dropped. Also disables analysis for some
warnings completely.
Differential Revision: https://reviews.llvm.org/D109884
Added:
Modified:
clang-tools-extra/clangd/ParsedAST.cpp
clang-tools-extra/clangd/unittests/ParsedASTTests.cpp
Removed:
################################################################################
diff --git a/clang-tools-extra/clangd/ParsedAST.cpp b/clang-tools-extra/clangd/ParsedAST.cpp
index d8351199d100f..9baa7000a18c1 100644
--- a/clang-tools-extra/clangd/ParsedAST.cpp
+++ b/clang-tools-extra/clangd/ParsedAST.cpp
@@ -283,15 +283,21 @@ ParsedAST::build(llvm::StringRef Filename, const ParseInputs &Inputs,
llvm::Optional<PreamblePatch> Patch;
bool PreserveDiags = true;
+ // We might use an ignoring diagnostic consumer if they are going to be
+ // dropped later on to not pay for extra latency by processing them.
+ DiagnosticConsumer *DiagConsumer = &ASTDiags;
+ IgnoreDiagnostics DropDiags;
if (Preamble) {
Patch = PreamblePatch::createFullPatch(Filename, Inputs, *Preamble);
Patch->apply(*CI);
PreserveDiags = Patch->preserveDiagnostics();
+ if (!PreserveDiags)
+ DiagConsumer = &DropDiags;
}
auto Clang = prepareCompilerInstance(
std::move(CI), PreamblePCH,
llvm::MemoryBuffer::getMemBufferCopy(Inputs.Contents, Filename), VFS,
- ASTDiags);
+ *DiagConsumer);
if (!Clang) {
// The last diagnostic contains information about the reason of this
// failure.
@@ -301,6 +307,10 @@ ParsedAST::build(llvm::StringRef Filename, const ParseInputs &Inputs,
: "unknown error");
return None;
}
+ if (!PreserveDiags) {
+ // Skips some analysis.
+ Clang->getDiagnosticOpts().IgnoreWarnings = true;
+ }
auto Action = std::make_unique<ClangdFrontendAction>();
const FrontendInputFile &MainInput = Clang->getFrontendOpts().Inputs[0];
@@ -329,7 +339,10 @@ ParsedAST::build(llvm::StringRef Filename, const ParseInputs &Inputs,
std::vector<std::unique_ptr<tidy::ClangTidyCheck>> CTChecks;
ast_matchers::MatchFinder CTFinder;
llvm::Optional<tidy::ClangTidyContext> CTContext;
- {
+ llvm::Optional<IncludeFixer> FixIncludes;
+ // No need to run clang-tidy or IncludeFixerif we are not going to surface
+ // diagnostics.
+ if (PreserveDiags) {
trace::Span Tracer("ClangTidyInit");
tidy::ClangTidyOptions ClangTidyOpts =
getTidyOptionsForFile(Inputs.ClangTidyProvider, Filename);
@@ -389,28 +402,28 @@ ParsedAST::build(llvm::StringRef Filename, const ParseInputs &Inputs,
}
return DiagLevel;
});
- }
- // Add IncludeFixer which can recover diagnostics caused by missing includes
- // (e.g. incomplete type) and attach include insertion fixes to diagnostics.
- llvm::Optional<IncludeFixer> FixIncludes;
- auto BuildDir = VFS->getCurrentWorkingDirectory();
- if (Inputs.Index && !BuildDir.getError()) {
- auto Style = getFormatStyleForFile(Filename, Inputs.Contents, *Inputs.TFS);
- auto Inserter = std::make_shared<IncludeInserter>(
- Filename, Inputs.Contents, Style, BuildDir.get(),
- &Clang->getPreprocessor().getHeaderSearchInfo());
- if (Preamble) {
- for (const auto &Inc : Preamble->Includes.MainFileIncludes)
- Inserter->addExisting(Inc);
+ // Add IncludeFixer which can recover diagnostics caused by missing includes
+ // (e.g. incomplete type) and attach include insertion fixes to diagnostics.
+ auto BuildDir = VFS->getCurrentWorkingDirectory();
+ if (Inputs.Index && !BuildDir.getError()) {
+ auto Style =
+ getFormatStyleForFile(Filename, Inputs.Contents, *Inputs.TFS);
+ auto Inserter = std::make_shared<IncludeInserter>(
+ Filename, Inputs.Contents, Style, BuildDir.get(),
+ &Clang->getPreprocessor().getHeaderSearchInfo());
+ if (Preamble) {
+ for (const auto &Inc : Preamble->Includes.MainFileIncludes)
+ Inserter->addExisting(Inc);
+ }
+ FixIncludes.emplace(Filename, Inserter, *Inputs.Index,
+ /*IndexRequestLimit=*/5);
+ ASTDiags.contributeFixes([&FixIncludes](DiagnosticsEngine::Level DiagLevl,
+ const clang::Diagnostic &Info) {
+ return FixIncludes->fix(DiagLevl, Info);
+ });
+ Clang->setExternalSemaSource(FixIncludes->unresolvedNameRecorder());
}
- FixIncludes.emplace(Filename, Inserter, *Inputs.Index,
- /*IndexRequestLimit=*/5);
- ASTDiags.contributeFixes([&FixIncludes](DiagnosticsEngine::Level DiagLevl,
- const clang::Diagnostic &Info) {
- return FixIncludes->fix(DiagLevl, Info);
- });
- Clang->setExternalSemaSource(FixIncludes->unresolvedNameRecorder());
}
IncludeStructure Includes;
diff --git a/clang-tools-extra/clangd/unittests/ParsedASTTests.cpp b/clang-tools-extra/clangd/unittests/ParsedASTTests.cpp
index ef0199a11ed90..a3c9f74e31757 100644
--- a/clang-tools-extra/clangd/unittests/ParsedASTTests.cpp
+++ b/clang-tools-extra/clangd/unittests/ParsedASTTests.cpp
@@ -460,77 +460,6 @@ TEST(ParsedASTTest, ReplayPreambleForTidyCheckers) {
Code.substr(FileRange.Begin - 1, FileRange.End - FileRange.Begin + 2));
EXPECT_EQ(SkippedFiles[I].kind(), tok::header_name);
}
-
- TU.AdditionalFiles["a.h"] = "";
- TU.AdditionalFiles["b.h"] = "";
- TU.AdditionalFiles["c.h"] = "";
- // Make sure replay logic works with patched preambles.
- llvm::StringLiteral Baseline = R"cpp(
- #include "a.h"
- #include "c.h")cpp";
- MockFS FS;
- TU.Code = Baseline.str();
- auto Inputs = TU.inputs(FS);
- auto BaselinePreamble = TU.preamble();
- ASSERT_TRUE(BaselinePreamble);
-
- // First make sure we don't crash on various modifications to the preamble.
- llvm::StringLiteral Cases[] = {
- // clang-format off
- // New include in middle.
- R"cpp(
- #include "a.h"
- #include "b.h"
- #include "c.h")cpp",
- // New include at top.
- R"cpp(
- #include "b.h"
- #include "a.h"
- #include "c.h")cpp",
- // New include at bottom.
- R"cpp(
- #include "a.h"
- #include "c.h"
- #include "b.h")cpp",
- // Same size with a missing include.
- R"cpp(
- #include "a.h"
- #include "b.h")cpp",
- // Smaller with no new includes.
- R"cpp(
- #include "a.h")cpp",
- // Smaller with a new includes.
- R"cpp(
- #include "b.h")cpp",
- // clang-format on
- };
- for (llvm::StringLiteral Case : Cases) {
- TU.Code = Case.str();
-
- IgnoreDiagnostics Diags;
- auto CI = buildCompilerInvocation(TU.inputs(FS), Diags);
- auto PatchedAST = ParsedAST::build(testPath(TU.Filename), TU.inputs(FS),
- std::move(CI), {}, BaselinePreamble);
- ASSERT_TRUE(PatchedAST);
- EXPECT_FALSE(PatchedAST->getDiagnostics());
- }
-
- // Then ensure correctness by making sure includes were seen only once.
- // Note that we first see the includes from the patch, as preamble includes
- // are replayed after exiting the built-in file.
- Includes.clear();
- TU.Code = R"cpp(
- #include "a.h"
- #include "b.h")cpp";
- IgnoreDiagnostics Diags;
- auto CI = buildCompilerInvocation(TU.inputs(FS), Diags);
- auto PatchedAST = ParsedAST::build(testPath(TU.Filename), TU.inputs(FS),
- std::move(CI), {}, BaselinePreamble);
- ASSERT_TRUE(PatchedAST);
- EXPECT_FALSE(PatchedAST->getDiagnostics());
- EXPECT_THAT(Includes,
- ElementsAre(WithFileName(testPath("__preamble_patch__.h")),
- WithFileName("b.h"), WithFileName("a.h")));
}
TEST(ParsedASTTest, PatchesAdditionalIncludes) {
More information about the cfe-commits
mailing list