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

[Julien Cabieces]

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
>>> QGIS-Developer at lists.osgeo.org
>>> List info: https://lists.osgeo.org/mailman/listinfo/qgis-developer
>>> Unsubscribe: https://lists.osgeo.org/mailman/listinfo/qgis-developer
>> 
>
> _______________________________________________
> QGIS-Developer mailing list
> QGIS-Developer at lists.osgeo.org
> 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
julien.cabieces at oslandia.com