Hi, > 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. >
I don't have any. And we may never face such an issue like the one I pointed in int-to-float conversion. But there is data loss, so I'd rather be (too) cautious. But you're probable right and it's maybe overkill indeed. Regards, Julien > 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 -- Julien Cabieces Senior Developer at Oslandia [email protected] _______________________________________________ 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
