On Mon, Nov 26, 2018 at 12:12 AM Alexander Grund via Boost < boost@lists.boost.org> wrote:
Am 25.11.18 um 04:31 schrieb Emil Dotchevski via Boost:
Here is my argument again:
1) A cast has different semantics than an implicit conversion.
2) If the implicit conversion is correct, replacing it with a cast to silence a warning is incorrect.
Which of these two points do you think is false?
I'd argue both are false because they are to general. If you add "may" then these arguments "may" be true.
I'll include your further reply:
The reason is that they have the wrong mindset when approaching the
The first is a fact, the second is basic logic. Both are true. problem.
They think: I get a warning, because the compiler doesn't know that this signed value is always positive, but I know that it is, so I'm going to cast, to tell the compiler that I know what I'm doing.
A better reasoning is: I'm converting a signed value to unsigned value, which will not be correct if the signed value is negative. With this in mind, you'll notice that the cast is utterly useless in catching the
actual
error the compiler is warning you about.
I can't really follow you here: The compiler not knowing that the signed value (which per definition CAN in general be negative) is always positive and alerting you of this is the whole point. IF you know absolutely sure that this signed value is always non-negative, then why not: a) Change the parameter type to unsigned to communicate this as a precondition of the function
This is not what unsigned is for. You should generally use signed ints even if the value can't be negative.
I don't see where this cast is wrong. It does the same as the implicit cast but explicit and hence conveys *intend*.
Fact: the cast does not do the same thing as the implicit conversion. (The cast converts to the type you specify in the cast. The implicit conversion converts to the target type.)
Disabling this warning might hide instances where such implicit conversions are in fact wrong.
What you're saying about disabling the warning is equally true about the cast. You're thinking that if you look at it and say it's ok, then the cast can't be wrong, but that's false. If you change the compiler or the platform, or under maintenance, the cast may become the cause of a bug, which it will then hide because you've told the compiler you know what you're doing.
I also see a contradiction in your argument: "this signed value is always positive, [...] I know that it is" is a valid reason to "converting a signed value to unsigned value, which will not be correct if the signed value is negative". So why is "the cast [...] utterly useless in catching the actual error"? If you know (or can ensure) the precondition then there is no error and you putting in the cast states that you thought about it.
unsigned f(); void g( int x ) { if( static_cast<unsigned>(x) < f() ) //x is guaranteed to be >=0 .... } Elsewhere: g(-1); Would the cast or the comment catch this bug? No. Compare with: void g( int x ) { assert(x>=0); if( x < f() ) .... } First, the assert will catch the bug, while the cast will hide it. Secondly, the implicit conversion is both correct and more likely to remain correct under maintenance, because the compiler will pick the best conversion if the types change. And if you worry about truncation under maintenance, same reasoning applies: add asserts rather than comments or casts.
In conclusion: These warnings are about suspicious code that *needs* to be inspected. After inspection relevant action must be taken which should result in the warning being solved or suppressed so the next reviewer does not need to inspect it again unless surrounding code has changed.
I'm not against doing something that suppresses the warning without changing the program semantically, though these things tend to be ugly and difficult to port, so they're impractical. But adding a cast changes the behavior of the program. It is not a good idea to break a correct program in order to suppress a bogus warning.