[PATCH] D42728: Add more warnings for implict conversions (e.g. double truncation to float).

[John McCall via Phabricator via llvm-commits]

rjmccall added inline comments.


================
Comment at: lib/Sema/SemaChecking.cpp:9218
+         "Must be compound assignment operation");
+  AnalyzeAssignment(S, E);
+
----------------
This feels likely to cause redundant or inappropriate diagnostics.


================
Comment at: lib/Sema/SemaChecking.cpp:9222
+  const BuiltinType *SourceBT = dyn_cast<BuiltinType>(
+      S.Context.getCanonicalType(E->getRHS()->getType()).getTypePtr());
+  const BuiltinType *TargetBT =
----------------
`E->getComputationResultType()->getType()->getAs<BuiltinType>()`


================
Comment at: lib/Sema/SemaChecking.cpp:9224
+  const BuiltinType *TargetBT =
+      dyn_cast<BuiltinType>(S.Context.getCanonicalType(T).getTypePtr());
+
----------------
`T->getAs<BuiltinType>()`


================
Comment at: lib/Sema/SemaChecking.cpp:9231
+    if (SourceBT->getKind() > TargetBT->getKind())
+      if (ShouldWarnFloatPrecision(S, E, SourceBT, TargetBT, E->getOperatorLoc()))
+        // warn about dropping FP rank.
----------------
Should we actually do a suppression for constant values here?  We suppress them for normal assignments because it's free to coerce a constant double to float, but [fixed]myFloat *= 2.0[/fixed] actually does do a computation in [fixed]double[/fixed] that has to be truncated to [fixed]float[/fixed].  Someone using this warning to find places where unwanted implicit floating-point coercions are being used would surely want to know about this location so that they can make the literal [fixed]2.0f[/fixed].


https://reviews.llvm.org/D42728