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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
