[PATCH] D50467: [SEMA] add more -Wfloat-conversion to compound assigment analysis

Nick Desaulniers via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Fri Aug 10 13:02:59 PDT 2018


nickdesaulniers added inline comments.


================
Comment at: lib/Sema/SemaChecking.cpp:10424
+      // We don't want to warn for system macro.
+      S.SourceMgr.isInSystemMacro(E->getOperatorLoc()))
+    // warn about dropping FP rank.
----------------
aaron.ballman wrote:
> This looks incorrect to me -- this is testing the rank difference and that it is in a system macro (the `!` was dropped). If this didn't cause any tests to fail, we should add a test that would fail for it.
Thanks for the code review!

Sorry, I don't understand what you mean by "rank difference"? F16 vs F32 vs F64 vs F128 etc?

When you say "this looks incorrect," are you commenting on the code I added (Lines 10415 to 10418) or the prexisting code that I moved (Lines 10420-10427)?

Good catch on dropping the `!`, that was definitely not intentional (and now I'm curious if `dw` in vim will delete `(!` together), will add back.

I'm happy to add a test for the system macro, but I also don't know what that is.  Can you explain?


Repository:
  rC Clang

https://reviews.llvm.org/D50467





More information about the cfe-commits mailing list