From a9605a92bdb82199d2113b9d0a840a0b4c82864b Mon Sep 17 00:00:00 2001 From: Joe Eagar Date: Wed, 18 Mar 2026 05:27:45 -0700 Subject: [PATCH] [clangd] Support suppressions for driver diagnostics (#182912) Rebase of https://reviews.llvm.org/D127844 Fixes [#1142](https://github.com/clangd/clangd/issues/1142) --- clang-tools-extra/clangd/Diagnostics.cpp | 64 +++++++++++-------- clang-tools-extra/clangd/Diagnostics.h | 8 +-- clang-tools-extra/clangd/ParsedAST.cpp | 5 -- clang-tools-extra/clangd/Preamble.cpp | 5 -- .../clangd/unittests/CompilerTests.cpp | 15 +++++ .../clangd/unittests/ConfigCompileTests.cpp | 37 ----------- .../clangd/unittests/DiagnosticsTests.cpp | 21 ++++++ 7 files changed, 75 insertions(+), 80 deletions(-) diff --git a/clang-tools-extra/clangd/Diagnostics.cpp b/clang-tools-extra/clangd/Diagnostics.cpp index 47c3fab03458..f68092b9a088 100644 --- a/clang-tools-extra/clangd/Diagnostics.cpp +++ b/clang-tools-extra/clangd/Diagnostics.cpp @@ -9,6 +9,7 @@ #include "Diagnostics.h" #include "../clang-tidy/ClangTidyDiagnosticConsumer.h" #include "Compiler.h" +#include "Config.h" #include "Protocol.h" #include "SourceCode.h" #include "support/Logger.h" @@ -679,6 +680,28 @@ static void fillNonLocationData(DiagnosticsEngine::Level DiagLevel, .str(); } +static bool isDiagnosticSuppressed(const clang::Diagnostic &Diag, + const llvm::StringSet<> &Suppress, + const std::optional &LangOpts) { + // Don't complain about header-only stuff in mainfiles if it's a header. + // FIXME: would be cleaner to suppress in clang, once we decide whether the + // behavior should be to silently-ignore or respect the pragma. + if (LangOpts && Diag.getID() == diag::pp_pragma_sysheader_in_main_file && + LangOpts->IsHeaderFile) + return true; + + if (const char *CodePtr = getDiagnosticCode(Diag.getID())) { + if (Suppress.contains(normalizeSuppressedCode(CodePtr))) + return true; + } + StringRef Warning = + Diag.getDiags()->getDiagnosticIDs()->getWarningOptionForDiag( + Diag.getID()); + if (!Warning.empty() && Suppress.contains(Warning)) + return true; + return false; +} + void StoreDiags::HandleDiagnostic(DiagnosticsEngine::Level DiagLevel, const clang::Diagnostic &Info) { // If the diagnostic was generated for a different SourceManager, skip it. @@ -696,6 +719,19 @@ void StoreDiags::HandleDiagnostic(DiagnosticsEngine::Level DiagLevel, Info.getDiags()->getDiagnosticIDs()->isDefaultMappingAsError( Info.getID()); + if (!isNote(DiagLevel)) { + const Config &Cfg = Config::current(); + // Check if diagnostics is suppressed (possibly by user), before doing any + // adjustments. + if (Cfg.Diagnostics.SuppressAll || + isDiagnosticSuppressed(Info, Cfg.Diagnostics.Suppress, LangOpts)) { + DiagLevel = DiagnosticsEngine::Ignored; + } else if (Adjuster) { + // FIXME: Merge with feature modules. + DiagLevel = Adjuster(DiagLevel, Info); + } + } + if (Info.getLocation().isInvalid()) { // Handle diagnostics coming from command-line arguments. The source manager // is *not* available at this point, so we cannot use it. @@ -806,8 +842,7 @@ void StoreDiags::HandleDiagnostic(DiagnosticsEngine::Level DiagLevel, } if (Message.empty()) // either !SyntheticMessage, or we failed to make one. Info.FormatDiagnostic(Message); - LastDiag->Fixes.push_back( - Fix{std::string(Message), std::move(Edits), {}}); + LastDiag->Fixes.push_back(Fix{std::string(Message), std::move(Edits), {}}); return true; }; @@ -816,9 +851,6 @@ void StoreDiags::HandleDiagnostic(DiagnosticsEngine::Level DiagLevel, flushLastDiag(); LastDiag = Diag(); - // FIXME: Merge with feature modules. - if (Adjuster) - DiagLevel = Adjuster(DiagLevel, Info); FillDiagBase(*LastDiag); if (isExcluded(LastDiag->ID)) @@ -905,28 +937,6 @@ void StoreDiags::flushLastDiag() { Output.push_back(std::move(*LastDiag)); } -bool isDiagnosticSuppressed(const clang::Diagnostic &Diag, - const llvm::StringSet<> &Suppress, - const LangOptions &LangOpts) { - // Don't complain about header-only stuff in mainfiles if it's a header. - // FIXME: would be cleaner to suppress in clang, once we decide whether the - // behavior should be to silently-ignore or respect the pragma. - if (Diag.getID() == diag::pp_pragma_sysheader_in_main_file && - LangOpts.IsHeaderFile) - return true; - - if (const char *CodePtr = getDiagnosticCode(Diag.getID())) { - if (Suppress.contains(normalizeSuppressedCode(CodePtr))) - return true; - } - StringRef Warning = - Diag.getDiags()->getDiagnosticIDs()->getWarningOptionForDiag( - Diag.getID()); - if (!Warning.empty() && Suppress.contains(Warning)) - return true; - return false; -} - llvm::StringRef normalizeSuppressedCode(llvm::StringRef Code) { Code.consume_front("err_"); Code.consume_front("-W"); diff --git a/clang-tools-extra/clangd/Diagnostics.h b/clang-tools-extra/clangd/Diagnostics.h index c45d8dc3aa6c..d433abb53015 100644 --- a/clang-tools-extra/clangd/Diagnostics.h +++ b/clang-tools-extra/clangd/Diagnostics.h @@ -173,17 +173,13 @@ private: std::vector Output; std::optional LangOpts; std::optional LastDiag; - std::optional LastDiagLoc; // Valid only when LastDiag is set. - bool LastDiagOriginallyError = false; // Valid only when LastDiag is set. + std::optional LastDiagLoc; // Valid only when LastDiag is set. + bool LastDiagOriginallyError = false; // Valid only when LastDiag is set. SourceManager *OrigSrcMgr = nullptr; llvm::DenseSet> IncludedErrorLocations; }; -/// Determine whether a (non-clang-tidy) diagnostic is suppressed by config. -bool isDiagnosticSuppressed(const clang::Diagnostic &Diag, - const llvm::StringSet<> &Suppressed, - const LangOptions &); /// Take a user-specified diagnostic code, and convert it to a normalized form /// stored in the config and consumed by isDiagnosticsSuppressed. /// diff --git a/clang-tools-extra/clangd/ParsedAST.cpp b/clang-tools-extra/clangd/ParsedAST.cpp index c23b5d7a38d3..4e873f1257a1 100644 --- a/clang-tools-extra/clangd/ParsedAST.cpp +++ b/clang-tools-extra/clangd/ParsedAST.cpp @@ -585,11 +585,6 @@ ParsedAST::build(llvm::StringRef Filename, const ParseInputs &Inputs, ASTDiags.setLevelAdjuster([&](DiagnosticsEngine::Level DiagLevel, const clang::Diagnostic &Info) { - if (Cfg.Diagnostics.SuppressAll || - isDiagnosticSuppressed(Info, Cfg.Diagnostics.Suppress, - Clang->getLangOpts())) - return DiagnosticsEngine::Ignored; - auto It = OverriddenSeverity.find(Info.getID()); if (It != OverriddenSeverity.end()) DiagLevel = It->second; diff --git a/clang-tools-extra/clangd/Preamble.cpp b/clang-tools-extra/clangd/Preamble.cpp index cdda0208a96d..f5e512793e98 100644 --- a/clang-tools-extra/clangd/Preamble.cpp +++ b/clang-tools-extra/clangd/Preamble.cpp @@ -598,13 +598,8 @@ buildPreamble(PathRef FileName, CompilerInvocation CI, CompilerInstance::createDiagnostics(*VFS, CI.getDiagnosticOpts(), &PreambleDiagnostics, /*ShouldOwnClient=*/false); - const Config &Cfg = Config::current(); PreambleDiagnostics.setLevelAdjuster([&](DiagnosticsEngine::Level DiagLevel, const clang::Diagnostic &Info) { - if (Cfg.Diagnostics.SuppressAll || - isDiagnosticSuppressed(Info, Cfg.Diagnostics.Suppress, - CI.getLangOpts())) - return DiagnosticsEngine::Ignored; switch (Info.getID()) { case diag::warn_no_newline_eof: // If the preamble doesn't span the whole file, drop the no newline at diff --git a/clang-tools-extra/clangd/unittests/CompilerTests.cpp b/clang-tools-extra/clangd/unittests/CompilerTests.cpp index ab9e85e983d1..9c8ad8d70b47 100644 --- a/clang-tools-extra/clangd/unittests/CompilerTests.cpp +++ b/clang-tools-extra/clangd/unittests/CompilerTests.cpp @@ -7,6 +7,8 @@ //===----------------------------------------------------------------------===// #include "Compiler.h" +#include "Config.h" +#include "Diagnostics.h" #include "TestTU.h" #include "clang/Frontend/DependencyOutputOptions.h" #include "clang/Frontend/FrontendOptions.h" @@ -113,6 +115,19 @@ TEST(BuildCompilerInvocation, EmptyArgs) { // No crash. EXPECT_EQ(buildCompilerInvocation(Inputs, Diags), nullptr); } +TEST(BuildCompilerInvocation, SuppressDiags) { + MockFS FS; + StoreDiags Diags; + TestTU TU; + TU.ExtraArgs = {"-funknown-arg"}; + auto Inputs = TU.inputs(FS); + + Config Cfg; + Cfg.Diagnostics.Suppress = {"drv_unknown_argument"}; + WithContextValue SuppressFilterWithCfg(Config::Key, std::move(Cfg)); + EXPECT_NE(buildCompilerInvocation(Inputs, Diags), nullptr); + EXPECT_THAT(Diags.take(), IsEmpty()); +} } // namespace } // namespace clangd } // namespace clang diff --git a/clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp b/clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp index a9b462980dd4..5fecf32f8e5a 100644 --- a/clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp +++ b/clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp @@ -298,43 +298,6 @@ TEST_F(ConfigCompileTests, DiagnosticSuppression) { "unreachable-code", "unused-variable", "typecheck_bool_condition", "unexpected_friend", "warn_alloca")); - clang::DiagnosticOptions DiagOpts; - clang::DiagnosticsEngine DiagEngine(DiagnosticIDs::create(), DiagOpts, - new clang::IgnoringDiagConsumer); - - using Diag = clang::Diagnostic; - { - auto D = DiagEngine.Report(diag::warn_unreachable); - EXPECT_TRUE(isDiagnosticSuppressed( - Diag{&DiagEngine, D}, Conf.Diagnostics.Suppress, LangOptions())); - } - // Subcategory not respected/suppressed. - { - auto D = DiagEngine.Report(diag::warn_unreachable_break); - EXPECT_FALSE(isDiagnosticSuppressed( - Diag{&DiagEngine, D}, Conf.Diagnostics.Suppress, LangOptions())); - } - { - auto D = DiagEngine.Report(diag::warn_unused_variable); - EXPECT_TRUE(isDiagnosticSuppressed( - Diag{&DiagEngine, D}, Conf.Diagnostics.Suppress, LangOptions())); - } - { - auto D = DiagEngine.Report(diag::err_typecheck_bool_condition); - EXPECT_TRUE(isDiagnosticSuppressed( - Diag{&DiagEngine, D}, Conf.Diagnostics.Suppress, LangOptions())); - } - { - auto D = DiagEngine.Report(diag::err_unexpected_friend); - EXPECT_TRUE(isDiagnosticSuppressed( - Diag{&DiagEngine, D}, Conf.Diagnostics.Suppress, LangOptions())); - } - { - auto D = DiagEngine.Report(diag::warn_alloca); - EXPECT_TRUE(isDiagnosticSuppressed( - Diag{&DiagEngine, D}, Conf.Diagnostics.Suppress, LangOptions())); - } - Frag.Diagnostics.Suppress.emplace_back("*"); EXPECT_TRUE(compileAndApply()); EXPECT_TRUE(Conf.Diagnostics.SuppressAll); diff --git a/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp b/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp index 84ceddbd4fc4..6fe48478e117 100644 --- a/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp +++ b/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp @@ -2170,6 +2170,27 @@ TEST(DiagnosticsTest, UnusedInHeader) { EXPECT_THAT(TU.build().getDiagnostics(), IsEmpty()); } +TEST(DiagnosticsTest, DontSuppressSubcategories) { + Annotations Source(R"cpp( + /*error-ok*/ + void bar(int x) { + switch(x) { + default: + break; + break; + } + })cpp"); + TestTU TU; + TU.ExtraArgs.push_back("-Wunreachable-code-aggressive"); + TU.Code = Source.code().str(); + Config Cfg; + // This shouldn't suppress subcategory unreachable-break. + Cfg.Diagnostics.Suppress = {"unreachable-code"}; + WithContextValue SuppressFilterWithCfg(Config::Key, std::move(Cfg)); + EXPECT_THAT(TU.build().getDiagnostics(), + ElementsAre(diagName("-Wunreachable-code-break"))); +} + } // namespace } // namespace clangd } // namespace clang