Skip to content

[clang-tidy] Suggest using reinterpret_cast in bugprone-casting-thro… #106784

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.

Sign up for GitHub

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
merged 1 commit into from
Sep 4, 2024
Merged

[clang-tidy] Suggest using reinterpret_cast in bugprone-casting-thro… #106784

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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: 3 additions & 1 deletion clang-tools-extra/clang-tidy/bugprone/CastingThroughVoidCheck.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,9 @@ void CastingThroughVoidCheck::check(const MatchFinder::MatchResult &Result) {
const auto ST = *Result.Nodes.getNodeAs<QualType>("source_type");
const auto VT = *Result.Nodes.getNodeAs<QualType>("void_type");
const auto *CE = Result.Nodes.getNodeAs<ExplicitCastExpr>("cast");
diag(CE->getExprLoc(), "do not cast %0 to %1 through %2") << ST << TT << VT;
diag(CE->getExprLoc(),
"do not cast %0 to %1 through %2; use reinterpret_cast instead")
<< ST << TT << VT;
}

} // namespace clang::tidy::bugprone
4 changes: 4 additions & 0 deletions clang-tools-extra/docs/ReleaseNotes.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,10 @@ New check aliases
Changes in existing checks
^^^^^^^^^^^^^^^^^^^^^^^^^^

- Improved :doc:`bugprone-casting-through-void
<clang-tidy/checks/bugprone/casting-through-void>` check to suggest replacing
the offending code with ``reinterpret_cast``, to more clearly express intent.

- Improved :doc:`modernize-use-std-format
<clang-tidy/checks/modernize/use-std-format>` check to support replacing
member function calls too.
Expand Down
21 changes: 19 additions & 2 deletions clang-tools-extra/docs/clang-tidy/checks/bugprone/casting-through-void.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,9 @@
bugprone-casting-through-void
=============================

Detects unsafe or redundant two-step casting operations involving ``void*``.
Detects unsafe or redundant two-step casting operations involving ``void*``,
which is equivalent to ``reinterpret_cast`` as per the
`C++ Standard <https://eel.is/c++draft/expr.reinterpret.cast#7>`_.

Two-step type conversions via ``void*`` are discouraged for several reasons.

Expand All @@ -16,7 +18,17 @@ Two-step type conversions via ``void*`` are discouraged for several reasons.

In summary, avoiding two-step type conversions through ``void*`` ensures clearer code,
maintains essential compiler warnings, and prevents ambiguity and potential runtime
errors, particularly in complex inheritance scenarios.
errors, particularly in complex inheritance scenarios. If such a cast is wanted,
it shall be done via ``reinterpret_cast``, to express the intent more clearly.

Note: it is expected that, after applying the suggested fix and using
``reinterpret_cast``, the check :doc:`cppcoreguidelines-pro-type-reinterpret-cast
<../cppcoreguidelines/pro-type-reinterpret-cast>` will emit a warning.
This is intentional: ``reinterpret_cast`` is a dangerous operation that can
easily break the strict aliasing rules when dereferencing the casted pointer,
invoking Undefined Behavior. The warning is there to prompt users to carefuly
analyze whether the usage of ``reinterpret_cast`` is safe, in which case the
warning may be suppressed.

Examples:

Expand All @@ -29,3 +41,8 @@ Examples:
reinterpret_cast<IntegerPointer>(reinterpret_cast<void *>(ptr)); // WRONG
(IntegerPointer)(void *)ptr; // WRONG
IntegerPointer(static_cast<void *>(ptr)); // WRONG

reinterpret_cast<IntegerPointer>(ptr); // OK, clearly expresses intent.
// NOTE: dereferencing this pointer violates
// the strict aliasing rules, invoking
// Undefined Behavior.
40 changes: 20 additions & 20 deletions clang-tools-extra/test/clang-tidy/checkers/bugprone/casting-through-void.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,54 +10,54 @@ const double cd = 100;

void normal_test() {
static_cast<int *>(static_cast<void *>(&d));
// CHECK-MESSAGES: :[[@LINE-1]]:22: warning: do not cast 'double *' to 'int *' through 'void *' [bugprone-casting-through-void]
// CHECK-MESSAGES: :[[@LINE-1]]:22: warning: do not cast 'double *' to 'int *' through 'void *'; use reinterpret_cast instead [bugprone-casting-through-void]
static_cast<int *>(static_cast<V>(&d));
// CHECK-MESSAGES: :[[@LINE-1]]:22: warning: do not cast 'double *' to 'int *' through 'V' (aka 'void *') [bugprone-casting-through-void]
// CHECK-MESSAGES: :[[@LINE-1]]:22: warning: do not cast 'double *' to 'int *' through 'V' (aka 'void *'); use reinterpret_cast instead [bugprone-casting-through-void]
static_cast<int *>(static_cast<void *>(&i));
// CHECK-MESSAGES: :[[@LINE-1]]:22: warning: do not cast 'int *' to 'int *' through 'void *' [bugprone-casting-through-void]
// CHECK-MESSAGES: :[[@LINE-1]]:22: warning: do not cast 'int *' to 'int *' through 'void *'; use reinterpret_cast instead [bugprone-casting-through-void]

static_cast<void *>(static_cast<void *>(&i));
}

void const_pointer_test() {
static_cast<int *const>(static_cast<void *>(&d));
// CHECK-MESSAGES: :[[@LINE-1]]:27: warning: do not cast 'double *' to 'int *const' through 'void *' [bugprone-casting-through-void]
// CHECK-MESSAGES: :[[@LINE-1]]:27: warning: do not cast 'double *' to 'int *const' through 'void *'; use reinterpret_cast instead [bugprone-casting-through-void]
static_cast<int *const>(static_cast<V>(&d));
// CHECK-MESSAGES: :[[@LINE-1]]:27: warning: do not cast 'double *' to 'int *const' through 'V' (aka 'void *') [bugprone-casting-through-void]
// CHECK-MESSAGES: :[[@LINE-1]]:27: warning: do not cast 'double *' to 'int *const' through 'V' (aka 'void *'); use reinterpret_cast instead [bugprone-casting-through-void]
static_cast<int *const>(static_cast<void *>(&i));
// CHECK-MESSAGES: :[[@LINE-1]]:27: warning: do not cast 'int *' to 'int *const' through 'void *' [bugprone-casting-through-void]
// CHECK-MESSAGES: :[[@LINE-1]]:27: warning: do not cast 'int *' to 'int *const' through 'void *'; use reinterpret_cast instead [bugprone-casting-through-void]

static_cast<void *const>(static_cast<void *>(&i));
}

void const_test() {
static_cast<const int *>(static_cast<const void *>(&d));
// CHECK-MESSAGES: :[[@LINE-1]]:28: warning: do not cast 'double *' to 'const int *' through 'const void *' [bugprone-casting-through-void]
// CHECK-MESSAGES: :[[@LINE-1]]:28: warning: do not cast 'double *' to 'const int *' through 'const void *'; use reinterpret_cast instead [bugprone-casting-through-void]
static_cast<const int *>(static_cast<const V>(&d));
// CHECK-MESSAGES: :[[@LINE-1]]:28: warning: do not cast 'double *' to 'const int *' through 'const V' (aka 'void *const') [bugprone-casting-through-void]
// CHECK-MESSAGES: :[[@LINE-1]]:28: warning: do not cast 'double *' to 'const int *' through 'const V' (aka 'void *const'); use reinterpret_cast instead [bugprone-casting-through-void]
static_cast<const int *>(static_cast<const void *>(&i));
// CHECK-MESSAGES: :[[@LINE-1]]:28: warning: do not cast 'int *' to 'const int *' through 'const void *' [bugprone-casting-through-void]
// CHECK-MESSAGES: :[[@LINE-1]]:28: warning: do not cast 'int *' to 'const int *' through 'const void *'; use reinterpret_cast instead [bugprone-casting-through-void]

static_cast<const void *>(static_cast<const void *>(&i));

static_cast<const int *>(static_cast<const void *>(&cd));
// CHECK-MESSAGES: :[[@LINE-1]]:28: warning: do not cast 'const double *' to 'const int *' through 'const void *' [bugprone-casting-through-void]
// CHECK-MESSAGES: :[[@LINE-1]]:28: warning: do not cast 'const double *' to 'const int *' through 'const void *'; use reinterpret_cast instead [bugprone-casting-through-void]
static_cast<const int *>(static_cast<const CV>(&cd));
// CHECK-MESSAGES: :[[@LINE-1]]:28: warning: do not cast 'const double *' to 'const int *' through 'const CV' (aka 'const void *const') [bugprone-casting-through-void]
// CHECK-MESSAGES: :[[@LINE-1]]:28: warning: do not cast 'const double *' to 'const int *' through 'const CV' (aka 'const void *const'); use reinterpret_cast instead [bugprone-casting-through-void]
static_cast<const int *>(static_cast<const void *>(&ci));
// CHECK-MESSAGES: :[[@LINE-1]]:28: warning: do not cast 'const int *' to 'const int *' through 'const void *' [bugprone-casting-through-void]
// CHECK-MESSAGES: :[[@LINE-1]]:28: warning: do not cast 'const int *' to 'const int *' through 'const void *'; use reinterpret_cast instead [bugprone-casting-through-void]

static_cast<const void *>(static_cast<const void *>(&ci));
}


void reinterpret_cast_test() {
static_cast<int *>(reinterpret_cast<void *>(&d));
// CHECK-MESSAGES: :[[@LINE-1]]:22: warning: do not cast 'double *' to 'int *' through 'void *' [bugprone-casting-through-void]
// CHECK-MESSAGES: :[[@LINE-1]]:22: warning: do not cast 'double *' to 'int *' through 'void *'; use reinterpret_cast instead [bugprone-casting-through-void]
reinterpret_cast<int *>(static_cast<void *>(&d));
// CHECK-MESSAGES: :[[@LINE-1]]:27: warning: do not cast 'double *' to 'int *' through 'void *' [bugprone-casting-through-void]
// CHECK-MESSAGES: :[[@LINE-1]]:27: warning: do not cast 'double *' to 'int *' through 'void *'; use reinterpret_cast instead [bugprone-casting-through-void]
reinterpret_cast<int *>(reinterpret_cast<void *>(&d));
// CHECK-MESSAGES: :[[@LINE-1]]:27: warning: do not cast 'double *' to 'int *' through 'void *' [bugprone-casting-through-void]
// CHECK-MESSAGES: :[[@LINE-1]]:27: warning: do not cast 'double *' to 'int *' through 'void *'; use reinterpret_cast instead [bugprone-casting-through-void]

static_cast<void *>(reinterpret_cast<void *>(&i));
reinterpret_cast<void *>(reinterpret_cast<void *>(&i));
Expand All @@ -66,11 +66,11 @@ void reinterpret_cast_test() {

void c_style_cast_test() {
static_cast<int *>((void *)&d);
// CHECK-MESSAGES: :[[@LINE-1]]:22: warning: do not cast 'double *' to 'int *' through 'void *' [bugprone-casting-through-void]
// CHECK-MESSAGES: :[[@LINE-1]]:22: warning: do not cast 'double *' to 'int *' through 'void *'; use reinterpret_cast instead [bugprone-casting-through-void]
(int *)(void *)&d;
// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: do not cast 'double *' to 'int *' through 'void *' [bugprone-casting-through-void]
// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: do not cast 'double *' to 'int *' through 'void *'; use reinterpret_cast instead [bugprone-casting-through-void]
static_cast<int *>((void *)&d);
// CHECK-MESSAGES: :[[@LINE-1]]:22: warning: do not cast 'double *' to 'int *' through 'void *' [bugprone-casting-through-void]
// CHECK-MESSAGES: :[[@LINE-1]]:22: warning: do not cast 'double *' to 'int *' through 'void *'; use reinterpret_cast instead [bugprone-casting-through-void]

static_cast<void *>((void *)&i);
}
Expand All @@ -82,12 +82,12 @@ using I = int *;
void cxx_functional_cast() {
A(static_cast<void*>(&d));
I(static_cast<void*>(&d));
// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: do not cast 'double *' to 'I' (aka 'int *') through 'void *' [bugprone-casting-through-void]
// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: do not cast 'double *' to 'I' (aka 'int *') through 'void *'; use reinterpret_cast instead [bugprone-casting-through-void]
}

void bit_cast() {
__builtin_bit_cast(int *, static_cast<void *>(&d));
// CHECK-MESSAGES: :[[@LINE-1]]:29: warning: do not cast 'double *' to 'int *' through 'void *' [bugprone-casting-through-void]
// CHECK-MESSAGES: :[[@LINE-1]]:29: warning: do not cast 'double *' to 'int *' through 'void *'; use reinterpret_cast instead [bugprone-casting-through-void]
}

namespace PR87069 {
Expand Down
Loading