[clang-tools-extra] 9198d04 - Allow disabling integer to floating-point narrowing conversions for cppcoreguidelines-narrowing-conversions

[Aaron Ballman via cfe-commits]

Author: Paul Altin
Date: 2021-12-16T08:24:09-05:00
New Revision: 9198d04c06b561cd78d9407cedd50f7b995ee6d8

URL: https://github.com/llvm/llvm-project/commit/9198d04c06b561cd78d9407cedd50f7b995ee6d8
DIFF: https://github.com/llvm/llvm-project/commit/9198d04c06b561cd78d9407cedd50f7b995ee6d8.diff

LOG: Allow disabling integer to floating-point narrowing conversions for cppcoreguidelines-narrowing-conversions

This change adds an option to disable warnings from the
cppcoreguidelines-narrowing-conversions check on integer to floating-
point conversions which may be narrowing.

An example of a case where this might be useful:
```
std::vector<double> v = {1, 2, 3, 4};
double mean = std::accumulate(v.cbegin(), v.cend(), 0.0) / v.size();
```
The conversion from std::size_t to double is technically narrowing on
64-bit systems, but v almost certainly does not have enough elements
for this to be a problem.

This option would allow the cppcoreguidelines-narrowing-conversions
check to be enabled on codebases which might otherwise turn it off
because of cases like the above.

Added: 
    clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-narrowing-conversions-narrowingintegertofloatingpoint-option.cpp

Modified: 
    clang-tools-extra/clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.cpp
    clang-tools-extra/clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.h
    clang-tools-extra/docs/ReleaseNotes.rst
    clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-narrowing-conversions.rst

Removed: 
    


################################################################################
diff  --git a/clang-tools-extra/clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.cpp b/clang-tools-extra/clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.cpp
index 4d67c33361c17..99965c9e5bb98 100644
--- a/clang-tools-extra/clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.cpp
+++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.cpp
@@ -37,6 +37,8 @@ NarrowingConversionsCheck::NarrowingConversionsCheck(StringRef Name,
     : ClangTidyCheck(Name, Context),
       WarnOnIntegerNarrowingConversion(
           Options.get("WarnOnIntegerNarrowingConversion", true)),
+      WarnOnIntegerToFloatingPointNarrowingConversion(
+          Options.get("WarnOnIntegerToFloatingPointNarrowingConversion", true)),
       WarnOnFloatingPointNarrowingConversion(
           Options.get("WarnOnFloatingPointNarrowingConversion", true)),
       WarnWithinTemplateInstantiation(
@@ -49,6 +51,8 @@ void NarrowingConversionsCheck::storeOptions(
     ClangTidyOptions::OptionMap &Opts) {
   Options.store(Opts, "WarnOnIntegerNarrowingConversion",
                 WarnOnIntegerNarrowingConversion);
+  Options.store(Opts, "WarnOnIntegerToFloatingPointNarrowingConversion",
+                WarnOnIntegerToFloatingPointNarrowingConversion);
   Options.store(Opts, "WarnOnFloatingPointNarrowingConversion",
                 WarnOnFloatingPointNarrowingConversion);
   Options.store(Opts, "WarnWithinTemplateInstantiation",
@@ -425,19 +429,21 @@ void NarrowingConversionsCheck::handleIntegralToBoolean(
 void NarrowingConversionsCheck::handleIntegralToFloating(
     const ASTContext &Context, SourceLocation SourceLoc, const Expr &Lhs,
     const Expr &Rhs) {
-  const BuiltinType *ToType = getBuiltinType(Lhs);
-  llvm::APSInt IntegerConstant;
-  if (getIntegerConstantExprValue(Context, Rhs, IntegerConstant)) {
-    if (!isWideEnoughToHold(Context, IntegerConstant, *ToType))
-      diagNarrowIntegerConstant(SourceLoc, Lhs, Rhs, IntegerConstant);
-    return;
-  }
+  if (WarnOnIntegerToFloatingPointNarrowingConversion) {
+    const BuiltinType *ToType = getBuiltinType(Lhs);
+    llvm::APSInt IntegerConstant;
+    if (getIntegerConstantExprValue(Context, Rhs, IntegerConstant)) {
+      if (!isWideEnoughToHold(Context, IntegerConstant, *ToType))
+        diagNarrowIntegerConstant(SourceLoc, Lhs, Rhs, IntegerConstant);
+      return;
+    }
 
-  const BuiltinType *FromType = getBuiltinType(Rhs);
-  if (isWarningInhibitedByEquivalentSize(Context, *FromType, *ToType))
-    return;
-  if (!isWideEnoughToHold(Context, *FromType, *ToType))
-    diagNarrowType(SourceLoc, Lhs, Rhs);
+    const BuiltinType *FromType = getBuiltinType(Rhs);
+    if (isWarningInhibitedByEquivalentSize(Context, *FromType, *ToType))
+      return;
+    if (!isWideEnoughToHold(Context, *FromType, *ToType))
+      diagNarrowType(SourceLoc, Lhs, Rhs);
+  }
 }
 
 void NarrowingConversionsCheck::handleFloatingToIntegral(

diff  --git a/clang-tools-extra/clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.h b/clang-tools-extra/clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.h
index c6d087f867bda..36ced698e869f 100644
--- a/clang-tools-extra/clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.h
+++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.h
@@ -98,6 +98,7 @@ class NarrowingConversionsCheck : public ClangTidyCheck {
                                           const BuiltinType &ToType) const;
 
   const bool WarnOnIntegerNarrowingConversion;
+  const bool WarnOnIntegerToFloatingPointNarrowingConversion;
   const bool WarnOnFloatingPointNarrowingConversion;
   const bool WarnWithinTemplateInstantiation;
   const bool WarnOnEquivalentBitWidth;

diff  --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index bcb9a0f40719c..6f8a25e0d04f5 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -153,6 +153,12 @@ Changes in existing checks
   <clang-tidy/checks/bugprone-throw-keyword-missing>` when creating an exception object
   using placement new
 
+- :doc:`cppcoreguidelines-narrowing-conversions <clang-tidy/checks/cppcoreguidelines-narrowing-conversions>`
+  check now supports a `WarnOnIntegerToFloatingPointNarrowingConversion`
+  option to control whether to warn on narrowing integer to floating-point
+  conversions.
+
+
 Removed checks
 ^^^^^^^^^^^^^^
 

diff  --git a/clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-narrowing-conversions.rst b/clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-narrowing-conversions.rst
index e750c76676c5a..3ba5bdcaf276e 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-narrowing-conversions.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-narrowing-conversions.rst
@@ -15,7 +15,8 @@ https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#es4
 We enforce only part of the guideline, more specifically, we flag narrowing conversions from:
  - an integer to a narrower integer (e.g. ``char`` to ``unsigned char``)
    if WarnOnIntegerNarrowingConversion Option is set,
- - an integer to a narrower floating-point (e.g. ``uint64_t`` to ``float``),
+ - an integer to a narrower floating-point (e.g. ``uint64_t`` to ``float``)
+   if WarnOnIntegerToFloatingPointNarrowingConversion Option is set,
  - a floating-point to an integer (e.g. ``double`` to ``int``),
  - a floating-point to a narrower floating-point (e.g. ``double`` to ``float``)
    if WarnOnFloatingPointNarrowingConversion Option is set.
@@ -36,6 +37,11 @@ Options
     When `true`, the check will warn on narrowing integer conversion
     (e.g. ``int`` to ``size_t``). `true` by default.
 
+.. option:: WarnOnIntegerToFloatingPointNarrowingConversion
+
+    When `true`, the check will warn on narrowing integer to floating-point
+    conversion (e.g. ``size_t`` to ``double``). `true` by default.
+
 .. option:: WarnOnFloatingPointNarrowingConversion
 
     When `true`, the check will warn on narrowing floating point conversion

diff  --git a/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-narrowing-conversions-narrowingintegertofloatingpoint-option.cpp b/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-narrowing-conversions-narrowingintegertofloatingpoint-option.cpp
new file mode 100644
index 0000000000000..e7f65f5c9982c
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-narrowing-conversions-narrowingintegertofloatingpoint-option.cpp
@@ -0,0 +1,19 @@
+// RUN: %check_clang_tidy -check-suffix=DEFAULT %s \
+// RUN: cppcoreguidelines-narrowing-conversions %t -- \
+// RUN: -config='{CheckOptions: [{key: cppcoreguidelines-narrowing-conversions.WarnOnIntegerToFloatingPointNarrowingConversion, value: true}]}'
+
+// RUN: %check_clang_tidy -check-suffix=DISABLED %s \
+// RUN: cppcoreguidelines-narrowing-conversions %t -- \
+// RUN: -config='{CheckOptions: [{key: cppcoreguidelines-narrowing-conversions.WarnOnIntegerToFloatingPointNarrowingConversion, value: false}]}'
+
+void foo(unsigned long long value) {
+  double a = value;
+  // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:14: warning: narrowing conversion from 'unsigned long long' to 'double' [cppcoreguidelines-narrowing-conversions]
+  // DISABLED: No warning for integer to floating-point narrowing conversions when WarnOnIntegerToFloatingPointNarrowingConversion = false.
+}
+
+void floating_point_to_integer_is_still_not_ok(double f) {
+  int a = f;
+  // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:11: warning: narrowing conversion from 'double' to 'int' [cppcoreguidelines-narrowing-conversions]
+  // CHECK-MESSAGES-DISABLED: :[[@LINE-2]]:11: warning: narrowing conversion from 'double' to 'int' [cppcoreguidelines-narrowing-conversions]
+}