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

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added inline comments.


================
Comment at: test/clang-tidy/cppcoreguidelines-narrowing-conversions.cpp:164
+  while (i) {
+    // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: narrowing conversion from 'int' to 'bool' [cppcoreguidelines-narrowing-conversions]
+  }
----------------
gchatelet wrote:
> JonasToth wrote:
> > lebedev.ri wrote:
> > > courbet wrote:
> > > > What about providing a fix for this one :
> > > > `while (i != 0) {`
> > > Is this //actually// a narrowing conversion as per the CPPCG?
> > > Conversion to bool is a special case, it's not a truncation, but a `x != 0`.
> > > I'd think cases like these where bool was pretty clearly meant (`if()`, `while()`, `for()`, ???) should be exempt.
> > No it is not an narrowing conversion the CPPCG are concerned: https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#es46-avoid-lossy-narrowing-truncating-arithmetic-conversions
> > 
> > Short: 
> > ```
> > ES.46: Avoid lossy (narrowing, truncating) arithmetic conversions
> > 
> > Enforcement
> > 
> > A good analyzer can detect all narrowing conversions. However, flagging all narrowing conversions will lead to a lot of false positives. Suggestions:
> > 
> >     - flag all floating-point to integer conversions (maybe only float->char and double->int. Here be dragons! we need data)
> >     - flag all long->char (I suspect int->char is very common. Here be dragons! we need data)
> >     - consider narrowing conversions for function arguments especially suspect
> > ```
> > 
> > We do have a readability-check that is concerned about int -> bool conversions. Because of that I think we could safely ignore the `int->bool` conversion. 
> > But `float -> bool` is still suspicious in my opinion. It has the same semantics, but comparing `float` on equality is dangerous and one could argue it shall be enforced through point 1 (all float-to-int conversions).
> @JonasToth Do you have the name of the readability check so I can document the behavior?
Should be https://clang.llvm.org/extra/clang-tidy/checks/readability-implicit-bool-conversion.html


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D53488