This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG204014ec7557: [clangd] Fix feature modules to drop 
diagnostics (authored by kadircet).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D103387/new/

https://reviews.llvm.org/D103387

Files:
  clang-tools-extra/clangd/Diagnostics.cpp
  clang-tools-extra/clangd/Diagnostics.h
  clang-tools-extra/clangd/unittests/FeatureModulesTests.cpp

Index: clang-tools-extra/clangd/unittests/FeatureModulesTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/FeatureModulesTests.cpp
+++ 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 @@
   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
Index: clang-tools-extra/clangd/Diagnostics.h
===================================================================
--- clang-tools-extra/clangd/Diagnostics.h
+++ clang-tools-extra/clangd/Diagnostics.h
@@ -171,7 +171,6 @@
   SourceManager *OrigSrcMgr = nullptr;
 
   llvm::DenseSet<std::pair<unsigned, unsigned>> IncludedErrorLocations;
-  bool LastPrimaryDiagnosticWasSuppressed = false;
 };
 
 /// Determine whether a (non-clang-tidy) diagnostic is suppressed by config.
Index: clang-tools-extra/clangd/Diagnostics.cpp
===================================================================
--- clang-tools-extra/clangd/Diagnostics.cpp
+++ clang-tools-extra/clangd/Diagnostics.cpp
@@ -76,10 +76,10 @@
   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 @@
     // 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 @@
     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) {
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to