Details
Diff Detail
- Repository
- rCXX libc++
Event Timeline
test/std/language.support/support.types/byteops/and.assign.pass.cpp | ||
---|---|---|
30 ↗ | (On Diff #140553) | Should Clang really be warning when the expression is in an unevaluated context? |
test/std/language.support/support.types/byteops/and.assign.pass.cpp | ||
---|---|---|
30 ↗ | (On Diff #140553) | At least that is what it currently already does: |
test/std/language.support/support.types/byteops/and.assign.pass.cpp | ||
---|---|---|
30 ↗ | (On Diff #140553) | 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. 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? |
test/std/language.support/support.types/byteops/and.assign.pass.cpp | ||
---|---|---|
30 ↗ | (On Diff #140553) |
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.
Absolutely, but static analyzer, not compiler diagnostic.
Not unless you want to completely disable -Wself-assign (as in, not just the new part from D44883) for tests, no. |
The diagnostic was adjusted not to warn in an unevaluated context.
The remaining diff will have to remain, unless you want to disable -Wself-assign altogether for tests...
If there are remaining problems with this diff, please let me know; otherwise it would be really great to get this accepted!
LGTM. Perhaps we could add a comment for the first such occurrence of the cast in each file explaining it.