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 -- 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
