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

[Guillaume Chatelet via Phabricator via cfe-commits]

gchatelet added inline comments.


================
Comment at: clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.cpp:178
+      return;
+    // Conversions to unsigned integer are well defined and follow modulo 2
+    // arithmetic.
----------------
JonasToth wrote:
> I am surprised by `following modulo 2 arithmetic` and think it's a bit misleading. Writing just `module arithmetic` is probably better, as `module 2` somewhat implies there a only 2 valid values (0, 1).
> 
> Is this the `int` -> `unsigned int` case path? That seems worth diagnosing too.
Yes, thx for noticing. I updated the comment, I think it's better now.

Indeed this is the `int` -> `unsigned int` case path. Warning here would lead to a lot of noise for everybody doing bitwise operations since `-1` is a compact way to represent the maximum value. Since the semantic is valid and well defined by the standard I'm unsure it's worth the pain. I'm open to suggestions though.

Maybe a good way to figure out is what would be the correct fix for say `unsigned long AllBits = -1;`


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D53488