lebedev.ri added inline comments.
================ Comment at: lib/Sema/SemaExpr.cpp:12093 + break; + } + ---------------- rjmccall wrote: > lebedev.ri wrote: > > rjmccall wrote: > > > I think doing this here can result in double-warning if the overload > > > resolves to a builtin operator. Now, it might not actually be possible > > > for that to combine with the requirements for self-assignment, but still, > > > I think the right place to diagnose this for C++ is the same place we > > > call DiagnoseSelfMove in CreateOverloadedBinOp. > > > > > > Can CheckIdentityFieldAssignment just be integrated with > > > DiagnoseSelfAssignment so that callers don't need to do call both? > > > I think the right place to diagnose this for C++ is the same place we > > > call DiagnoseSelfMove in CreateOverloadedBinOp. > > > > ``` > > switch (Opc) { > > case BO_Assign: > > case BO_DivAssign: > > case BO_SubAssign: > > case BO_AndAssign: > > case BO_OrAssign: > > case BO_XorAssign: > > DiagnoseSelfAssignment(Args[0], Args[1], OpLoc); > > CheckIdentityFieldAssignment(Args[0], Args[1], OpLoc); > > break; > > default: > > break; > > } > > > > // Check for a self move. > > if (Op == OO_Equal) > > DiagnoseSelfMove(Args[0], Args[1], OpLoc); > > ``` > > > > > > ^ That does not appear to work. Pretty much all these tests start to fail. > > > Okay. It's possible that my suggestion is wrong. Can you explain more how > they fail? Right, i should have been verbose :) There are no test changes as compared to the current diff. Here is the output of `$ ninja check-clang-sema check-clang-semacxx` {F5920055} It is also totally possible that i'm missing something obvious on my end... Repository: rC Clang https://reviews.llvm.org/D44883 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits