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

[Haojian Wu via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG4ea066acc928: [clangd] Fix assertion crashes on unmatched NOLINTBEGIN comments. (authored by hokein).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113999

Files:
  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


Index: clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
+++ clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
@@ -476,6 +476,13 @@
       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());
Index: clang-tools-extra/clangd/ParsedAST.cpp
===================================================================
--- clang-tools-extra/clangd/ParsedAST.cpp
+++ clang-tools-extra/clangd/ParsedAST.cpp
@@ -391,9 +391,13 @@
           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;
           }
 
Index: clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h
===================================================================
--- clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h
+++ clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h
@@ -223,10 +223,6 @@
 /// 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,
Index: clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
===================================================================
--- clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
+++ clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
@@ -517,16 +517,6 @@
 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,


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D113999.387928.patch
Type: text/x-patch
Size: 3129 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20211117/b2d3cf10/attachment.bin>