[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