Re: [boost] Current Guidance on Compiler Warnings?

Subject: Re: [boost] Current Guidance on Compiler Warnings?
From: Gavin Lambert (boost_at_[hidden])
Date: 2018-11-27 05:30:02

On 27/11/2018 16:31, Emil Dotchevski wrote:
> If you have:
> 
> void f( unsigned );
> 
> void g( int x )
> {
>    f(x);
> }
> 
> I don't think you'll get a warning. 

Hmm, that's true.  That seems worrisome.

You do get a warning if you try to pass a negative constant value (eg. 
calling f(-2)).  And I agree that you should not get a warning when 
passing a positive constant value even where it would normally assume a 
signed type.

But for the case where you're passing a parameter or other variable of 
the wrong type, it really should have at least the option to generate a 
warning, even if it's not enabled by default.

> But I might be wrong, so let's say you
> do get a warning, so you do:
> 
> void g( int x )
> {
>    f( static_cast<unsigned>(x) );
> }
> 
> How does this help in detecting the logic error of passing a negative x to
> g? Can you quantify the exact mechanics by which this cast makes your code
> safer? By what measure is the code less safe without the cast?

By itself, it doesn't help; you're correct.  But that wasn't what I was 
saying; I was saying that the warning would help you to find that code 
so that you can then rewrite g to one of the following:

void g( unsigned x )
{
     f(x);
}

void g( int x )
{
     assert(x >= 0);
     f(unsigned(x));  // or static_cast, if you prefer
}

Both of these are correct.  Performing conversion (regardless of whether 
it is implicit or explicit) without the assert is incorrect.  Performing 
implicit conversion with the assert is safe, but not easily 
distinguishable from the original unsafe code unless the compiler can 
inspect the assert even in NDEBUG builds.

And again, if someone later refactors f to have a different parameter 
type (which is not able to exactly represent every possible value of 
'unsigned'), then either of these *should* cause a warning to be raised 
again, requiring someone to look at it.

(Though as you noted before, that doesn't appear to currently be the 
case.  That seems like a compiler defect.)