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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits