[PATCH] D64666: [Sema] Enable -Wimplicit-float-conversion for integral to floating point precision loss

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman accepted this revision.
aaron.ballman added a comment.

In D64666#1583629 <https://reviews.llvm.org/D64666#1583629>, @jfb wrote:

> I think you want to default-ignore the "may lose precision" warnings, but not the ones that you know always lose precision.


We don't often add off-by-default diagnostics because users rarely enable them, but given that this is for GCC compatibility and that matches the behavior of GCC, I think it's a reasonable idea.

LGTM aside from the small nits I commented on.



================
Comment at: clang/lib/Driver/ToolChains/Arch/AArch64.cpp:1
 //===--- AArch64.cpp - AArch64 (not ARM) Helpers for Tools ------*- C++ -*-===//
 //
----------------
Please revert whatever has changed in this file -- it seems to be whitespace only (perhaps line endings?).


================
Comment at: clang/lib/Sema/SemaChecking.cpp:11437
+        // Issue constant integer to float precision loss warning.
+        S.DiagRuntimeBehavior(
+          E->getExprLoc(), E,
----------------
nickdesaulniers wrote:
> Should this just be a generic `Diag` rather than a `DiagRuntimeBehavior`?
`DiagRuntimeBehavior()` seems reasonable to me -- we don't want to warn users about this in unevaluated contexts like a `sizeof()`, at least that I can think of.


================
Comment at: clang/test/Sema/implicit-float-conversion.c:3
+
+
+long testReturn(long a, float b) {
----------------
You can remove some of the spurious newlines from this file.


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

https://reviews.llvm.org/D64666