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



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

Reply via email to