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

2016-11-01 Thread 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);
 


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 (ShouldIgnoreNo

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

2016-11-02 Thread Nikita Kakuev via cfe-commits 
nkakuev added a reviewer: malcolm.parsons.
nkakuev updated this revision to Diff 76718.
nkakuev added a comment.

Fixed review comments: replaced a static variable with a class member.


https://reviews.llvm.org/D26218

Files:
  clang-tidy/ClangTidyDiagnosticConsumer.cpp
  clang-tidy/ClangTidyDiagnosticConsumer.h
  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.h
===
--- clang-tidy/ClangTidyDiagnosticConsumer.h
+++ clang-tidy/ClangTidyDiagnosticConsumer.h
@@ -286,6 +286,7 @@
   std::unique_ptr HeaderFilter;
   bool LastErrorRelatesToUserCode;
   bool LastErrorPassesLineFilter;
+  bool LastErrorWasIgnored;
 };
 
 } // end namespace tidy
Index: clang-tidy/ClangTidyDiagnosticConsumer.cpp
===
--- clang-tidy/ClangTidyDiagnosticConsumer.cpp
+++ clang-tidy/ClangTidyDiagnosticConsumer.cpp
@@ -250,7 +250,7 @@
 
 ClangTidyDiagnosticConsumer::ClangTidyDiagnosticConsumer(ClangTidyContext &Ctx)
 : Context(Ctx), LastErrorRelatesToUserCode(false),
-  LastErrorPassesLineFilter(false) {
+  LastErrorPassesLineFilter(false), LastErrorWasIgnored(false) {
   IntrusiveRefCntPtr DiagOpts = new DiagnosticOptions();
   Diags.reset(new DiagnosticsEngine(
   IntrusiveRefCntPtr(new DiagnosticIDs), &*DiagOpts, this,
@@ -309,13 +309,20 @@
 
 void ClangTidyDiagnosticConsumer::HandleDiagnostic(
 DiagnosticsEngine::Level DiagLevel, const Diagnostic &Info) {
+  if (LastErrorWasIgnored && 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
+LastErrorWasIgnored = true;
 return;
   }
+
+  LastErrorWasIgnored = false;
   // Count warnings/errors.
   DiagnosticConsumer::HandleDiagnostic(DiagLevel, Info);
 


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.h
===
--- clang-tidy/ClangT

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

2016-11-02 Thread Nikita Kakuev via cfe-commits 
nkakuev updated this revision to Diff 76769.
nkakuev marked an inline comment as done.
nkakuev added a comment.

Fix a typo.


https://reviews.llvm.org/D26218

Files:
  clang-tidy/ClangTidyDiagnosticConsumer.cpp
  clang-tidy/ClangTidyDiagnosticConsumer.h
  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: trigger_warning.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.h
===
--- clang-tidy/ClangTidyDiagnosticConsumer.h
+++ clang-tidy/ClangTidyDiagnosticConsumer.h
@@ -286,6 +286,7 @@
   std::unique_ptr HeaderFilter;
   bool LastErrorRelatesToUserCode;
   bool LastErrorPassesLineFilter;
+  bool LastErrorWasIgnored;
 };
 
 } // end namespace tidy
Index: clang-tidy/ClangTidyDiagnosticConsumer.cpp
===
--- clang-tidy/ClangTidyDiagnosticConsumer.cpp
+++ clang-tidy/ClangTidyDiagnosticConsumer.cpp
@@ -250,7 +250,7 @@
 
 ClangTidyDiagnosticConsumer::ClangTidyDiagnosticConsumer(ClangTidyContext &Ctx)
 : Context(Ctx), LastErrorRelatesToUserCode(false),
-  LastErrorPassesLineFilter(false) {
+  LastErrorPassesLineFilter(false), LastErrorWasIgnored(false) {
   IntrusiveRefCntPtr DiagOpts = new DiagnosticOptions();
   Diags.reset(new DiagnosticsEngine(
   IntrusiveRefCntPtr(new DiagnosticIDs), &*DiagOpts, this,
@@ -309,13 +309,20 @@
 
 void ClangTidyDiagnosticConsumer::HandleDiagnostic(
 DiagnosticsEngine::Level DiagLevel, const Diagnostic &Info) {
+  if (LastErrorWasIgnored && 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
+LastErrorWasIgnored = true;
 return;
   }
+
+  LastErrorWasIgnored = false;
   // Count warnings/errors.
   DiagnosticConsumer::HandleDiagnostic(DiagLevel, Info);
 


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: trigger_warning.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.h
===
--- clang-tidy/ClangTidyDiagnosticConsumer.h
+++ clang-tidy/Cla

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

2016-11-02 Thread Nikita Kakuev via cfe-commits 
nkakuev added a comment.

@malcolm.parsons, can you please commit this patch? I don't have commit rights.


https://reviews.llvm.org/D26218



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

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

2016-11-08 Thread Nikita Kakuev via cfe-commits 
nkakuev added a comment.

@alexfh I'll fix it in a separate review.


https://reviews.llvm.org/D26218



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

[PATCH] D26418: Add '-suppress-checks-filter' option to suppress diagnostics from certain files

2016-11-08 Thread Nikita Kakuev via cfe-commits 
nkakuev created this revision.
nkakuev added reviewers: malcolm.parsons, alexfh.
nkakuev added a subscriber: cfe-commits.

Currently clang-tidy doesn't provide a practical way to suppress diagnostics 
from headers you don't have control over. If using a function defined in such 
header causes a false positive, there is no easy way to deal with this issue.

Since you //cannot// modify a header, you //cannot// add NOLINT (or any other 
source annotations) to it. And adding NOLINT to each invocation of such 
function is either impossible or hugely impractical if invocations are 
ubiquitous.

Using the '-header-filter' doesn't help to address this issue as well. Unless 
your own headers are strategically named, it is virtually impossible to craft a 
regex that will filter out only the third-party ones.

The '-line-filter' can be used to suppress such warnings, but it's not very 
convenient. Instead of excluding a line that produces a warning you have to 
include all other lines. Plus, it provides no way to suppress only certain 
warnings from a file.

The option I propose solves aforementioned problems. It allows a user to 
specify a set of regular expressions to suppress diagnostics from files whose 
names match these expressions. Additionally, a set of checks can be specified 
to suppress only certain diagnostics.

The option can be used in the following way:
`clang-tidy 
-suppress-checks-filter='[{"name":"falty_header.h"},{"name":"third-party/","checks":"google-explicit-constructor"}]'
 source.cpp -- -c`


https://reviews.llvm.org/D26418

Files:
  clang-tidy/ClangTidyDiagnosticConsumer.cpp
  clang-tidy/ClangTidyDiagnosticConsumer.h
  clang-tidy/ClangTidyOptions.cpp
  clang-tidy/ClangTidyOptions.h
  clang-tidy/tool/ClangTidyMain.cpp
  test/clang-tidy/Inputs/suppress-checks-filter/1/header1.h
  test/clang-tidy/Inputs/suppress-checks-filter/1/header2.h
  test/clang-tidy/Inputs/suppress-checks-filter/2/header3.h
  test/clang-tidy/Inputs/suppress-checks-filter/2/header4.h
  test/clang-tidy/suppress-checks-filter.cpp

Index: test/clang-tidy/suppress-checks-filter.cpp
===
--- test/clang-tidy/suppress-checks-filter.cpp
+++ test/clang-tidy/suppress-checks-filter.cpp
@@ -0,0 +1,17 @@
+// RUN: clang-tidy -checks='-*,google-explicit-constructor,google-readability-casting' -header-filter='header*' -suppress-checks-filter='[{"name":"header1.h"},{"name":"header2.h","checks":"google-explicit-constructor"},{"name":"2/"}]' %s -- -I %S/Inputs/suppress-checks-filter 2>&1 | FileCheck %s
+
+#include "1/header1.h"
+// CHECK-NOT: header1.h:{{.*}} warning
+
+#include "1/header2.h"
+// CHECK-NOT: 1/header2.h:{{.*}} warning: single-argument constructors {{.*}}
+// CHECK: 1/header2.h:{{.*}} warning: redundant cast to the same type {{.*}}
+
+#include "2/header3.h"
+// CHECK-NOT: 2/header3.h:{{.*}} warning: single-argument constructors
+
+#include "2/header4.h"
+// CHECK-NOT: 2/header4.h:{{.*}} warning: single-argument constructors
+
+// CHECK: Suppressed 4 warnings (4 with suppressed checks filters)
+
Index: test/clang-tidy/Inputs/suppress-checks-filter/2/header4.h
===
--- test/clang-tidy/Inputs/suppress-checks-filter/2/header4.h
+++ test/clang-tidy/Inputs/suppress-checks-filter/2/header4.h
@@ -0,0 +1,2 @@
+class A4 { A4(int); };
+
Index: test/clang-tidy/Inputs/suppress-checks-filter/2/header3.h
===
--- test/clang-tidy/Inputs/suppress-checks-filter/2/header3.h
+++ test/clang-tidy/Inputs/suppress-checks-filter/2/header3.h
@@ -0,0 +1,2 @@
+class A3 { A3(int); };
+
Index: test/clang-tidy/Inputs/suppress-checks-filter/1/header2.h
===
--- test/clang-tidy/Inputs/suppress-checks-filter/1/header2.h
+++ test/clang-tidy/Inputs/suppress-checks-filter/1/header2.h
@@ -0,0 +1,6 @@
+class A2 { A2(int); };
+
+class B2 {
+  B2(int &In, int &Out) { Out = (int)In; }
+};
+
Index: test/clang-tidy/Inputs/suppress-checks-filter/1/header1.h
===
--- test/clang-tidy/Inputs/suppress-checks-filter/1/header1.h
+++ test/clang-tidy/Inputs/suppress-checks-filter/1/header1.h
@@ -0,0 +1,2 @@
+class A1 { A1(int); };
+
Index: clang-tidy/tool/ClangTidyMain.cpp
===
--- clang-tidy/tool/ClangTidyMain.cpp
+++ clang-tidy/tool/ClangTidyMain.cpp
@@ -105,6 +105,20 @@
cl::init(""),
cl::cat(ClangTidyCategory));
 
+static cl::opt
+SuppressWarningsFromHeaders("suppress-checks-filter", cl::desc(R"(
+Suppress diagnostics from files whose names match
+provided regular expression. If a list of checks
+is specified, only diagnostics produces by these
+checks will be suppressed. The format of
+the argument is a JSON array of objec

[PATCH] D26418: [clang-tidy] Add '-suppress-checks-filter' option to suppress diagnostics from certain files

2016-11-08 Thread Nikita Kakuev via cfe-commits 
nkakuev added a comment.

In https://reviews.llvm.org/D26418#590170, @alexfh wrote:

> I also don't understand the use case for turning off only some checks for 
> third-party headers. Either you care about third-party stuff or not, why only 
> switch off certain checks?


The use case is ignoring false positives that originate from third-party 
headers. Because even if you don't care about third-party stuff, you can't 
suppress all diagnostics from it. Here's an example:

  // header.h
  void TriggerWarning(const int& In, int& Out) {
if (In > 123)
  Out = 123;
  }



  // source.cpp
  #include "header.h"
  void MaybeInitialize(int &Out) {
int In;
TriggerWarning(In, Out);
  }

The warning is caused by a third-party code, but header filter won't help you 
to suppress it since it now relates to your sources.

But let's say this warning is a false-positive. What can you do to suppress it? 
You can turn off a faulty check, but you need to turn it off for //your// code.

With '-suppress-checks-filter' you can turn it off for third-party headers 
only, without sabotaging your own sources.


https://reviews.llvm.org/D26418



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

[PATCH] D26418: [clang-tidy] Add '-suppress-checks-filter' option to suppress diagnostics from certain files

2016-11-08 Thread Nikita Kakuev via cfe-commits 
nkakuev added a comment.

In https://reviews.llvm.org/D26418#590168, @alexfh wrote:

> What's the biggest complexity with -header-filter? Lack of way to specify 
> ? Will it help to make -header-filter a 
> GlobList?


See my previous comment. Header filter isn't to help when a (false positive) 
warning, caused by a third-party header, results in diagnostics for your source 
file.


https://reviews.llvm.org/D26418



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

[PATCH] D26418: [clang-tidy] Add '-suppress-checks-filter' option to suppress diagnostics from certain files

2016-11-09 Thread Nikita Kakuev via cfe-commits 
nkakuev added a comment.

In https://reviews.llvm.org/D26418#590417, @malcolm.parsons wrote:

> In https://reviews.llvm.org/D26418#590383, @nkakuev wrote:
>
> > The warning is caused by a third-party code, but header filter won't help 
> > you to suppress it since it now relates to your sources.
>
>
> The header filter suppresses the warning location, but the note locations are 
> in the main file so they unsuppress the warning.
>
> It sounds like you want note locations to be ignored.


I want a convenient instrument to suppress unwanted diagnostics. Even if I 
change clang-tidy to ignore note locations, it would still be impractical to 
use '-header-filter' to //exclude// diagnostics from certain headers. Header 
filter was designed to //include// headers, not to //exclude// them. Trying to 
exclude "these m directories and that n files" is close to impossible using a 
single regular expression. Plus, even if I manage to do this, I'll be ignoring 
all diagnostics and not only the faulty ones.

Suppress-check-filter is specifically designed to //exclude// headers and 
doesn't require abusing regular expressions to do this. Plus, it allows a user 
to specify what diagnostics he doesn't want, instead of ignoring everything.


https://reviews.llvm.org/D26418



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

[PATCH] D26418: [clang-tidy] Add '-suppress-checks-filter' option to suppress diagnostics from certain files

2016-11-09 Thread Nikita Kakuev via cfe-commits 
nkakuev added a comment.

In https://reviews.llvm.org/D26418#590417, @malcolm.parsons wrote:

> In https://reviews.llvm.org/D26418#590383, @nkakuev wrote:
>
> > The warning is caused by a third-party code, but header filter won't help 
> > you to suppress it since it now relates to your sources.
>
>
> The header filter suppresses the warning location, but the note locations are 
> in the main file so they unsuppress the warning.
>
> It sounds like you want note locations to be ignored.


On a second thought, ignoring note locations might be a good enough solution 
for me.
How should I do this? Add a new option (e.g. '-ignore-note-locations')?


https://reviews.llvm.org/D26418



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

[PATCH] D26466: [clang-tidy] Fix NOLINT test

2016-11-09 Thread Nikita Kakuev via cfe-commits 
nkakuev created this revision.
nkakuev added a reviewer: alexfh.
nkakuev added a subscriber: cfe-commits.

Test cases I've added in review https://reviews.llvm.org/D26218 were too 
brittle and weren't working properly.
This patch fixes this.


https://reviews.llvm.org/D26466

Files:
  test/clang-tidy/nolint.cpp


Index: test/clang-tidy/nolint.cpp
===
--- test/clang-tidy/nolint.cpp
+++ test/clang-tidy/nolint.cpp
@@ -1,5 +1,12 @@
 // 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
 
+#include "trigger_warning.h"
+void I(int& Out) {
+  int In;
+  A1(In, Out);
+}
+// CHECK-MESSAGES-NOT: trigger_warning.h:{{.*}} warning
+// CHECK-MESSAGES-NOT: :[[@LINE-4]]:{{.*}} note
 
 class A { A(int i); };
 // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: single-argument constructors must 
be marked explicit
@@ -28,12 +35,4 @@
 #define DOUBLE_MACRO MACRO(H) // NOLINT
 DOUBLE_MACRO
 
-#include "trigger_warning.h"
-void I(int& Out) {
-  int In;
-  A1(In, Out);
-}
-// CHECK-NOT: trigger_warning.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/nolint.cpp
===
--- test/clang-tidy/nolint.cpp
+++ test/clang-tidy/nolint.cpp
@@ -1,5 +1,12 @@
 // 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
 
+#include "trigger_warning.h"
+void I(int& Out) {
+  int In;
+  A1(In, Out);
+}
+// CHECK-MESSAGES-NOT: trigger_warning.h:{{.*}} warning
+// CHECK-MESSAGES-NOT: :[[@LINE-4]]:{{.*}} note
 
 class A { A(int i); };
 // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: single-argument constructors must be marked explicit
@@ -28,12 +35,4 @@
 #define DOUBLE_MACRO MACRO(H) // NOLINT
 DOUBLE_MACRO
 
-#include "trigger_warning.h"
-void I(int& Out) {
-  int In;
-  A1(In, Out);
-}
-// CHECK-NOT: trigger_warning.h:{{.*}} warning: The left operand of '>' is a garbage value
-// CHECK-NOT: :[[@LINE-4]]:{{.*}} note
-
 // CHECK-MESSAGES: Suppressed 8 warnings (8 NOLINT)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

2016-11-09 Thread Nikita Kakuev via cfe-commits 
nkakuev marked an inline comment as done.
nkakuev added inline comments.



Comment at: test/clang-tidy/nolint.cpp:36
+}
+// CHECK-NOT: trigger_warning.h:{{.*}} warning: The left operand of '>' is a 
garbage value
+// CHECK-NOT: :[[@LINE-4]]:{{.*}} note

alexfh wrote:
> The test is too brittle: if the functionality breaks after the text of the 
> message changes, it will still pass.
> 
> Two ways to deal with it:
> 1. add a positive test to ensure this pattern actually triggers a warning
> 2. make the CHECK-NOT pattern broader: `// CHECK-NOT: warning:`.
Fixed in review D26466.


https://reviews.llvm.org/D26218



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

[PATCH] D26466: [clang-tidy] Fix NOLINT test

2016-11-09 Thread Nikita Kakuev via cfe-commits 
nkakuev added a comment.

@alexfh, you're welcome!
Can you please commit this patch?


https://reviews.llvm.org/D26466



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

[PATCH] D26418: [clang-tidy] Add '-suppress-checks-filter' option to suppress diagnostics from certain files

2016-11-14 Thread Nikita Kakuev via cfe-commits 
nkakuev added a comment.

Ping.

Ignoring note locations is a coarse-grained solution that will suppress both 
false and //true// positives. Suppress-checks-filter is a finer-grained 
instrument that can be targetted on particular false positives precisely.

My sympathies are with suppress-checks-filter, but if it sounds like too much, 
I can try to implement note locations ignoring instead.


https://reviews.llvm.org/D26418



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