Skip to content

Commit

Permalink
[clang-tidy] Suggest using reinterpret_cast in bugprone-casting-thro… (…
Browse files Browse the repository at this point in the history 
…#106784)

…ugh-void

reinterpret_cast is the equivalent construct, and more clearly expresses
intent.

Co-authored-by: Carlos Gálvez <[email protected]>
  • Loading branch information
@carlosgalvezp
carlosgalvezp and Carlos Gálvez authored Sep 4, 2024
1 parent 5a658ee commit d25e24a
Show file tree
Hide file tree
Showing 4 changed files with 46 additions and 23 deletions.
4 changes: 3 additions & 1 deletion clang-tools-extra/clang-tidy/bugprone/CastingThroughVoidCheck.cpp
View file
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
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
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
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

0 comments on commit d25e24a

Please sign in to comment.