[PATCH] D26218: [clang-tidy] Ignore notes along with a warning when processing NOLINT

[Nikita Kakuev via cfe-commits]

nkakuev created this revision.
nkakuev added reviewers: alexfh, mgehre.
nkakuev added a subscriber: cfe-commits.

When clang-tidy suppresses a warning, ignored by NOLINT, it doesn't suppress related notes. This leads to an assertion/crash, as described in this bug: https://llvm.org/bugs/show_bug.cgi?id=30565

The minimal reproducible example is as follows (run with 'clang-tidy source.cpp -- -c'):

- header.h:

void TriggerWarning(const int& In, int& Out) {

  if (In > 123) // NOLINT
    Out = 123;

}

- source.cpp:

#include "header.h"
void MaybeInitialize(int &Out) {

  int In;
  TriggerWarning(In, Out);

}

Suppressing notes after suppressing a warning fixes the problem and produces a consistent diagnostics output.


https://reviews.llvm.org/D26218

Files:
  clang-tidy/ClangTidyDiagnosticConsumer.cpp
  test/clang-tidy/Inputs/nolint/trigger_warning.h
  test/clang-tidy/nolint.cpp


Index: test/clang-tidy/nolint.cpp
===================================================================
--- test/clang-tidy/nolint.cpp
+++ test/clang-tidy/nolint.cpp
@@ -1,4 +1,5 @@
-// RUN: %check_clang_tidy %s google-explicit-constructor,clang-diagnostic-unused-variable %t -- -extra-arg=-Wunused-variable --
+// RUN: %check_clang_tidy %s google-explicit-constructor,clang-diagnostic-unused-variable,clang-analyzer-core.UndefinedBinaryOperatorResult %t -- -extra-arg=-Wunused-variable -- -I%S/Inputs/nolint
+
 
 class A { A(int i); };
 // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: single-argument constructors must be marked explicit
@@ -27,4 +28,12 @@
 #define DOUBLE_MACRO MACRO(H) // NOLINT
 DOUBLE_MACRO
 
-// CHECK-MESSAGES: Suppressed 7 warnings (7 NOLINT)
+#include "trigger_warning.h"
+void I(int& Out) {
+  int In;
+  A1(In, Out);
+}
+// CHECK-NOT: header.h:{{.*}} warning: The left operand of '>' is a garbage value
+// CHECK-NOT: :[[@LINE-4]]:{{.*}} note
+
+// CHECK-MESSAGES: Suppressed 8 warnings (8 NOLINT)
Index: test/clang-tidy/Inputs/nolint/trigger_warning.h
===================================================================
--- test/clang-tidy/Inputs/nolint/trigger_warning.h
+++ test/clang-tidy/Inputs/nolint/trigger_warning.h
@@ -0,0 +1,5 @@
+void A1(const int &In, int &Out) {
+  if (In > 123) // NOLINT
+    Out = 123;
+}
+
Index: clang-tidy/ClangTidyDiagnosticConsumer.cpp
===================================================================
--- clang-tidy/ClangTidyDiagnosticConsumer.cpp
+++ clang-tidy/ClangTidyDiagnosticConsumer.cpp
@@ -309,13 +309,21 @@
 
 void ClangTidyDiagnosticConsumer::HandleDiagnostic(
     DiagnosticsEngine::Level DiagLevel, const Diagnostic &Info) {
+  static bool ShouldIgnoreNotes = false;
+  if (ShouldIgnoreNotes && DiagLevel == DiagnosticsEngine::Note)
+    return;
+
   if (Info.getLocation().isValid() &&
       DiagLevel != DiagnosticsEngine::Error &&
       DiagLevel != DiagnosticsEngine::Fatal &&
       LineIsMarkedWithNOLINTinMacro(Diags->getSourceManager(), Info.getLocation())) {
     ++Context.Stats.ErrorsIgnoredNOLINT;
+    // Ignored a warning, should ignore related notes as well
+    ShouldIgnoreNotes = true;
     return;
   }
+
+  ShouldIgnoreNotes = false;
   // Count warnings/errors.
   DiagnosticConsumer::HandleDiagnostic(DiagLevel, Info);
 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D26218.76639.patch
Type: text/x-patch
Size: 2324 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20161101/d8f1e554/attachment.bin>