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