Avoid casts that don't change the pointer type #6785
Merged
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
In C, explicit pointer casts are mostly needed to work with the byte representation of a larger type. In other cases, either the implicit cast (e.g. to and from
void*
) is already fine and won't cause any warnings, or the code is doing something suspicious (e.g.Rboolean*
↔int*
) and we'd rather have the warning from the implicit cast.For example, adding an explicit cast from
LOGICAL(...)
toint*
doesn't prevent a warning, becauseLOGICAL(...)
already returns anint*
. If the pointer type returned byLOGICAL(...)
ever changes, the absence of the cast will cause the compiler to issue a diagnostic: then it would be an implicit cast to a different pointer type, likely a violation of the "strict aliasing" rule, something bad to warn about. An explicit cast, on the other hand, tells the compiler to silence the warning and do as written.Initially found while working on #6782. More examples located using the following Coccinelle script:
and the following command line:
The remaining examples found by
spatch
are all insnprintf(...)
calls casting non-pointer values like anuint64_t
corresponding to thePRIu64
conversion specifier. It's probably fine to keep the code defensive and leave these casts in. If the non-pointer type does change, the cast should prevent most problems, unless the cast causes a signed overflow (to be caught by UBSan).