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