gsl::narrow warnings for integer to float cast

[beinhaerter]

Given this code

#include <gsl/narrow>

constexpr void f() {
    constexpr double d{ 3. };
    constexpr unsigned u1{ static_cast<unsigned>(d) };
    constexpr unsigned u2{ gsl::narrow<unsigned>(d) };
    constexpr double d1{ static_cast<double>(u1) };
    constexpr double d2{ gsl::narrow<double>(u1) };
}

MSVC warngs:

warning C26467: Converting from floating point to unsigned integral types results in non-portable code if the double/float has a negative value. Use gsl::narrow_cast or gsl::narrow instead to guard against undefined behavior and potential data loss (es.46).

The warning is issued on the line defining u1. It is also issued in gsl::narrow for the line defining d2.

g++-10 -Wfloat-equal warns in gsl::narrow for the line defining u2.

What do the GSL maintainers think about narrow[_cast] from double to unsigned (and vice versa)? Is it something that shall not be done? Shall we suppress some of the warnings in GSL?

[dmitrykobets-msft]

Hi @beinhaerter,

I can't find the warning for using a static_cast from an integer to a floating point - the only related warning/rule I could find is ES.46 but that does not include integer -> floating in its enforcement. Did you mean floating -> int?

As for the clang warning, that looks like it only triggers for gsl::narrow and not gsl::narrow_cast, and triggers for either direction between floating <-> int:

gsl::narrow<float>(42); // Wfloat-equal
gsl::narrow<int>(42.0f); // Wfloat-equal

Hopefully this is just an implementation caveat with regards to gsl::narrow and will either require a change in the implementation, or a suppression of the warning. I'll investigate this.

Thanks for noticing this, Dmitry

[beinhaerter]

@dmitrykobets-msft Sorry for the confusion. I now reproduced it again here. The warning says it itself, it was for converting double to unsigned. My question was too general because I was writing about integer, not unsigned integer. I adjusted the question and title and now provide a short code snippet. I also adjusted gcc to g++10 because that is what I used right now and it makes more clear which version emits the warning (in case this is version deopendant).

[dmitrykobets-msft]

@beinhaerter thanks for the clarification.

I'll respond to the re-iterated points:

The MSVC warning on the line defining u1 is expected, because that line is using static_cast The MSVC warning for gsl::narrow when defining d2 is the false positive here: https://github.com/microsoft/GSL/issues/1036 The -Wfloat-equal warning is on the static_cast<U>(t) != u code. I'll need to investigate this to see if it's a legitimate warning about our implementation of gsl::narrow, or should be suppressed, as you suggested. The last time floating->integral was discussed it was concluded that it will be permitted: https://github.com/microsoft/GSL/issues/786. But perhaps the -Wfloat-equal warning here may change things.

Thanks, Dmitry