kadircet updated this revision to Diff 334079.
kadircet added a comment.
- Have 2 separate hooks for preamble and mainfile ASTs, as they are produced
async.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D98499/new/
https://reviews.llvm.org/D98499
Files:
clang-tools-extra/clangd/ClangdServer.cpp
clang-tools-extra/clangd/ClangdServer.h
clang-tools-extra/clangd/Compiler.h
clang-tools-extra/clangd/Diagnostics.cpp
clang-tools-extra/clangd/Diagnostics.h
clang-tools-extra/clangd/FeatureModule.h
clang-tools-extra/clangd/ParsedAST.cpp
clang-tools-extra/clangd/Preamble.cpp
clang-tools-extra/clangd/TUScheduler.cpp
clang-tools-extra/clangd/tool/Check.cpp
clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp
clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
clang-tools-extra/clangd/unittests/TestTU.cpp
clang-tools-extra/clangd/unittests/TestTU.h
Index: clang-tools-extra/clangd/unittests/TestTU.h
===================================================================
--- clang-tools-extra/clangd/unittests/TestTU.h
+++ clang-tools-extra/clangd/unittests/TestTU.h
@@ -25,6 +25,7 @@
#include "support/Path.h"
#include "llvm/ADT/StringMap.h"
#include "gtest/gtest.h"
+#include <memory>
#include <string>
#include <utility>
#include <vector>
@@ -76,6 +77,8 @@
// to eliminate this option some day.
bool OverlayRealFileSystemForModules = false;
+ std::vector<std::shared_ptr<FeatureModule::ASTListener>> Hooks;
+
// By default, build() will report Error diagnostics as GTest errors.
// Suppress this behavior by adding an 'error-ok' comment to the code.
// The result will always have getDiagnostics() populated.
Index: clang-tools-extra/clangd/unittests/TestTU.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/TestTU.cpp
+++ clang-tools-extra/clangd/unittests/TestTU.cpp
@@ -36,6 +36,8 @@
FS.Files[ImportThunk] = ThunkContents;
ParseInputs Inputs;
+ Inputs.PreambleHooks = Hooks;
+ Inputs.MainFileHooks = Hooks;
auto &Argv = Inputs.CompileCommand.CommandLine;
Argv = {"clang"};
// FIXME: this shouldn't need to be conditional, but it breaks a
@@ -97,6 +99,7 @@
MockFS FS;
auto Inputs = inputs(FS);
StoreDiags Diags;
+ Diags.setASTHooks(Inputs.MainFileHooks);
auto CI = buildCompilerInvocation(Inputs, Diags);
assert(CI && "Failed to build compilation invocation.");
if (OverlayRealFileSystemForModules)
Index: clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
+++ clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
@@ -1346,6 +1346,24 @@
});
}
+TEST(ParsedASTTest, ModuleSawDiag) {
+ static constexpr const llvm::StringLiteral KDiagMsg = "StampedDiag";
+ struct Hooks : public FeatureModule::ASTListener {
+ void sawDiagnostic(const clang::Diagnostic &Info,
+ clangd::Diag &Diag) override {
+ Diag.Message = KDiagMsg.str();
+ }
+ };
+
+ Annotations Code("[[test]]; /* error-ok */");
+ TestTU TU;
+ TU.Code = Code.code().str();
+ TU.Hooks.emplace_back(new Hooks);
+
+ auto AST = TU.build();
+ EXPECT_THAT(*AST.getDiagnostics(),
+ testing::Contains(Diag(Code.range(), KDiagMsg.str())));
+}
} // namespace
} // namespace clangd
} // namespace clang
Index: clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp
+++ clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp
@@ -365,6 +365,27 @@
// And immediately shut down. FeatureModule destructor verifies we blocked.
}
+TEST_F(LSPTest, DiagModuleTest) {
+ static constexpr llvm::StringLiteral DiagMsg = "DiagMsg";
+ class DiagModule final : public FeatureModule {
+ struct DiagHooks : public ASTListener {
+ void sawDiagnostic(const clang::Diagnostic &, clangd::Diag &D) override {
+ D.Message = DiagMsg.str();
+ }
+ };
+
+ public:
+ std::unique_ptr<ASTListener> astHooks() override {
+ return std::make_unique<DiagHooks>();
+ }
+ };
+ FeatureModules.add(std::make_unique<DiagModule>());
+
+ auto &Client = start();
+ Client.didOpen("foo.cpp", "test;");
+ EXPECT_THAT(Client.diagnostics("foo.cpp"),
+ llvm::ValueIs(testing::ElementsAre(DiagMessage(DiagMsg))));
+}
} // namespace
} // namespace clangd
} // namespace clang
Index: clang-tools-extra/clangd/tool/Check.cpp
===================================================================
--- clang-tools-extra/clangd/tool/Check.cpp
+++ clang-tools-extra/clangd/tool/Check.cpp
@@ -46,6 +46,7 @@
#include "llvm/ADT/Optional.h"
#include "llvm/ADT/StringExtras.h"
#include "llvm/Support/Path.h"
+#include <memory>
namespace clang {
namespace clangd {
@@ -119,13 +120,18 @@
}
// Prepare inputs and build CompilerInvocation (parsed compile command).
- bool buildInvocation(const ThreadsafeFS &TFS,
- llvm::Optional<std::string> Contents) {
- StoreDiags CaptureInvocationDiags;
+ bool buildInvocation(
+ const ThreadsafeFS &TFS, llvm::Optional<std::string> Contents,
+ llvm::ArrayRef<std::shared_ptr<FeatureModule::ASTListener>> Hooks) {
std::vector<std::string> CC1Args;
Inputs.CompileCommand = Cmd;
Inputs.TFS = &TFS;
Inputs.ClangTidyProvider = Opts.ClangTidyProvider;
+ Inputs.PreambleHooks = Hooks.vec();
+ Inputs.MainFileHooks = Hooks.vec();
+
+ StoreDiags CaptureInvocationDiags;
+ CaptureInvocationDiags.setASTHooks(Hooks);
if (Contents.hasValue()) {
Inputs.Contents = *Contents;
log("Imaginary source file contents:\n{0}", Inputs.Contents);
@@ -252,7 +258,15 @@
Opts.ConfigProvider, nullptr);
WithContext Ctx(ContextProvider(""));
Checker C(File, Opts);
- if (!C.buildCommand(TFS) || !C.buildInvocation(TFS, Contents) ||
+ std::vector<std::shared_ptr<FeatureModule::ASTListener>> Hooks;
+ if (Opts.FeatureModules) {
+ for (auto &M : *Opts.FeatureModules) {
+ if (auto H = M.astHooks())
+ Hooks.push_back(std::move(H));
+ }
+ }
+
+ if (!C.buildCommand(TFS) || !C.buildInvocation(TFS, Contents, Hooks) ||
!C.buildAST())
return false;
C.testLocationFeatures();
Index: clang-tools-extra/clangd/TUScheduler.cpp
===================================================================
--- clang-tools-extra/clangd/TUScheduler.cpp
+++ clang-tools-extra/clangd/TUScheduler.cpp
@@ -804,6 +804,7 @@
printArgv(Inputs.CompileCommand.CommandLine));
StoreDiags CompilerInvocationDiagConsumer;
+ CompilerInvocationDiagConsumer.setASTHooks(Inputs.MainFileHooks);
std::vector<std::string> CC1Args;
std::unique_ptr<CompilerInvocation> Invocation = buildCompilerInvocation(
Inputs, CompilerInvocationDiagConsumer, &CC1Args);
@@ -868,6 +869,7 @@
IdleASTs.take(this, &ASTAccessForRead);
if (!AST) {
StoreDiags CompilerInvocationDiagConsumer;
+ CompilerInvocationDiagConsumer.setASTHooks(FileInputs.MainFileHooks);
std::unique_ptr<CompilerInvocation> Invocation =
buildCompilerInvocation(FileInputs, CompilerInvocationDiagConsumer);
// Try rebuilding the AST.
Index: clang-tools-extra/clangd/Preamble.cpp
===================================================================
--- clang-tools-extra/clangd/Preamble.cpp
+++ clang-tools-extra/clangd/Preamble.cpp
@@ -326,6 +326,7 @@
trace::Span Tracer("BuildPreamble");
SPAN_ATTACH(Tracer, "File", FileName);
StoreDiags PreambleDiagnostics;
+ PreambleDiagnostics.setASTHooks(Inputs.PreambleHooks);
llvm::IntrusiveRefCntPtr<DiagnosticsEngine> PreambleDiagsEngine =
CompilerInstance::createDiagnostics(&CI.getDiagnosticOpts(),
&PreambleDiagnostics, false);
Index: clang-tools-extra/clangd/ParsedAST.cpp
===================================================================
--- clang-tools-extra/clangd/ParsedAST.cpp
+++ clang-tools-extra/clangd/ParsedAST.cpp
@@ -262,6 +262,7 @@
CI->getLangOpts()->DelayedTemplateParsing = false;
StoreDiags ASTDiags;
+ ASTDiags.setASTHooks(Inputs.MainFileHooks);
llvm::Optional<PreamblePatch> Patch;
bool PreserveDiags = true;
@@ -318,7 +319,7 @@
Check->registerMatchers(&CTFinder);
}
- const Config& Cfg = Config::current();
+ const Config &Cfg = Config::current();
ASTDiags.setLevelAdjuster([&](DiagnosticsEngine::Level DiagLevel,
const clang::Diagnostic &Info) {
if (Cfg.Diagnostics.SuppressAll ||
Index: clang-tools-extra/clangd/FeatureModule.h
===================================================================
--- clang-tools-extra/clangd/FeatureModule.h
+++ clang-tools-extra/clangd/FeatureModule.h
@@ -11,6 +11,7 @@
#include "support/Function.h"
#include "support/Threading.h"
+#include "clang/Basic/Diagnostic.h"
#include "llvm/ADT/FunctionExtras.h"
#include "llvm/ADT/StringRef.h"
#include "llvm/Support/Compiler.h"
@@ -21,6 +22,7 @@
namespace clang {
namespace clangd {
+struct Diag;
class LSPBinder;
class SymbolIndex;
class ThreadsafeFS;
@@ -96,6 +98,20 @@
/// enumerating or applying code actions.
virtual void contributeTweaks(std::vector<std::unique_ptr<Tweak>> &Out) {}
+ /// Various hooks that can be invoked by ASTWorker thread while building an
+ /// AST. These hooks are always called from a single thread, so
+ /// implementations don't need to be thread-safe.
+ struct ASTListener {
+ virtual ~ASTListener() = default;
+
+ /// Called everytime a diagnostic is encountered. Modules can use this
+ /// modify the final diagnostic, or store some information to surface code
+ /// actions later on.
+ virtual void sawDiagnostic(const clang::Diagnostic &, clangd::Diag &) {}
+ };
+ /// Can be called asynchronously before building an AST.
+ virtual std::unique_ptr<ASTListener> astHooks() { return nullptr; }
+
protected:
/// Accessors for modules to access shared server facilities they depend on.
Facilities &facilities();
Index: clang-tools-extra/clangd/Diagnostics.h
===================================================================
--- clang-tools-extra/clangd/Diagnostics.h
+++ clang-tools-extra/clangd/Diagnostics.h
@@ -9,6 +9,7 @@
#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_DIAGNOSTICS_H
#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_DIAGNOSTICS_H
+#include "FeatureModule.h"
#include "Protocol.h"
#include "support/Path.h"
#include "clang/Basic/Diagnostic.h"
@@ -22,6 +23,7 @@
#include "llvm/ADT/StringSet.h"
#include "llvm/Support/SourceMgr.h"
#include <cassert>
+#include <memory>
#include <string>
namespace clang {
@@ -142,6 +144,10 @@
/// diagnostics, such as promoting warnings to errors, or ignoring
/// diagnostics.
void setLevelAdjuster(LevelAdjuster Adjuster) { this->Adjuster = Adjuster; }
+ void setASTHooks(
+ llvm::ArrayRef<std::shared_ptr<FeatureModule::ASTListener>> Hooks) {
+ this->Hooks = Hooks;
+ }
private:
void flushLastDiag();
@@ -157,6 +163,7 @@
llvm::DenseSet<std::pair<unsigned, unsigned>> IncludedErrorLocations;
bool LastPrimaryDiagnosticWasSuppressed = false;
+ llvm::ArrayRef<std::shared_ptr<FeatureModule::ASTListener>> Hooks;
};
/// Determine whether a (non-clang-tidy) diagnostic is suppressed by config.
Index: clang-tools-extra/clangd/Diagnostics.cpp
===================================================================
--- clang-tools-extra/clangd/Diagnostics.cpp
+++ clang-tools-extra/clangd/Diagnostics.cpp
@@ -744,6 +744,8 @@
LastDiag->Fixes.insert(LastDiag->Fixes.end(), ExtraFixes.begin(),
ExtraFixes.end());
}
+ for (auto &Hook : Hooks)
+ Hook->sawDiagnostic(Info, *LastDiag);
} else {
// Handle a note to an existing diagnostic.
Index: clang-tools-extra/clangd/Compiler.h
===================================================================
--- clang-tools-extra/clangd/Compiler.h
+++ clang-tools-extra/clangd/Compiler.h
@@ -15,6 +15,7 @@
#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_COMPILER_H
#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_COMPILER_H
+#include "FeatureModule.h"
#include "GlobalCompilationDatabase.h"
#include "TidyProvider.h"
#include "index/Index.h"
@@ -22,6 +23,8 @@
#include "clang/Frontend/CompilerInstance.h"
#include "clang/Frontend/PrecompiledPreamble.h"
#include "clang/Tooling/CompilationDatabase.h"
+#include <memory>
+#include <vector>
namespace clang {
namespace clangd {
@@ -54,6 +57,10 @@
const SymbolIndex *Index = nullptr;
ParseOptions Opts = ParseOptions();
TidyProviderRef ClangTidyProvider = {};
+ // Using shared_ptr since ParseInputs are copied between threads. But none of
+ // these hooks are accessed simultaneously.
+ std::vector<std::shared_ptr<FeatureModule::ASTListener>> PreambleHooks = {};
+ std::vector<std::shared_ptr<FeatureModule::ASTListener>> MainFileHooks = {};
};
/// Builds compiler invocation that could be used to build AST or preamble.
Index: clang-tools-extra/clangd/ClangdServer.h
===================================================================
--- clang-tools-extra/clangd/ClangdServer.h
+++ clang-tools-extra/clangd/ClangdServer.h
@@ -38,6 +38,7 @@
#include "llvm/ADT/Optional.h"
#include "llvm/ADT/StringRef.h"
#include <functional>
+#include <memory>
#include <string>
#include <type_traits>
#include <utility>
Index: clang-tools-extra/clangd/ClangdServer.cpp
===================================================================
--- clang-tools-extra/clangd/ClangdServer.cpp
+++ clang-tools-extra/clangd/ClangdServer.cpp
@@ -231,6 +231,14 @@
Inputs.Opts = std::move(Opts);
Inputs.Index = Index;
Inputs.ClangTidyProvider = ClangTidyProvider;
+ if (FeatureModules) {
+ for (auto &Mod : *FeatureModules) {
+ if (auto Hook = Mod.astHooks()) {
+ Inputs.PreambleHooks.emplace_back(std::move(Hook));
+ Inputs.MainFileHooks.emplace_back(Mod.astHooks());
+ }
+ }
+ }
bool NewFile = WorkScheduler->update(File, Inputs, WantDiags);
// If we loaded Foo.h, we want to make sure Foo.cpp is indexed.
if (NewFile && BackgroundIdx)
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
