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

Haojian Wu via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Oct 25 07:58:48 PDT 2018


hokein added a comment.

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`).


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D53488





More information about the cfe-commits mailing list