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

[Andrew V. Tischenko via Phabricator via llvm-commits]

avt77 added inline comments.


================
Comment at: lib/Sema/SemaChecking.cpp:9152
 
+static bool ShouldWarnFloatPrecision(Sema &S, Expr *E,
+                                     const BuiltinType *SourceBT,
----------------
ABataev wrote:
> Function names should follow coding standard, `shouldWarnFloatPrecision`.
That's a problem. Look around - they use capital.


================
Comment at: lib/Sema/SemaChecking.cpp:9158
+  // representable in the target type.
+  Expr::EvalResult result;
+  if (E->EvaluateAsRValue(result, S.Context)) {
----------------
ABataev wrote:
> The names of the variables should start from capital letter, `Result`
And they use 'result'. Ok, I'll follow standard here.


================
Comment at: lib/Sema/SemaChecking.cpp:9166-9167
+  }
+  if (S.SourceMgr.isInSystemMacro(CC))
+    return true;
+  return true;
----------------
ABataev wrote:
> Do you need this check? With or without it you just return `true`.
Many tnx - it's a bug; I should return false.


https://reviews.llvm.org/D42728