[PATCH] D79923: [clangd] Only emit default error/fatal diagnostices from included files.

[Haojian Wu via Phabricator via cfe-commits]

hokein created this revision.
hokein added a reviewer: sammccall.
Herald added subscribers: usaxena95, kadircet, arphaman, jkorous, MaskRay, 
ilya-biryukov.
Herald added a project: clang.

This would avoid adding too much noise when there is a "-Wall" in the
compile command.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D79923

Files:
  clang-tools-extra/clangd/Diagnostics.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
@@ -1033,6 +1033,20 @@
                   WithNote(Diag(Header.range(), "error occurred here")))));
 }
 
+TEST(DiagsInHeaders, OnlyDefaultErrorOrFatal) {
+  Annotations Main(R"cpp(
+    #include [["a.h"]] // get unused "foo" warning when building preamble.
+    )cpp");
+  Annotations Header(R"cpp(
+    namespace { void foo() {} }
+    void func() {foo();} ;)cpp");
+  TestTU TU = TestTU::withCode(Main.code());
+  TU.AdditionalFiles = {{"a.h", std::string(Header.code())}};
+  // promote warnings to errors.
+  TU.ExtraArgs = {"-Werror", "-Wunused"};
+  EXPECT_THAT(TU.build().getDiagnostics(), IsEmpty());
+}
+
 TEST(DiagsInHeaders, FromNonWrittenSources) {
   Annotations Main(R"cpp(
     #include [["a.h"]]
Index: clang-tools-extra/clangd/Diagnostics.cpp
===================================================================
--- clang-tools-extra/clangd/Diagnostics.cpp
+++ clang-tools-extra/clangd/Diagnostics.cpp
@@ -124,7 +124,10 @@
 bool adjustDiagFromHeader(Diag &D, const clang::Diagnostic &Info,
                           const LangOptions &LangOpts) {
   // We only report diagnostics with at least error severity from headers.
-  if (D.Severity < DiagnosticsEngine::Level::Error)
+  // Note that we only use the default serverity to avoid adding too much 
noise,
+  // especially there is a "-Werror" flag in the compile command.
+  if (!Info.getDiags()->getDiagnosticIDs()->isDefaultMappingAsError(
+          Info.getID()))
     return false;
 
   const SourceManager &SM = Info.getSourceManager();
@@ -514,7 +517,8 @@
   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.
-    if (DiagLevel < DiagnosticsEngine::Level::Error) {
+    if (!Info.getDiags()->getDiagnosticIDs()->isDefaultMappingAsError(
+            Info.getID())) {
       IgnoreDiagnostics::log(DiagLevel, Info);
       return; // non-errors add too much noise, do not show them.
     }



Index: clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
+++ clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
@@ -1033,6 +1033,20 @@
                   WithNote(Diag(Header.range(), "error occurred here")))));
 }
 
+TEST(DiagsInHeaders, OnlyDefaultErrorOrFatal) {
+  Annotations Main(R"cpp(
+    #include [["a.h"]] // get unused "foo" warning when building preamble.
+    )cpp");
+  Annotations Header(R"cpp(
+    namespace { void foo() {} }
+    void func() {foo();} ;)cpp");
+  TestTU TU = TestTU::withCode(Main.code());
+  TU.AdditionalFiles = {{"a.h", std::string(Header.code())}};
+  // promote warnings to errors.
+  TU.ExtraArgs = {"-Werror", "-Wunused"};
+  EXPECT_THAT(TU.build().getDiagnostics(), IsEmpty());
+}
+
 TEST(DiagsInHeaders, FromNonWrittenSources) {
   Annotations Main(R"cpp(
     #include [["a.h"]]
Index: clang-tools-extra/clangd/Diagnostics.cpp
===================================================================
--- clang-tools-extra/clangd/Diagnostics.cpp
+++ clang-tools-extra/clangd/Diagnostics.cpp
@@ -124,7 +124,10 @@
 bool adjustDiagFromHeader(Diag &D, const clang::Diagnostic &Info,
                           const LangOptions &LangOpts) {
   // We only report diagnostics with at least error severity from headers.
-  if (D.Severity < DiagnosticsEngine::Level::Error)
+  // Note that we only use the default serverity to avoid adding too much noise,
+  // especially there is a "-Werror" flag in the compile command.
+  if (!Info.getDiags()->getDiagnosticIDs()->isDefaultMappingAsError(
+          Info.getID()))
     return false;
 
   const SourceManager &SM = Info.getSourceManager();
@@ -514,7 +517,8 @@
   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.
-    if (DiagLevel < DiagnosticsEngine::Level::Error) {
+    if (!Info.getDiags()->getDiagnosticIDs()->isDefaultMappingAsError(
+            Info.getID())) {
       IgnoreDiagnostics::log(DiagLevel, Info);
       return; // non-errors add too much noise, do not show them.
     }

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits