I have a question concerning the benefit of this diagnostic.
One user suggested that we implemented search of all the explicit type
conversions in C style in the PVS-Studio analyzer.
That is, a diagnostic to detect constructs of this kind:

int *x = (int *)y;
float a = float(b);
float c = (float)(d);

Its purpose is to replace all these conversions with their safer
versions - reinterpret_cast/static_cast/const_cast. During the process
of such refactoring, some defects in code may well be detected.

Of course, this is not detection of crucial errors, and if we
implement this diagnostic, it will be in the section [Customer's
Specific Requests] and disabled by default.

But I even doubt the benefit of this diagnostic. So I decided to ask
other users: does anybody else need this option of searching for explicit
type conversions in C style? Would anybody like to perform this kind
of refactoring in their code?

Solution 2

I agree with Carlo, you should definitely do this if you are moving from C to C++. The old C-style casts are prone to causing unexplained errors which can be very hard to discover in running applications.

Solution 4

If you can correctly guess what they are, case by case, replacing them with C++ proper casts makes the code more robust.

The only danger I see is where you cannot guess in a certain way, which cast you should use (it happens with (A*)&b, where b is a B instance and A and B are both template parameters. What the C cast will do depends on what A and B actually are on instantiation, and is not obvious that -if static_cast works for A and B as related types- the reinterpret_cast in case A & B are unrelated must be treated as error.

In other words, the weaker behavior of C cast, in certain old code, may had been consciously used as a "feature" (not error!): in such case, removing it, may alter the behavior of correct (In computation theory sense) code.

Of course, C++ community will criticize such a code, but if it exist and it is proved it works... you can make it no more working!

I don't get your template example, or at leat the conclusion you draw from it. How can it be correct to cast on object of one type to another when the two are not related? At the very least it is a very bad style, similar to abusing a pointer variable to store an integer value, but in most cases I expect that would be aa clear and destructive error.

Of course, 90% of the cases it is.
But there are cases (especially for thing that work closely to the hardware) that interpreting a given bunch of byte (because, despite what language layers and guru can say, that's what the hardware gives you) sometime as a struct, sometimes as another is the only way to go, since that's the way the underlying hardware works.
I agree that is not C++ "good style": it is -in fact- "good old bad C hack" but is what makes the interface to underlying operating system to work.
If you are in one of these situations, considering C cast bad becomes considering low level abstraction bad. It depends on which abstraction level your code sits on.

Ok, so what you were thinking of is stuff like some low, low hardware (or close) libraries that instead of returning a struct, just return a void* (or equivalent). I can agree to that. But then, you could in this case make a template specialization that uses static_cast (which is valid for casting from void*), and then use no cast for everything else. If anything breaks, but would be valid code, make another specilization for that conversion, using reinterpret_cast if required.

Sure: you can always do better. My point is that there are case where is not just a matter of "find and replace". What you are in fact proposing in your comment is a (sort of) "rethinking", not just a syntax adjustment.
The fact that such a rethink is "convenient" depends case by case, and it's not always obvious.

Solution 3

The problem is that the declaration of the variable (or expression) being cast can be anywhere else in the codebase, and if that declaration ever changes, the cast will be silently corrupted. The declaration may even be in another library, so, exchanging that library might silently break your code. I've seen that happen, and had to find and fix such cases.

I wonder however if such a task can be automated for casts to user-defined types as well. Certainly for casts to one specific type, but how do you handle a legacy app with hundreds or thousands of such types? While only a few of them might actually be used in type casts, how can you find out which? Maybe a grep with a clever search pattern (and some method to remove duplicates) could help?