Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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"
Expand Down Expand Up @@ -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>(
Expand Down
1 change: 1 addition & 0 deletions clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ add_clang_library(clangTidyBugproneModule STATIC
MultiLevelImplicitPointerConversionCheck.cpp
MultipleNewInOneExpressionCheck.cpp
MultipleStatementMacroCheck.cpp
NarrowingConversionsCheck.cpp
NoEscapeCheck.cpp
NonZeroEnumToBoolConversionCheck.cpp
NondeterministicPointerIterationOrderCheck.cpp
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@

using namespace clang::ast_matchers;

namespace clang::tidy::cppcoreguidelines {
namespace clang::tidy::bugprone {

namespace {

Expand Down Expand Up @@ -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
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ add_clang_library(clangTidyCppCoreGuidelinesModule STATIC
MacroUsageCheck.cpp
MisleadingCaptureDefaultByValueCheck.cpp
MissingStdForwardCheck.cpp
NarrowingConversionsCheck.cpp
NoMallocCheck.cpp
NoSuspendWithLockCheck.cpp
OwningMemoryCheck.cpp
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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"
Expand Down Expand Up @@ -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>(
Expand Down
7 changes: 7 additions & 0 deletions clang-tools-extra/docs/ReleaseNotes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -356,6 +356,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
-----------------------------

Expand Down
Original file line number Diff line number Diff line change
@@ -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.
Original file line number Diff line number Diff line change
@@ -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.
Loading
Loading