Skip to content

Commit bce3ac4

Browse files
committed
[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
1 parent bb6d96c commit bce3ac4

12 files changed

+116
-1
lines changed

Diff for: clang-tools-extra/clangd/ClangdServer.cpp

+1
Original file line numberDiff line numberDiff line change
@@ -231,6 +231,7 @@ void ClangdServer::addDocument(PathRef File, llvm::StringRef Contents,
231231
Inputs.Opts = std::move(Opts);
232232
Inputs.Index = Index;
233233
Inputs.ClangTidyProvider = ClangTidyProvider;
234+
Inputs.FeatureModules = FeatureModules;
234235
bool NewFile = WorkScheduler->update(File, Inputs, WantDiags);
235236
// If we loaded Foo.h, we want to make sure Foo.cpp is indexed.
236237
if (NewFile && BackgroundIdx)

Diff for: clang-tools-extra/clangd/ClangdServer.h

+1
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@
3838
#include "llvm/ADT/Optional.h"
3939
#include "llvm/ADT/StringRef.h"
4040
#include <functional>
41+
#include <memory>
4142
#include <string>
4243
#include <type_traits>
4344
#include <utility>

Diff for: clang-tools-extra/clangd/Compiler.h

+5
Original file line numberDiff line numberDiff line change
@@ -15,13 +15,16 @@
1515
#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_COMPILER_H
1616
#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_COMPILER_H
1717

18+
#include "FeatureModule.h"
1819
#include "GlobalCompilationDatabase.h"
1920
#include "TidyProvider.h"
2021
#include "index/Index.h"
2122
#include "support/ThreadsafeFS.h"
2223
#include "clang/Frontend/CompilerInstance.h"
2324
#include "clang/Frontend/PrecompiledPreamble.h"
2425
#include "clang/Tooling/CompilationDatabase.h"
26+
#include <memory>
27+
#include <vector>
2528

2629
namespace clang {
2730
namespace clangd {
@@ -54,6 +57,8 @@ struct ParseInputs {
5457
const SymbolIndex *Index = nullptr;
5558
ParseOptions Opts = ParseOptions();
5659
TidyProviderRef ClangTidyProvider = {};
60+
// Used to acquire ASTListeners when parsing files.
61+
FeatureModuleSet *FeatureModules = nullptr;
5762
};
5863

5964
/// Builds compiler invocation that could be used to build AST or preamble.

Diff for: clang-tools-extra/clangd/Diagnostics.cpp

+2
Original file line numberDiff line numberDiff line change
@@ -744,6 +744,8 @@ void StoreDiags::HandleDiagnostic(DiagnosticsEngine::Level DiagLevel,
744744
LastDiag->Fixes.insert(LastDiag->Fixes.end(), ExtraFixes.begin(),
745745
ExtraFixes.end());
746746
}
747+
if (DiagCB)
748+
DiagCB(Info, *LastDiag);
747749
} else {
748750
// Handle a note to an existing diagnostic.
749751

Diff for: clang-tools-extra/clangd/Diagnostics.h

+9
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,10 @@
2222
#include "llvm/ADT/StringSet.h"
2323
#include "llvm/Support/SourceMgr.h"
2424
#include <cassert>
25+
#include <functional>
26+
#include <memory>
2527
#include <string>
28+
#include <utility>
2629

2730
namespace clang {
2831
namespace tidy {
@@ -136,18 +139,24 @@ class StoreDiags : public DiagnosticConsumer {
136139
const clang::Diagnostic &)>;
137140
using LevelAdjuster = std::function<DiagnosticsEngine::Level(
138141
DiagnosticsEngine::Level, const clang::Diagnostic &)>;
142+
using DiagCallback =
143+
std::function<void(const clang::Diagnostic &, clangd::Diag &)>;
139144
/// If set, possibly adds fixes for diagnostics using \p Fixer.
140145
void contributeFixes(DiagFixer Fixer) { this->Fixer = Fixer; }
141146
/// If set, this allows the client of this class to adjust the level of
142147
/// diagnostics, such as promoting warnings to errors, or ignoring
143148
/// diagnostics.
144149
void setLevelAdjuster(LevelAdjuster Adjuster) { this->Adjuster = Adjuster; }
150+
/// Invokes a callback every time a diagnostics is completely formed. Handler
151+
/// of the callback can also mutate the diagnostic.
152+
void setDiagCallback(DiagCallback CB) { DiagCB = std::move(CB); }
145153

146154
private:
147155
void flushLastDiag();
148156

149157
DiagFixer Fixer = nullptr;
150158
LevelAdjuster Adjuster = nullptr;
159+
DiagCallback DiagCB = nullptr;
151160
std::vector<Diag> Output;
152161
llvm::Optional<LangOptions> LangOpts;
153162
llvm::Optional<Diag> LastDiag;

Diff for: clang-tools-extra/clangd/FeatureModule.h

+18
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111

1212
#include "support/Function.h"
1313
#include "support/Threading.h"
14+
#include "clang/Basic/Diagnostic.h"
1415
#include "llvm/ADT/FunctionExtras.h"
1516
#include "llvm/ADT/StringRef.h"
1617
#include "llvm/Support/Compiler.h"
@@ -21,6 +22,7 @@
2122

2223
namespace clang {
2324
namespace clangd {
25+
struct Diag;
2426
class LSPBinder;
2527
class SymbolIndex;
2628
class ThreadsafeFS;
@@ -96,6 +98,22 @@ class FeatureModule {
9698
/// enumerating or applying code actions.
9799
virtual void contributeTweaks(std::vector<std::unique_ptr<Tweak>> &Out) {}
98100

101+
/// Extension point that allows modules to observe and modify an AST build.
102+
/// One instance is created each time clangd produces a ParsedAST or
103+
/// PrecompiledPreamble. For a given instance, lifecycle methods are always
104+
/// called on a single thread.
105+
struct ASTListener {
106+
/// Listeners are destroyed once the AST is built.
107+
virtual ~ASTListener() = default;
108+
109+
/// Called everytime a diagnostic is encountered. Modules can use this
110+
/// modify the final diagnostic, or store some information to surface code
111+
/// actions later on.
112+
virtual void sawDiagnostic(const clang::Diagnostic &, clangd::Diag &) {}
113+
};
114+
/// Can be called asynchronously before building an AST.
115+
virtual std::unique_ptr<ASTListener> astListeners() { return nullptr; }
116+
99117
protected:
100118
/// Accessors for modules to access shared server facilities they depend on.
101119
Facilities &facilities();

Diff for: clang-tools-extra/clangd/ParsedAST.cpp

+15-1
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
#include "Compiler.h"
1515
#include "Config.h"
1616
#include "Diagnostics.h"
17+
#include "FeatureModule.h"
1718
#include "Headers.h"
1819
#include "IncludeFixer.h"
1920
#include "Preamble.h"
@@ -25,6 +26,7 @@
2526
#include "support/Trace.h"
2627
#include "clang/AST/ASTContext.h"
2728
#include "clang/AST/Decl.h"
29+
#include "clang/Basic/Diagnostic.h"
2830
#include "clang/Basic/LangOptions.h"
2931
#include "clang/Basic/SourceLocation.h"
3032
#include "clang/Basic/SourceManager.h"
@@ -261,7 +263,19 @@ ParsedAST::build(llvm::StringRef Filename, const ParseInputs &Inputs,
261263
// breaks many features. Disable it for the main-file (not preamble).
262264
CI->getLangOpts()->DelayedTemplateParsing = false;
263265

266+
std::vector<std::unique_ptr<FeatureModule::ASTListener>> ASTListeners;
267+
if (Inputs.FeatureModules) {
268+
for (auto &M : *Inputs.FeatureModules) {
269+
if (auto Listener = M.astListeners())
270+
ASTListeners.emplace_back(std::move(Listener));
271+
}
272+
}
264273
StoreDiags ASTDiags;
274+
ASTDiags.setDiagCallback(
275+
[&ASTListeners](const clang::Diagnostic &D, clangd::Diag &Diag) {
276+
llvm::for_each(ASTListeners,
277+
[&](const auto &L) { L->sawDiagnostic(D, Diag); });
278+
});
265279

266280
llvm::Optional<PreamblePatch> Patch;
267281
bool PreserveDiags = true;
@@ -318,7 +332,7 @@ ParsedAST::build(llvm::StringRef Filename, const ParseInputs &Inputs,
318332
Check->registerMatchers(&CTFinder);
319333
}
320334

321-
const Config& Cfg = Config::current();
335+
const Config &Cfg = Config::current();
322336
ASTDiags.setLevelAdjuster([&](DiagnosticsEngine::Level DiagLevel,
323337
const clang::Diagnostic &Info) {
324338
if (Cfg.Diagnostics.SuppressAll ||

Diff for: clang-tools-extra/clangd/Preamble.cpp

+12
Original file line numberDiff line numberDiff line change
@@ -325,7 +325,19 @@ buildPreamble(PathRef FileName, CompilerInvocation CI,
325325

326326
trace::Span Tracer("BuildPreamble");
327327
SPAN_ATTACH(Tracer, "File", FileName);
328+
std::vector<std::unique_ptr<FeatureModule::ASTListener>> ASTListeners;
329+
if (Inputs.FeatureModules) {
330+
for (auto &M : *Inputs.FeatureModules) {
331+
if (auto Listener = M.astListeners())
332+
ASTListeners.emplace_back(std::move(Listener));
333+
}
334+
}
328335
StoreDiags PreambleDiagnostics;
336+
PreambleDiagnostics.setDiagCallback(
337+
[&ASTListeners](const clang::Diagnostic &D, clangd::Diag &Diag) {
338+
llvm::for_each(ASTListeners,
339+
[&](const auto &L) { L->sawDiagnostic(D, Diag); });
340+
});
329341
llvm::IntrusiveRefCntPtr<DiagnosticsEngine> PreambleDiagsEngine =
330342
CompilerInstance::createDiagnostics(&CI.getDiagnosticOpts(),
331343
&PreambleDiagnostics, false);

Diff for: clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp

+21
Original file line numberDiff line numberDiff line change
@@ -365,6 +365,27 @@ TEST_F(LSPTest, FeatureModulesThreadingTest) {
365365
// And immediately shut down. FeatureModule destructor verifies we blocked.
366366
}
367367

368+
TEST_F(LSPTest, DiagModuleTest) {
369+
static constexpr llvm::StringLiteral DiagMsg = "DiagMsg";
370+
class DiagModule final : public FeatureModule {
371+
struct DiagHooks : public ASTListener {
372+
void sawDiagnostic(const clang::Diagnostic &, clangd::Diag &D) override {
373+
D.Message = DiagMsg.str();
374+
}
375+
};
376+
377+
public:
378+
std::unique_ptr<ASTListener> astListeners() override {
379+
return std::make_unique<DiagHooks>();
380+
}
381+
};
382+
FeatureModules.add(std::make_unique<DiagModule>());
383+
384+
auto &Client = start();
385+
Client.didOpen("foo.cpp", "test;");
386+
EXPECT_THAT(Client.diagnostics("foo.cpp"),
387+
llvm::ValueIs(testing::ElementsAre(DiagMessage(DiagMsg))));
388+
}
368389
} // namespace
369390
} // namespace clangd
370391
} // namespace clang

Diff for: clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp

+27
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
#include "Annotations.h"
1010
#include "Config.h"
1111
#include "Diagnostics.h"
12+
#include "FeatureModule.h"
1213
#include "ParsedAST.h"
1314
#include "Protocol.h"
1415
#include "SourceCode.h"
@@ -26,6 +27,7 @@
2627
#include "gmock/gmock.h"
2728
#include "gtest/gtest.h"
2829
#include <algorithm>
30+
#include <memory>
2931

3032
namespace clang {
3133
namespace clangd {
@@ -1346,6 +1348,31 @@ TEST(ToLSPDiag, RangeIsInMain) {
13461348
});
13471349
}
13481350

1351+
TEST(ParsedASTTest, ModuleSawDiag) {
1352+
static constexpr const llvm::StringLiteral KDiagMsg = "StampedDiag";
1353+
struct DiagModifierModule final : public FeatureModule {
1354+
struct Listener : public FeatureModule::ASTListener {
1355+
void sawDiagnostic(const clang::Diagnostic &Info,
1356+
clangd::Diag &Diag) override {
1357+
Diag.Message = KDiagMsg.str();
1358+
}
1359+
};
1360+
std::unique_ptr<ASTListener> astListeners() override {
1361+
return std::make_unique<Listener>();
1362+
};
1363+
};
1364+
FeatureModuleSet FMS;
1365+
FMS.add(std::make_unique<DiagModifierModule>());
1366+
1367+
Annotations Code("[[test]]; /* error-ok */");
1368+
TestTU TU;
1369+
TU.Code = Code.code().str();
1370+
TU.FeatureModules = &FMS;
1371+
1372+
auto AST = TU.build();
1373+
EXPECT_THAT(*AST.getDiagnostics(),
1374+
testing::Contains(Diag(Code.range(), KDiagMsg.str())));
1375+
}
13491376
} // namespace
13501377
} // namespace clangd
13511378
} // namespace clang

Diff for: clang-tools-extra/clangd/unittests/TestTU.cpp

+1
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ ParseInputs TestTU::inputs(MockFS &FS) const {
3636
FS.Files[ImportThunk] = ThunkContents;
3737

3838
ParseInputs Inputs;
39+
Inputs.FeatureModules = FeatureModules;
3940
auto &Argv = Inputs.CompileCommand.CommandLine;
4041
Argv = {"clang"};
4142
// FIXME: this shouldn't need to be conditional, but it breaks a

Diff for: clang-tools-extra/clangd/unittests/TestTU.h

+4
Original file line numberDiff line numberDiff line change
@@ -19,12 +19,14 @@
1919

2020
#include "../TidyProvider.h"
2121
#include "Compiler.h"
22+
#include "FeatureModule.h"
2223
#include "ParsedAST.h"
2324
#include "TestFS.h"
2425
#include "index/Index.h"
2526
#include "support/Path.h"
2627
#include "llvm/ADT/StringMap.h"
2728
#include "gtest/gtest.h"
29+
#include <memory>
2830
#include <string>
2931
#include <utility>
3032
#include <vector>
@@ -76,6 +78,8 @@ struct TestTU {
7678
// to eliminate this option some day.
7779
bool OverlayRealFileSystemForModules = false;
7880

81+
FeatureModuleSet *FeatureModules = nullptr;
82+
7983
// By default, build() will report Error diagnostics as GTest errors.
8084
// Suppress this behavior by adding an 'error-ok' comment to the code.
8185
// The result will always have getDiagnostics() populated.

0 commit comments

Comments
 (0)