sammccall created this revision.
sammccall added a reviewer: kadircet.
Herald added subscribers: cfe-commits, usaxena95, jfb, arphaman, jkorous,
MaskRay, javed.absar, ilya-biryukov.
Herald added a project: clang.
This saves something like 20% latency on the AST build with a typical load of
clang-tidy checks.
The tradeoff is the AST is never cached for diagnostics, but this is less
latency sensitive and rarely happens anyway.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D81069
Files:
clang-tools-extra/clangd/ParsedAST.cpp
clang-tools-extra/clangd/ParsedAST.h
clang-tools-extra/clangd/TUScheduler.cpp
clang-tools-extra/clangd/TUScheduler.h
clang-tools-extra/clangd/test/metrics.test
clang-tools-extra/clangd/unittests/ParsedASTTests.cpp
clang-tools-extra/clangd/unittests/PreambleTests.cpp
clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
clang-tools-extra/clangd/unittests/TestTU.cpp
Index: clang-tools-extra/clangd/unittests/TestTU.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/TestTU.cpp
+++ clang-tools-extra/clangd/unittests/TestTU.cpp
@@ -84,8 +84,9 @@
auto Preamble = clang::clangd::buildPreamble(testPath(Filename), *CI, Inputs,
/*StoreInMemory=*/true,
/*PreambleCallback=*/nullptr);
- auto AST = ParsedAST::build(testPath(Filename), Inputs, std::move(CI),
- Diags.take(), Preamble);
+ auto AST =
+ ParsedAST::build(testPath(Filename), Inputs, std::move(CI),
+ /*ProduceDiagnostics=*/true, Diags.take(), Preamble);
if (!AST.hasValue()) {
ADD_FAILURE() << "Failed to build code:\n" << Code;
llvm_unreachable("Failed to build TestTU!");
Index: clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
+++ clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
@@ -521,7 +521,6 @@
}
TEST_F(TUSchedulerTests, EvictedAST) {
- std::atomic<int> BuiltASTCounter(0);
auto Opts = optsForTest();
Opts.AsyncThreadsCount = 1;
Opts.RetentionPolicy.MaxRetainedASTs = 2;
@@ -532,52 +531,45 @@
int* a;
double* b = a;
)cpp";
- llvm::StringLiteral OtherSourceContents = R"cpp(
- int* a;
- double* b = a + 0;
- )cpp";
auto Foo = testPath("foo.cpp");
auto Bar = testPath("bar.cpp");
auto Baz = testPath("baz.cpp");
- EXPECT_THAT(Tracer.takeMetric("ast_access_diag", "hit"), SizeIs(0));
- EXPECT_THAT(Tracer.takeMetric("ast_access_diag", "miss"), SizeIs(0));
- // Build one file in advance. We will not access it later, so it will be the
- // one that the cache will evict.
- updateWithCallback(S, Foo, SourceContents, WantDiagnostics::Yes,
- [&BuiltASTCounter]() { ++BuiltASTCounter; });
+ // Build one file in advance.
+ S.update(Foo, getInputs(Foo, SourceContents.str()), WantDiagnostics::Yes);
ASSERT_TRUE(S.blockUntilIdle(timeoutSeconds(10)));
- ASSERT_EQ(BuiltASTCounter.load(), 1);
- EXPECT_THAT(Tracer.takeMetric("ast_access_diag", "hit"), SizeIs(0));
- EXPECT_THAT(Tracer.takeMetric("ast_access_diag", "miss"), SizeIs(1));
-
- // Build two more files. Since we can retain only 2 ASTs, these should be
- // the ones we see in the cache later.
- updateWithCallback(S, Bar, SourceContents, WantDiagnostics::Yes,
- [&BuiltASTCounter]() { ++BuiltASTCounter; });
- updateWithCallback(S, Baz, SourceContents, WantDiagnostics::Yes,
- [&BuiltASTCounter]() { ++BuiltASTCounter; });
+ // AST should be cached after diagnostics are produced.
+ ASSERT_THAT(S.getFilesWithCachedAST(), UnorderedElementsAre(Foo));
+
+ // Build two more files.
+ S.update(Bar, getInputs(Foo, SourceContents.str()), WantDiagnostics::Yes);
+ S.update(Baz, getInputs(Foo, SourceContents.str()), WantDiagnostics::Yes);
ASSERT_TRUE(S.blockUntilIdle(timeoutSeconds(10)));
- ASSERT_EQ(BuiltASTCounter.load(), 3);
- EXPECT_THAT(Tracer.takeMetric("ast_access_diag", "hit"), SizeIs(0));
- EXPECT_THAT(Tracer.takeMetric("ast_access_diag", "miss"), SizeIs(2));
+ // We can retain only 2 ASTs, Foo should have been evicted.
+ ASSERT_THAT(S.getFilesWithCachedAST(), UnorderedElementsAre(Bar, Baz));
- // Check only the last two ASTs are retained.
+ // No reads yet.
+ EXPECT_THAT(Tracer.takeMetric("ast_access_read", "hit"), SizeIs(0));
+ EXPECT_THAT(Tracer.takeMetric("ast_access_read", "miss"), SizeIs(0));
+
+ // Run action on file in AST cache.
+ S.runWithAST("Cached", Bar,
+ [](llvm::Expected<InputsAndAST> E) { EXPECT_TRUE(bool(E)); });
+ ASSERT_TRUE(S.blockUntilIdle(timeoutSeconds(10)));
+ // Should have hit the cache, same files still present.
ASSERT_THAT(S.getFilesWithCachedAST(), UnorderedElementsAre(Bar, Baz));
+ EXPECT_THAT(Tracer.takeMetric("ast_access_read", "hit"), SizeIs(1));
+ EXPECT_THAT(Tracer.takeMetric("ast_access_read", "miss"), SizeIs(0));
// Access the old file again.
- updateWithCallback(S, Foo, OtherSourceContents, WantDiagnostics::Yes,
- [&BuiltASTCounter]() { ++BuiltASTCounter; });
+ S.runWithAST("Uncached", Foo,
+ [](llvm::Expected<InputsAndAST> E) { EXPECT_TRUE(bool(E)); });
ASSERT_TRUE(S.blockUntilIdle(timeoutSeconds(10)));
- ASSERT_EQ(BuiltASTCounter.load(), 4);
- EXPECT_THAT(Tracer.takeMetric("ast_access_diag", "hit"), SizeIs(0));
- EXPECT_THAT(Tracer.takeMetric("ast_access_diag", "miss"), SizeIs(1));
-
- // Check the AST for foo.cpp is retained now and one of the others got
- // evicted.
- EXPECT_THAT(S.getFilesWithCachedAST(),
- UnorderedElementsAre(Foo, AnyOf(Bar, Baz)));
+ // We missed the cache. Now Foo is retained and Baz got evicted (LRU).
+ EXPECT_THAT(Tracer.takeMetric("ast_access_read", "hit"), SizeIs(0));
+ EXPECT_THAT(Tracer.takeMetric("ast_access_read", "miss"), SizeIs(1));
+ EXPECT_THAT(S.getFilesWithCachedAST(), UnorderedElementsAre(Foo, Bar));
}
// We send "empty" changes to TUScheduler when we think some external event
@@ -798,8 +790,6 @@
EXPECT_THAT(Tracer.takeMetric("ast_access_read", "hit"), SizeIs(0));
EXPECT_THAT(Tracer.takeMetric("ast_access_read", "miss"), SizeIs(0));
- EXPECT_THAT(Tracer.takeMetric("ast_access_diag", "hit"), SizeIs(0));
- EXPECT_THAT(Tracer.takeMetric("ast_access_diag", "miss"), SizeIs(0));
updateWithDiags(
S, FooCpp, Contents, WantDiagnostics::No,
[](std::vector<Diag>) { ADD_FAILURE() << "Should not be called."; });
@@ -811,15 +801,13 @@
EXPECT_THAT(Tracer.takeMetric("ast_access_read", "hit"), SizeIs(0));
EXPECT_THAT(Tracer.takeMetric("ast_access_read", "miss"), SizeIs(1));
- // Even though the inputs didn't change and AST can be reused, we need to
+ // Even though the inputs didn't change, we need to
// report the diagnostics, as they were not reported previously.
std::atomic<bool> SeenDiags(false);
updateWithDiags(S, FooCpp, Contents, WantDiagnostics::Auto,
[&](std::vector<Diag>) { SeenDiags = true; });
ASSERT_TRUE(S.blockUntilIdle(timeoutSeconds(10)));
ASSERT_TRUE(SeenDiags);
- EXPECT_THAT(Tracer.takeMetric("ast_access_diag", "hit"), SizeIs(1));
- EXPECT_THAT(Tracer.takeMetric("ast_access_diag", "miss"), SizeIs(0));
// Subsequent request does not get any diagnostics callback because the same
// diags have previously been reported and the inputs didn't change.
Index: clang-tools-extra/clangd/unittests/PreambleTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/PreambleTests.cpp
+++ clang-tools-extra/clangd/unittests/PreambleTests.cpp
@@ -205,8 +205,8 @@
ADD_FAILURE() << "Failed to build compiler invocation";
return llvm::None;
}
- return ParsedAST::build(testPath(TU.Filename), TU.inputs(), std::move(CI), {},
- BaselinePreamble);
+ return ParsedAST::build(testPath(TU.Filename), TU.inputs(), std::move(CI),
+ /*ProduceDiagnostics=*/false, {}, BaselinePreamble);
}
std::string getPreamblePatch(llvm::StringRef Baseline,
Index: clang-tools-extra/clangd/unittests/ParsedASTTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/ParsedASTTests.cpp
+++ clang-tools-extra/clangd/unittests/ParsedASTTests.cpp
@@ -24,6 +24,7 @@
#include "TestFS.h"
#include "TestTU.h"
#include "clang/AST/DeclTemplate.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
#include "clang/Basic/SourceLocation.h"
#include "clang/Basic/SourceManager.h"
#include "clang/Basic/TokenKinds.h"
@@ -44,6 +45,7 @@
using ::testing::AllOf;
using ::testing::ElementsAre;
using ::testing::ElementsAreArray;
+using ::testing::IsEmpty;
MATCHER_P(DeclNamed, Name, "") {
if (NamedDecl *ND = dyn_cast<NamedDecl>(arg))
@@ -56,6 +58,8 @@
return false;
}
+MATCHER_P(Diag, Message, "") { return arg.Message == Message; }
+
// Matches if the Decl has template args equal to ArgName. If the decl is a
// NamedDecl and ArgName is an empty string it also matches.
MATCHER_P(WithTemplateArgs, ArgName, "") {
@@ -452,8 +456,9 @@
ASSERT_TRUE(EmptyPreamble);
TU.Code = "#include <a.h>";
Includes.clear();
- auto PatchedAST = ParsedAST::build(testPath(TU.Filename), TU.inputs(),
- std::move(CI), {}, EmptyPreamble);
+ auto PatchedAST =
+ ParsedAST::build(testPath(TU.Filename), TU.inputs(), std::move(CI),
+ /*ProduceDiagnostics=*/true, {}, EmptyPreamble);
ASSERT_TRUE(PatchedAST);
// Make sure includes were seen only once.
EXPECT_THAT(Includes,
@@ -461,6 +466,66 @@
WithFileName("a.h")));
}
+TEST(ParsedASTTest, NoDiagnostics) {
+ static bool TidyCheckIsError;
+ // A simple clang-tidy check, which we can verify:
+ // - is used when ProduceDiagnostics is on
+ // - does no work (isn't instantiated) when ProduceDiagnostics is off.
+ struct NoVars : public tidy::ClangTidyCheck {
+ NoVars(StringRef N, tidy::ClangTidyContext *C) : ClangTidyCheck(N, C) {
+ EXPECT_FALSE(TidyCheckIsError) << "Shouldn't even be instantiated!";
+ }
+ void registerMatchers(ast_matchers::MatchFinder *Finder) override {
+ Finder->addMatcher(ast_matchers::varDecl().bind("var"), this);
+ }
+ void check(const ast_matchers::MatchFinder::MatchResult &Result) override {
+ diag(Result.Nodes.getNodeAs<Decl>("var")->getLocation(), "tidy");
+ }
+ };
+ struct NoVarsModule : public tidy::ClangTidyModule {
+ void addCheckFactories(tidy::ClangTidyCheckFactories &CF) override {
+ CF.registerCheck<NoVars>("no-vars");
+ }
+ };
+ static tidy::ClangTidyModuleRegistry::Add<NoVarsModule> X("no-vars-mod", "");
+
+ StoreDiags StoreDriverDiags;
+ TestTU TU;
+ TU.ExtraArgs.push_back("-flag");
+ TU.AdditionalFiles["header.h"] = "#error header";
+ TU.Code = R"cpp(
+ #include "header.h"
+ #error preamble
+ int foo;
+ #error body
+ )cpp";
+ TU.ClangTidyChecks = "no-vars";
+
+ auto CI = *buildCompilerInvocation(TU.inputs(), StoreDriverDiags);
+ auto Preamble = TU.preamble();
+ auto Inputs = TU.inputs();
+ auto DriverDiags = StoreDriverDiags.take();
+
+ // Build with diagnostics, to sanity-check the inputs.
+ TidyCheckIsError = false;
+ auto AST = ParsedAST::build(
+ testPath(TU.Filename), Inputs, std::make_unique<CompilerInvocation>(CI),
+ /*ProduceDiagnostics=*/true, DriverDiags, Preamble);
+ EXPECT_THAT(AST->getDiagnostics(),
+ ElementsAre(Diag("unknown argument: '-flag'"),
+ Diag("in included file: header"), Diag("preamble"),
+ Diag("body"), Diag("tidy")));
+ EXPECT_EQ(Decl::Var, findDecl(*AST, "foo").getKind());
+
+ // Build with no diagnostics.
+ TidyCheckIsError = true; // Verify we don't do *any* clang-tidy work.
+ AST = ParsedAST::build(testPath(TU.Filename), Inputs,
+ std::make_unique<CompilerInvocation>(CI),
+ /*ProduceDiagnostics=*/false, DriverDiags, Preamble);
+ EXPECT_THAT(AST->getDiagnostics(), IsEmpty()) << "ProduceDiagnostics = false";
+ EXPECT_EQ(Decl::Var, findDecl(*AST, "foo").getKind()) << "AST still good";
+}
+
TEST(ParsedASTTest, PatchesAdditionalIncludes) {
llvm::StringLiteral ModifiedContents = R"cpp(
#include "baz.h"
@@ -489,13 +554,14 @@
auto EmptyPreamble =
buildPreamble(testPath("foo.cpp"), *CI, Inputs, true, nullptr);
ASSERT_TRUE(EmptyPreamble);
- EXPECT_THAT(EmptyPreamble->Includes.MainFileIncludes, testing::IsEmpty());
+ EXPECT_THAT(EmptyPreamble->Includes.MainFileIncludes, IsEmpty());
// Now build an AST using empty preamble and ensure patched includes worked.
TU.Code = ModifiedContents.str();
Inputs = TU.inputs();
- auto PatchedAST = ParsedAST::build(testPath("foo.cpp"), Inputs, std::move(CI),
- {}, EmptyPreamble);
+ auto PatchedAST =
+ ParsedAST::build(testPath("foo.cpp"), Inputs, std::move(CI),
+ /*ProduceDiagnostics=*/false, {}, EmptyPreamble);
ASSERT_TRUE(PatchedAST);
ASSERT_TRUE(PatchedAST->getDiagnostics().empty());
@@ -538,8 +604,9 @@
// correctly parsed.
TU.Code = "";
Inputs = TU.inputs();
- auto PatchedAST = ParsedAST::build(testPath("foo.cpp"), Inputs, std::move(CI),
- {}, BaselinePreamble);
+ auto PatchedAST =
+ ParsedAST::build(testPath("foo.cpp"), Inputs, std::move(CI),
+ /*ProduceDiagnostics=*/false, {}, BaselinePreamble);
ASSERT_TRUE(PatchedAST);
// Ensure source location information is correct.
Index: clang-tools-extra/clangd/test/metrics.test
===================================================================
--- clang-tools-extra/clangd/test/metrics.test
+++ clang-tools-extra/clangd/test/metrics.test
@@ -4,7 +4,7 @@
{"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"test:///foo.c","languageId":"c","version":1,"text":"void main() {}"}}}
# Don't verify value, timestamp, or order.
# CHECK-DAG: d,lsp_latency,textDocument/didOpen,
-# CHECK-DAG: c,ast_access_diag,miss,
+# CHECK-DAG: d,span_latency,BuildAST,
---
{"jsonrpc":"2.0","id":5,"method":"shutdown"}
---
Index: clang-tools-extra/clangd/TUScheduler.h
===================================================================
--- clang-tools-extra/clangd/TUScheduler.h
+++ clang-tools-extra/clangd/TUScheduler.h
@@ -110,6 +110,7 @@
/// Indicates whether clang failed to build the TU.
bool BuildFailed = false;
/// Indicates whether we reused the prebuilt AST.
+ /// FIXME: always false, diagnostics are always built from fresh ASTs.
bool ReuseAST = false;
};
/// Serialize this to an LSP file status item.
Index: clang-tools-extra/clangd/TUScheduler.cpp
===================================================================
--- clang-tools-extra/clangd/TUScheduler.cpp
+++ clang-tools-extra/clangd/TUScheduler.cpp
@@ -675,9 +675,9 @@
llvm::Optional<std::unique_ptr<ParsedAST>> AST =
IdleASTs.take(this, &ASTAccessForRead);
if (!AST) {
- StoreDiags CompilerInvocationDiagConsumer;
+ IgnoreDiagnostics IgnoreDiags;
std::unique_ptr<CompilerInvocation> Invocation =
- buildCompilerInvocation(FileInputs, CompilerInvocationDiagConsumer);
+ buildCompilerInvocation(FileInputs, IgnoreDiags);
// Try rebuilding the AST.
vlog("ASTWorker rebuilding evicted AST to run {0}: {1} version {2}", Name,
FileName, FileInputs.Version);
@@ -687,7 +687,7 @@
llvm::Optional<ParsedAST> NewAST;
if (Invocation) {
NewAST = ParsedAST::build(FileName, FileInputs, std::move(Invocation),
- CompilerInvocationDiagConsumer.take(),
+ /*ProduceDiagnostics=*/false, {},
getPossiblyStalePreamble());
++ASTBuildCount;
}
@@ -796,9 +796,6 @@
void ASTWorker::generateDiagnostics(
std::unique_ptr<CompilerInvocation> Invocation, ParseInputs Inputs,
std::vector<Diag> CIDiags) {
- // Tracks ast cache accesses for publishing diags.
- static constexpr trace::Metric ASTAccessForDiag(
- "ast_access_diag", trace::Metric::Counter, "result");
assert(Invocation);
// No need to rebuild the AST if we won't send the diagnostics.
{
@@ -824,40 +821,27 @@
Status.ASTActivity.K = ASTAction::Building;
Status.ASTActivity.Name = std::move(TaskName);
});
- // We might be able to reuse the last we've built for a read request.
- // FIXME: It might be better to not reuse this AST. That way queued AST builds
- // won't be required for diags.
- llvm::Optional<std::unique_ptr<ParsedAST>> AST =
- IdleASTs.take(this, &ASTAccessForDiag);
- if (!AST || !InputsAreLatest) {
- auto RebuildStartTime = DebouncePolicy::clock::now();
- llvm::Optional<ParsedAST> NewAST = ParsedAST::build(
- FileName, Inputs, std::move(Invocation), CIDiags, LatestPreamble);
- auto RebuildDuration = DebouncePolicy::clock::now() - RebuildStartTime;
- ++ASTBuildCount;
- // Try to record the AST-build time, to inform future update debouncing.
- // This is best-effort only: if the lock is held, don't bother.
- std::unique_lock<std::mutex> Lock(Mutex, std::try_to_lock);
- if (Lock.owns_lock()) {
- // Do not let RebuildTimes grow beyond its small-size (i.e.
- // capacity).
- if (RebuildTimes.size() == RebuildTimes.capacity())
- RebuildTimes.erase(RebuildTimes.begin());
- RebuildTimes.push_back(RebuildDuration);
- Lock.unlock();
- }
- Status.update([&](TUStatus &Status) {
- Status.Details.ReuseAST = false;
- Status.Details.BuildFailed = !NewAST.hasValue();
- });
- AST = NewAST ? std::make_unique<ParsedAST>(std::move(*NewAST)) : nullptr;
- } else {
- log("Skipping rebuild of the AST for {0}, inputs are the same.", FileName);
- Status.update([](TUStatus &Status) {
- Status.Details.ReuseAST = true;
- Status.Details.BuildFailed = false;
- });
+ auto RebuildStartTime = DebouncePolicy::clock::now();
+ llvm::Optional<ParsedAST> AST =
+ ParsedAST::build(FileName, Inputs, std::move(Invocation),
+ /*ProduceDiagnostics=*/true, CIDiags, LatestPreamble);
+ auto RebuildDuration = DebouncePolicy::clock::now() - RebuildStartTime;
+ ++ASTBuildCount;
+ // Try to record the AST-build time, to inform future update debouncing.
+ // This is best-effort only: if the lock is held, don't bother.
+ std::unique_lock<std::mutex> Lock(Mutex, std::try_to_lock);
+ if (Lock.owns_lock()) {
+ // Do not let RebuildTimes grow beyond its small-size (i.e.
+ // capacity).
+ if (RebuildTimes.size() == RebuildTimes.capacity())
+ RebuildTimes.erase(RebuildTimes.begin());
+ RebuildTimes.push_back(RebuildDuration);
+ Lock.unlock();
}
+ Status.update([&](TUStatus &Status) {
+ Status.Details.ReuseAST = false;
+ Status.Details.BuildFailed = !AST.hasValue();
+ });
// Publish diagnostics.
auto RunPublish = [&](llvm::function_ref<void()> Publish) {
@@ -867,9 +851,9 @@
if (CanPublishResults)
Publish();
};
- if (*AST) {
+ if (AST) {
trace::Span Span("Running main AST callback");
- Callbacks.onMainAST(FileName, **AST, RunPublish);
+ Callbacks.onMainAST(FileName, *AST, RunPublish);
} else {
// Failed to build the AST, at least report diagnostics from the
// command line if there were any.
@@ -882,8 +866,10 @@
// queue raced ahead while we were waiting on the preamble. In that case the
// queue can't reuse the AST.
if (InputsAreLatest) {
- RanASTCallback = *AST != nullptr;
- IdleASTs.put(this, std::move(*AST));
+ RanASTCallback = AST.hasValue();
+ IdleASTs.take(this); // Put requires the entry doesn't exist.
+ IdleASTs.put(this,
+ AST ? std::make_unique<ParsedAST>(std::move(*AST)) : nullptr);
}
}
Index: clang-tools-extra/clangd/ParsedAST.h
===================================================================
--- clang-tools-extra/clangd/ParsedAST.h
+++ clang-tools-extra/clangd/ParsedAST.h
@@ -52,7 +52,7 @@
/// This function does not check if preamble is valid to reuse.
static llvm::Optional<ParsedAST>
build(llvm::StringRef Filename, const ParseInputs &Inputs,
- std::unique_ptr<clang::CompilerInvocation> CI,
+ std::unique_ptr<clang::CompilerInvocation> CI, bool ProduceDiagnostics,
llvm::ArrayRef<Diag> CompilerInvocationDiags,
std::shared_ptr<const PreambleData> Preamble);
Index: clang-tools-extra/clangd/ParsedAST.cpp
===================================================================
--- clang-tools-extra/clangd/ParsedAST.cpp
+++ clang-tools-extra/clangd/ParsedAST.cpp
@@ -22,6 +22,7 @@
#include "support/Trace.h"
#include "clang/AST/ASTContext.h"
#include "clang/AST/Decl.h"
+#include "clang/Basic/Diagnostic.h"
#include "clang/Basic/LangOptions.h"
#include "clang/Basic/SourceLocation.h"
#include "clang/Basic/SourceManager.h"
@@ -243,6 +244,7 @@
llvm::Optional<ParsedAST>
ParsedAST::build(llvm::StringRef Filename, const ParseInputs &Inputs,
std::unique_ptr<clang::CompilerInvocation> CI,
+ bool ProduceDiagnostics,
llvm::ArrayRef<Diag> CompilerInvocationDiags,
std::shared_ptr<const PreambleData> Preamble) {
trace::Span Tracer("BuildAST");
@@ -267,8 +269,11 @@
// This is on-by-default in windows to allow parsing SDK headers, but it
// breaks many features. Disable it for the main-file (not preamble).
CI->getLangOpts()->DelayedTemplateParsing = false;
+ if (!ProduceDiagnostics)
+ CI->getDiagnosticOpts().IgnoreWarnings = true;
StoreDiags ASTDiags;
+ IgnoringDiagConsumer IgnoreDiags;
llvm::Optional<PreamblePatch> Patch;
if (Preamble) {
@@ -278,7 +283,8 @@
auto Clang = prepareCompilerInstance(
std::move(CI), PreamblePCH,
llvm::MemoryBuffer::getMemBufferCopy(Inputs.Contents, Filename), VFS,
- ASTDiags);
+ ProduceDiagnostics ? llvm::cast<DiagnosticConsumer>(ASTDiags)
+ : llvm::cast<DiagnosticConsumer>(IgnoreDiags));
if (!Clang)
return None;
@@ -297,9 +303,9 @@
// ancestors outside this scope).
// In practice almost all checks work well without modifications.
std::vector<std::unique_ptr<tidy::ClangTidyCheck>> CTChecks;
- ast_matchers::MatchFinder CTFinder;
+ llvm::Optional<ast_matchers::MatchFinder> CTFinder;
llvm::Optional<tidy::ClangTidyContext> CTContext;
- {
+ if (ProduceDiagnostics) {
trace::Span Tracer("ClangTidyInit");
dlog("ClangTidy configuration for file {0}: {1}", Filename,
tidy::configurationAsText(Inputs.Opts.ClangTidyOpts));
@@ -343,13 +349,14 @@
return DiagLevel;
});
Preprocessor *PP = &Clang->getPreprocessor();
+ CTFinder.emplace();
for (const auto &Check : CTChecks) {
if (!Check->isLanguageVersionSupported(CTContext->getLangOpts()))
continue;
// FIXME: the PP callbacks skip the entire preamble.
// Checks that want to see #includes in the main file do not see them.
Check->registerPPCallbacks(Clang->getSourceManager(), PP, PP);
- Check->registerMatchers(&CTFinder);
+ Check->registerMatchers(CTFinder.getPointer());
}
}
@@ -357,8 +364,8 @@
// (e.g. incomplete type) and attach include insertion fixes to diagnostics.
llvm::Optional<IncludeFixer> FixIncludes;
auto BuildDir = VFS->getCurrentWorkingDirectory();
- if (Inputs.Opts.SuggestMissingIncludes && Inputs.Index &&
- !BuildDir.getError()) {
+ if (ProduceDiagnostics && Inputs.Opts.SuggestMissingIncludes &&
+ Inputs.Index && !BuildDir.getError()) {
auto Style = getFormatStyleForFile(Filename, Inputs.Contents, VFS.get());
auto Inserter = std::make_shared<IncludeInserter>(
Filename, Inputs.Contents, Style, BuildDir.get(),
@@ -424,15 +431,14 @@
std::vector<Decl *> ParsedDecls = Action->takeTopLevelDecls();
// AST traversals should exclude the preamble, to avoid performance cliffs.
Clang->getASTContext().setTraversalScope(ParsedDecls);
- {
+ if (CTFinder.hasValue()) {
// Run the AST-dependent part of the clang-tidy checks.
// (The preprocessor part ran already, via PPCallbacks).
trace::Span Tracer("ClangTidyMatch");
- CTFinder.matchAST(Clang->getASTContext());
+ CTFinder->matchAST(Clang->getASTContext());
}
- // UnitDiagsConsumer is local, we can not store it in CompilerInstance that
- // has a longer lifetime.
+ // Our diagnostic consumer is local, CompilerInstance must outlive it.
Clang->getDiagnostics().setClient(new IgnoreDiagnostics);
// CompilerInstance won't run this callback, do it directly.
ASTDiags.EndSourceFile();
@@ -441,14 +447,17 @@
// So just inform the preprocessor of EOF, while keeping everything alive.
Clang->getPreprocessor().EndSourceFile();
- std::vector<Diag> Diags = CompilerInvocationDiags;
- // Add diagnostics from the preamble, if any.
- if (Preamble)
- Diags.insert(Diags.end(), Preamble->Diags.begin(), Preamble->Diags.end());
- // Finally, add diagnostics coming from the AST.
- {
- std::vector<Diag> D = ASTDiags.take(CTContext.getPointer());
- Diags.insert(Diags.end(), D.begin(), D.end());
+ std::vector<Diag> Diags;
+ if (ProduceDiagnostics) {
+ Diags = CompilerInvocationDiags;
+ // Add diagnostics from the preamble, if any.
+ if (Preamble)
+ Diags.insert(Diags.end(), Preamble->Diags.begin(), Preamble->Diags.end());
+ // Finally, add diagnostics coming from the AST.
+ {
+ std::vector<Diag> D = ASTDiags.take(CTContext.getPointer());
+ Diags.insert(Diags.end(), D.begin(), D.end());
+ }
}
return ParsedAST(Inputs.Version, std::move(Preamble), std::move(Clang),
std::move(Action), std::move(Tokens), std::move(Macros),
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
