CppCoreGuidelines
CppCoreGuidelines copied to clipboard
Suggest gsl::narrow/gsl::narrow_cast for float/double to integral conversions in ES.46
One of the things ES.46 touches upon is the following:
Enforcement A good analyzer can detect all narrowing conversions. However, flagging all narrowing conversions will lead to a lot of false positives. Suggestions:
Flag all floating-point to integer conversions (maybe only float->char and double->int. Here be dragons! we need data).
We have recently ran into an issue where directly casting a negative double to an unsigned integral type results in inconsistent code generation when targeting a specific architecture. Therefore, we want to implement a static check that flags all conversions from negative double/float to unsigned integral types and suggests to use gsl::narrow or gsl::narrow_cast. My hope is we can then update the existing implementation of gsl::narrow/gsl::narrow_cast to guard against such conversions by first going from float/double to signed integral type and then convert the result to unsigned, guarding against data loss during the process.
Can we improve the documentation of ES.46 such that it suggests people to use gsl::narrow and gsl::narrow_cast when doing float/double to integral conversions?
The guideline does include the line
We also include lossy arithmetic casts, such as from a negative floating point type to an unsigned integral type:
followed by example code which seems to show the issue you're describing.
I believe the intention of the guideline is that gsl::narrow handles this case in the way you'd expect. i.e. if your value is negative it's going to raise an error (of some sort) and the microsoft implementation seems to reflect that (from looking at the code. I haven't tested) https://github.com/microsoft/GSL/blob/master/include/gsl/narrow .
Can you clarify what you're asking for because as far as I can tell what you're asking for already exists?
Yes, data would be great. The very first C++ (and C with classes) got away with banning long->char and float->char.
Editors call: Are you suggesting that we require narrow or narrow_cast for all float->unsigned conversions, and that narrow gives "the right" value (as if it was an integer value) and narrow_cast gives a run-time error?
Can we improve the documentation of ES.46 such that it suggests people to use gsl::narrow and gsl::narrow_cast when doing float/double to integral conversions?
We think the guideline does say that, including the part about "We also include lossy arithmetic casts, such as from a negative floating point type to an unsigned integral type"... can you elaborate what wording change you would like to see? Is the request to make the quoted Enforcement unconditional, that is:
- Flag all floating-point to integer conversions.