Re: [boost] Current Guidance on Compiler Warnings?

Am 26.11.18 um 14:50 schrieb degski:

On Mon, 26 Nov 2018 at 10:12, Alexander Grund via Boost mailto:boost@lists.boost.org> wrote:

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

b) add a cast to the unsigned parameter type and potentially an assert if this helps future readers and catch precondition violations

I don't see where this cast is wrong. It does the same as the implicit cast but explicit and hence conveys *intend*.

But it [any of those options] will never alert you to a violation of that precondition. Making std::size_t unsigned is a mistake IMO.

- In option a) you get a compiler warning in the calling code when the precondition is violated (conversion from signed to unsigned) - In option b) the assert will tell you that I might agree with the size_t mistake: Using it for sizes may be ok, but as an index type might have been wrong.

If you do as Emil proposes, run a debug build with [that type of] asserts you'll **know** that **your** surrounding code [or **your** input ] is wrong and you'll **know** something needs fixing (not with a cast, but **your** [surrounding] logic). To me his [Emil's] argument is convincing, casting just hides the problem

Correct: The surrounding code needs fixing, not the code with the now-added cast. The warning about the "missing" cast can never point you to the surrounding code. It is a violation of the contract of the function to be called with a negative value. You can catch this with an assert or move the responsibility up to the caller (change param type) There is another point about this: This function might be an internal one where you have full control over, maybe even a lambda. There *may* be cases where you can guarantee that this is not called with negative values (the local lambda, an internal class function only locally accessible, ...) but normally you either assert-and-cast or change the param type leading to e.g.: for(int i=0; i<42;i++) foo(static_cast<unsigned>(i)); The cast can be easily validated locally, no assert required.

coz you just shot yourself in the foot and you don't know where the gun is (carrying a concealed weapon is a criminal offense in the US AFAIK ;-) )]. If you do not suppress the warnings (after careful investigation) how do you find real issues highlighted by warnings? Using your metaphor: How do you find the serial killer with the gun if everyone has a gun and shoots wildly around? We should have a dynamic_assert b.t.w., i.e. the converse of static_assert (in C++, not the C macro).

I prefer something like this: template enable_if_t checked_cast(U val){ assert(val >= 0); return static_cast<T>(val); } Similar for conditions for narrowing casts or unsigned->signed casts. If appropriate use an own define for assert that may stay enabled in release builds. Note that for my loop example above the check will be optimized out by teh compiler as it knows that i>=0