[PATCH] D45128: [libcxx][test] Silence -Wself-assign diagnostics
Arthur O'Dwyer via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Sat Mar 31 14:54:41 PDT 2018
Quuxplusone 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), "" );
----------------
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`?
Repository:
rCXX libc++
https://reviews.llvm.org/D45128
More information about the cfe-commits
mailing list