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

[Oksana Shadura via Phabricator via cfe-commits]

ksu.shadura added a comment.

Hi,

we see the false-positive behavior of -Wno-self-assign-overloaded flag in case 
of subtraction assignment operator. 
The minimal reproducer that we got: https://godbolt.org/g/8PQMpR

  typedef int Int_t;
  typedef double Double_t;
  class TObject {};
  template  class TMatrixTBase : public TObject {};
  template  class TMatrixT : public TMatrixTBase {
  public:
enum EMatrixCreatorsOp1 {
  kZero,
  kUnit,
  kTransposed,
};
TMatrixT(){};
TMatrixT(Int_t row_lwb, Int_t row_upb, Int_t col_lwb, Int_t col_upb){};
TMatrixT(EMatrixCreatorsOp1 op, const TMatrixT &prototype){};
TMatrixT &operator-=(const TMatrixT &source){return 
*this;};
  };
  
  typedef TMatrixT TMatrixD;
  void stress_binary_ebe_op(Int_t rsize, Int_t csize) {
TMatrixD m(2, rsize + 1, 0, csize - 1);
TMatrixD m1(TMatrixD::kZero, m);
m1 -= m1;
  }

Thanks in advance!


Repository:
  rL LLVM

https://reviews.llvm.org/D45766



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Oksana Shadura via Phabricator via cfe-commits]

ksu.shadura added a comment.

Thank you for the test example! I got your point, but I wanted to ask if it 
should be like this for commutative operations?
In our case it is actually matrix, and subtraction of matrices is not 
commutative operation..


Repository:
  rL LLVM

https://reviews.llvm.org/D45766



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Oksana Shadura via Phabricator via cfe-commits]

ksu.shadura added a comment.

In https://reviews.llvm.org/D45766#1098823, @Quuxplusone wrote:

> In https://reviews.llvm.org/D45766#1097797, @ksu.shadura wrote:
>
> > Thank you for the test example! I got your point, but I wanted to ask if it 
> > should be like this for commutative operations?
> >  In our case it is actually matrix, and subtraction of matrices is not 
> > commutative operation..
>
>
> Mathematical commutativity has nothing to do with this diagnostic. The 
> diagnostic is complaining that `m1 -= m1` produces a trivial effect (in this 
> case `m1.clear()`) via a syntax that might indicate that the programmer made 
> a typo (i.e. the compiler suspects that you meant `m -= m1` or something).
>  From the function name `stress_...`, I infer that this is a unit-test. This 
> warning is known to give "false positives" in unit-test code, where one often 
> actually wants to achieve trivial effects through complicated code. The 
> current recommendation there, AFAIK, is either to write something like
>
>   m1 -= static_cast(m1);  // hide the self-subtraction from the 
> compiler
>   
>
> or else to add `-Wno-self-assign-overloaded` to your compiler flags. 
> (libc++'s test suite already passes `-Wno-unused-command-line-argument 
> -Wno-attributes -Wno-pessimizing-move -Wno-c++11-extensions 
> -Wno-user-defined-literals -Wno-conversion -Wno-unused-local-typedef 
> -Wno-#warnings`, for example.)
>  (That said, I continue to think that this diagnostic produces more noise 
> than signal, and wish it weren't in `-Wall`...)


Thanks for your opinion, your guess was right! It is a unit-test, so seems we 
need to try to suppress warnings on our side.


Repository:
  rL LLVM

https://reviews.llvm.org/D45766



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits