Search of explicit type conversions in C style: a = (int)b;

Question

5.00/5 (3 votes)

See more:

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:

C++

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?

Posted 28-Sep-11 2:01am

Karpov Andrey

Updated 28-Sep-11 2:16am

v3

Add a Solution

5 solutions

Add a Solution

Add your solution here

Treat my content as plain text, not as HTML

Preview 0

…

Existing Members

Sign in to your account

...or Join us

Download, Vote, Comment, Publish.

Your Email
Password
Forgot your password?

Your Email
This email is in use. Do you need your password?
Optional Password

I have read and agree to the Terms of Service and Privacy Policy
Please subscribe me to the CodeProject newsletters

When answering a question please:

Read the question carefully.
Understand that English isn't everyone's first language so be lenient of bad spelling and grammar.
If a question is poorly phrased then either ask for clarification, ignore it, or edit the question and fix the problem. Insults are not welcome.
Don't tell someone to read the manual. Chances are they have and don't get it. Provide an answer or move on to the next question.

Let's work to help developers, not make them feel stupid.

This content, along with any associated source code and files, is licensed under The Code Project Open License (CPOL)

CPallini · Answer 1 · 2011-09-28T02:07:00

Solution 1

It might be useful, those are actually 'dangerous' areas of the code.
I mean, revising C-like casts is always a good practice, in my opinion.

Posted 28-Sep-11 2:07am

CPallini

Updated 28-Sep-11 2:15am

v3

Richard MacCutchan · Answer 2 · 2011-09-28T04:38:00

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.

Posted 28-Sep-11 4:38am

Richard MacCutchan

Emilio Garavaglia · Answer 3 · 2011-09-28T10:45:00

Solution 4

C casts can be sometimes static_cast (for related types), sometime reinterpret_cast (for unrelated types).

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!

Posted 28-Sep-11 10:45am

Emilio Garavaglia

Comments

Stefan_Lang 29-Sep-11 5:15am

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.

Emilio Garavaglia 29-Sep-11 10:25am

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.

Stefan_Lang 29-Sep-11 10:42am

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.

Emilio Garavaglia 29-Sep-11 16:49pm

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.

Stefan_Lang · Answer 4 · 2011-09-28T05:41:00

That makes us three :)

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?

Karpov Andrey · Answer 5 · 2011-09-28T19:39:00

Solution 5

Thanks to everyone. I think we implement this.

Posted 28-Sep-11 19:39pm