[QGIS-Developer] clang-tidy warning for narrowing to float

[David Koňařík]

Hi all,
I'd like to propose a change to QGIS's clang-tidy settings. Currently, 
we have bugprone-narrowing-conversions [0] turned on by wildcard, which 
by default warns on converting between integer types, between floating 
point types, and from integers to floating points, if the full range of 
the source type won't fit into the destination type without loss of 
precision.

I fully agree that conversions between different integer types should be 
conscious decisions, since they can lead to security issues [1]. I find 
the floating point warnings to have dubious usefulness, though.

In QGIS' 3D code, we often need to use integer values (e.g. screen size) 
in floating point calculations, with practically no chance that the 
integer value won't be representable as a float, and zero risk even if 
it is - creating a security issue would require a very creative abuse of 
floats. Fixing the warning leads to a visual spam of 
static_cast<float>()s everywhere, obscuring any actual bugs.

Converting doubles to floats can actually cause issues, but it's also 
inevitable in many places, seeing as Qt3D uses floats, but we need to 
use doubles ourselves for precision reasons. Given this, I also think 
the warning isn't very useful here.

I propose turning off WarnOnIntegerToFloatingPointNarrowingConversion 
and possibly WarnOnFloatingPointNarrowingConversion, based on your feedback.

David Koňařík

[1]: 
https://clang.llvm.org/extra/clang-tidy/checks/bugprone/narrowing-conversions.html
[0]: e.g. char *copy_string(char *str, uint64_t str_size) {
   uint32_t size_plus_null = str_size + 1;
   char *copy = malloc(size_plus_null);
   memcpy(copy, str, str_size);
   copy[str_size] = '\0';
   return copy;
}