Author: Kadir Cetinkaya Date: 2021-06-17T09:29:29+02:00 New Revision: 204014ec7557cde214ee8aae66661927bf9976f2
URL: https://github.com/llvm/llvm-project/commit/204014ec7557cde214ee8aae66661927bf9976f2 DIFF: https://github.com/llvm/llvm-project/commit/204014ec7557cde214ee8aae66661927bf9976f2.diff LOG: [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 Added: Modified: clang-tools-extra/clangd/Diagnostics.cpp clang-tools-extra/clangd/Diagnostics.h clang-tools-extra/clangd/unittests/FeatureModulesTests.cpp Removed: ################################################################################ diff --git a/clang-tools-extra/clangd/Diagnostics.cpp b/clang-tools-extra/clangd/Diagnostics.cpp index 88ec7aaa9b26..089802db4f69 100644 --- a/clang-tools-extra/clangd/Diagnostics.cpp +++ b/clang-tools-extra/clangd/Diagnostics.cpp @@ -76,10 +76,10 @@ bool mentionsMainFile(const Diag &D) { 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 @@ void StoreDiags::HandleDiagnostic(DiagnosticsEngine::Level DiagLevel, // 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 @@ void StoreDiags::flushLastDiag() { 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) { diff --git a/clang-tools-extra/clangd/Diagnostics.h b/clang-tools-extra/clangd/Diagnostics.h index 9c2235dce9a9..169b4aee6895 100644 --- a/clang-tools-extra/clangd/Diagnostics.h +++ b/clang-tools-extra/clangd/Diagnostics.h @@ -171,7 +171,6 @@ class StoreDiags : public DiagnosticConsumer { SourceManager *OrigSrcMgr = nullptr; llvm::DenseSet<std::pair<unsigned, unsigned>> IncludedErrorLocations; - bool LastPrimaryDiagnosticWasSuppressed = false; }; /// Determine whether a (non-clang-tidy) diagnostic is suppressed by config. diff --git a/clang-tools-extra/clangd/unittests/FeatureModulesTests.cpp b/clang-tools-extra/clangd/unittests/FeatureModulesTests.cpp index 2686c43b3530..124cf9320c3e 100644 --- a/clang-tools-extra/clangd/unittests/FeatureModulesTests.cpp +++ b/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 @@ TEST(FeatureModulesTest, ContributesTweak) { 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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
