[PATCH] D103894: NarrowingConversionsCheck should support inhibiting conversions of mixed integer and floating point types with WarnOnEquivalentBitWidth=0.

2021-06-09 Thread Stephen Concannon via Phabricator via cfe-commits
Stephen updated this revision to Diff 351030.
Stephen added a comment.

Try again.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103894

Files:
  clang-tools-extra/clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.cpp
  clang-tools-extra/clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.h
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-narrowing-conversions-equivalentbitwidth-option.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-narrowing-conversions-equivalentbitwidth-option.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-narrowing-conversions-equivalentbitwidth-option.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-narrowing-conversions-equivalentbitwidth-option.cpp
@@ -11,16 +11,35 @@
 
 void narrowing_equivalent_bitwidth() {
   int i;
-  unsigned int j;
-  i = j;
+  unsigned int ui;
+  i = ui;
   // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:7: warning: narrowing conversion from 'unsigned int' to signed type 'int' is implementation-defined [cppcoreguidelines-narrowing-conversions]
   // DISABLED: Warning disabled with WarnOnEquivalentBitWidth=0.
+
+  float f;
+  i = f;
+  // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:7: warning: narrowing conversion from 'float' to 'int' [cppcoreguidelines-narrowing-conversions]
+  // DISABLED: Warning disabled with WarnOnEquivalentBitWidth=0.
+
+  f = i;
+  // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:7: warning: narrowing conversion from 'int' to 'float' [cppcoreguidelines-narrowing-conversions]
+  // DISABLED: Warning disabled with WarnOnEquivalentBitWidth=0.
+
+  long long ll;
+  double d;
+  ll = d;
+  // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:8: warning: narrowing conversion from 'double' to 'long long' [cppcoreguidelines-narrowing-conversions]
+  // DISABLED: Warning disabled with WarnOnEquivalentBitWidth=0.
+
+  d = ll;
+  // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:7: warning: narrowing conversion from 'long long' to 'double' [cppcoreguidelines-narrowing-conversions]
+  // DISABLED: Warning disabled with WarnOnEquivalentBitWidth=0.
 }
 
 void most_narrowing_is_not_ok() {
   int i;
-  long long j;
-  i = j;
+  long long ui;
+  i = ui;
   // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:7: warning: narrowing conversion from 'long long' to signed type 'int' is implementation-defined [cppcoreguidelines-narrowing-conversions]
   // CHECK-MESSAGES-DISABLED: :[[@LINE-2]]:7: warning: narrowing conversion from 'long long' to signed type 'int' is implementation-defined [cppcoreguidelines-narrowing-conversions]
 }
Index: clang-tools-extra/clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.h
===
--- clang-tools-extra/clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.h
+++ clang-tools-extra/clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.h
@@ -93,6 +93,10 @@
   void handleBinaryOperator(const ASTContext ,
 const BinaryOperator );
 
+  bool isWarningInhibitedByEquivalentSize(const ASTContext ,
+  const BuiltinType ,
+  const BuiltinType ) const;
+
   const bool WarnOnFloatingPointNarrowingConversion;
   const bool WarnWithinTemplateInstantiation;
   const bool WarnOnEquivalentBitWidth;
Index: clang-tools-extra/clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.cpp
===
--- clang-tools-extra/clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.cpp
+++ clang-tools-extra/clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.cpp
@@ -67,8 +67,9 @@
   hasType(namedDecl(hasAnyListedName(IgnoreConversionFromTypes)));
 
   // `IsConversionFromIgnoredType` will ignore narrowing calls from those types,
-  // but not expressions that are promoted to `int64` due to a binary expression
-  // with one of those types. For example, it will continue to reject:
+  // but not expressions that are promoted to an ignored type as a result of a
+  // binary expression with one of those types.
+  // For example, it will continue to reject:
   // `int narrowed = int_value + container.size()`.
   // We attempt to address common incidents of compound expressions with
   // `IsIgnoredTypeTwoLevelsDeep`, allowing binary expressions that have one
@@ -217,6 +218,22 @@
   return ToIntegerRange.contains(IntegerConstant);
 }
 
+// Returns true iff the floating point constant can be losslessly represented
+// by an integer in the given destination type. eg. 2.0 can be accurately
+// represented by an int32_t, but neither 2^33 nor 2.001 can.
+static bool isFloatExactlyRepresentable(const ASTContext ,
+const llvm::APFloat ,
+const QualType ) {
+  unsigned 

[PATCH] D103894: NarrowingConversionsCheck should support inhibiting conversions of mixed integer and floating point types with WarnOnEquivalentBitWidth=0.

2021-06-09 Thread Stephen Concannon via Phabricator via cfe-commits
Stephen updated this revision to Diff 351029.
Stephen added a comment.

Try specifying the commit range.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103894

Files:
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-narrowing-conversions-equivalentbitwidth-option.cpp


Index: 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-narrowing-conversions-equivalentbitwidth-option.cpp
===
--- 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-narrowing-conversions-equivalentbitwidth-option.cpp
+++ 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-narrowing-conversions-equivalentbitwidth-option.cpp
@@ -18,19 +18,21 @@
 
   float f;
   i = f;
-  // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:7: warning: narrowing conversion 
from 'float' to signed type 'int' is implementation-defined 
[cppcoreguidelines-narrowing-conversions]
+  // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:7: warning: narrowing conversion 
from 'float' to 'int' [cppcoreguidelines-narrowing-conversions]
   // DISABLED: Warning disabled with WarnOnEquivalentBitWidth=0.
+
   f = i;
-  // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:7: warning: narrowing conversion 
from 'int' to signed type 'float' is implementation-defined 
[cppcoreguidelines-narrowing-conversions]
+  // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:7: warning: narrowing conversion 
from 'int' to 'float' [cppcoreguidelines-narrowing-conversions]
   // DISABLED: Warning disabled with WarnOnEquivalentBitWidth=0.
 
   long long ll;
   double d;
   ll = d;
-  // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:8: warning: narrowing conversion 
from 'double' to signed type 'long long' is implementation-defined 
[cppcoreguidelines-narrowing-conversions]
+  // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:8: warning: narrowing conversion 
from 'double' to 'long long' [cppcoreguidelines-narrowing-conversions]
   // DISABLED: Warning disabled with WarnOnEquivalentBitWidth=0.
+
   d = ll;
-  // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:7: warning: narrowing conversion 
from 'long long' to signed type 'double' is implementation-defined 
[cppcoreguidelines-narrowing-conversions]
+  // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:7: warning: narrowing conversion 
from 'long long' to 'double' [cppcoreguidelines-narrowing-conversions]
   // DISABLED: Warning disabled with WarnOnEquivalentBitWidth=0.
 }
 


Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-narrowing-conversions-equivalentbitwidth-option.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-narrowing-conversions-equivalentbitwidth-option.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-narrowing-conversions-equivalentbitwidth-option.cpp
@@ -18,19 +18,21 @@
 
   float f;
   i = f;
-  // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:7: warning: narrowing conversion from 'float' to signed type 'int' is implementation-defined [cppcoreguidelines-narrowing-conversions]
+  // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:7: warning: narrowing conversion from 'float' to 'int' [cppcoreguidelines-narrowing-conversions]
   // DISABLED: Warning disabled with WarnOnEquivalentBitWidth=0.
+
   f = i;
-  // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:7: warning: narrowing conversion from 'int' to signed type 'float' is implementation-defined [cppcoreguidelines-narrowing-conversions]
+  // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:7: warning: narrowing conversion from 'int' to 'float' [cppcoreguidelines-narrowing-conversions]
   // DISABLED: Warning disabled with WarnOnEquivalentBitWidth=0.
 
   long long ll;
   double d;
   ll = d;
-  // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:8: warning: narrowing conversion from 'double' to signed type 'long long' is implementation-defined [cppcoreguidelines-narrowing-conversions]
+  // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:8: warning: narrowing conversion from 'double' to 'long long' [cppcoreguidelines-narrowing-conversions]
   // DISABLED: Warning disabled with WarnOnEquivalentBitWidth=0.
+
   d = ll;
-  // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:7: warning: narrowing conversion from 'long long' to signed type 'double' is implementation-defined [cppcoreguidelines-narrowing-conversions]
+  // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:7: warning: narrowing conversion from 'long long' to 'double' [cppcoreguidelines-narrowing-conversions]
   // DISABLED: Warning disabled with WarnOnEquivalentBitWidth=0.
 }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D103894: NarrowingConversionsCheck should support inhibiting conversions of mixed integer and floating point types with WarnOnEquivalentBitWidth=0.

2021-06-09 Thread Stephen Concannon via Phabricator via cfe-commits
Stephen updated this revision to Diff 351028.
Stephen added a comment.

Undo that last move, since that didn't work.
I'm absolutely incompetent with this tool.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103894

Files:
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-narrowing-conversions-equivalentbitwidth-option.cpp


Index: 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-narrowing-conversions-equivalentbitwidth-option.cpp
===
--- 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-narrowing-conversions-equivalentbitwidth-option.cpp
+++ 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-narrowing-conversions-equivalentbitwidth-option.cpp
@@ -18,19 +18,21 @@
 
   float f;
   i = f;
-  // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:7: warning: narrowing conversion 
from 'float' to signed type 'int' is implementation-defined 
[cppcoreguidelines-narrowing-conversions]
+  // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:7: warning: narrowing conversion 
from 'float' to 'int' [cppcoreguidelines-narrowing-conversions]
   // DISABLED: Warning disabled with WarnOnEquivalentBitWidth=0.
+
   f = i;
-  // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:7: warning: narrowing conversion 
from 'int' to signed type 'float' is implementation-defined 
[cppcoreguidelines-narrowing-conversions]
+  // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:7: warning: narrowing conversion 
from 'int' to 'float' [cppcoreguidelines-narrowing-conversions]
   // DISABLED: Warning disabled with WarnOnEquivalentBitWidth=0.
 
   long long ll;
   double d;
   ll = d;
-  // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:8: warning: narrowing conversion 
from 'double' to signed type 'long long' is implementation-defined 
[cppcoreguidelines-narrowing-conversions]
+  // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:8: warning: narrowing conversion 
from 'double' to 'long long' [cppcoreguidelines-narrowing-conversions]
   // DISABLED: Warning disabled with WarnOnEquivalentBitWidth=0.
+
   d = ll;
-  // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:7: warning: narrowing conversion 
from 'long long' to signed type 'double' is implementation-defined 
[cppcoreguidelines-narrowing-conversions]
+  // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:7: warning: narrowing conversion 
from 'long long' to 'double' [cppcoreguidelines-narrowing-conversions]
   // DISABLED: Warning disabled with WarnOnEquivalentBitWidth=0.
 }
 


Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-narrowing-conversions-equivalentbitwidth-option.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-narrowing-conversions-equivalentbitwidth-option.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-narrowing-conversions-equivalentbitwidth-option.cpp
@@ -18,19 +18,21 @@
 
   float f;
   i = f;
-  // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:7: warning: narrowing conversion from 'float' to signed type 'int' is implementation-defined [cppcoreguidelines-narrowing-conversions]
+  // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:7: warning: narrowing conversion from 'float' to 'int' [cppcoreguidelines-narrowing-conversions]
   // DISABLED: Warning disabled with WarnOnEquivalentBitWidth=0.
+
   f = i;
-  // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:7: warning: narrowing conversion from 'int' to signed type 'float' is implementation-defined [cppcoreguidelines-narrowing-conversions]
+  // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:7: warning: narrowing conversion from 'int' to 'float' [cppcoreguidelines-narrowing-conversions]
   // DISABLED: Warning disabled with WarnOnEquivalentBitWidth=0.
 
   long long ll;
   double d;
   ll = d;
-  // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:8: warning: narrowing conversion from 'double' to signed type 'long long' is implementation-defined [cppcoreguidelines-narrowing-conversions]
+  // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:8: warning: narrowing conversion from 'double' to 'long long' [cppcoreguidelines-narrowing-conversions]
   // DISABLED: Warning disabled with WarnOnEquivalentBitWidth=0.
+
   d = ll;
-  // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:7: warning: narrowing conversion from 'long long' to signed type 'double' is implementation-defined [cppcoreguidelines-narrowing-conversions]
+  // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:7: warning: narrowing conversion from 'long long' to 'double' [cppcoreguidelines-narrowing-conversions]
   // DISABLED: Warning disabled with WarnOnEquivalentBitWidth=0.
 }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D103894: NarrowingConversionsCheck should support inhibiting conversions of mixed integer and floating point types with WarnOnEquivalentBitWidth=0.

2021-06-09 Thread Stephen Concannon via Phabricator via cfe-commits
Stephen updated this revision to Diff 351024.
Stephen added a comment.

Fix the diff.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103894

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 [cppcoreguidelines-narrowing-conversions]
+  // IGNORED: Warning is disabled with IgnoreConversionFromTypes=global_size_t.
+}
+
+void 

[PATCH] D103894: NarrowingConversionsCheck should support inhibiting conversions of mixed integer and floating point types with WarnOnEquivalentBitWidth=0.

2021-06-09 Thread Stephen Concannon via Phabricator via cfe-commits
Stephen updated this revision to Diff 350950.
Stephen added a comment.

Fix the expected error messages.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103894

Files:
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-narrowing-conversions-equivalentbitwidth-option.cpp


Index: 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-narrowing-conversions-equivalentbitwidth-option.cpp
===
--- 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-narrowing-conversions-equivalentbitwidth-option.cpp
+++ 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-narrowing-conversions-equivalentbitwidth-option.cpp
@@ -18,19 +18,21 @@
 
   float f;
   i = f;
-  // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:7: warning: narrowing conversion 
from 'float' to signed type 'int' is implementation-defined 
[cppcoreguidelines-narrowing-conversions]
+  // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:7: warning: narrowing conversion 
from 'float' to 'int' [cppcoreguidelines-narrowing-conversions]
   // DISABLED: Warning disabled with WarnOnEquivalentBitWidth=0.
+
   f = i;
-  // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:7: warning: narrowing conversion 
from 'int' to signed type 'float' is implementation-defined 
[cppcoreguidelines-narrowing-conversions]
+  // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:7: warning: narrowing conversion 
from 'int' to 'float' [cppcoreguidelines-narrowing-conversions]
   // DISABLED: Warning disabled with WarnOnEquivalentBitWidth=0.
 
   long long ll;
   double d;
   ll = d;
-  // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:8: warning: narrowing conversion 
from 'double' to signed type 'long long' is implementation-defined 
[cppcoreguidelines-narrowing-conversions]
+  // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:8: warning: narrowing conversion 
from 'double' to 'long long' [cppcoreguidelines-narrowing-conversions]
   // DISABLED: Warning disabled with WarnOnEquivalentBitWidth=0.
+
   d = ll;
-  // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:7: warning: narrowing conversion 
from 'long long' to signed type 'double' is implementation-defined 
[cppcoreguidelines-narrowing-conversions]
+  // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:7: warning: narrowing conversion 
from 'long long' to 'double' [cppcoreguidelines-narrowing-conversions]
   // DISABLED: Warning disabled with WarnOnEquivalentBitWidth=0.
 }
 


Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-narrowing-conversions-equivalentbitwidth-option.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-narrowing-conversions-equivalentbitwidth-option.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-narrowing-conversions-equivalentbitwidth-option.cpp
@@ -18,19 +18,21 @@
 
   float f;
   i = f;
-  // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:7: warning: narrowing conversion from 'float' to signed type 'int' is implementation-defined [cppcoreguidelines-narrowing-conversions]
+  // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:7: warning: narrowing conversion from 'float' to 'int' [cppcoreguidelines-narrowing-conversions]
   // DISABLED: Warning disabled with WarnOnEquivalentBitWidth=0.
+
   f = i;
-  // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:7: warning: narrowing conversion from 'int' to signed type 'float' is implementation-defined [cppcoreguidelines-narrowing-conversions]
+  // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:7: warning: narrowing conversion from 'int' to 'float' [cppcoreguidelines-narrowing-conversions]
   // DISABLED: Warning disabled with WarnOnEquivalentBitWidth=0.
 
   long long ll;
   double d;
   ll = d;
-  // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:8: warning: narrowing conversion from 'double' to signed type 'long long' is implementation-defined [cppcoreguidelines-narrowing-conversions]
+  // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:8: warning: narrowing conversion from 'double' to 'long long' [cppcoreguidelines-narrowing-conversions]
   // DISABLED: Warning disabled with WarnOnEquivalentBitWidth=0.
+
   d = ll;
-  // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:7: warning: narrowing conversion from 'long long' to signed type 'double' is implementation-defined [cppcoreguidelines-narrowing-conversions]
+  // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:7: warning: narrowing conversion from 'long long' to 'double' [cppcoreguidelines-narrowing-conversions]
   // DISABLED: Warning disabled with WarnOnEquivalentBitWidth=0.
 }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D103894: NarrowingConversionsCheck should support inhibiting conversions of mixed integer and floating point types with WarnOnEquivalentBitWidth=0.

2021-06-08 Thread Stephen Concannon via Phabricator via cfe-commits
Stephen created this revision.
Stephen added a reviewer: hokein.
Herald added subscribers: kbarton, nemanjai.
Stephen requested review of this revision.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.

Also standardize control flow of handleX conversion functions to make it easier 
to be consistent.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D103894

Files:
  clang-tools-extra/clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.cpp
  clang-tools-extra/clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.h
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-narrowing-conversions-equivalentbitwidth-option.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-narrowing-conversions-equivalentbitwidth-option.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-narrowing-conversions-equivalentbitwidth-option.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-narrowing-conversions-equivalentbitwidth-option.cpp
@@ -11,16 +11,33 @@
 
 void narrowing_equivalent_bitwidth() {
   int i;
-  unsigned int j;
-  i = j;
+  unsigned int ui;
+  i = ui;
   // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:7: warning: narrowing conversion from 'unsigned int' to signed type 'int' is implementation-defined [cppcoreguidelines-narrowing-conversions]
   // DISABLED: Warning disabled with WarnOnEquivalentBitWidth=0.
+
+  float f;
+  i = f;
+  // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:7: warning: narrowing conversion from 'float' to signed type 'int' is implementation-defined [cppcoreguidelines-narrowing-conversions]
+  // DISABLED: Warning disabled with WarnOnEquivalentBitWidth=0.
+  f = i;
+  // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:7: warning: narrowing conversion from 'int' to signed type 'float' is implementation-defined [cppcoreguidelines-narrowing-conversions]
+  // DISABLED: Warning disabled with WarnOnEquivalentBitWidth=0.
+
+  long long ll;
+  double d;
+  ll = d;
+  // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:8: warning: narrowing conversion from 'double' to signed type 'long long' is implementation-defined [cppcoreguidelines-narrowing-conversions]
+  // DISABLED: Warning disabled with WarnOnEquivalentBitWidth=0.
+  d = ll;
+  // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:7: warning: narrowing conversion from 'long long' to signed type 'double' is implementation-defined [cppcoreguidelines-narrowing-conversions]
+  // DISABLED: Warning disabled with WarnOnEquivalentBitWidth=0.
 }
 
 void most_narrowing_is_not_ok() {
   int i;
-  long long j;
-  i = j;
+  long long ui;
+  i = ui;
   // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:7: warning: narrowing conversion from 'long long' to signed type 'int' is implementation-defined [cppcoreguidelines-narrowing-conversions]
   // CHECK-MESSAGES-DISABLED: :[[@LINE-2]]:7: warning: narrowing conversion from 'long long' to signed type 'int' is implementation-defined [cppcoreguidelines-narrowing-conversions]
 }
Index: clang-tools-extra/clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.h
===
--- clang-tools-extra/clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.h
+++ clang-tools-extra/clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.h
@@ -93,6 +93,10 @@
   void handleBinaryOperator(const ASTContext ,
 const BinaryOperator );
 
+  bool isWarningInhibitedByEquivalentSize(const ASTContext ,
+  const BuiltinType ,
+  const BuiltinType ) const;
+
   const bool WarnOnFloatingPointNarrowingConversion;
   const bool WarnWithinTemplateInstantiation;
   const bool WarnOnEquivalentBitWidth;
Index: clang-tools-extra/clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.cpp
===
--- clang-tools-extra/clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.cpp
+++ clang-tools-extra/clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.cpp
@@ -67,8 +67,9 @@
   hasType(namedDecl(hasAnyListedName(IgnoreConversionFromTypes)));
 
   // `IsConversionFromIgnoredType` will ignore narrowing calls from those types,
-  // but not expressions that are promoted to `int64` due to a binary expression
-  // with one of those types. For example, it will continue to reject:
+  // but not expressions that are promoted to an ignored type as a result of a
+  // binary expression with one of those types.
+  // For example, it will continue to reject:
   // `int narrowed = int_value + container.size()`.
   // We attempt to address common incidents of compound expressions with
   // `IsIgnoredTypeTwoLevelsDeep`, allowing binary expressions that have one
@@ -217,6 +218,22 @@
   return ToIntegerRange.contains(IntegerConstant);
 }
 
+// Returns true iff the floating point constant can be losslessly represented
+// 

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