subject:"\[PATCH\] D95349\: \[clangd\] Allow diagnostics to be suppressed with configuration"

[PATCH] D95349: [clangd] Allow diagnostics to be suppressed with configuration

2021-01-27 Thread Sam McCall via Phabricator via cfe-commits

sammccall added inline comments.



Comment at: clang-tools-extra/clangd/ParsedAST.cpp:319
 
-if (!CTChecks.empty()) {
-  ASTDiags.setLevelAdjuster([](DiagnosticsEngine::Level 
DiagLevel,
- const clang::Diagnostic ) {
+ASTDiags.setLevelAdjuster([&, (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([&, (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

2021-01-27 Thread Aleksandr Platonov via Phabricator via cfe-commits

ArcsinX added inline comments.



Comment at: clang-tools-extra/clangd/ParsedAST.cpp:319
 
-if (!CTChecks.empty()) {
-  ASTDiags.setLevelAdjuster([](DiagnosticsEngine::Level 
DiagLevel,
- const clang::Diagnostic ) {
+ASTDiags.setLevelAdjuster([&, (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([&, (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

2021-01-25 Thread Sam McCall via Phabricator via cfe-commits

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=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,
+

[PATCH] D95349: [clangd] Allow diagnostics to be suppressed with configuration

2021-01-25 Thread Haojian Wu via Phabricator via cfe-commits

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

2021-01-25 Thread Sam McCall via Phabricator via cfe-commits

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));
+

[PATCH] D95349: [clangd] Allow diagnostics to be suppressed with configuration

[PATCH] D95349: [clangd] Allow diagnostics to be suppressed with configuration

[PATCH] D95349: [clangd] Allow diagnostics to be suppressed with configuration

[PATCH] D95349: [clangd] Allow diagnostics to be suppressed with configuration

[PATCH] D95349: [clangd] Allow diagnostics to be suppressed with configuration

5 matches

Site Navigation

Mail list logo

Footer information