[clang-tidy] swap cppcoreguidelines-narrowing-conversions and bugprone-narrowing-conversions (#120245)

According to #116591.
> Coding guidelines should "cherry-pick" (and posddsibly
configure/harden/make more strict) base checks.
We should move narrowing conversion to bugprone and keep alias in
cppcoreguidelines

Author: HerrCai0907

clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp

@@ -9,7 +9,6 @@
 #include "../ClangTidy.h"
 #include "../ClangTidyModule.h"
 #include "../ClangTidyModuleRegistry.h"
-#include "../cppcoreguidelines/NarrowingConversionsCheck.h"
 #include "ArgumentCommentCheck.h"
 #include "AssertSideEffectCheck.h"
 #include "AssignmentInIfConditionCheck.h"
@@ -47,6 +46,7 @@
 #include "MultiLevelImplicitPointerConversionCheck.h"
 #include "MultipleNewInOneExpressionCheck.h"
 #include "MultipleStatementMacroCheck.h"
+#include "NarrowingConversionsCheck.h"
 #include "NoEscapeCheck.h"
 #include "NonZeroEnumToBoolConversionCheck.h"
 #include "NondeterministicPointerIterationOrderCheck.h"
@@ -183,7 +183,7 @@ class BugproneModule : public ClangTidyModule {
         "bugprone-pointer-arithmetic-on-polymorphic-object");
     CheckFactories.registerCheck<RedundantBranchConditionCheck>(
         "bugprone-redundant-branch-condition");
-    CheckFactories.registerCheck<cppcoreguidelines::NarrowingConversionsCheck>(
+    CheckFactories.registerCheck<NarrowingConversionsCheck>(
         "bugprone-narrowing-conversions");
     CheckFactories.registerCheck<NoEscapeCheck>("bugprone-no-escape");
     CheckFactories.registerCheck<NonZeroEnumToBoolConversionCheck>(

clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt

@@ -42,6 +42,7 @@ add_clang_library(clangTidyBugproneModule STATIC
   MultiLevelImplicitPointerConversionCheck.cpp
   MultipleNewInOneExpressionCheck.cpp
   MultipleStatementMacroCheck.cpp
+  NarrowingConversionsCheck.cpp
   NoEscapeCheck.cpp
   NonZeroEnumToBoolConversionCheck.cpp
   NondeterministicPointerIterationOrderCheck.cpp

clang-tools-extra/clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.cpp renamed to clang-tools-extra/clang-tidy/bugprone/NarrowingConversionsCheck.cpp

@@ -22,7 +22,7 @@
 
 using namespace clang::ast_matchers;
 
-namespace clang::tidy::cppcoreguidelines {
+namespace clang::tidy::bugprone {
 
 namespace {
 
@@ -614,4 +614,4 @@ void NarrowingConversionsCheck::check(const MatchFinder::MatchResult &Result) {
     return handleImplicitCast(*Result.Context, *Cast);
   llvm_unreachable("must be binary operator or cast expression");
 }
-} // namespace clang::tidy::cppcoreguidelines
+} // namespace clang::tidy::bugprone

clang-tools-extra/clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.h renamed to clang-tools-extra/clang-tidy/bugprone/NarrowingConversionsCheck.h

@@ -6,19 +6,19 @@
 //
 //===----------------------------------------------------------------------===//
 
-#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CPPCOREGUIDELINES_NARROWING_CONVERSIONS_H
-#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CPPCOREGUIDELINES_NARROWING_CONVERSIONS_H
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_NARROWING_CONVERSIONS_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_NARROWING_CONVERSIONS_H
 
 #include "../ClangTidyCheck.h"
 
-namespace clang::tidy::cppcoreguidelines {
+namespace clang::tidy::bugprone {
 
 /// Checks for narrowing conversions, e.g:
 ///   int i = 0;
 ///   i += 0.1;
 ///
 /// For the user-facing documentation see:
-/// http://clang.llvm.org/extra/clang-tidy/checks/cppcoreguidelines/narrowing-conversions.html
+/// http://clang.llvm.org/extra/clang-tidy/checks/bugprone/narrowing-conversions.html
 class NarrowingConversionsCheck : public ClangTidyCheck {
 public:
   NarrowingConversionsCheck(StringRef Name, ClangTidyContext *Context);
@@ -104,6 +104,6 @@ class NarrowingConversionsCheck : public ClangTidyCheck {
   const bool PedanticMode;
 };
 
-} // namespace clang::tidy::cppcoreguidelines
+} // namespace clang::tidy::bugprone
 
-#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CPPCOREGUIDELINES_NARROWING_CONVERSIONS_H
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_NARROWING_CONVERSIONS_H

clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt

@@ -16,7 +16,6 @@ add_clang_library(clangTidyCppCoreGuidelinesModule STATIC
   MacroUsageCheck.cpp
   MisleadingCaptureDefaultByValueCheck.cpp
   MissingStdForwardCheck.cpp
-  NarrowingConversionsCheck.cpp
   NoMallocCheck.cpp
   NoSuspendWithLockCheck.cpp
   OwningMemoryCheck.cpp

clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp

@@ -9,6 +9,7 @@
 #include "../ClangTidy.h"
 #include "../ClangTidyModule.h"
 #include "../ClangTidyModuleRegistry.h"
+#include "../bugprone/NarrowingConversionsCheck.h"
 #include "../misc/NonPrivateMemberVariablesInClassesCheck.h"
 #include "../misc/UnconventionalAssignOperatorCheck.h"
 #include "../modernize/AvoidCArraysCheck.h"
@@ -30,7 +31,6 @@
 #include "MacroUsageCheck.h"
 #include "MisleadingCaptureDefaultByValueCheck.h"
 #include "MissingStdForwardCheck.h"
-#include "NarrowingConversionsCheck.h"
 #include "NoMallocCheck.h"
 #include "NoSuspendWithLockCheck.h"
 #include "OwningMemoryCheck.h"
@@ -87,7 +87,7 @@ class CppCoreGuidelinesModule : public ClangTidyModule {
         "cppcoreguidelines-misleading-capture-default-by-value");
     CheckFactories.registerCheck<MissingStdForwardCheck>(
         "cppcoreguidelines-missing-std-forward");
-    CheckFactories.registerCheck<NarrowingConversionsCheck>(
+    CheckFactories.registerCheck<bugprone::NarrowingConversionsCheck>(
         "cppcoreguidelines-narrowing-conversions");
     CheckFactories.registerCheck<NoMallocCheck>("cppcoreguidelines-no-malloc");
     CheckFactories.registerCheck<NoSuspendWithLockCheck>(

clang-tools-extra/docs/ReleaseNotes.rst

@@ -361,6 +361,13 @@ Removed checks
 Miscellaneous
 ^^^^^^^^^^^^^
 
+- The :doc:`bugprone-narrowing-conversions <clang-tidy/checks/bugprone/narrowing-conversions>`
+  check is no longer an alias of :doc:`cppcoreguidelines-narrowing-conversions
+  <clang-tidy/checks/cppcoreguidelines/narrowing-conversions>`. Instead,
+  :doc:`cppcoreguidelines-narrowing-conversions
+  <clang-tidy/checks/cppcoreguidelines/narrowing-conversions>` is now an alias
+  of :doc:`bugprone-narrowing-conversions <clang-tidy/checks/bugprone/narrowing-conversions>`.
+
 Improvements to include-fixer
 -----------------------------
 

clang-tools-extra/docs/clang-tidy/checks/bugprone/narrowing-conversions.rst

@@ -1,10 +1,126 @@
 .. title:: clang-tidy - bugprone-narrowing-conversions
-.. meta::
-   :http-equiv=refresh: 5;URL=../cppcoreguidelines/narrowing-conversions.html
 
 bugprone-narrowing-conversions
 ==============================
 
-The bugprone-narrowing-conversions check is an alias, please see
-:doc:`cppcoreguidelines-narrowing-conversions <../cppcoreguidelines/narrowing-conversions>`
-for more information.
+`cppcoreguidelines-narrowing-conversions` redirects here as an alias for this check.
+
+Checks for silent narrowing conversions, e.g: ``int i = 0; i += 0.1;``. While
+the issue is obvious in this former example, it might not be so in the
+following: ``void MyClass::f(double d) { int_member_ += d; }``.
+
+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``)
+   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.
+
+This check will flag:
+ - All narrowing conversions that are not marked by an explicit cast (c-style or
+   ``static_cast``). For example: ``int i = 0; i += 0.1;``,
+   ``void f(int); f(0.1);``,
+ - All applications of binary operators with a narrowing conversions.
+   For example: ``int i; i+= 0.1;``.
+
+Arithmetic with smaller integer types than ``int`` trigger implicit conversions,
+as explained under `"Integral Promotion" on cppreference.com
+<https://en.cppreference.com/w/cpp/language/implicit_conversion>`_.
+This check diagnoses more instances of narrowing than the compiler warning
+`-Wconversion` does. The example below demonstrates this behavior.
+
+.. code-block:: c++
+
+  // The following function definition demonstrates usage of arithmetic with
+  // integer types smaller than `int` and how the narrowing conversion happens
+  // implicitly.
+  void computation(short argument1, short argument2) {
+    // Arithmetic written by humans:
+    short result = argument1 + argument2;
+    // Arithmetic actually performed by C++:
+    short result = static_cast<short>(static_cast<int>(argument1) + static_cast<int>(argument2));
+  }
+
+  void recommended_resolution(short argument1, short argument2) {
+    short result = argument1 + argument2;
+    //           ^ warning: narrowing conversion from 'int' to signed type 'short' is implementation-defined
+
+    // The cppcoreguidelines recommend to resolve this issue by using the GSL
+    // in one of two ways. Either by a cast that throws if a loss of precision
+    // would occur.
+    short result = gsl::narrow<short>(argument1 + argument2);
+    // Or it can be resolved without checking the result risking invalid results.
+    short result = gsl::narrow_cast<short>(argument1 + argument2);
+
+    // A classical `static_cast` will silence the warning as well if the GSL
+    // is not available.
+    short result = static_cast<short>(argument1 + argument2);
+  }
+
+Options
+-------
+
+.. option:: WarnOnIntegerNarrowingConversion
+
+    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
+    (e.g. ``double`` to ``float``). `true` by default.
+
+.. option:: WarnWithinTemplateInstantiation
+
+    When `true`, the check will warn on narrowing conversions within template
+    instantiations. `false` by default.
+
+.. option:: WarnOnEquivalentBitWidth
+
+    When `true`, the check will warn on narrowing conversions that arise from
+    casting between types of equivalent bit width. (e.g.
+    `int n = uint(0);` or `long long n = double(0);`) `true` by default.
+
+.. option:: IgnoreConversionFromTypes
+
+   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
+   problematic assignments such as `int n = std::vector<char>().size();` or
+   `int n = std::difference(it1, it2);`. The default list is empty, but one
+   suggested list for a legacy codebase would be
+   `size_t;ptrdiff_t;size_type;difference_type`.
+
+.. option:: PedanticMode
+
+    When `true`, the check will warn on assigning a floating point constant
+    to an integer value even if the floating point value is exactly
+    representable in the destination type (e.g. ``int i = 1.0;``).
+    `false` by default.
+
+FAQ
+---
+
+ - What does "narrowing conversion from 'int' to 'float'" mean?
+
+An IEEE754 Floating Point number can represent all integer values in the range
+[-2^PrecisionBits, 2^PrecisionBits] where PrecisionBits is the number of bits in
+the mantissa.
+
+For ``float`` this would be [-2^23, 2^23], where ``int`` can represent values in
+the range [-2^31, 2^31-1].
+
+ - What does "implementation-defined" mean?
+
+You may have encountered messages like "narrowing conversion from 'unsigned int'
+to signed type 'int' is implementation-defined".
+The C/C++ standard does not mandate two's complement for signed integers, and so
+the compiler is free to define what the semantics are for converting an unsigned
+integer to signed integer. Clang's implementation uses the two's complement
+format.

clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/narrowing-conversions.rst

@@ -1,129 +1,14 @@
 .. title:: clang-tidy - cppcoreguidelines-narrowing-conversions
+.. meta::
+   :http-equiv=refresh: 5;URL=../bugprone/narrowing-conversions.html
 
 cppcoreguidelines-narrowing-conversions
 =======================================
 
-Checks for silent narrowing conversions, e.g: ``int i = 0; i += 0.1;``. While
-the issue is obvious in this former example, it might not be so in the
-following: ``void MyClass::f(double d) { int_member_ += d; }``.
-
-This check implements `ES.46
+This check implements part of  `ES.46
 <https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#es46-avoid-lossy-narrowing-truncating-arithmetic-conversions>`_
 from the C++ Core Guidelines.
 
-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``)
-   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.
-
-This check will flag:
- - All narrowing conversions that are not marked by an explicit cast (c-style or
-   ``static_cast``). For example: ``int i = 0; i += 0.1;``,
-   ``void f(int); f(0.1);``,
- - All applications of binary operators with a narrowing conversions.
-   For example: ``int i; i+= 0.1;``.
-
-Arithmetic with smaller integer types than ``int`` trigger implicit conversions,
-as explained under `"Integral Promotion" on cppreference.com
-<https://en.cppreference.com/w/cpp/language/implicit_conversion>`_.
-This check diagnoses more instances of narrowing than the compiler warning
-`-Wconversion` does. The example below demonstrates this behavior.
-
-.. code-block:: c++
-
-   // The following function definition demonstrates usage of arithmetic with
-   // integer types smaller than `int` and how the narrowing conversion happens
-   // implicitly.
-   void computation(short argument1, short argument2) {
-     // Arithmetic written by humans:
-     short result = argument1 + argument2;
-     // Arithmetic actually performed by C++:
-     short result = static_cast<short>(static_cast<int>(argument1) + static_cast<int>(argument2));
-   }
-
-   void recommended_resolution(short argument1, short argument2) {
-     short result = argument1 + argument2;
-     //           ^ warning: narrowing conversion from 'int' to signed type 'short' is implementation-defined
-
-     // The cppcoreguidelines recommend to resolve this issue by using the GSL
-     // in one of two ways. Either by a cast that throws if a loss of precision
-     // would occur.
-     short result = gsl::narrow<short>(argument1 + argument2);
-     // Or it can be resolved without checking the result risking invalid results.
-     short result = gsl::narrow_cast<short>(argument1 + argument2);
-
-     // A classical `static_cast` will silence the warning as well if the GSL
-     // is not available.
-     short result = static_cast<short>(argument1 + argument2);
-   }
-
-
-Options
--------
-
-.. option:: WarnOnIntegerNarrowingConversion
-
-    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
-    (e.g. ``double`` to ``float``). `true` by default.
-
-.. option:: WarnWithinTemplateInstantiation
-
-    When `true`, the check will warn on narrowing conversions within template
-    instantiations. `false` by default.
-
-.. option:: WarnOnEquivalentBitWidth
-
-    When `true`, the check will warn on narrowing conversions that arise from
-    casting between types of equivalent bit width. (e.g.
-    `int n = uint(0);` or `long long n = double(0);`) `true` by default.
-
-.. option:: IgnoreConversionFromTypes
-
-   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
-   problematic assignments such as `int n = std::vector<char>().size();` or
-   `int n = std::difference(it1, it2);`. The default list is empty, but one
-   suggested list for a legacy codebase would be
-   `size_t;ptrdiff_t;size_type;difference_type`.
-
-.. option:: PedanticMode
-
-    When `true`, the check will warn on assigning a floating point constant
-    to an integer value even if the floating point value is exactly
-    representable in the destination type (e.g. ``int i = 1.0;``).
-    `false` by default.
-
-FAQ
----
-
- - What does "narrowing conversion from 'int' to 'float'" mean?
-
-An IEEE754 Floating Point number can represent all integer values in the range
-[-2^PrecisionBits, 2^PrecisionBits] where PrecisionBits is the number of bits in
-the mantissa.
-
-For ``float`` this would be [-2^23, 2^23], where ``int`` can represent values in
-the range [-2^31, 2^31-1].
-
- - What does "implementation-defined" mean?
-
-You may have encountered messages like "narrowing conversion from 'unsigned int'
-to signed type 'int' is implementation-defined".
-The C/C++ standard does not mandate two's complement for signed integers, and so
-the compiler is free to define what the semantics are for converting an unsigned
-integer to signed integer. Clang's implementation uses the two's complement
-format.
+The cppcoreguidelines-narrowing-conversions check is an alias, please see
+:doc:`bugprone-narrowing-conversions <../bugprone/narrowing-conversions>`
+for more information.