[PATCH] D45163: [Sema] -Wunused-value: diagnose unused std::move() call results.

2018-04-03 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment. In https://reviews.llvm.org/D45163#1056044, @rjmccall wrote: > It seems reasonable to ask libc++ to use > `__attribute__((warn_unused_result))` if `[[nodiscard]]` is unavailable. https://reviews.llvm.org/D45179 should hopefully add an **opt-in** define to allow doi

[PATCH] D45163: [Sema] -Wunused-value: diagnose unused std::move() call results.

2018-04-03 Thread Arthur O'Dwyer via Phabricator via cfe-commits
Quuxplusone added a comment. In https://reviews.llvm.org/D45163#1055957, @lebedev.ri wrote: > In https://reviews.llvm.org/D45163#1055856, @rjmccall wrote: > > > Well, we should feel welcome to submit patches to the C++ library > > implementations, I think. I can understand why Marshall is waiti

[PATCH] D45163: [Sema] -Wunused-value: diagnose unused std::move() call results.

2018-04-03 Thread John McCall via Phabricator via cfe-commits
rjmccall added a comment. Trust me, I understand that this is an important function, but a lot of functions are important, and we're not going to hardcode knowledge about all of them in the compiler. It seems reasonable to ask libc++ to use `__attribute__((warn_unused_result))` if `[[nodiscard

[PATCH] D45163: [Sema] -Wunused-value: diagnose unused std::move() call results.

2018-04-03 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment. In https://reviews.llvm.org/D45163#1055856, @rjmccall wrote: > Well, we should feel welcome to submit patches to the C++ library > implementations, I think. I can understand why Marshall is waiting to just > do this until he gets a comprehensive committee paper,

[PATCH] D45163: [Sema] -Wunused-value: diagnose unused std::move() call results.

2018-04-03 Thread John McCall via Phabricator via cfe-commits
rjmccall added a comment. Well, we should feel welcome to submit patches to the C++ library implementations, I think. I can understand why Marshall is waiting to just do this until he gets a comprehensive committee paper, but he might consider taking a patch in the meantime. But I'm not sure

[PATCH] D45163: [Sema] -Wunused-value: diagnose unused std::move() call results.

2018-04-03 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment. In https://reviews.llvm.org/D45163#1054994, @Quuxplusone wrote: > `std::move` would definitely be special in this regard if there were a > pressing benefit to be gained — i.e., if people were currently getting bitten > by accidentally discarded calls of `std::move(x)

[PATCH] D45163: [Sema] -Wunused-value: diagnose unused std::move() call results.

2018-04-02 Thread Arthur O'Dwyer via Phabricator via cfe-commits
Quuxplusone added a comment. `std::move` would definitely be special in this regard if there were a pressing benefit to be gained — i.e., if people were currently getting bitten by accidentally discarded calls of `std::move(x)`. But you haven't shown that people are getting bitten today; in fac

[PATCH] D45163: [Sema] -Wunused-value: diagnose unused std::move() call results.

2018-04-02 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment. In https://reviews.llvm.org/D45163#1054925, @Quuxplusone wrote: > > I think it wouldn't be unreasonable to ask standard library maintainers to > > add `[[nodiscard]]` to `std::move` > > +1. Also `std::forward`, for sure. Basically any metaprogramming function > that

[PATCH] D45163: [Sema] -Wunused-value: diagnose unused std::move() call results.

2018-04-02 Thread Arthur O'Dwyer via Phabricator via cfe-commits
Quuxplusone added a comment. > I think it wouldn't be unreasonable to ask standard library maintainers to > add `[[nodiscard]]` to `std::move` +1. Also `std::forward`, for sure. Basically any metaprogramming function that is statically known to return a reference to its argument. This knowledge

[PATCH] D45163: [Sema] -Wunused-value: diagnose unused std::move() call results.

2018-04-02 Thread John McCall via Phabricator via cfe-commits
rjmccall added a comment. That seems reasonable. And this summer would still be in time for the next release. Repository: rC Clang https://reviews.llvm.org/D45163 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi

[PATCH] D45163: [Sema] -Wunused-value: diagnose unused std::move() call results.

2018-04-02 Thread Marshall Clow via Phabricator via cfe-commits
mclow.lists added a comment. So. https://bugs.llvm.org/show_bug.cgi?id=10011 was resolved by https://wg21.link/P0600 (which added `[[nodiscard]]` to `string.empty()` We can do the same for `move`. However, I have been promised a comprehensive paper listing all the (100s?) of places in the stand

[PATCH] D45163: [Sema] -Wunused-value: diagnose unused std::move() call results.

2018-04-02 Thread John McCall via Phabricator via cfe-commits
rjmccall added a comment. Yeah, actually, I'm second-guessing myself. Maybe this should just be a libc++ / libstdc++ bug. Repository: rC Clang https://reviews.llvm.org/D45163 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.

[PATCH] D45163: [Sema] -Wunused-value: diagnose unused std::move() call results.

2018-04-02 Thread Nico Weber via Phabricator via cfe-commits
thakis added a comment. (See also https://bugs.llvm.org/show_bug.cgi?id=10011 for a somewhat related discussion.) Repository: rC Clang https://reviews.llvm.org/D45163 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/

[PATCH] D45163: [Sema] -Wunused-value: diagnose unused std::move() call results.

2018-04-02 Thread John McCall via Phabricator via cfe-commits
rjmccall added a comment. LGTM. I think it wouldn't be unreasonable to ask standard library maintainers to add `[[nodiscard]]` to `std::move`, but it's also not unreasonable for us to special-case some functions. Repository: rC Clang https://reviews.llvm.org/D45163 _

[PATCH] D45163: [Sema] -Wunused-value: diagnose unused std::move() call results.

2018-04-02 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri created this revision. lebedev.ri added reviewers: aaron.ballman, rsmith, rtrieu, rjmccall, dblaikie. I have seen such a problem when reviewing https://reviews.llvm.org/D43341. https://godbolt.org/g/aJYcaa #include struct S {}; void test(S a) { std::move(a); } Sin