[Sema] Enable -Wimplicit-float-conversion for integral to floating po…

…int precision loss

Issue an warning when the code tries to do an implicit int -> float
conversion, where the float type ha a narrower significant than the
float type.

The new warning is controlled by flag -Wimplicit-int-float-conversion,
under -Wimplicit-float-conversion and -Wconversion. It is also silenced
when c++11 narrowing warning is issued.

Differential Revision: https://reviews.llvm.org/D64666

llvm-svn: 367497

clang/include/clang/Basic/DiagnosticGroups.td

@@ -62,7 +62,8 @@ def BoolConversion : DiagGroup<"bool-conversion", [PointerBoolConversion,
 def IntConversion : DiagGroup<"int-conversion">;
 def EnumConversion : DiagGroup<"enum-conversion">;
 def ImplicitIntConversion : DiagGroup<"implicit-int-conversion">;
-def ImplicitFloatConversion : DiagGroup<"implicit-float-conversion">;
+def ImplicitIntFloatConversion : DiagGroup<"implicit-int-float-conversion">;
+def ImplicitFloatConversion : DiagGroup<"implicit-float-conversion", [ImplicitIntFloatConversion]>;
 def ImplicitFixedPointConversion : DiagGroup<"implicit-fixed-point-conversion">;
 
 def FloatOverflowConversion : DiagGroup<"float-overflow-conversion">;

clang/include/clang/Basic/DiagnosticSemaKinds.td

@@ -3266,6 +3266,14 @@ def warn_impcast_float_integer : Warning<
   "implicit conversion turns floating-point number into integer: %0 to %1">,
   InGroup<FloatConversion>, DefaultIgnore;
 
+// Implicit int -> float conversion precision loss warnings.
+def warn_impcast_integer_float_precision : Warning<
+  "implicit conversion from %0 to %1 may lose precision">,
+  InGroup<ImplicitIntFloatConversion>, DefaultIgnore;
+def warn_impcast_integer_float_precision_constant : Warning<
+  "implicit conversion from %2 to %3 changes value from %0 to %1">,
+  InGroup<ImplicitIntFloatConversion>;
+
 def warn_impcast_float_to_integer : Warning<
   "implicit conversion from %0 to %1 changes value from %2 to %3">,
   InGroup<FloatOverflowConversion>, DefaultIgnore;

clang/lib/Sema/SemaChecking.cpp

@@ -10200,7 +10200,8 @@ static bool IsSameFloatAfterCast(const APValue &value,
           IsSameFloatAfterCast(value.getComplexFloatImag(), Src, Tgt));
 }
 
-static void AnalyzeImplicitConversions(Sema &S, Expr *E, SourceLocation CC);
+static void AnalyzeImplicitConversions(Sema &S, Expr *E, SourceLocation CC,
+                                       bool IsListInit = false);
 
 static bool IsEnumConstOrFromMacro(Sema &S, Expr *E) {
   // Suppress cases where we are comparing against an enum constant.
@@ -11161,9 +11162,10 @@ static bool isObjCSignedCharBool(Sema &S, QualType Ty) {
          S.getLangOpts().ObjC && S.NSAPIObj->isObjCBOOLType(Ty);
 }
 
-static void
-CheckImplicitConversion(Sema &S, Expr *E, QualType T, SourceLocation CC,
-                        bool *ICContext = nullptr) {
+static void CheckImplicitConversion(Sema &S, Expr *E, QualType T,
+                                    SourceLocation CC,
+                                    bool *ICContext = nullptr,
+                                    bool IsListInit = false) {
   if (E->isTypeDependent() || E->isValueDependent()) return;
 
   const Type *Source = S.Context.getCanonicalType(E->getType()).getTypePtr();
@@ -11405,6 +11407,54 @@ CheckImplicitConversion(Sema &S, Expr *E, QualType T, SourceLocation CC,
     }
   }
 
+  // If we are casting an integer type to a floating point type without
+  // initialization-list syntax, we might lose accuracy if the floating
+  // point type has a narrower significand than the integer type.
+  if (SourceBT && TargetBT && SourceBT->isIntegerType() &&
+      TargetBT->isFloatingType() && !IsListInit) {
+    // Determine the number of precision bits in the source integer type.
+    IntRange SourceRange = GetExprRange(S.Context, E, S.isConstantEvaluated());
+    unsigned int SourcePrecision = SourceRange.Width;
+
+    // Determine the number of precision bits in the
+    // target floating point type.
+    unsigned int TargetPrecision = llvm::APFloatBase::semanticsPrecision(
+        S.Context.getFloatTypeSemantics(QualType(TargetBT, 0)));
+
+    if (SourcePrecision > 0 && TargetPrecision > 0 &&
+        SourcePrecision > TargetPrecision) {
+
+      llvm::APSInt SourceInt;
+      if (E->isIntegerConstantExpr(SourceInt, S.Context)) {
+        // If the source integer is a constant, convert it to the target
+        // floating point type. Issue a warning if the value changes
+        // during the whole conversion.
+        llvm::APFloat TargetFloatValue(
+            S.Context.getFloatTypeSemantics(QualType(TargetBT, 0)));
+        llvm::APFloat::opStatus ConversionStatus =
+            TargetFloatValue.convertFromAPInt(
+                SourceInt, SourceBT->isSignedInteger(),
+                llvm::APFloat::rmNearestTiesToEven);
+
+        if (ConversionStatus != llvm::APFloat::opOK) {
+          std::string PrettySourceValue = SourceInt.toString(10);
+          SmallString<32> PrettyTargetValue;
+          TargetFloatValue.toString(PrettyTargetValue, TargetPrecision);
+
+          S.DiagRuntimeBehavior(
+              E->getExprLoc(), E,
+              S.PDiag(diag::warn_impcast_integer_float_precision_constant)
+                  << PrettySourceValue << PrettyTargetValue << E->getType() << T
+                  << E->getSourceRange() << clang::SourceRange(CC));
+        }
+      } else {
+        // Otherwise, the implicit conversion may lose precision.
+        DiagnoseImpCast(S, E, T, CC,
+                        diag::warn_impcast_integer_float_precision);
+      }
+    }
+  }
+
   DiagnoseNullConversion(S, E, T, CC);
 
   S.DiscardMisalignedMemberAddress(Target, E);
@@ -11595,11 +11645,17 @@ static void CheckBoolLikeConversion(Sema &S, Expr *E, SourceLocation CC) {
 /// AnalyzeImplicitConversions - Find and report any interesting
 /// implicit conversions in the given expression.  There are a couple
 /// of competing diagnostics here, -Wconversion and -Wsign-compare.
-static void AnalyzeImplicitConversions(Sema &S, Expr *OrigE,
-                                       SourceLocation CC) {
+static void AnalyzeImplicitConversions(Sema &S, Expr *OrigE, SourceLocation CC,
+                                       bool IsListInit/*= false*/) {
   QualType T = OrigE->getType();
   Expr *E = OrigE->IgnoreParenImpCasts();
 
+  // Propagate whether we are in a C++ list initialization expression.
+  // If so, we do not issue warnings for implicit int-float conversion
+  // precision loss, because C++11 narrowing already handles it.
+  IsListInit =
+      IsListInit || (isa<InitListExpr>(OrigE) && S.getLangOpts().CPlusPlus);
+
   if (E->isTypeDependent() || E->isValueDependent())
     return;
 
@@ -11619,7 +11675,7 @@ static void AnalyzeImplicitConversions(Sema &S, Expr *OrigE,
   // The non-canonical typecheck is just an optimization;
   // CheckImplicitConversion will filter out dead implicit conversions.
   if (E->getType() != T)
-    CheckImplicitConversion(S, E, T, CC);
+    CheckImplicitConversion(S, E, T, CC, nullptr, IsListInit);
 
   // Now continue drilling into this expression.
 
@@ -11629,15 +11685,15 @@ static void AnalyzeImplicitConversions(Sema &S, Expr *OrigE,
     // FIXME: Use a more uniform representation for this.
     for (auto *SE : POE->semantics())
       if (auto *OVE = dyn_cast<OpaqueValueExpr>(SE))
-        AnalyzeImplicitConversions(S, OVE->getSourceExpr(), CC);
+        AnalyzeImplicitConversions(S, OVE->getSourceExpr(), CC, IsListInit);
   }
 
   // Skip past explicit casts.
   if (auto *CE = dyn_cast<ExplicitCastExpr>(E)) {
     E = CE->getSubExpr()->IgnoreParenImpCasts();
     if (!CE->getType()->isVoidType() && E->getType()->isAtomicType())
       S.Diag(E->getBeginLoc(), diag::warn_atomic_implicit_seq_cst);
-    return AnalyzeImplicitConversions(S, E, CC);
+    return AnalyzeImplicitConversions(S, E, CC, IsListInit);
   }
 
   if (BinaryOperator *BO = dyn_cast<BinaryOperator>(E)) {
@@ -11676,7 +11732,7 @@ static void AnalyzeImplicitConversions(Sema &S, Expr *OrigE,
       // Ignore checking string literals that are in logical and operators.
       // This is a common pattern for asserts.
       continue;
-    AnalyzeImplicitConversions(S, ChildExpr, CC);
+    AnalyzeImplicitConversions(S, ChildExpr, CC, IsListInit);
   }
 
   if (BO && BO->isLogicalOp()) {

clang/test/Sema/conversion.c

@@ -233,7 +233,7 @@ void test8(int v) {
   takes_int(v);
   takes_long(v);
   takes_longlong(v);
-  takes_float(v);
+  takes_float(v); // expected-warning {{implicit conversion from 'int' to 'float' may lose precision}}
   takes_double(v);
   takes_longdouble(v);
 }
@@ -244,8 +244,8 @@ void test9(long v) {
   takes_int(v); // expected-warning {{implicit conversion loses integer precision}}
   takes_long(v);
   takes_longlong(v);
-  takes_float(v);
-  takes_double(v);
+  takes_float(v);  // expected-warning {{implicit conversion from 'long' to 'float' may lose precision}}
+  takes_double(v); // expected-warning {{implicit conversion from 'long' to 'double' may lose precision}}
   takes_longdouble(v);
 }
 
@@ -255,8 +255,8 @@ void test10(long long v) {
   takes_int(v); // expected-warning {{implicit conversion loses integer precision}}
   takes_long(v);
   takes_longlong(v);
-  takes_float(v);
-  takes_double(v);
+  takes_float(v);  // expected-warning {{implicit conversion from 'long long' to 'float' may lose precision}}
+  takes_double(v); // expected-warning {{implicit conversion from 'long long' to 'double' may lose precision}}
   takes_longdouble(v);
 }
 

clang/test/Sema/ext_vector_casts.c

@@ -115,12 +115,12 @@ static void splats(int i, long l, __uint128_t t, float f, double d) {
   vl = vl + t; // expected-warning {{implicit conversion loses integer precision}}
 
   vf = 1 + vf;
-  vf = l + vf;
+  vf = l + vf; // expected-warning {{implicit conversion from 'long' to 'float2' (vector of 2 'float' values) may lose precision}}
   vf = 2.0 + vf;
   vf = d + vf; // expected-warning {{implicit conversion loses floating-point precision}}
-  vf = vf + 0xffffffff;
+  vf = vf + 0xffffffff; // expected-warning {{implicit conversion from 'unsigned int' to 'float2' (vector of 2 'float' values) changes value from 4294967295 to 4294967296}}
   vf = vf + 2.1; // expected-warning {{implicit conversion loses floating-point precision}}
-  
-  vd = l + vd;
-  vd = vd + t;
+
+  vd = l + vd; // expected-warning {{implicit conversion from 'long' to 'double2' (vector of 2 'double' values) may lose precision}}
+  vd = vd + t; // expected-warning {{implicit conversion from '__uint128_t' (aka 'unsigned __int128') to 'double2' (vector of 2 'double' values) may lose precision}}
 }

clang/test/Sema/implicit-int-float-conversion.c

@@ -0,0 +1,53 @@
+// RUN: %clang_cc1 %s -verify -Wno-conversion -Wimplicit-int-float-conversion
+
+long testReturn(long a, float b) {
+  return a + b; // expected-warning {{implicit conversion from 'long' to 'float' may lose precision}}
+}
+
+void testAssignment() {
+  float f = 222222;
+  double b = 222222222222L;
+
+#ifndef __ILP32__
+  float ff = 222222222222L;    // expected-warning {{implicit conversion from 'long' to 'float' changes value from 222222222222 to 222222221312}}
+  float ffff = 222222222222UL; // expected-warning {{implicit conversion from 'unsigned long' to 'float' changes value from 222222222222 to 222222221312}}
+#else
+  float ff = 222222222222L;    // expected-warning {{implicit conversion from 'long long' to 'float' changes value from 222222222222 to 222222221312}}
+  float ffff = 222222222222UL; // expected-warning {{implicit conversion from 'unsigned long long' to 'float' changes value from 222222222222 to 222222221312}}
+#endif
+
+  long l = 222222222222L;
+  float fff = l; // expected-warning {{implicit conversion from 'long' to 'float' may lose precision}}
+}
+
+void testExpression() {
+  float a = 0.0f;
+
+#ifndef __ILP32__
+  float b = 222222222222L + a; // expected-warning {{implicit conversion from 'long' to 'float' changes value from 222222222222 to 222222221312}}
+#else
+  float b = 222222222222L + a; // expected-warning {{implicit conversion from 'long long' to 'float' changes value from 222222222222 to 222222221312}}
+#endif
+
+  float g = 22222222 + 22222222;
+  float c = 22222222 + 22222223; // expected-warning {{implicit conversion from 'int' to 'float' changes value from 44444445 to 44444444}}
+
+  int i = 0;
+  float d = i + a; // expected-warning {{implicit conversion from 'int' to 'float' may lose precision}}
+
+  double e = 0.0;
+  double f = i + e;
+}
+
+void testCNarrowing() {
+  // Since this is a C file. C++11 narrowing is not in effect.
+  // In this case, we should issue warnings.
+#ifndef __ILP32__
+  float a = {222222222222L}; // expected-warning {{implicit conversion from 'long' to 'float' changes value from 222222222222 to 222222221312}}
+#else
+  float a = {222222222222L};   // expected-warning {{implicit conversion from 'long long' to 'float' changes value from 222222222222 to 222222221312}}
+#endif
+
+  long b = 222222222222L;
+  float c = {b}; // expected-warning {{implicit conversion from 'long' to 'float' may lose precision}}
+}

clang/test/Sema/implicit-int-float-narrowing.cpp

@@ -0,0 +1,10 @@
+// RUN: %clang_cc1 %s -verify -Wno-conversion -Wno-c++11-narrowing -Wimplicit-int-float-conversion
+
+void testNoWarningOnNarrowing() {
+  // Test that we do not issue duplicated warnings for
+  // C++11 narrowing.
+  float a = {222222222222L}; // expected-no-diagnostics
+
+  long b = 222222222222L;
+  float c = {b}; // expected-no-diagnostics
+}