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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
