Correct warning on Float->Integer conversions.

[erichkeane]

Summary

As identified and briefly discussed here:
https://bugs.llvm.org/show_bug.cgi?id=37305

Converting a floating point number to an integer type when
the integral part is out of the range of the integer type is
undefined behavior in C. Additionally, CodeGen emits an undef
in this situation.

HOWEVER, we've been giving a warning that says that the value is
changed. This patch corrects the warning to list that it is actually
undefined behavior.

[aaron.ballman]

lib/Sema/SemaChecking.cpp

9432

floating point -> floating-point

9440–9441

I think you can combine all of the predicates into:

if (!IsBool && ((IntegerValue.isSigned() && (...) || (IntegerValue.isUnsigned() && (...))))
  return DiagnoseImpCast(S, E, T, CContext, diag::warn_impcast_literal_float_to_integer_out_of_range);

[erichkeane]

lib/Sema/SemaChecking.cpp

9440–9441

I'd waffled on whether to combine those ore not, since I was 50/50 and you have an opinion, combining it :)

[erichkeane]

Aaron's comments :)

[aaron.ballman]

LGTM!

[efriedma]

The check for whether an input is "out of range" doesn't handle fractions correctly. Testcase:

int a() { return 2147483647.5; }
unsigned b() { return -.5; }

[erichkeane]

In D46535#1091787, @efriedma wrote:

The check for whether an input is "out of range" doesn't handle fractions correctly. Testcase:

int a() { return 2147483647.5; }
unsigned b() { return -.5; }

Hrm... For some reaosn I had it in my head that both of those were UB... Interestingly, EDG/ICC thinks the second one is. I'll look into this more to see if I can refine the warning.

[xbolva00]

Hello, @erichkeane

I am working on int to float checker but I haven't found yet a way how to check if integer fits to float's significant bits. Maybe you can recommend me something? I already tried some LLVM APIs but no success. (cc @spatel as you will probably know the right function to do it)

[erichkeane]

In D46535#1253473, @xbolva00 wrote:

Hello, @erichkeane

I am working on int to float checker but I haven't found yet a way how to check if integer fits to float's significant bits. Maybe you can recommend me something? I already tried some LLVM APIs but no success. (cc @spatel as you will probably know the right function to do it)

I'm not sure what you mean by that... Every Integer is representable (lossy of course) as a float as far as I know. You'll obviously lose the LSDs, but it'll be representable. Are you trying to check whether it is round-trip-able? If thats the case, I think you'll need to make sure it entirely fits in the mantissa. You can get the amount of mantissa bits by checking the APFloat's fltSemantics. There are a couple of builtin ones that I think end up being useful, but the 'precision' field seems like what you want.

[xbolva00]

Yeah, I want to check if it fits into mantissa.

[efriedma]

Every Integer is representable (lossy of course) as a float as far as I know.

Casting a uint128_t to float can overflow. And overflowing a _Float16 is easy. (Of course, both uint128_t and _Float16 are rare in normal C/C++ code.)

include/clang/Basic/DiagnosticSemaKinds.td

def warn_impcast_integer_precision_constant : Warning<
   InGroup<ConstantConversion>;
 def warn_impcast_bitfield_precision_constant : Warning<
   "implicit truncation from %2 to bit-field changes value from %0 to %1">,
   InGroup<BitFieldConstantConversion>;
 
 def warn_impcast_literal_float_to_integer : Warning<
   "implicit conversion from %0 to %1 changes value from %2 to %3">,
   InGroup<LiteralConversion>;
+def warn_impcast_literal_float_to_integer_out_of_range : Warning<
+  "implicit conversion of out of range value from %0 to %1 is undefined">,
+  InGroup<LiteralConversion>;
 def warn_impcast_float_integer : Warning<
   "implicit conversion turns floating-point number into integer: %0 to %1">,
   InGroup<FloatConversion>, DefaultIgnore;
 
 def warn_impcast_float_to_integer : Warning<
   "implicit conversion of out of range value from %0 to %1 changes value "
   "from %2 to %3">,
   InGroup<FloatOverflowConversion>, DefaultIgnore;
+def warn_impcast_float_to_integer_out_of_range : Warning<
+  "implicit conversion of out of range value from %0 to %1 is undefined">,
+  InGroup<FloatOverflowConversion>, DefaultIgnore;
 def warn_impcast_float_to_integer_zero : Warning<
   "implicit conversion from %0 to %1 changes non-zero value from %2 to %3">,
   InGroup<FloatZeroConversion>, DefaultIgnore;
 
 def warn_impcast_string_literal_to_bool : Warning<
   "implicit conversion turns string literal into bool: %0 to %1">,
   InGroup<StringConversion>, DefaultIgnore;
 def warn_impcast_different_enum_types : Warning<

lib/Sema/SemaChecking.cpp

if (Value.convertToInteger(IntegerValue, llvm::APFloat::rmTowardZero,
       isExact) {
     if (IsLiteral) return;
     return DiagnoseImpCast(S, E, T, CContext, diag::warn_impcast_float_integer,
                            PruneWarnings);
   }
 
   unsigned DiagID = 0;
   if (IsLiteral) {
+    // Conversion of a floating-point value to a non-bool integer where the
+    // integral part cannot be represented by the integer type is undefined.
+    if (!IsBool &&
+        ((IntegerValue.isSigned() && (IntegerValue.isMaxSignedValue() ||
+                                      IntegerValue.isMinSignedValue())) ||
+         (IntegerValue.isUnsigned() &&
+          (IntegerValue.isMaxValue() || IntegerValue.isMinValue()))))
+      return DiagnoseImpCast(
+          S, E, T, CContext,
+          diag::warn_impcast_literal_float_to_integer_out_of_range);
     // Warn on floating point literal to integer.
     DiagID = diag::warn_impcast_literal_float_to_integer;
   } else if (IntegerValue == 0) {
     if (Value.isZero()) {  // Skip -0.0 to 0 conversion.
       return DiagnoseImpCast(S, E, T, CContext,
                              diag::warn_impcast_float_integer, PruneWarnings);
     }
     // Warn on non-zero to zero conversion.
     DiagID = diag::warn_impcast_float_to_integer_zero;
   } else {
     if (IntegerValue.isUnsigned()) {
       if (!IntegerValue.isMaxValue()) {
         return DiagnoseImpCast(S, E, T, CContext,
                                diag::warn_impcast_float_integer, PruneWarnings);
       }
+      if (!IsBool && (IntegerValue.isMaxValue() || IntegerValue.isMinValue()))
+        return DiagnoseImpCast(S, E, T, CContext,
+                               diag::warn_impcast_float_to_integer_out_of_range,
+                               PruneWarnings);
     } else {  // IntegerValue.isSigned()
       if (!IntegerValue.isMaxSignedValue() &&
           !IntegerValue.isMinSignedValue()) {
         return DiagnoseImpCast(S, E, T, CContext,
                                diag::warn_impcast_float_integer, PruneWarnings);
       }
+      return DiagnoseImpCast(S, E, T, CContext,
+                             diag::warn_impcast_float_to_integer_out_of_range,
+                             PruneWarnings);
     }
     // Warn on evaluatable floating point expression to integer conversion.
     DiagID = diag::warn_impcast_float_to_integer;
   }
 
   // FIXME: Force the precision of the source value down so we don't print
   // digits which are usually useless (we don't really care here if we
   // truncate a digit by accident in edge cases).  Ideally, APFloat::toString

test/SemaCXX/coroutines.cpp

struct coroutine_handle<void> {
   coroutine_handle(coroutine_handle<PromiseType>);
   static coroutine_handle from_address(void *);
 };
 }} // namespace std::experimental
 
 void yield() {
   co_yield 0;
   co_yield {"foo", 1, 2};
-  co_yield {1e100}; // expected-error {{cannot be narrowed}} expected-note {{explicit cast}} expected-warning {{changes value}} expected-warning {{braces around scalar}}
+  co_yield {1e100}; // expected-error {{cannot be narrowed}} expected-note {{explicit cast}} expected-warning {{implicit conversion}} expected-warning {{braces around scalar}}
   co_yield {"foo", __LONG_LONG_MAX__}; // expected-error {{cannot be narrowed}} expected-note {{explicit cast}} expected-warning {{changes value}}
   co_yield {"foo"};
   co_yield "foo"; // expected-error {{no matching}}
   co_yield 1.0;
   co_yield yield; // expected-error {{no member named 'await_ready' in 'not_awaitable'}}
 }
 
 void check_auto_await_suspend() {

test/SemaCXX/warn-float-conversion.cpp

 #endif  // ZERO
 
 #ifdef OVERFLOW
 void TestOverflow() {
   char a = 500.0;  // caught by -Wliteral-conversion
   char b = -500.0;  // caught by -Wliteral-conversion
 
   const float LargeNumber = 1024;
-  char c = LargeNumber;  // expected-warning{{implicit conversion of out of range value from 'const float' to 'char' changes value from 1024 to 127}}
-  char d = 400.0 + 400.0;  // expected-warning{{implicit conversion of out of range value from 'double' to 'char' changes value from 800 to 127}}
+  char c = LargeNumber;  // expected-warning{{implicit conversion of out of range value from 'const float' to 'char' is undefined}}
+  char d = 400.0 + 400.0;  // expected-warning{{implicit conversion of out of range value from 'double' to 'char' is undefined}}
 
-  char e = 1.0 / 0.0;  // expected-warning{{implicit conversion of out of range value from 'double' to 'char' changes value from +Inf to 127}}
+  char e = 1.0 / 0.0;  // expected-warning{{implicit conversion of out of range value from 'double' to 'char' is undefined}}
 }
 #endif  // OVERFLOW

test/SemaCXX/warn-literal-conversion.cpp

 // are converted to false; everything else is converted to true.
 void test1() {
   bool b1 = 0.99f; // expected-warning {{implicit conversion from 'float' to 'bool' changes value from 0.99 to true}}
   bool b2 = 0.99; // expected-warning {{implicit conversion from 'double' to 'bool' changes value from 0.99 to true}}
   // These do not warn because they can be directly converted to integral
   // values.
   bool b3 = 0.0f;
   bool b4 = 0.0;
+
+  // These all warn because they overflow the target type.
+  short s = 32768.0; // expected-warning{{implicit conversion of out of range value from 'double' to 'short' is undefined}}
+  unsigned short us = 65536.0; // expected-warning{{implicit conversion of out of range value from 'double' to 'unsigned short' is undefined}}
+
+  short s2 = -32769.0; // expected-warning{{implicit conversion of out of range value from 'double' to 'short' is undefined}}
+  unsigned short us2 = -65537.0; // expected-warning{{implicit conversion of out of range value from 'double' to 'unsigned short' is undefined}}
 }