[clang-tools-extra] 4ea066a - [clangd] Fix assertion crashes on unmatched NOLINTBEGIN comments.

[Haojian Wu via cfe-commits]

Author: Haojian Wu
Date: 2021-11-17T15:31:38+01:00
New Revision: 4ea066acc928d772c2a610024b7f0656b36e6afd

URL: https://github.com/llvm/llvm-project/commit/4ea066acc928d772c2a610024b7f0656b36e6afd
DIFF: https://github.com/llvm/llvm-project/commit/4ea066acc928d772c2a610024b7f0656b36e6afd.diff

LOG: [clangd] Fix assertion crashes on unmatched NOLINTBEGIN comments.

The overload shouldSuppressDiagnostic seems unnecessary, and it is only
used in clangd.

This patch removes it and use the real one (suppression diagnostics are
discarded in clangd at the moment).

Fixes https://github.com/clangd/clangd/issues/929

Differential Revision: https://reviews.llvm.org/D113999

Added: 
    

Modified: 
    clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
    clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h
    clang-tools-extra/clangd/ParsedAST.cpp
    clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp

Removed: 
    


################################################################################
diff  --git a/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp b/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
index dd590f7c79fb0..5b410ba9d2ea4 100644
--- a/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
+++ b/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
@@ -517,16 +517,6 @@ static bool lineIsMarkedWithNOLINTinMacro(
 namespace clang {
 namespace tidy {
 
-bool shouldSuppressDiagnostic(DiagnosticsEngine::Level DiagLevel,
-                              const Diagnostic &Info, ClangTidyContext &Context,
-                              bool AllowIO) {
-  SmallVector<ClangTidyError, 1> Unused;
-  bool ShouldSuppress =
-      shouldSuppressDiagnostic(DiagLevel, Info, Context, Unused, AllowIO);
-  assert(Unused.empty());
-  return ShouldSuppress;
-}
-
 bool shouldSuppressDiagnostic(
     DiagnosticsEngine::Level DiagLevel, const Diagnostic &Info,
     ClangTidyContext &Context,

diff  --git a/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h b/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h
index 234455c5bea21..9f519fb605ad6 100644
--- a/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h
+++ b/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h
@@ -223,10 +223,6 @@ class ClangTidyContext {
 /// for example, the use of a "NOLINTBEGIN" comment that is not followed by a
 /// "NOLINTEND" comment - a diagnostic regarding the improper use is returned
 /// via the output argument `SuppressionErrors`.
-bool shouldSuppressDiagnostic(DiagnosticsEngine::Level DiagLevel,
-                              const Diagnostic &Info, ClangTidyContext &Context,
-                              bool AllowIO = true);
-
 bool shouldSuppressDiagnostic(
     DiagnosticsEngine::Level DiagLevel, const Diagnostic &Info,
     ClangTidyContext &Context,

diff  --git a/clang-tools-extra/clangd/ParsedAST.cpp b/clang-tools-extra/clangd/ParsedAST.cpp
index 8fce756b83c1d..719c374d8b859 100644
--- a/clang-tools-extra/clangd/ParsedAST.cpp
+++ b/clang-tools-extra/clangd/ParsedAST.cpp
@@ -391,9 +391,13 @@ ParsedAST::build(llvm::StringRef Filename, const ParseInputs &Inputs,
           bool IsInsideMainFile =
               Info.hasSourceManager() &&
               isInsideMainFile(Info.getLocation(), Info.getSourceManager());
+          SmallVector<tidy::ClangTidyError, 1> TidySuppressedErrors;
           if (IsInsideMainFile &&
               tidy::shouldSuppressDiagnostic(DiagLevel, Info, *CTContext,
+                                             TidySuppressedErrors,
                                              /*AllowIO=*/false)) {
+            // FIXME: should we expose the suppression error (invalid use of
+            // NOLINT comments)?
             return DiagnosticsEngine::Ignored;
           }
 

diff  --git a/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp b/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
index 277b9b181d9b6..89a3cc060f925 100644
--- a/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
+++ b/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
@@ -476,6 +476,13 @@ TEST(DiagnosticTest, ClangTidySuppressionComment) {
       double g = [[8]] / i;
       #define BAD2 BAD
       double h = BAD2;  // NOLINT
+      // NOLINTBEGIN
+      double x = BAD2;
+      double y = BAD2;
+      // NOLINTEND
+
+      // verify no crashes on unmatched nolints.
+      // NOLINTBEIGN
     }
   )cpp");
   TestTU TU = TestTU::withCode(Main.code());