Hi,
thanks for your input. We can, however, disable only some aspects of the
warning, and the only ones I'd like to disable are warnings on floating
point conversions.
The PR you linked fixed a bug with an unwanted integer-to-integer
conversion. I'm convinced something like this couldn't happen with float
conversions, but feel free to send me counterexamples.
David Koňařík
On 10/28/25 09:09, Julien Cabieces via QGIS-Developer wrote:
Hi,
I totally get your point, and I already faced such a situation where I
have to introduce (too) many static_cast<>. On the other side, I already
faced a situation where the use of this warning would have prevented a
real issue [0].
So, I'm not really in favor of your proposal because it would disable
the warning for the whole codebase, on some situation where it could be
relevant. Could we not instead:
- introduce float getters/setters to have the conversion at only one place
- just have some converted variable "const float fvar =
static_cast<float>( var )". I don't think it makes the code so difficult
to read.
Regards,
Julien
[0] https://github.com/qgis/QGIS/pull/50016
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;
}
_______________________________________________
QGIS-Developer mailing list
[email protected]
List info: https://lists.osgeo.org/mailman/listinfo/qgis-developer
Unsubscribe: https://lists.osgeo.org/mailman/listinfo/qgis-developer
_______________________________________________
QGIS-Developer mailing list
[email protected]
List info: https://lists.osgeo.org/mailman/listinfo/qgis-developer
Unsubscribe: https://lists.osgeo.org/mailman/listinfo/qgis-developer
