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

Guillaume Chatelet via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Nov 7 00:50:02 PST 2018

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;`

  rCTE Clang Tools Extra


More information about the cfe-commits mailing list