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

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman 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.
----------------
nickdesaulniers wrote:
> 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?
Sorry, my explanation may have been confusing. A better way to phrase it would have been "this isn't doing what the old code used to do, and I don't think you intended to change it." I was talking about the `!` that disappeared.

I think you can use linemarker directives to get this to happen:
```
# 1 "systemheader.h" 1 3
#define MACRO

# 1 "test_file.c" 2
// Your code that uses MACRO.
```


Repository:
  rC Clang

https://reviews.llvm.org/D50467