<div dir="ltr">FWIW I don't fundamentalyl object to also having something like -wtest. Probably needs a better name though (unfortunately the double-negative gets confusing... - like you want to describe the set of diagnostics that should not be used in test code, so that as a group might be "-Wnon-test" but then "-Wno-non-test" is pretty awkward) - probably worth chatting to Richard Smith about that, I reckon.</div><br><div class="gmail_quote"><div dir="ltr">On Mon, Apr 23, 2018 at 2:39 PM Roman Lebedev via Phabricator <<a href="mailto:reviews@reviews.llvm.org">reviews@reviews.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">lebedev.ri added inline comments.<br>
<br>
<br>
================<br>
Comment at: lib/Sema/SemaExpr.cpp:11527-11528<br>
<br>
-  S.Diag(OpLoc, diag::warn_self_assignment)<br>
-      << LHSDeclRef->getType()<br>
-      << LHSExpr->getSourceRange() << RHSExpr->getSourceRange();<br>
+  S.Diag(OpLoc, IsBuiltin ? diag::warn_self_assignment_builtin<br>
+                          : diag::warn_self_assignment_overloaded)<br>
+      << LHSDeclRef->getType() << LHSExpr->getSourceRange()<br>
----------------<br>
dblaikie wrote:<br>
> lebedev.ri wrote:<br>
> > dblaikie wrote:<br>
> > > Presumably this also changes how the warning is enabled? But that doesn't seem to be tested in this patch?<br>
> > What testing do you have in mind?<br>
> > The `test/SemaCXX/warn-self-assign-overloaded.cpp` change was supposed to show how it is enabled..<br>
> ah, misread those - figured they were testing the negative case (given the name of this patch) but I see they're testing the positive case.<br>
> <br>
> Maybe testing the negative case would be useful too?<br>
Added & committed.<br>
Thank you for the review!<br>
<br>
<br>
Repository:<br>
  rL LLVM<br>
<br>
<a href="https://reviews.llvm.org/D45766" rel="noreferrer" target="_blank">https://reviews.llvm.org/D45766</a><br>
<br>
<br>
<br>
</blockquote></div>