[PATCH] D53488: [clang-tidy] Catching narrowing from double to float.

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added a comment.

That one looks interesting :)

Am 25.10.18 um 17:13 schrieb Guillaume Chatelet via Phabricator:

> gchatelet added a comment.
> 
> In https://reviews.llvm.org/D53488#1275786, @hokein wrote:
> 
>> In https://reviews.llvm.org/D53488#1275750, @gchatelet wrote:
>> 
>>> In https://reviews.llvm.org/D53488#1274205, @JonasToth wrote:
>>> 
>>>> Did you run this code over a real-world code-base and did you find new stuff and/or false positives or the like?
>>> 
>>> Yes I did run it over our code base. I didn't find false positive but 98% of the warnings are from large generated lookup table initializers, e.g. `const static float kTable[] = {0.0, 2.0, ... };`
>>> 
>>>   Since every number in the list triggers the warning, it accounts for most of them.
>>> 
>>> I scrutinized a few hundreds of the rest: none were actual bugs (although it's hard to tell sometimes), most are legit like `float value = 0.0;` but I also found some oddities https://github.com/ARM-software/astc-encoder/blob/master/Source/vectypes.h#L13999 from generated headers.
>>> 
>>> To me the warnings are useful and if it were my code I'd be willing to fix them. That said, I'd totally understand that many people would find them useless or annoying.
>>> 
>>>   What do you think? Shall we still commit this as is?
>> 
>> It would be nice to know how many new findings does this patch introduce (number of findings before the patch vs after). If it is not too much, it is fine the commit as it is.
>> 
>>   I'd suggest to run the check on llvm code repository (using `clang-tidy/tool/run-clang-tidy.py`, and only enable `cppcoreguidelines-narrowing-conversions`).
> 
> I'll get back with some numbers.
> 
> In the meantime I found this one which is interesting
>  https://github.com/intel/ipmctl/blob/master/src/os/efi_shim/lnx_efi_api.c#L45
>  `spec.tv_nsec` (which is signed long) is converted to double and back to int64. There surely can be some loss in the process since `double` can //only// express 2^52 integers (2^52ns is about 52 days)
> 
> Repository:
> 
>   rCTE Clang Tools Extra
> 
> https://reviews.llvm.org/D53488


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D53488