[PATCH] D45766: [Sema] Add -Wno-self-assign-overloaded

David Blaikie via cfe-commits cfe-commits at lists.llvm.org
Mon Apr 23 14:41:47 PDT 2018


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.

On Mon, Apr 23, 2018 at 2:39 PM Roman Lebedev via Phabricator <
reviews at reviews.llvm.org> wrote:

> lebedev.ri added inline comments.
>
>
> ================
> Comment at: lib/Sema/SemaExpr.cpp:11527-11528
>
> -  S.Diag(OpLoc, diag::warn_self_assignment)
> -      << LHSDeclRef->getType()
> -      << LHSExpr->getSourceRange() << RHSExpr->getSourceRange();
> +  S.Diag(OpLoc, IsBuiltin ? diag::warn_self_assignment_builtin
> +                          : diag::warn_self_assignment_overloaded)
> +      << LHSDeclRef->getType() << LHSExpr->getSourceRange()
> ----------------
> dblaikie wrote:
> > lebedev.ri wrote:
> > > dblaikie wrote:
> > > > Presumably this also changes how the warning is enabled? But that
> doesn't seem to be tested in this patch?
> > > What testing do you have in mind?
> > > The `test/SemaCXX/warn-self-assign-overloaded.cpp` change was supposed
> to show how it is enabled..
> > 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.
> >
> > Maybe testing the negative case would be useful too?
> Added & committed.
> Thank you for the review!
>
>
> Repository:
>   rL LLVM
>
> https://reviews.llvm.org/D45766
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20180423/76a3f0b6/attachment.html>


More information about the cfe-commits mailing list