[PATCH] D95349: [clangd] Allow diagnostics to be suppressed with configuration
sammccall added inline comments. Comment at: clang-tools-extra/clangd/ParsedAST.cpp:319 -if (!CTChecks.empty()) { - ASTDiags.setLevelAdjuster([&CTContext](DiagnosticsEngine::Level DiagLevel, - const clang::Diagnostic &Info) { +ASTDiags.setLevelAdjuster([&, &Cfg(Config::current())]( + DiagnosticsEngine::Level DiagLevel, ArcsinX wrote: > This line breaks GCC 5.4 build with the following log: > ``` > llvm-project/clang-tools-extra/clangd/ParsedAST.cpp:319:55: error: binding > ‘const clang::clangd::Config’ to reference of type ‘clang::clangd::Config&’ > discards qualifiers > ASTDiags.setLevelAdjuster([&, &Cfg(Config::current())]( >^ > ``` > Unsure is it a compiler bug or not. Moving `Cfg` out of the capture list or > using a cast to `Config &` solves the problem. Thanks! Appears to be https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66735 Landed the workaround as 12de8e1399fecf691639ba430b3824acb1311e70 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D95349/new/ https://reviews.llvm.org/D95349 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D95349: [clangd] Allow diagnostics to be suppressed with configuration
ArcsinX added inline comments. Comment at: clang-tools-extra/clangd/ParsedAST.cpp:319 -if (!CTChecks.empty()) { - ASTDiags.setLevelAdjuster([&CTContext](DiagnosticsEngine::Level DiagLevel, - const clang::Diagnostic &Info) { +ASTDiags.setLevelAdjuster([&, &Cfg(Config::current())]( + DiagnosticsEngine::Level DiagLevel, This line breaks GCC 5.4 build with the following log: ``` llvm-project/clang-tools-extra/clangd/ParsedAST.cpp:319:55: error: binding ‘const clang::clangd::Config’ to reference of type ‘clang::clangd::Config&’ discards qualifiers ASTDiags.setLevelAdjuster([&, &Cfg(Config::current())]( ^ ``` Unsure is it a compiler bug or not. Moving `Cfg` out of the capture list or using a cast to `Config &` solves the problem. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D95349/new/ https://reviews.llvm.org/D95349 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D95349: [clangd] Allow diagnostics to be suppressed with configuration
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG7e506b30a1e1: [clangd] Allow diagnostics to be suppressed with configuration (authored by sammccall). Changed prior to commit: https://reviews.llvm.org/D95349?vs=318972&id=318997#toc Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D95349/new/ https://reviews.llvm.org/D95349 Files: clang-tools-extra/clangd/Config.h clang-tools-extra/clangd/ConfigCompile.cpp clang-tools-extra/clangd/ConfigFragment.h clang-tools-extra/clangd/Diagnostics.cpp clang-tools-extra/clangd/Diagnostics.h clang-tools-extra/clangd/ParsedAST.cpp clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp clang-tools-extra/clangd/unittests/ConfigYAMLTests.cpp clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp Index: clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp === --- clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp +++ clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp @@ -7,6 +7,7 @@ //===--===// #include "Annotations.h" +#include "Config.h" #include "Diagnostics.h" #include "ParsedAST.h" #include "Protocol.h" @@ -16,6 +17,7 @@ #include "TestTU.h" #include "TidyProvider.h" #include "index/MemIndex.h" +#include "support/Context.h" #include "support/Path.h" #include "clang/Basic/Diagnostic.h" #include "clang/Basic/DiagnosticSema.h" @@ -371,6 +373,28 @@ DiagSource(Diag::ClangTidy), DiagName("modernize-loop-convert"; } +TEST(DiagnosticTest, RespectsDiagnosticConfig) { + Annotations Main(R"cpp( +// error-ok +void x() { + [[unknown]](); + $ret[[return]] 42; +} + )cpp"); + auto TU = TestTU::withCode(Main.code()); + EXPECT_THAT( + TU.build().getDiagnostics(), + ElementsAre(Diag(Main.range(), "use of undeclared identifier 'unknown'"), + Diag(Main.range("ret"), + "void function 'x' should not return a value"))); + Config Cfg; + Cfg.Diagnostics.Suppress.insert("return-type"); + WithContextValue WithCfg(Config::Key, std::move(Cfg)); + EXPECT_THAT(TU.build().getDiagnostics(), + ElementsAre(Diag(Main.range(), + "use of undeclared identifier 'unknown'"))); +} + TEST(DiagnosticTest, ClangTidySuppressionComment) { Annotations Main(R"cpp( int main() { Index: clang-tools-extra/clangd/unittests/ConfigYAMLTests.cpp === --- clang-tools-extra/clangd/unittests/ConfigYAMLTests.cpp +++ clang-tools-extra/clangd/unittests/ConfigYAMLTests.cpp @@ -98,7 +98,7 @@ YAML.range()); } -TEST(ParseYAML, Diagnostics) { +TEST(ParseYAML, ConfigDiagnostics) { CapturedDiags Diags; Annotations YAML(R"yaml( If: Index: clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp === --- clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp +++ clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp @@ -11,6 +11,7 @@ #include "ConfigTesting.h" #include "Features.inc" #include "TestFS.h" +#include "clang/Basic/DiagnosticSema.h" #include "llvm/ADT/None.h" #include "llvm/ADT/Optional.h" #include "llvm/ADT/StringRef.h" @@ -30,6 +31,7 @@ using ::testing::IsEmpty; using ::testing::SizeIs; using ::testing::StartsWith; +using ::testing::UnorderedElementsAre; class ConfigCompileTests : public ::testing::Test { protected: @@ -183,6 +185,39 @@ } } +TEST_F(ConfigCompileTests, DiagnosticSuppression) { + Frag.Diagnostics.Suppress.emplace_back("bugprone-use-after-move"); + Frag.Diagnostics.Suppress.emplace_back("unreachable-code"); + Frag.Diagnostics.Suppress.emplace_back("-Wunused-variable"); + Frag.Diagnostics.Suppress.emplace_back("typecheck_bool_condition"); + Frag.Diagnostics.Suppress.emplace_back("err_unexpected_friend"); + Frag.Diagnostics.Suppress.emplace_back("warn_alloca"); + EXPECT_TRUE(compileAndApply()); + EXPECT_THAT(Conf.Diagnostics.Suppress.keys(), + UnorderedElementsAre("bugprone-use-after-move", + "unreachable-code", "unused-variable", + "typecheck_bool_condition", + "unexpected_friend", "warn_alloca")); + EXPECT_TRUE(isBuiltinDiagnosticSuppressed(diag::warn_unreachable, +Conf.Diagnostics.Suppress)); + // Subcategory not respected/suppressed. + EXPECT_FALSE(isBuiltinDiagnosticSuppressed(diag::warn_unreachable_break, + Conf.Diagnostics.Suppress)); + EXPECT_TRUE(isBuiltinDiagnosticSuppressed(diag::warn_unused_variable, +Conf.Diagn
[PATCH] D95349: [clangd] Allow diagnostics to be suppressed with configuration
hokein accepted this revision. hokein added inline comments. This revision is now accepted and ready to land. Comment at: clang-tools-extra/clangd/Diagnostics.cpp:808 +llvm::StringRef Code(CodePtr); +Code.consume_front("err_"); +if (Suppress.contains(Code)) nit: use `normalizeSuppressedCode(Code)`? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D95349/new/ https://reviews.llvm.org/D95349 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D95349: [clangd] Allow diagnostics to be suppressed with configuration
sammccall created this revision. sammccall added a reviewer: hokein. Herald added subscribers: usaxena95, kadircet, arphaman. sammccall requested review of this revision. Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov. Herald added a project: clang. This has been specifically requested: https://github.com/clangd/vscode-clangd/issues/114 and various issues can be addressed with this as a workaround, e.g.: https://github.com/clangd/clangd/issues/662 Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D95349 Files: clang-tools-extra/clangd/Config.h clang-tools-extra/clangd/ConfigCompile.cpp clang-tools-extra/clangd/ConfigFragment.h clang-tools-extra/clangd/Diagnostics.cpp clang-tools-extra/clangd/Diagnostics.h clang-tools-extra/clangd/ParsedAST.cpp clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp clang-tools-extra/clangd/unittests/ConfigYAMLTests.cpp clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp Index: clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp === --- clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp +++ clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp @@ -7,6 +7,7 @@ //===--===// #include "Annotations.h" +#include "Config.h" #include "Diagnostics.h" #include "ParsedAST.h" #include "Protocol.h" @@ -16,6 +17,7 @@ #include "TestTU.h" #include "TidyProvider.h" #include "index/MemIndex.h" +#include "support/Context.h" #include "support/Path.h" #include "clang/Basic/Diagnostic.h" #include "clang/Basic/DiagnosticSema.h" @@ -371,6 +373,28 @@ DiagSource(Diag::ClangTidy), DiagName("modernize-loop-convert"; } +TEST(DiagnosticTest, RespectsDiagnosticConfig) { + Annotations Main(R"cpp( +// error-ok +void x() { + [[unknown]](); + $ret[[return]] 42; +} + )cpp"); + auto TU = TestTU::withCode(Main.code()); + EXPECT_THAT( + TU.build().getDiagnostics(), + ElementsAre(Diag(Main.range(), "use of undeclared identifier 'unknown'"), + Diag(Main.range("ret"), + "void function 'x' should not return a value"))); + Config Cfg; + Cfg.Diagnostics.Suppress.insert("return-type"); + WithContextValue WithCfg(Config::Key, std::move(Cfg)); + EXPECT_THAT(TU.build().getDiagnostics(), + ElementsAre(Diag(Main.range(), + "use of undeclared identifier 'unknown'"))); +} + TEST(DiagnosticTest, ClangTidySuppressionComment) { Annotations Main(R"cpp( int main() { Index: clang-tools-extra/clangd/unittests/ConfigYAMLTests.cpp === --- clang-tools-extra/clangd/unittests/ConfigYAMLTests.cpp +++ clang-tools-extra/clangd/unittests/ConfigYAMLTests.cpp @@ -98,7 +98,7 @@ YAML.range()); } -TEST(ParseYAML, Diagnostics) { +TEST(ParseYAML, ConfigDiagnostics) { CapturedDiags Diags; Annotations YAML(R"yaml( If: Index: clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp === --- clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp +++ clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp @@ -11,6 +11,7 @@ #include "ConfigTesting.h" #include "Features.inc" #include "TestFS.h" +#include "clang/Basic/DiagnosticSema.h" #include "llvm/ADT/None.h" #include "llvm/ADT/Optional.h" #include "llvm/ADT/StringRef.h" @@ -30,6 +31,7 @@ using ::testing::IsEmpty; using ::testing::SizeIs; using ::testing::StartsWith; +using ::testing::UnorderedElementsAre; class ConfigCompileTests : public ::testing::Test { protected: @@ -183,6 +185,39 @@ } } +TEST_F(ConfigCompileTests, DiagnosticSuppression) { + Frag.Diagnostics.Suppress.emplace_back("bugprone-use-after-move"); + Frag.Diagnostics.Suppress.emplace_back("unreachable-code"); + Frag.Diagnostics.Suppress.emplace_back("-Wunused-variable"); + Frag.Diagnostics.Suppress.emplace_back("typecheck_bool_condition"); + Frag.Diagnostics.Suppress.emplace_back("err_unexpected_friend"); + Frag.Diagnostics.Suppress.emplace_back("warn_alloca"); + EXPECT_TRUE(compileAndApply()); + EXPECT_THAT(Conf.Diagnostics.Suppress.keys(), + UnorderedElementsAre("bugprone-use-after-move", + "unreachable-code", "unused-variable", + "typecheck_bool_condition", + "unexpected_friend", "warn_alloca")); + EXPECT_TRUE(isBuiltinDiagnosticSuppressed(diag::warn_unreachable, +Conf.Diagnostics.Suppress)); + // Subcategory not respected/suppressed. + EXPECT_FALSE(isBuiltinDiagnosticSuppressed(diag::warn_unreachable_break, + Conf.Diagnostics.Suppress)); + EXPECT_TRUE(isBuiltinDiagnosticSuppressed(diag::war