[PATCH] D45128: [libcxx][test] Silence -Wself-assign diagnostics

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added inline comments.


================
Comment at: test/std/language.support/support.types/byteops/and.assign.pass.cpp:30
 
-    static_assert(noexcept(b &= b), "" );
+    static_assert(noexcept(b &= (std::byte &)b), "" );
 
----------------
Quuxplusone wrote:
> lebedev.ri wrote:
> > EricWF wrote:
> > > Should Clang really be warning when the expression is in an unevaluated context?
> > At least that is what it currently already does:
> > https://godbolt.org/g/RpcgBi
> > 
> Clang already warns on `noexcept(b = b)` that "expression with side effects has no effect in an unevaluated context [-Wunevaluated-expression]", so adding one more warning should be acceptable, I think.
> Both warnings are (IMO correctly) disabled if the expression `b = b` is dependently typed, e.g. in a template, where expressions like `noexcept(b = b)` are practically unavoidable.
> 
> Personally I think the warnings are acceptable, but these proposed fixes (inserting C-style casts to `T&`) are not good. The cast hides the intent of the code (especially in the self-assignment tests for `function` and `any`). We shouldn't be encouraging that particular workaround in user code. And besides, a good static analyzer (or even Clang itself) should be able to see that "casting `a` to the type of `a`" doesn't change the object referred to, and so the self-assignment is still happening, regardless.
> 
> How do the `std::byte` tests avoid the existing warning for `-Wunevaluated-expression`? Could we use the same mechanism to avoid the new warning for `-Wself-assign`?
> 
> Could we reuse that mechanism in the tests for `byte`'s `-=` and `/=` and `%=` and `^=` operators, which may one day start complaining as loudly as `&=` and `|=` when you have the same thing on the left and right hand sides?
> 
> Could we reuse that mechanism in the tests for `function` and `any`?
> but these proposed fixes (inserting C-style casts to T&) are not good. The cast hides the intent of the code (especially in the self-assignment tests for function and any). We shouldn't be encouraging that particular workaround in user code. 

Please refer to the D45082 for disscussion on that, and on whether or not to add a more fine-grained sub-flag to `-Wself-assign`.

> And besides, a good static analyzer (or even Clang itself) should be able to see that "casting a to the type of a" doesn't change the object referred to, and so the self-assignment is still happening, regardless.

Absolutely, but static analyzer, not compiler diagnostic.

> Could we use the same mechanism to avoid the new warning for `-Wself-assign`?

Not unless you want to completely disable `-Wself-assign` (as in, not just the new part from D44883) for tests, no.



Repository:
  rCXX libc++

https://reviews.llvm.org/D45128