[PATCH] D52835: [Diagnostics] Check integer to floating point number implicit conversions

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

It looks like you removed a considerable amount of testing coverage; why?



================
Comment at: lib/Sema/SemaChecking.cpp:10920-10921
+    if (E->EvaluateAsInt(IntValue, S.Context, Expr::SE_AllowSideEffects)) {
+      if (S.SourceMgr.isInSystemMacro(CC))
+        return;
+      const llvm::fltSemantics &FloatSemantics =
----------------
aaron.ballman wrote:
> It seems wrong to early return here -- that means none of the later checks are run on system macros, but we've also not diagnosed anything as being wrong with the user's code yet.
This changes the behavior of your patch -- I think it made sense to not trigger this diagnostic in a system macro. I was suggesting that you replace the early return with braces and flip the logic around so that you only do the diagnostic work if you're not in a system macro.


https://reviews.llvm.org/D52835