Skip to content

Commit 204014e

Browse files
committed
[clangd] Fix feature modules to drop diagnostics
Ignored diagnostics were only checked after level adjusters and assumed it would stay the same for the rest. But it can also be modified by FeatureModules. Differential Revision: https://reviews.llvm.org/D103387
1 parent b662651 commit 204014e

File tree

3 files changed

+53
-24
lines changed

3 files changed

+53
-24
lines changed

clang-tools-extra/clangd/Diagnostics.cpp

Lines changed: 21 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -76,10 +76,10 @@ bool mentionsMainFile(const Diag &D) {
7676
return false;
7777
}
7878

79-
bool isExcluded(const Diag &D) {
79+
bool isExcluded(unsigned DiagID) {
8080
// clang will always fail parsing MS ASM, we don't link in desc + asm parser.
81-
if (D.ID == clang::diag::err_msasm_unable_to_create_target ||
82-
D.ID == clang::diag::err_msasm_unsupported_arch)
81+
if (DiagID == clang::diag::err_msasm_unable_to_create_target ||
82+
DiagID == clang::diag::err_msasm_unsupported_arch)
8383
return true;
8484
return false;
8585
}
@@ -726,44 +726,42 @@ void StoreDiags::HandleDiagnostic(DiagnosticsEngine::Level DiagLevel,
726726
// Handle the new main diagnostic.
727727
flushLastDiag();
728728

729-
if (Adjuster) {
729+
LastDiag = Diag();
730+
// FIXME: Merge with feature modules.
731+
if (Adjuster)
730732
DiagLevel = Adjuster(DiagLevel, Info);
731-
if (DiagLevel == DiagnosticsEngine::Ignored) {
732-
LastPrimaryDiagnosticWasSuppressed = true;
733-
return;
734-
}
735-
}
736-
LastPrimaryDiagnosticWasSuppressed = false;
737733

738-
LastDiag = Diag();
739734
FillDiagBase(*LastDiag);
735+
if (isExcluded(LastDiag->ID))
736+
LastDiag->Severity = DiagnosticsEngine::Ignored;
737+
if (DiagCB)
738+
DiagCB(Info, *LastDiag);
739+
// Don't bother filling in the rest if diag is going to be dropped.
740+
if (LastDiag->Severity == DiagnosticsEngine::Ignored)
741+
return;
742+
740743
LastDiagLoc.emplace(Info.getLocation(), Info.getSourceManager());
741744
LastDiagOriginallyError = OriginallyError;
742-
743745
if (!Info.getFixItHints().empty())
744746
AddFix(true /* try to invent a message instead of repeating the diag */);
745747
if (Fixer) {
746-
auto ExtraFixes = Fixer(DiagLevel, Info);
748+
auto ExtraFixes = Fixer(LastDiag->Severity, Info);
747749
LastDiag->Fixes.insert(LastDiag->Fixes.end(), ExtraFixes.begin(),
748750
ExtraFixes.end());
749751
}
750-
if (DiagCB)
751-
DiagCB(Info, *LastDiag);
752752
} else {
753753
// Handle a note to an existing diagnostic.
754-
755-
// If a diagnostic was suppressed due to the suppression filter,
756-
// also suppress notes associated with it.
757-
if (LastPrimaryDiagnosticWasSuppressed) {
758-
return;
759-
}
760-
761754
if (!LastDiag) {
762755
assert(false && "Adding a note without main diagnostic");
763756
IgnoreDiagnostics::log(DiagLevel, Info);
764757
return;
765758
}
766759

760+
// If a diagnostic was suppressed due to the suppression filter,
761+
// also suppress notes associated with it.
762+
if (LastDiag->Severity == DiagnosticsEngine::Ignored)
763+
return;
764+
767765
if (!Info.getFixItHints().empty()) {
768766
// A clang note with fix-it is not a separate diagnostic in clangd. We
769767
// attach it as a Fix to the main diagnostic instead.
@@ -788,7 +786,7 @@ void StoreDiags::flushLastDiag() {
788786
LastDiag.reset();
789787
});
790788

791-
if (isExcluded(*LastDiag))
789+
if (LastDiag->Severity == DiagnosticsEngine::Ignored)
792790
return;
793791
// Move errors that occur from headers into main file.
794792
if (!LastDiag->InsideMainFile && LastDiagLoc && LastDiagOriginallyError) {

clang-tools-extra/clangd/Diagnostics.h

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -171,7 +171,6 @@ class StoreDiags : public DiagnosticConsumer {
171171
SourceManager *OrigSrcMgr = nullptr;
172172

173173
llvm::DenseSet<std::pair<unsigned, unsigned>> IncludedErrorLocations;
174-
bool LastPrimaryDiagnosticWasSuppressed = false;
175174
};
176175

177176
/// Determine whether a (non-clang-tidy) diagnostic is suppressed by config.

clang-tools-extra/clangd/unittests/FeatureModulesTests.cpp

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
//
77
//===----------------------------------------------------------------------===//
88

9+
#include "Annotations.h"
910
#include "FeatureModule.h"
1011
#include "Selection.h"
1112
#include "TestTU.h"
@@ -53,6 +54,37 @@ TEST(FeatureModulesTest, ContributesTweak) {
5354
EXPECT_EQ(Actual->get()->id(), TweakID);
5455
}
5556

57+
TEST(FeatureModulesTest, SuppressDiags) {
58+
struct DiagModifierModule final : public FeatureModule {
59+
struct Listener : public FeatureModule::ASTListener {
60+
void sawDiagnostic(const clang::Diagnostic &Info,
61+
clangd::Diag &Diag) override {
62+
Diag.Severity = DiagnosticsEngine::Ignored;
63+
}
64+
};
65+
std::unique_ptr<ASTListener> astListeners() override {
66+
return std::make_unique<Listener>();
67+
};
68+
};
69+
FeatureModuleSet FMS;
70+
FMS.add(std::make_unique<DiagModifierModule>());
71+
72+
Annotations Code("[[test]]; /* error-ok */");
73+
TestTU TU;
74+
TU.Code = Code.code().str();
75+
76+
{
77+
auto AST = TU.build();
78+
EXPECT_THAT(*AST.getDiagnostics(), testing::Not(testing::IsEmpty()));
79+
}
80+
81+
TU.FeatureModules = &FMS;
82+
{
83+
auto AST = TU.build();
84+
EXPECT_THAT(*AST.getDiagnostics(), testing::IsEmpty());
85+
}
86+
}
87+
5688
} // namespace
5789
} // namespace clangd
5890
} // namespace clang

0 commit comments

Comments
 (0)