[PATCH] D44883: [Sema] Extend -Wself-assign and -Wself-assign-field to warn on overloaded self-assignment (classes)

[Roman Lebedev via Phabricator via cfe-commits]

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