[clang-tools-extra] bce3ac4 - [clangd] Introduce ASTHooks to FeatureModules
Kadir Cetinkaya via cfe-commits
cfe-commits at lists.llvm.org
Tue Apr 13 08:50:31 PDT 2021
Author: Kadir Cetinkaya
Date: 2021-04-13T17:45:09+02:00
New Revision: bce3ac4f224aa7da0b253852ce8a28ad5a39c31f
URL: https://github.com/llvm/llvm-project/commit/bce3ac4f224aa7da0b253852ce8a28ad5a39c31f
DIFF: https://github.com/llvm/llvm-project/commit/bce3ac4f224aa7da0b253852ce8a28ad5a39c31f.diff
LOG: [clangd] Introduce ASTHooks to FeatureModules
These can be invoked at different stages while building an AST to let
FeatureModules implement features on top of it. The patch also
introduces a sawDiagnostic hook, which can mutate the final clangd::Diag
while reading a clang::Diagnostic.
Differential Revision: https://reviews.llvm.org/D98499
Added:
Modified:
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/unittests/ClangdLSPServerTests.cpp
clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
clang-tools-extra/clangd/unittests/TestTU.cpp
clang-tools-extra/clangd/unittests/TestTU.h
Removed:
################################################################################
diff --git a/clang-tools-extra/clangd/ClangdServer.cpp b/clang-tools-extra/clangd/ClangdServer.cpp
index ec69c6a71a34..f045cd1b4f8d 100644
--- a/clang-tools-extra/clangd/ClangdServer.cpp
+++ b/clang-tools-extra/clangd/ClangdServer.cpp
@@ -231,6 +231,7 @@ void ClangdServer::addDocument(PathRef File, llvm::StringRef Contents,
Inputs.Opts = std::move(Opts);
Inputs.Index = Index;
Inputs.ClangTidyProvider = ClangTidyProvider;
+ Inputs.FeatureModules = FeatureModules;
bool NewFile = WorkScheduler->update(File, Inputs, WantDiags);
// If we loaded Foo.h, we want to make sure Foo.cpp is indexed.
if (NewFile && BackgroundIdx)
diff --git a/clang-tools-extra/clangd/ClangdServer.h b/clang-tools-extra/clangd/ClangdServer.h
index 37ac30f70cb4..2dbf14455272 100644
--- a/clang-tools-extra/clangd/ClangdServer.h
+++ b/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>
diff --git a/clang-tools-extra/clangd/Compiler.h b/clang-tools-extra/clangd/Compiler.h
index 13fd4da33e3c..035106968315 100644
--- a/clang-tools-extra/clangd/Compiler.h
+++ b/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,8 @@ struct ParseInputs {
const SymbolIndex *Index = nullptr;
ParseOptions Opts = ParseOptions();
TidyProviderRef ClangTidyProvider = {};
+ // Used to acquire ASTListeners when parsing files.
+ FeatureModuleSet *FeatureModules = nullptr;
};
/// Builds compiler invocation that could be used to build AST or preamble.
diff --git a/clang-tools-extra/clangd/Diagnostics.cpp b/clang-tools-extra/clangd/Diagnostics.cpp
index d2982636c807..49574dc3b2ac 100644
--- a/clang-tools-extra/clangd/Diagnostics.cpp
+++ b/clang-tools-extra/clangd/Diagnostics.cpp
@@ -744,6 +744,8 @@ void StoreDiags::HandleDiagnostic(DiagnosticsEngine::Level DiagLevel,
LastDiag->Fixes.insert(LastDiag->Fixes.end(), ExtraFixes.begin(),
ExtraFixes.end());
}
+ if (DiagCB)
+ DiagCB(Info, *LastDiag);
} else {
// Handle a note to an existing diagnostic.
diff --git a/clang-tools-extra/clangd/Diagnostics.h b/clang-tools-extra/clangd/Diagnostics.h
index 6a432db23cc2..ea55132a9ca8 100644
--- a/clang-tools-extra/clangd/Diagnostics.h
+++ b/clang-tools-extra/clangd/Diagnostics.h
@@ -22,7 +22,10 @@
#include "llvm/ADT/StringSet.h"
#include "llvm/Support/SourceMgr.h"
#include <cassert>
+#include <functional>
+#include <memory>
#include <string>
+#include <utility>
namespace clang {
namespace tidy {
@@ -136,18 +139,24 @@ class StoreDiags : public DiagnosticConsumer {
const clang::Diagnostic &)>;
using LevelAdjuster = std::function<DiagnosticsEngine::Level(
DiagnosticsEngine::Level, const clang::Diagnostic &)>;
+ using DiagCallback =
+ std::function<void(const clang::Diagnostic &, clangd::Diag &)>;
/// If set, possibly adds fixes for diagnostics using \p Fixer.
void contributeFixes(DiagFixer Fixer) { this->Fixer = Fixer; }
/// If set, this allows the client of this class to adjust the level of
/// diagnostics, such as promoting warnings to errors, or ignoring
/// diagnostics.
void setLevelAdjuster(LevelAdjuster Adjuster) { this->Adjuster = Adjuster; }
+ /// Invokes a callback every time a diagnostics is completely formed. Handler
+ /// of the callback can also mutate the diagnostic.
+ void setDiagCallback(DiagCallback CB) { DiagCB = std::move(CB); }
private:
void flushLastDiag();
DiagFixer Fixer = nullptr;
LevelAdjuster Adjuster = nullptr;
+ DiagCallback DiagCB = nullptr;
std::vector<Diag> Output;
llvm::Optional<LangOptions> LangOpts;
llvm::Optional<Diag> LastDiag;
diff --git a/clang-tools-extra/clangd/FeatureModule.h b/clang-tools-extra/clangd/FeatureModule.h
index 13d7b7711829..82c9134f9272 100644
--- a/clang-tools-extra/clangd/FeatureModule.h
+++ b/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,22 @@ class FeatureModule {
/// enumerating or applying code actions.
virtual void contributeTweaks(std::vector<std::unique_ptr<Tweak>> &Out) {}
+ /// Extension point that allows modules to observe and modify an AST build.
+ /// One instance is created each time clangd produces a ParsedAST or
+ /// PrecompiledPreamble. For a given instance, lifecycle methods are always
+ /// called on a single thread.
+ struct ASTListener {
+ /// Listeners are destroyed once the AST is built.
+ 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> astListeners() { return nullptr; }
+
protected:
/// Accessors for modules to access shared server facilities they depend on.
Facilities &facilities();
diff --git a/clang-tools-extra/clangd/ParsedAST.cpp b/clang-tools-extra/clangd/ParsedAST.cpp
index fca19428192e..6cb76f32ced0 100644
--- a/clang-tools-extra/clangd/ParsedAST.cpp
+++ b/clang-tools-extra/clangd/ParsedAST.cpp
@@ -14,6 +14,7 @@
#include "Compiler.h"
#include "Config.h"
#include "Diagnostics.h"
+#include "FeatureModule.h"
#include "Headers.h"
#include "IncludeFixer.h"
#include "Preamble.h"
@@ -25,6 +26,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"
@@ -261,7 +263,19 @@ ParsedAST::build(llvm::StringRef Filename, const ParseInputs &Inputs,
// breaks many features. Disable it for the main-file (not preamble).
CI->getLangOpts()->DelayedTemplateParsing = false;
+ std::vector<std::unique_ptr<FeatureModule::ASTListener>> ASTListeners;
+ if (Inputs.FeatureModules) {
+ for (auto &M : *Inputs.FeatureModules) {
+ if (auto Listener = M.astListeners())
+ ASTListeners.emplace_back(std::move(Listener));
+ }
+ }
StoreDiags ASTDiags;
+ ASTDiags.setDiagCallback(
+ [&ASTListeners](const clang::Diagnostic &D, clangd::Diag &Diag) {
+ llvm::for_each(ASTListeners,
+ [&](const auto &L) { L->sawDiagnostic(D, Diag); });
+ });
llvm::Optional<PreamblePatch> Patch;
bool PreserveDiags = true;
@@ -318,7 +332,7 @@ ParsedAST::build(llvm::StringRef Filename, const ParseInputs &Inputs,
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 ||
diff --git a/clang-tools-extra/clangd/Preamble.cpp b/clang-tools-extra/clangd/Preamble.cpp
index 88cc0b3a2905..73b1c900cfa5 100644
--- a/clang-tools-extra/clangd/Preamble.cpp
+++ b/clang-tools-extra/clangd/Preamble.cpp
@@ -325,7 +325,19 @@ buildPreamble(PathRef FileName, CompilerInvocation CI,
trace::Span Tracer("BuildPreamble");
SPAN_ATTACH(Tracer, "File", FileName);
+ std::vector<std::unique_ptr<FeatureModule::ASTListener>> ASTListeners;
+ if (Inputs.FeatureModules) {
+ for (auto &M : *Inputs.FeatureModules) {
+ if (auto Listener = M.astListeners())
+ ASTListeners.emplace_back(std::move(Listener));
+ }
+ }
StoreDiags PreambleDiagnostics;
+ PreambleDiagnostics.setDiagCallback(
+ [&ASTListeners](const clang::Diagnostic &D, clangd::Diag &Diag) {
+ llvm::for_each(ASTListeners,
+ [&](const auto &L) { L->sawDiagnostic(D, Diag); });
+ });
llvm::IntrusiveRefCntPtr<DiagnosticsEngine> PreambleDiagsEngine =
CompilerInstance::createDiagnostics(&CI.getDiagnosticOpts(),
&PreambleDiagnostics, false);
diff --git a/clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp b/clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp
index f596f94bd6cd..6955fa0caa25 100644
--- a/clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp
+++ b/clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp
@@ -365,6 +365,27 @@ TEST_F(LSPTest, FeatureModulesThreadingTest) {
// 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> astListeners() 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
diff --git a/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp b/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
index d5b4a08a4229..d5096cba7e74 100644
--- a/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
+++ b/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
@@ -9,6 +9,7 @@
#include "Annotations.h"
#include "Config.h"
#include "Diagnostics.h"
+#include "FeatureModule.h"
#include "ParsedAST.h"
#include "Protocol.h"
#include "SourceCode.h"
@@ -26,6 +27,7 @@
#include "gmock/gmock.h"
#include "gtest/gtest.h"
#include <algorithm>
+#include <memory>
namespace clang {
namespace clangd {
@@ -1346,6 +1348,31 @@ TEST(ToLSPDiag, RangeIsInMain) {
});
}
+TEST(ParsedASTTest, ModuleSawDiag) {
+ static constexpr const llvm::StringLiteral KDiagMsg = "StampedDiag";
+ struct DiagModifierModule final : public FeatureModule {
+ struct Listener : public FeatureModule::ASTListener {
+ void sawDiagnostic(const clang::Diagnostic &Info,
+ clangd::Diag &Diag) override {
+ Diag.Message = KDiagMsg.str();
+ }
+ };
+ std::unique_ptr<ASTListener> astListeners() override {
+ return std::make_unique<Listener>();
+ };
+ };
+ FeatureModuleSet FMS;
+ FMS.add(std::make_unique<DiagModifierModule>());
+
+ Annotations Code("[[test]]; /* error-ok */");
+ TestTU TU;
+ TU.Code = Code.code().str();
+ TU.FeatureModules = &FMS;
+
+ auto AST = TU.build();
+ EXPECT_THAT(*AST.getDiagnostics(),
+ testing::Contains(Diag(Code.range(), KDiagMsg.str())));
+}
} // namespace
} // namespace clangd
} // namespace clang
diff --git a/clang-tools-extra/clangd/unittests/TestTU.cpp b/clang-tools-extra/clangd/unittests/TestTU.cpp
index 1c6e54774c03..b0c0d1b7c2a1 100644
--- a/clang-tools-extra/clangd/unittests/TestTU.cpp
+++ b/clang-tools-extra/clangd/unittests/TestTU.cpp
@@ -36,6 +36,7 @@ ParseInputs TestTU::inputs(MockFS &FS) const {
FS.Files[ImportThunk] = ThunkContents;
ParseInputs Inputs;
+ Inputs.FeatureModules = FeatureModules;
auto &Argv = Inputs.CompileCommand.CommandLine;
Argv = {"clang"};
// FIXME: this shouldn't need to be conditional, but it breaks a
diff --git a/clang-tools-extra/clangd/unittests/TestTU.h b/clang-tools-extra/clangd/unittests/TestTU.h
index 169cab045ea1..3e0c089dc5be 100644
--- a/clang-tools-extra/clangd/unittests/TestTU.h
+++ b/clang-tools-extra/clangd/unittests/TestTU.h
@@ -19,12 +19,14 @@
#include "../TidyProvider.h"
#include "Compiler.h"
+#include "FeatureModule.h"
#include "ParsedAST.h"
#include "TestFS.h"
#include "index/Index.h"
#include "support/Path.h"
#include "llvm/ADT/StringMap.h"
#include "gtest/gtest.h"
+#include <memory>
#include <string>
#include <utility>
#include <vector>
@@ -76,6 +78,8 @@ struct TestTU {
// to eliminate this option some day.
bool OverlayRealFileSystemForModules = false;
+ FeatureModuleSet *FeatureModules = nullptr;
+
// 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.
More information about the cfe-commits
mailing list