[PATCH] D109884: [clangd] Dont work on diags if we are not going to emit
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGea79b77da3ee: [clangd] Dont work on diags if we are not
going to emit (authored by kadircet).
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D109884/new/
https://reviews.llvm.org/D109884
Files:
clang-tools-extra/clangd/ParsedAST.cpp
clang-tools-extra/clangd/unittests/ParsedASTTests.cpp
Index: clang-tools-extra/clangd/unittests/ParsedASTTests.cpp
===
--- clang-tools-extra/clangd/unittests/ParsedASTTests.cpp
+++ clang-tools-extra/clangd/unittests/ParsedASTTests.cpp
@@ -460,77 +460,6 @@
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) {
Index: clang-tools-extra/clangd/ParsedAST.cpp
===
--- clang-tools-extra/clangd/ParsedAST.cpp
+++ clang-tools-extra/clangd/ParsedAST.cpp
@@ -283,15 +283,21 @@
llvm::Optional 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 =
+ IgnoreDiagnostics DropDiags;
if (Preamble) {
Patch = PreamblePatch::createFullPatch(Filename, Inputs, *Preamble);
Patch->apply(*CI);
PreserveDiags = Patch->preserveDiagnostics();
+if (!PreserveDiags)
+ DiagConsumer =
}
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 @@
: "unknown error");
return None;
}
+ if (!PreserveDiags) {
+// Skips some analysis.
+Clang->getDiagnosticOpts().IgnoreWarnings = true;
+ }
auto Action = std::make_unique();
const FrontendInputFile = Clang->getFrontendOpts().Inputs[0];
@@ -329,7 +339,10 @@
std::vector> CTChecks;
ast_matchers::MatchFinder CTFinder;
llvm::Optional CTContext;
- {
+ llvm::Optional FixIncludes;
+ // No need to run clang-tidy or IncludeFixerif we are not going to surface
+ // diagnostics.
+ if (PreserveDiags) {
trace::Span
[PATCH] D109884: [clangd] Dont work on diags if we are not going to emit
kadircet updated this revision to Diff 372947.
kadircet added a comment.
- Revert revert of IncludeFixer changes
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D109884/new/
https://reviews.llvm.org/D109884
Files:
clang-tools-extra/clangd/ParsedAST.cpp
clang-tools-extra/clangd/unittests/ParsedASTTests.cpp
Index: clang-tools-extra/clangd/unittests/ParsedASTTests.cpp
===
--- clang-tools-extra/clangd/unittests/ParsedASTTests.cpp
+++ clang-tools-extra/clangd/unittests/ParsedASTTests.cpp
@@ -460,77 +460,6 @@
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) {
Index: clang-tools-extra/clangd/ParsedAST.cpp
===
--- clang-tools-extra/clangd/ParsedAST.cpp
+++ clang-tools-extra/clangd/ParsedAST.cpp
@@ -283,15 +283,21 @@
llvm::Optional 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 =
+ IgnoreDiagnostics DropDiags;
if (Preamble) {
Patch = PreamblePatch::createFullPatch(Filename, Inputs, *Preamble);
Patch->apply(*CI);
PreserveDiags = Patch->preserveDiagnostics();
+if (!PreserveDiags)
+ DiagConsumer =
}
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 @@
: "unknown error");
return None;
}
+ if (!PreserveDiags) {
+// Skips some analysis.
+Clang->getDiagnosticOpts().IgnoreWarnings = true;
+ }
auto Action = std::make_unique();
const FrontendInputFile = Clang->getFrontendOpts().Inputs[0];
@@ -329,7 +339,10 @@
std::vector> CTChecks;
ast_matchers::MatchFinder CTFinder;
llvm::Optional CTContext;
- {
+ llvm::Optional 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);
@@
[PATCH] D109884: [clangd] Dont work on diags if we are not going to emit
kadircet updated this revision to Diff 372943.
kadircet marked 2 inline comments as done.
kadircet added a comment.
- Get rid of static_casts
- Update comments
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D109884/new/
https://reviews.llvm.org/D109884
Files:
clang-tools-extra/clangd/ParsedAST.cpp
clang-tools-extra/clangd/unittests/ParsedASTTests.cpp
Index: clang-tools-extra/clangd/unittests/ParsedASTTests.cpp
===
--- clang-tools-extra/clangd/unittests/ParsedASTTests.cpp
+++ clang-tools-extra/clangd/unittests/ParsedASTTests.cpp
@@ -460,77 +460,6 @@
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) {
Index: clang-tools-extra/clangd/ParsedAST.cpp
===
--- clang-tools-extra/clangd/ParsedAST.cpp
+++ clang-tools-extra/clangd/ParsedAST.cpp
@@ -283,15 +283,21 @@
llvm::Optional 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 =
+ IgnoreDiagnostics DropDiags;
if (Preamble) {
Patch = PreamblePatch::createFullPatch(Filename, Inputs, *Preamble);
Patch->apply(*CI);
PreserveDiags = Patch->preserveDiagnostics();
+if (!PreserveDiags)
+ DiagConsumer =
}
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 @@
: "unknown error");
return None;
}
+ if (!PreserveDiags) {
+// Skips some analysis.
+Clang->getDiagnosticOpts().IgnoreWarnings = true;
+ }
auto Action = std::make_unique();
const FrontendInputFile = Clang->getFrontendOpts().Inputs[0];
@@ -329,7 +339,8 @@
std::vector> CTChecks;
ast_matchers::MatchFinder CTFinder;
llvm::Optional CTContext;
- {
+ // No need to run clang-tidy if we are not going to surface diagnostics.
+ if (PreserveDiags) {
trace::Span Tracer("ClangTidyInit");
tidy::ClangTidyOptions ClangTidyOpts =
getTidyOptionsForFile(Inputs.ClangTidyProvider, Filename);
[PATCH] D109884: [clangd] Dont work on diags if we are not going to emit
sammccall accepted this revision. sammccall added inline comments. This revision is now accepted and ready to land. Comment at: clang-tools-extra/clangd/unittests/ParsedASTTests.cpp:464 - - TU.AdditionalFiles["a.h"] = ""; - TU.AdditionalFiles["b.h"] = ""; kadircet wrote: > sammccall wrote: > > Why are these tests deleted? > they rely on the fact that clang-tidy checkers PPcallbacks are run with > patched asts, but it is no longer the case, hence it becomes impossible to > satisfy them. > > currently we never replay includes with patched preambles, as ReplayPreamble > bails out when there are no existing PPCallbacks and that's always the case. > we can still try and test it via complicated means like enabling > ClangdFeatures to register PPCallbacks, but it will be testing a feature that > doesn't exist in practice + would be quite some work for just testing, so I'd > rather leave it as-is (probably by adding a comment around > ReplayPreamble::attach saying that we should test this once there are > non-clang-tidy users). Oops, I didn't scroll up fast enough (reviewing on phone lol). SGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D109884/new/ https://reviews.llvm.org/D109884 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D109884: [clangd] Dont work on diags if we are not going to emit
kadircet added inline comments. Comment at: clang-tools-extra/clangd/unittests/ParsedASTTests.cpp:464 - - TU.AdditionalFiles["a.h"] = ""; - TU.AdditionalFiles["b.h"] = ""; sammccall wrote: > Why are these tests deleted? they rely on the fact that clang-tidy checkers PPcallbacks are run with patched asts, but it is no longer the case, hence it becomes impossible to satisfy them. currently we never replay includes with patched preambles, as ReplayPreamble bails out when there are no existing PPCallbacks and that's always the case. we can still try and test it via complicated means like enabling ClangdFeatures to register PPCallbacks, but it will be testing a feature that doesn't exist in practice + would be quite some work for just testing, so I'd rather leave it as-is (probably by adding a comment around ReplayPreamble::attach saying that we should test this once there are non-clang-tidy users). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D109884/new/ https://reviews.llvm.org/D109884 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D109884: [clangd] Dont work on diags if we are not going to emit
sammccall added a comment.
Great!
Comment at: clang-tools-extra/clangd/ParsedAST.cpp:295
llvm::MemoryBuffer::getMemBufferCopy(Inputs.Contents, Filename), VFS,
- ASTDiags);
+ PreserveDiags ? static_cast(ASTDiags)
+: static_cast(DropDiags));
These explicit upcasts seem ugly enough that I might extract a pointer var to
avoid them...
Comment at: clang-tools-extra/clangd/ParsedAST.cpp:338
llvm::Optional CTContext;
- {
+ // No need to initalize a clang-tidy context if we are not going to surface
+ // diagnostics.
s/initialize a clang-tidy context/run clang-tidy/
Comment at: clang-tools-extra/clangd/unittests/ParsedASTTests.cpp:464
-
- TU.AdditionalFiles["a.h"] = "";
- TU.AdditionalFiles["b.h"] = "";
Why are these tests deleted?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D109884/new/
https://reviews.llvm.org/D109884
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D109884: [clangd] Dont work on diags if we are not going to emit
kadircet updated this revision to Diff 372938.
kadircet added a comment.
- Disable IncludeFixer too.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D109884/new/
https://reviews.llvm.org/D109884
Files:
clang-tools-extra/clangd/ParsedAST.cpp
clang-tools-extra/clangd/unittests/ParsedASTTests.cpp
Index: clang-tools-extra/clangd/unittests/ParsedASTTests.cpp
===
--- clang-tools-extra/clangd/unittests/ParsedASTTests.cpp
+++ clang-tools-extra/clangd/unittests/ParsedASTTests.cpp
@@ -460,77 +460,6 @@
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) {
Index: clang-tools-extra/clangd/ParsedAST.cpp
===
--- clang-tools-extra/clangd/ParsedAST.cpp
+++ clang-tools-extra/clangd/ParsedAST.cpp
@@ -288,10 +288,12 @@
Patch->apply(*CI);
PreserveDiags = Patch->preserveDiagnostics();
}
+ IgnoreDiagnostics DropDiags;
auto Clang = prepareCompilerInstance(
std::move(CI), PreamblePCH,
llvm::MemoryBuffer::getMemBufferCopy(Inputs.Contents, Filename), VFS,
- ASTDiags);
+ PreserveDiags ? static_cast(ASTDiags)
+: static_cast(DropDiags));
if (!Clang) {
// The last diagnostic contains information about the reason of this
// failure.
@@ -301,6 +303,10 @@
: "unknown error");
return None;
}
+ if (!PreserveDiags) {
+// Skips some analysis.
+Clang->getDiagnosticOpts().IgnoreWarnings = true;
+ }
auto Action = std::make_unique();
const FrontendInputFile = Clang->getFrontendOpts().Inputs[0];
@@ -329,7 +335,10 @@
std::vector> CTChecks;
ast_matchers::MatchFinder CTFinder;
llvm::Optional CTContext;
- {
+ llvm::Optional FixIncludes;
+ // No need to initalize a clang-tidy context and include fixerif we are not
+ // going to surface diagnostics.
+ if (PreserveDiags) {
trace::Span Tracer("ClangTidyInit");
tidy::ClangTidyOptions ClangTidyOpts =
getTidyOptionsForFile(Inputs.ClangTidyProvider, Filename);
@@ -389,28 +398,28 @@
}
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 FixIncludes;
- auto BuildDir =
[PATCH] D109884: [clangd] Dont work on diags if we are not going to emit
kadircet created this revision.
kadircet added a reviewer: sammccall.
Herald added subscribers: usaxena95, arphaman.
kadircet requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov.
Herald added a project: clang-tools-extra.
Doesn't install clang-tidy checks & process clang diags when they're
going to be dropped. Also disables analysis for some warnings completely.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D109884
Files:
clang-tools-extra/clangd/ParsedAST.cpp
clang-tools-extra/clangd/unittests/ParsedASTTests.cpp
Index: clang-tools-extra/clangd/unittests/ParsedASTTests.cpp
===
--- clang-tools-extra/clangd/unittests/ParsedASTTests.cpp
+++ clang-tools-extra/clangd/unittests/ParsedASTTests.cpp
@@ -460,77 +460,6 @@
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) {
Index: clang-tools-extra/clangd/ParsedAST.cpp
===
--- clang-tools-extra/clangd/ParsedAST.cpp
+++ clang-tools-extra/clangd/ParsedAST.cpp
@@ -288,10 +288,12 @@
Patch->apply(*CI);
PreserveDiags = Patch->preserveDiagnostics();
}
+ IgnoreDiagnostics DropDiags;
auto Clang = prepareCompilerInstance(
std::move(CI), PreamblePCH,
llvm::MemoryBuffer::getMemBufferCopy(Inputs.Contents, Filename), VFS,
- ASTDiags);
+ PreserveDiags ? static_cast(ASTDiags)
+: static_cast(DropDiags));
if (!Clang) {
// The last diagnostic contains information about the reason of this
// failure.
@@ -301,6 +303,10 @@
: "unknown error");
return None;
}
+ if (!PreserveDiags) {
+// Skips some analysis.
+Clang->getDiagnosticOpts().IgnoreWarnings = true;
+ }
auto Action = std::make_unique();
const FrontendInputFile = Clang->getFrontendOpts().Inputs[0];
@@ -329,7 +335,9 @@
std::vector> CTChecks;
ast_matchers::MatchFinder CTFinder;
llvm::Optional CTContext;
- {
+ // No need to initalize a clang-tidy context if we are not going to surface
+ // diagnostics.
+ if (PreserveDiags) {
trace::Span Tracer("ClangTidyInit");
tidy::ClangTidyOptions ClangTidyOpts =
getTidyOptionsForFile(Inputs.ClangTidyProvider, Filename);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
