[PATCH] D64666: Allow Clang -Wconversion, -Wimplicit-float-conversion warns about integer type -> floating point type implicit conversion precision loss.

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers added a comment.

Thanks for the patch @ziangwan !



================
Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:3245
+def warn_impcast_integer_float_precision : Warning<
+  "implicit conversion from %0 to %1 may loses integer precision">,
+  InGroup<ImplicitFloatConversion>, DefaultIgnore;
----------------
`may lose` or just `loses`.


================
Comment at: clang/lib/Sema/SemaChecking.cpp:11404
+  // If we are casting an integer type to a floating point type, we might
+  // lose accuracy if the floating point type has a narrower signicand
+  // than the floating point type. Issue warnings for that accuracy loss. 
----------------
`significand`


================
Comment at: clang/lib/Sema/SemaChecking.cpp:11412
+
+    // Determine the number of precision bits in the target floating point type
+    // Also create the APFloat object to represent the converted value.
----------------
End the sentence w/ punctuation.


================
Comment at: clang/lib/Sema/SemaChecking.cpp:11417
+    llvm::APFloat SourceToFloatValue(
+      S.Context.getFloatTypeSemantics(QualType(TargetBT, 0)));
+
----------------
should this use `SourceBT` rather than `TargetBT`?


================
Comment at: clang/lib/Sema/SemaChecking.cpp:11420
+    // If we manage to find out the precisions for both types, perform
+    // acccuracy loss check and issue warning if necessary.
+    if (SourcePrecision > 0 && TargetPrecision > 0 &&
----------------
`accuracy`

`:set spell` if you're using vim


================
Comment at: clang/lib/Sema/SemaChecking.cpp:11421
+    // acccuracy loss check and issue warning if necessary.
+    if (SourcePrecision > 0 && TargetPrecision > 0 &&
+        SourcePrecision > TargetPrecision) {
----------------
Maybe negate this conditional and return early?


================
Comment at: clang/lib/Sema/SemaChecking.cpp:11437
+        // Issue constant integer to float precision loss warning.
+        S.DiagRuntimeBehavior(
+          E->getExprLoc(), E,
----------------
Should this just be a generic `Diag` rather than a `DiagRuntimeBehavior`?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D64666/new/

https://reviews.llvm.org/D64666