[PATCH] D99543: [clang-tidy] Allow opt-in or out of some commonly occuring patterns in NarrowingConversionsCheck.

2021-05-12 Thread Haojian Wu via Phabricator via cfe-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rG211761332e43: [clang-tidy] Allow opt-in or out of some 
commonly occuring patterns in… (authored by Stephen, committed by hokein).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99543

Files:
  clang-tools-extra/clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.cpp
  clang-tools-extra/clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.h
  
clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-narrowing-conversions.rst
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-narrowing-conversions-equivalentbitwidth-option.cpp
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-narrowing-conversions-ignoreconversionfromtypes-option.cpp
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-narrowing-conversions-intemplates-option.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-narrowing-conversions-intemplates-option.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-narrowing-conversions-intemplates-option.cpp
@@ -0,0 +1,35 @@
+// RUN: %check_clang_tidy -check-suffix=DEFAULT %s \
+// RUN: cppcoreguidelines-narrowing-conversions %t -- \
+// RUN: -config='{CheckOptions: [ \
+// RUN: ]}'
+
+// RUN: %check_clang_tidy -check-suffix=WARN %s \
+// RUN: cppcoreguidelines-narrowing-conversions %t -- \
+// RUN: -config='{CheckOptions: [ \
+// RUN:   {key: cppcoreguidelines-narrowing-conversions.WarnWithinTemplateInstantiation, value: 1} \
+// RUN: ]}'
+
+template 
+void assign_in_template(OrigType jj) {
+  int ii;
+  ii = jj;
+  // DEFAULT: Warning disabled because WarnWithinTemplateInstantiation=0.
+  // CHECK-MESSAGES-WARN: :[[@LINE-2]]:8: warning: narrowing conversion from 'long long' to signed type 'int' is implementation-defined [cppcoreguidelines-narrowing-conversions]
+}
+
+void narrow_inside_template_not_ok() {
+  long long j = 123;
+  assign_in_template(j);
+}
+
+void assign_outside_template(long long jj) {
+  int ii;
+  ii = jj;
+  // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:8: warning: narrowing conversion from 'long long' to signed type 'int' is implementation-defined [cppcoreguidelines-narrowing-conversions]
+  // CHECK-MESSAGES-WARN: :[[@LINE-2]]:8: warning: narrowing conversion from 'long long' to signed type 'int' is implementation-defined [cppcoreguidelines-narrowing-conversions]
+}
+
+void narrow_outside_template_not_ok() {
+  long long j = 123;
+  assign_outside_template(j);
+}
Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-narrowing-conversions-ignoreconversionfromtypes-option.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-narrowing-conversions-ignoreconversionfromtypes-option.cpp
@@ -0,0 +1,76 @@
+// RUN: %check_clang_tidy -check-suffix=DEFAULT %s \
+// RUN: cppcoreguidelines-narrowing-conversions %t -- \
+// RUN: -config='{CheckOptions: [ \
+// RUN: ]}'
+
+// RUN: %check_clang_tidy -check-suffix=IGNORED %s \
+// RUN: cppcoreguidelines-narrowing-conversions %t -- \
+// RUN: -config='{CheckOptions: [ \
+// RUN:   {key: cppcoreguidelines-narrowing-conversions.IgnoreConversionFromTypes, value: "global_size_t;nested_size_type"} \
+// RUN: ]}'
+
+// We use global_size_t instead of 'size_t' because windows predefines size_t.
+typedef long long global_size_t;
+
+struct vector {
+  typedef long long nested_size_type;
+
+  global_size_t size() const { return 0; }
+};
+
+void narrowing_global_size_t() {
+  int i;
+  global_size_t j;
+  i = j;
+  // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:7: warning: narrowing conversion from 'global_size_t' (aka 'long long') to signed type 'int' is implementation-defined [cppcoreguidelines-narrowing-conversions]
+  // IGNORED: Warning is disabled with IgnoreConversionFromTypes=global_size_t.
+}
+
+void narrowing_size_type() {
+  int i;
+  vector::nested_size_type j;
+  i = j;
+  // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:7: warning: narrowing conversion from 'vector::nested_size_type' (aka 'long long') to signed type 'int' is implementation-defined [cppcoreguidelines-narrowing-conversions]
+  // IGNORED: Warning is disabled with IgnoreConversionFromTypes=nested_size_type.
+}
+
+void narrowing_size_method() {
+  vector v;
+  int i, j;
+  i = v.size();
+  // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:7: warning: narrowing conversion from 'global_size_t' (aka 'long long') to signed type 'int' is implementation-defined [cppcoreguidelines-narrowing-conversions]
+  // IGNORED: Warning is disabled with IgnoreConversionFromTypes=global_size_t.
+
+  i = j + v.size();
+  // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:7: warning: narrowing conversion from 'long long' to signed type 'int' is implementation-defined

[PATCH] D99543: [clang-tidy] Allow opt-in or out of some commonly occuring patterns in NarrowingConversionsCheck.

2021-05-12 Thread Haojian Wu via Phabricator via cfe-commits 
hokein accepted this revision.
hokein added a comment.
This revision is now accepted and ready to land.

thanks, looks good. Sorry for the delay.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99543

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

[PATCH] D99543: [clang-tidy] Allow opt-in or out of some commonly occuring patterns in NarrowingConversionsCheck.

2021-05-03 Thread Stephen Concannon via Phabricator via cfe-commits 
Stephen marked an inline comment as done.
Stephen added inline comments.



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-narrowing-conversions-ignoreconversionfromtypes-option.cpp:12
+
+typedef long long size_t;
+

njames93 wrote:
> This is breaking tests on windows, It seems like its pre-defined to be 
> `unsigned long long` on windows.
> Simplest fix is to rename size_t to something else.
> I guess changing the typedef to `unsigned long long` will change behaviour of 
> tests.
> Failing that an `// UNSUPPORTED: system-windows` directive at the top of the 
> file would also work.
Thanks, njames93. I've switched to custom type names to avoid these sorts of 
conflicts. I've also removed the difference_type test, since it didn't test 
distinct functionality from size_type, once they use custom names.

Sorry for the inconvenience.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99543

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

[PATCH] D99543: [clang-tidy] Allow opt-in or out of some commonly occuring patterns in NarrowingConversionsCheck.

2021-05-03 Thread Stephen Concannon via Phabricator via cfe-commits 
Stephen updated this revision to Diff 342472.
Stephen added a comment.

1. Updating D99543 : [clang-tidy] Allow opt-in 
or out of some commonly occuring patterns in NarrowingConversionsCheck. #
2. Enter a brief description of the changes included in this update.
3. The first line is used as subject, next lines as comment. #
4. If you intended to create a new revision, use:
5. $ arc diff --create

Within clang-tidy's NarrowingConversionsCheck.

- Allow opt-out of some common occurring patterns, such as:
  - Implicit casts between types of equivalent bit widths.
  - Implicit casts occurring from the return of a ::size() method.
  - Implicit casts on size_type and difference_type.
- Allow opt-in of errors within template instantiations.

This will help projects adopt these guidelines iteratively.
Developed in conjunction with Yitzhak Mandelbaum (ymandel).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99543

Files:
  clang-tools-extra/clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.cpp
  clang-tools-extra/clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.h
  
clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-narrowing-conversions.rst
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-narrowing-conversions-equivalentbitwidth-option.cpp
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-narrowing-conversions-ignoreconversionfromtypes-option.cpp
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-narrowing-conversions-intemplates-option.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-narrowing-conversions-intemplates-option.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-narrowing-conversions-intemplates-option.cpp
@@ -0,0 +1,35 @@
+// RUN: %check_clang_tidy -check-suffix=DEFAULT %s \
+// RUN: cppcoreguidelines-narrowing-conversions %t -- \
+// RUN: -config='{CheckOptions: [ \
+// RUN: ]}'
+
+// RUN: %check_clang_tidy -check-suffix=WARN %s \
+// RUN: cppcoreguidelines-narrowing-conversions %t -- \
+// RUN: -config='{CheckOptions: [ \
+// RUN:   {key: cppcoreguidelines-narrowing-conversions.WarnWithinTemplateInstantiation, value: 1} \
+// RUN: ]}'
+
+template 
+void assign_in_template(OrigType jj) {
+  int ii;
+  ii = jj;
+  // DEFAULT: Warning disabled because WarnWithinTemplateInstantiation=0.
+  // CHECK-MESSAGES-WARN: :[[@LINE-2]]:8: warning: narrowing conversion from 'long long' to signed type 'int' is implementation-defined [cppcoreguidelines-narrowing-conversions]
+}
+
+void narrow_inside_template_not_ok() {
+  long long j = 123;
+  assign_in_template(j);
+}
+
+void assign_outside_template(long long jj) {
+  int ii;
+  ii = jj;
+  // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:8: warning: narrowing conversion from 'long long' to signed type 'int' is implementation-defined [cppcoreguidelines-narrowing-conversions]
+  // CHECK-MESSAGES-WARN: :[[@LINE-2]]:8: warning: narrowing conversion from 'long long' to signed type 'int' is implementation-defined [cppcoreguidelines-narrowing-conversions]
+}
+
+void narrow_outside_template_not_ok() {
+  long long j = 123;
+  assign_outside_template(j);
+}
Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-narrowing-conversions-ignoreconversionfromtypes-option.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-narrowing-conversions-ignoreconversionfromtypes-option.cpp
@@ -0,0 +1,76 @@
+// RUN: %check_clang_tidy -check-suffix=DEFAULT %s \
+// RUN: cppcoreguidelines-narrowing-conversions %t -- \
+// RUN: -config='{CheckOptions: [ \
+// RUN: ]}'
+
+// RUN: %check_clang_tidy -check-suffix=IGNORED %s \
+// RUN: cppcoreguidelines-narrowing-conversions %t -- \
+// RUN: -config='{CheckOptions: [ \
+// RUN:   {key: cppcoreguidelines-narrowing-conversions.IgnoreConversionFromTypes, value: "global_size_t;nested_size_type"} \
+// RUN: ]}'
+
+// We use global_size_t instead of 'size_t' because windows predefines size_t.
+typedef long long global_size_t;
+
+struct vector {
+  typedef long long nested_size_type;
+
+  global_size_t size() const { return 0; }
+};
+
+void narrowing_global_size_t() {
+  int i;
+  global_size_t j;
+  i = j;
+  // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:7: warning: narrowing conversion from 'global_size_t' (aka 'long long') to signed type 'int' is implementation-defined [cppcoreguidelines-narrowing-conversions]
+  // IGNORED: Warning is disabled with IgnoreConversionFromTypes=global_size_t.
+}
+
+void narrowing_size_type() {
+  int i;
+  vector::nested_size_type j;
+  i = j;
+  // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:7: warning: narrowing conversion from 'vector::nested_size_type' (aka 'long long') to signed type 'int' is implementation-defined

[PATCH] D99543: [clang-tidy] Allow opt-in or out of some commonly occuring patterns in NarrowingConversionsCheck.

2021-05-03 Thread Nathan James via Phabricator via cfe-commits 
njames93 added inline comments.



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-narrowing-conversions-ignoreconversionfromtypes-option.cpp:12
+
+typedef long long size_t;
+

This is breaking tests on windows, It seems like its pre-defined to be 
`unsigned long long` on windows.
Simplest fix is to rename size_t to something else.
I guess changing the typedef to `unsigned long long` will change behaviour of 
tests.
Failing that an `// UNSUPPORTED: system-windows` directive at the top of the 
file would also work.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99543

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

[PATCH] D99543: [clang-tidy] Allow opt-in or out of some commonly occuring patterns in NarrowingConversionsCheck.

2021-05-03 Thread Stephen Concannon via Phabricator via cfe-commits 
Stephen requested review of this revision.
Stephen added a comment.

Thanks, ymandel!

Anything else, aaron.ballman or hokein?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99543

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

[PATCH] D99543: [clang-tidy] Allow opt-in or out of some commonly occuring patterns in NarrowingConversionsCheck.

2021-05-03 Thread Stephen Concannon via Phabricator via cfe-commits 
Stephen updated this revision to Diff 342409.
Stephen marked 2 inline comments as done.
Stephen added a comment.

Within clang-tidy's NarrowingConversionsCheck.

- Allow opt-out of some common occurring patterns, such as:
  - Implicit casts between types of equivalent bit widths.
  - Implicit casts occurring from the return of a ::size() method.
  - Implicit casts on size_type and difference_type.
- Allow opt-in of errors within template instantiations.

This will help projects adopt these guidelines iteratively.
Developed in conjunction with Yitzhak Mandelbaum (ymandel).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99543

Files:
  clang-tools-extra/clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.cpp
  clang-tools-extra/clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.h
  
clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-narrowing-conversions.rst
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-narrowing-conversions-equivalentbitwidth-option.cpp
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-narrowing-conversions-ignoreconversionfromtypes-option.cpp
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-narrowing-conversions-intemplates-option.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-narrowing-conversions-intemplates-option.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-narrowing-conversions-intemplates-option.cpp
@@ -0,0 +1,35 @@
+// RUN: %check_clang_tidy -check-suffix=DEFAULT %s \
+// RUN: cppcoreguidelines-narrowing-conversions %t -- \
+// RUN: -config='{CheckOptions: [ \
+// RUN: ]}'
+
+// RUN: %check_clang_tidy -check-suffix=WARN %s \
+// RUN: cppcoreguidelines-narrowing-conversions %t -- \
+// RUN: -config='{CheckOptions: [ \
+// RUN:   {key: cppcoreguidelines-narrowing-conversions.WarnWithinTemplateInstantiation, value: 1} \
+// RUN: ]}'
+
+template 
+void assign_in_template(OrigType jj) {
+  int ii;
+  ii = jj;
+  // DEFAULT: Warning disabled because WarnWithinTemplateInstantiation=0.
+  // CHECK-MESSAGES-WARN: :[[@LINE-2]]:8: warning: narrowing conversion from 'long long' to signed type 'int' is implementation-defined [cppcoreguidelines-narrowing-conversions]
+}
+
+void narrow_inside_template_not_ok() {
+  long long j = 123;
+  assign_in_template(j);
+}
+
+void assign_outside_template(long long jj) {
+  int ii;
+  ii = jj;
+  // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:8: warning: narrowing conversion from 'long long' to signed type 'int' is implementation-defined [cppcoreguidelines-narrowing-conversions]
+  // CHECK-MESSAGES-WARN: :[[@LINE-2]]:8: warning: narrowing conversion from 'long long' to signed type 'int' is implementation-defined [cppcoreguidelines-narrowing-conversions]
+}
+
+void narrow_outside_template_not_ok() {
+  long long j = 123;
+  assign_outside_template(j);
+}
Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-narrowing-conversions-ignoreconversionfromtypes-option.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-narrowing-conversions-ignoreconversionfromtypes-option.cpp
@@ -0,0 +1,84 @@
+// RUN: %check_clang_tidy -check-suffix=DEFAULT %s \
+// RUN: cppcoreguidelines-narrowing-conversions %t -- \
+// RUN: -config='{CheckOptions: [ \
+// RUN: ]}'
+
+// RUN: %check_clang_tidy -check-suffix=IGNORED %s \
+// RUN: cppcoreguidelines-narrowing-conversions %t -- \
+// RUN: -config='{CheckOptions: [ \
+// RUN:   {key: cppcoreguidelines-narrowing-conversions.IgnoreConversionFromTypes, value: "size_t;ptrdiff_t;size_type;difference_type"} \
+// RUN: ]}'
+
+typedef long long size_t;
+
+struct vector {
+  typedef long long size_type;
+  typedef long long difference_type;
+
+  size_t size() const { return 0; }
+};
+
+void narrowing_size_t() {
+  int i;
+  size_t j;
+  i = j;
+  // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:7: warning: narrowing conversion from 'size_t' (aka 'long long') to signed type 'int' is implementation-defined [cppcoreguidelines-narrowing-conversions]
+  // IGNORED: Warning is disabled with IgnoreConversionFromTypes=size_t.
+}
+
+void narrowing_size_type() {
+  int i;
+  vector::size_type j;
+  i = j;
+  // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:7: warning: narrowing conversion from 'vector::size_type' (aka 'long long') to signed type 'int' is implementation-defined [cppcoreguidelines-narrowing-conversions]
+  // IGNORED: Warning is disabled with IgnoreConversionFromTypes=size_type.
+}
+
+void narrowing_difference_type() {
+  int i;
+  vector::difference_type j;
+  i = j;
+  // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:7: warning: narrowing conversion from 'vector::difference_type' (aka 'long long') to signed type 'int' is implementation-defined

[PATCH] D99543: [clang-tidy] Allow opt-in or out of some commonly occuring patterns in NarrowingConversionsCheck.

2021-04-29 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits 
ymandel accepted this revision.
ymandel added inline comments.
This revision is now accepted and ready to land.



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-narrowing-conversions.rst:51
+
+   Narrowing conversions from any type in this semicolon separated list will be
+   ignored. This may be useful to weed out commonly occurring, but less 
commonly

nit: add dash "semicolon-separated"


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99543

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

[PATCH] D99543: [clang-tidy] Allow opt-in or out of some commonly occuring patterns in NarrowingConversionsCheck.

2021-04-16 Thread Stephen Concannon via Phabricator via cfe-commits 
Stephen updated this revision to Diff 338273.
Stephen added a comment.

(fix patch to include all local changes)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99543

Files:
  clang-tools-extra/clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.cpp
  clang-tools-extra/clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.h
  
clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-narrowing-conversions.rst
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-narrowing-conversions-equivalentbitwidth-option.cpp
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-narrowing-conversions-ignoreconversionfromtypes-option.cpp
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-narrowing-conversions-intemplates-option.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-narrowing-conversions-intemplates-option.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-narrowing-conversions-intemplates-option.cpp
@@ -0,0 +1,35 @@
+// RUN: %check_clang_tidy -check-suffix=DEFAULT %s \
+// RUN: cppcoreguidelines-narrowing-conversions %t -- \
+// RUN: -config='{CheckOptions: [ \
+// RUN: ]}'
+
+// RUN: %check_clang_tidy -check-suffix=WARN %s \
+// RUN: cppcoreguidelines-narrowing-conversions %t -- \
+// RUN: -config='{CheckOptions: [ \
+// RUN:   {key: cppcoreguidelines-narrowing-conversions.WarnWithinTemplateInstantiation, value: 1} \
+// RUN: ]}'
+
+template 
+void assign_in_template(OrigType jj) {
+  int ii;
+  ii = jj;
+  // DEFAULT: Warning disabled because WarnWithinTemplateInstantiation=0.
+  // CHECK-MESSAGES-WARN: :[[@LINE-2]]:8: warning: narrowing conversion from 'long long' to signed type 'int' is implementation-defined [cppcoreguidelines-narrowing-conversions]
+}
+
+void narrow_inside_template_not_ok() {
+  long long j = 123;
+  assign_in_template(j);
+}
+
+void assign_outside_template(long long jj) {
+  int ii;
+  ii = jj;
+  // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:8: warning: narrowing conversion from 'long long' to signed type 'int' is implementation-defined [cppcoreguidelines-narrowing-conversions]
+  // CHECK-MESSAGES-WARN: :[[@LINE-2]]:8: warning: narrowing conversion from 'long long' to signed type 'int' is implementation-defined [cppcoreguidelines-narrowing-conversions]
+}
+
+void narrow_outside_template_not_ok() {
+  long long j = 123;
+  assign_outside_template(j);
+}
Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-narrowing-conversions-ignoreconversionfromtypes-option.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-narrowing-conversions-ignoreconversionfromtypes-option.cpp
@@ -0,0 +1,84 @@
+// RUN: %check_clang_tidy -check-suffix=DEFAULT %s \
+// RUN: cppcoreguidelines-narrowing-conversions %t -- \
+// RUN: -config='{CheckOptions: [ \
+// RUN: ]}'
+
+// RUN: %check_clang_tidy -check-suffix=IGNORED %s \
+// RUN: cppcoreguidelines-narrowing-conversions %t -- \
+// RUN: -config='{CheckOptions: [ \
+// RUN:   {key: cppcoreguidelines-narrowing-conversions.IgnoreConversionFromTypes, value: "size_t;ptrdiff_t;size_type;difference_type"} \
+// RUN: ]}'
+
+typedef long long size_t;
+
+struct vector {
+  typedef long long size_type;
+  typedef long long difference_type;
+
+  size_t size() const { return 0; }
+};
+
+void narrowing_size_t() {
+  int i;
+  size_t j;
+  i = j;
+  // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:7: warning: narrowing conversion from 'size_t' (aka 'long long') to signed type 'int' is implementation-defined [cppcoreguidelines-narrowing-conversions]
+  // IGNORED: Warning is disabled with IgnoreConversionFromTypes=size_t.
+}
+
+void narrowing_size_type() {
+  int i;
+  vector::size_type j;
+  i = j;
+  // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:7: warning: narrowing conversion from 'vector::size_type' (aka 'long long') to signed type 'int' is implementation-defined [cppcoreguidelines-narrowing-conversions]
+  // IGNORED: Warning is disabled with IgnoreConversionFromTypes=size_type.
+}
+
+void narrowing_difference_type() {
+  int i;
+  vector::difference_type j;
+  i = j;
+  // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:7: warning: narrowing conversion from 'vector::difference_type' (aka 'long long') to signed type 'int' is implementation-defined [cppcoreguidelines-narrowing-conversions]
+  // IGNORED: Warning is disabled with IgnoreConversionFromTypes=difference_type.
+}
+
+void narrowing_size_method() {
+  vector v;
+  int i, j;
+  i = v.size();
+  // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:7: warning: narrowing conversion from 'size_t' (aka 'long long') to signed type 'int' is implementation-defined [cppcoreguidelines-narrowing-conversions]
+  // IGNORED: Warning is disabled with IgnoreConversionFromTypes=size_t.
+
+  i = j + v.size();
+  //

[PATCH] D99543: [clang-tidy] Allow opt-in or out of some commonly occuring patterns in NarrowingConversionsCheck.

2021-04-16 Thread Stephen Concannon via Phabricator via cfe-commits 
Stephen requested review of this revision.
Stephen added a comment.

Thanks for the review! I think I've addressed the comments -- hopefully I kept 
it inline with what you're thinking. Please have another look at your leisure.




Comment at: 
clang-tools-extra/clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.cpp:82
+  const auto IsSizeTypeOrDifferenceType = hasType(
+  namedDecl(anyOf(hasName("size_type"), hasName("difference_type";
+

aaron.ballman wrote:
> Should this also support `size_t` and `ptrdiff_t` given that those are 
> usually the same as `size_type` and `difference_type`?
Thanks, yeah, that's a good idea. I was somewhat apprehensive to ignore all 
conversions involving size_t, but it's probably ok. Now that we are, we don't 
need the ::size() method stuff, so I removed that.

And it seemed like I might as well make the list of types configurable, so I 
rolled that in here too. So it's a bigger change that you maybe had intended, 
though hopefully not too far off the mark.



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-narrowing-conversions-equivalentbitwidth-off.cpp:1
+// RUN: %check_clang_tidy %s cppcoreguidelines-narrowing-conversions %t \
+// RUN: -config="{CheckOptions: [ \

hokein wrote:
> instead of having multiple individual test files, I think we can group them 
> into a single file, you can use the flag `-check-suffix=`, like
> 
> ```
> // RUN: %check_clang_tidy -check-suffix=TESTCASE1 ...
> // RUN: %check_clang_tidy -check-suffix=TESTCASE2 ...
> 
> 
> // some warning code for test-case1
> CHECK-MESSAGES-TESTCASE1: 
> ``` 
Thanks! Besides cutting down the number of files, it really helped clarify the 
distinction in behavior with the warning on/off.

I coalesced the on/off pairs into a single, but kept the options tested in 
separate files, as that seemed to be the pattern in place.

Let me know if you'd prefer to squeeze them into fewer.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99543

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

[PATCH] D99543: [clang-tidy] Allow opt-in or out of some commonly occuring patterns in NarrowingConversionsCheck.

2021-04-16 Thread Stephen Concannon via Phabricator via cfe-commits 
Stephen updated this revision to Diff 338272.
Stephen added a comment.

- Add ability to pass semi-colon separated list of types to ignore, instead of 
assuming only size_type and difference_type.
- Remove ability to avoid warnings on ::size() method, since it's subsumed by 
allowing narrowing conversions of size_t.
- Coalesce on/off option tests into per-option test files.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99543

Files:
  clang-tools-extra/clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.cpp
  clang-tools-extra/clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.h
  
clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-narrowing-conversions.rst
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-narrowing-conversions-equivalentbitwidth-off.cpp
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-narrowing-conversions-equivalentbitwidth-on.cpp
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-narrowing-conversions-equivalentbitwidth-option.cpp
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-narrowing-conversions-ignoreconversionfromtypes-option.cpp
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-narrowing-conversions-intemplates-off.cpp
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-narrowing-conversions-intemplates-on.cpp
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-narrowing-conversions-intemplates-option.cpp
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-narrowing-conversions-sizemethod-off.cpp
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-narrowing-conversions-sizemethod-on.cpp
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-narrowing-conversions-sizetypes-off.cpp
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-narrowing-conversions-sizetypes-on.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-narrowing-conversions-sizetypes-on.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-narrowing-conversions-sizetypes-on.cpp
+++ /dev/null
@@ -1,24 +0,0 @@
-// RUN: %check_clang_tidy %s cppcoreguidelines-narrowing-conversions %t \
-// RUN: -config="{CheckOptions: [ \
-// RUN:   {key: "cppcoreguidelines-narrowing-conversions.WarnOnSizeTypeAndDifferenceType", value: 1} \
-// RUN: ]}" \
-// RUN: -- -target x86_64-unknown-linux -fsigned-char
-
-struct vector {
-  typedef long long size_type;
-  typedef long long difference_type;
-};
-
-void narrowing_size_type_is_not_ok() {
-  int i;
-  vector::size_type j;
-  i = j;
-  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: narrowing conversion from 'vector::size_type' (aka 'long long') to signed type 'int' is implementation-defined [cppcoreguidelines-narrowing-conversions]
-}
-
-void narrowing_difference_type_is_not_ok() {
-  int i;
-  vector::difference_type j;
-  i = j;
-  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: narrowing conversion from 'vector::difference_type' (aka 'long long') to signed type 'int' is implementation-defined [cppcoreguidelines-narrowing-conversions]
-}
Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-narrowing-conversions-sizetypes-off.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-narrowing-conversions-sizetypes-off.cpp
+++ /dev/null
@@ -1,27 +0,0 @@
-// RUN: %check_clang_tidy %s cppcoreguidelines-narrowing-conversions %t \
-// RUN: -config="{CheckOptions: [ \
-// RUN:   {key: "cppcoreguidelines-narrowing-conversions.WarnOnSizeTypeAndDifferenceType", value: 0} \
-// RUN: ]}" \
-// RUN: -- -target x86_64-unknown-linux -fsigned-char
-struct vector {
-  typedef long long size_type;
-  typedef long long difference_type;
-};
-void narrowing_size_type_is_ok() {
-  int i;
-  vector::size_type j;
-  i = j;
-  // Warning disabled with WarnOnSizeTypeAndDifferenceType=0.
-}
-void narrowing_difference_type_is_ok() {
-  int i;
-  vector::difference_type j;
-  i = j;
-  // Warning disabled with WarnOnSizeTypeAndDifferenceType=0.
-}
-void most_narrowing_is_not_ok() {
-  int i;
-  long long j;
-  i = j;
-  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: narrowing conversion from 'long long' to signed type 'int' is implementation-defined [cppcoreguidelines-narrowing-conversions]
-}
Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-narrowing-conversions-sizemethod-on.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-narrowing-conversions-sizemethod-on.cpp
+++ /dev/null
@@ -1,32 +0,0 @@
-
-// RUN: %check_clang_tidy %s cppcoreguidelines-narrowing-conversions %t \
-// RUN: -config="{CheckOptions: [ \
-// RUN:   {key:

[PATCH] D99543: [clang-tidy] Allow opt-in or out of some commonly occuring patterns in NarrowingConversionsCheck.

2021-04-14 Thread Haojian Wu via Phabricator via cfe-commits 
hokein added a comment.

a small suggestion to simplify the tests.




Comment at: 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-narrowing-conversions-equivalentbitwidth-off.cpp:1
+// RUN: %check_clang_tidy %s cppcoreguidelines-narrowing-conversions %t \
+// RUN: -config="{CheckOptions: [ \

instead of having multiple individual test files, I think we can group them 
into a single file, you can use the flag `-check-suffix=`, like

```
// RUN: %check_clang_tidy -check-suffix=TESTCASE1 ...
// RUN: %check_clang_tidy -check-suffix=TESTCASE2 ...


// some warning code for test-case1
CHECK-MESSAGES-TESTCASE1: 
``` 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99543

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

[PATCH] D99543: [clang-tidy] Allow opt-in or out of some commonly occuring patterns in NarrowingConversionsCheck.

2021-04-13 Thread Aaron Ballman via Phabricator via cfe-commits 
aaron.ballman added a comment.

Thanks! I think the changes generally LG, but I did have a question that might 
generate a small change.




Comment at: 
clang-tools-extra/clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.cpp:82
+  const auto IsSizeTypeOrDifferenceType = hasType(
+  namedDecl(anyOf(hasName("size_type"), hasName("difference_type";
+

Should this also support `size_t` and `ptrdiff_t` given that those are usually 
the same as `size_type` and `difference_type`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99543

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

[PATCH] D99543: [clang-tidy] Allow opt-in or out of some commonly occuring patterns in NarrowingConversionsCheck.

2021-04-13 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits 
ymandel accepted this revision.
ymandel added a comment.
This revision is now accepted and ready to land.

But please wait for additional "Accept" from either `hokein` or 
`aaron.ballman`. Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99543

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