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

Jonas Toth via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Nov 6 09:40:57 PST 2018


JonasToth added a comment.

I think the check is really close.
If the other reviewers want to take a look at it again, now is a good moment :)



================
Comment at: clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.cpp:178
+      return;
+    // Conversions to unsigned integer are well defined and follow modulo 2
+    // arithmetic.
----------------
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.


================
Comment at: clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.cpp:224
+  if (ToType->isBooleanType())
+    return; // conversion to bool value is well defined.
+
----------------
Nit: Please move these two comment above the `if` and make `s/conversion/Conversion/`.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D53488





More information about the cfe-commits mailing list