kadircet updated this revision to Diff 352628.
kadircet marked 2 inline comments as done.
kadircet added a comment.
- Bail out early before filling in diag info
- Move isExcluded check into handleDiagnostics, rather than handling it during
flushing
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D103387/new/
https://reviews.llvm.org/D103387
Files:
clang-tools-extra/clangd/Diagnostics.cpp
clang-tools-extra/clangd/Diagnostics.h
clang-tools-extra/clangd/unittests/FeatureModulesTests.cpp
Index: clang-tools-extra/clangd/unittests/FeatureModulesTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/FeatureModulesTests.cpp
+++ clang-tools-extra/clangd/unittests/FeatureModulesTests.cpp
@@ -6,6 +6,7 @@
//
//===----------------------------------------------------------------------===//
+#include "Annotations.h"
#include "FeatureModule.h"
#include "Selection.h"
#include "TestTU.h"
@@ -53,6 +54,37 @@
EXPECT_EQ(Actual->get()->id(), TweakID);
}
+TEST(FeatureModulesTest, SuppressDiags) {
+ struct DiagModifierModule final : public FeatureModule {
+ struct Listener : public FeatureModule::ASTListener {
+ void sawDiagnostic(const clang::Diagnostic &Info,
+ clangd::Diag &Diag) override {
+ Diag.Severity = DiagnosticsEngine::Ignored;
+ }
+ };
+ 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();
+
+ {
+ auto AST = TU.build();
+ EXPECT_THAT(*AST.getDiagnostics(), testing::Not(testing::IsEmpty()));
+ }
+
+ TU.FeatureModules = &FMS;
+ {
+ auto AST = TU.build();
+ EXPECT_THAT(*AST.getDiagnostics(), testing::IsEmpty());
+ }
+}
+
} // namespace
} // namespace clangd
} // namespace clang
Index: clang-tools-extra/clangd/Diagnostics.h
===================================================================
--- clang-tools-extra/clangd/Diagnostics.h
+++ clang-tools-extra/clangd/Diagnostics.h
@@ -171,7 +171,6 @@
SourceManager *OrigSrcMgr = nullptr;
llvm::DenseSet<std::pair<unsigned, unsigned>> IncludedErrorLocations;
- bool LastPrimaryDiagnosticWasSuppressed = false;
};
/// 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
@@ -76,10 +76,10 @@
return false;
}
-bool isExcluded(const Diag &D) {
+bool isExcluded(unsigned DiagID) {
// clang will always fail parsing MS ASM, we don't link in desc + asm parser.
- if (D.ID == clang::diag::err_msasm_unable_to_create_target ||
- D.ID == clang::diag::err_msasm_unsupported_arch)
+ if (DiagID == clang::diag::err_msasm_unable_to_create_target ||
+ DiagID == clang::diag::err_msasm_unsupported_arch)
return true;
return false;
}
@@ -726,44 +726,42 @@
// Handle the new main diagnostic.
flushLastDiag();
- if (Adjuster) {
+ LastDiag = Diag();
+ // FIXME: Merge with feature modules.
+ if (Adjuster)
DiagLevel = Adjuster(DiagLevel, Info);
- if (DiagLevel == DiagnosticsEngine::Ignored) {
- LastPrimaryDiagnosticWasSuppressed = true;
- return;
- }
- }
- LastPrimaryDiagnosticWasSuppressed = false;
- LastDiag = Diag();
FillDiagBase(*LastDiag);
+ if (isExcluded(LastDiag->ID))
+ LastDiag->Severity = DiagnosticsEngine::Ignored;
+ if (DiagCB)
+ DiagCB(Info, *LastDiag);
+ // Don't bother filling in the rest if diag is going to be dropped.
+ if (LastDiag->Severity == DiagnosticsEngine::Ignored)
+ return;
+
LastDiagLoc.emplace(Info.getLocation(), Info.getSourceManager());
LastDiagOriginallyError = OriginallyError;
-
if (!Info.getFixItHints().empty())
AddFix(true /* try to invent a message instead of repeating the diag */);
if (Fixer) {
- auto ExtraFixes = Fixer(DiagLevel, Info);
+ auto ExtraFixes = Fixer(LastDiag->Severity, Info);
LastDiag->Fixes.insert(LastDiag->Fixes.end(), ExtraFixes.begin(),
ExtraFixes.end());
}
- if (DiagCB)
- DiagCB(Info, *LastDiag);
} else {
// Handle a note to an existing diagnostic.
-
- // If a diagnostic was suppressed due to the suppression filter,
- // also suppress notes associated with it.
- if (LastPrimaryDiagnosticWasSuppressed) {
- return;
- }
-
if (!LastDiag) {
assert(false && "Adding a note without main diagnostic");
IgnoreDiagnostics::log(DiagLevel, Info);
return;
}
+ // If a diagnostic was suppressed due to the suppression filter,
+ // also suppress notes associated with it.
+ if (LastDiag->Severity == DiagnosticsEngine::Ignored)
+ return;
+
if (!Info.getFixItHints().empty()) {
// A clang note with fix-it is not a separate diagnostic in clangd. We
// attach it as a Fix to the main diagnostic instead.
@@ -788,7 +786,7 @@
LastDiag.reset();
});
- if (isExcluded(*LastDiag))
+ if (LastDiag->Severity == DiagnosticsEngine::Ignored)
return;
// Move errors that occur from headers into main file.
if (!LastDiag->InsideMainFile && LastDiagLoc && LastDiagOriginallyError) {
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
