[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 a comment.

In https://reviews.llvm.org/D44883#1054326, @thakis wrote:

> In https://reviews.llvm.org/D44883#1048751, @dblaikie wrote:
>
> > Historically Clang's policy on warnings was, I think, much more
> >  conservative than it seems to be today. There was a strong desire not to
> >  implement off-by-default warnings, and to have warnings with an
> >  exceptionally low false-positive rate - maybe the user-defined operator
> >  detection was either assumed to, or demonstrated to, have a sufficiently
> >  high false positive rate to not meet that high bar.
>
>
> This is still the case. For a new warning, you should evaluate some large open-source codebase and measure true positive and false positive rate and post the numbers here.


I did run it on stage-2 of LLVM itself. The only 'false-positives' so far are already present in trunk:
https://godbolt.org/g/SXc4Wd

  void test_int () {
      int b;
      static_assert(noexcept(b &= b), "" );
  }
  void test_byte () {
      std::byte b;
      static_assert(noexcept(b &= (std::byte &)b), "" );
  }

  <source>:4:30: warning: explicitly assigning value of variable of type 'int' to itself [-Wself-assign]
  
      static_assert(noexcept(b &= b), "" );
  
                             ~ ^  ~

Should it really warn when in unevaluated context?
Dunno, but trunk already does that. Should that be changed?

Which other large codebase do you want me to evaluate, so we can talk facts?

>> (as for the flag splitting - that was sometimes done if the new variant of
>>  a flag had enough bug-finding power that an existing codebase using the
>>  existing flag behavior would need significant cleanup - by having the new
>>  functionality under another flag name, existing codebases upgrading to a
>>  newer compiler wouldn't be forced to either do all that cleanup up-front or
>>  disable the flag & risk regressions... )




Repository:
  rC Clang

https://reviews.llvm.org/D44883