search

microsoft / GSL

Guidelines Support Library
Other
6.14k stars 738 forks source link

es.46 on gsl::narrow/gsl::narrow_cast #1036

Closed beinhaerter closed 2 years ago

beinhaerter commented 2 years ago

When calling gsl::narrow<double>(size_t) under VS 16.11.10 I get the warning

gsl\narrow(42): 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).

It looks like this warning should be suppressed inside gsl::narrow and gsl::narrow_cast.

When adding GSL_SUPPRESS(es.46) // NO-FORMAT: attribute to both functions directly after the already existing GSL_SUPPRESS(f.6) I still get the warning.

Is this something that the VS team should look at? Is this a warning that we want to suppress in the GSL? If so, with GSL_SUPPRESS or with #pragma warning(disable: ...) as a workaround?

dmitrykobets-msft commented 2 years ago

Hi @beinhaerter I'll look into this. By the way, does the warning get suppressed if you add it right before the narrow cast, instead of before the function definition?

dmitrykobets-msft commented 2 years ago

Update: I tried it myself and I can't find a way to suppress the warning using GSL_SUPPRESS. That appears to be a VS bug, which I'll follow up on. GSL_SUPPRESS should be the method of suppression in this case. And yes this warning should be suppressed in this context, for the same reason that the const T t = narrow_cast<T>(u); in the definition of gsl::narrow has warning suppressions (see https://github.com/microsoft/GSL/issues/786)

dmitrykobets-msft commented 2 years ago

Closing this since suppressing es.46 will be fixed in VS 17.3, and the suppression was added the latest PR on this issue