It would be nice to have something like -Wno-effect which will issue warnings on usage of std::move for objects that cannot be moved. See also http://stackoverflow.com/q/28595117/230744 I assume that it may require introducing differentiation between explicit-rvalue (through std::move()/cast) and implicit-rvalue. This may save from errors like: ``` const Frame& x = stack.top(); Value y = std::move(x.value); // x.value - non-mutable ``` Though there might be a false-positives in template functions or when developer expects that in future function will have move overload for that argument.
Could you please provide a complete self-contained example? The snippet you posted looks like it's missing some declarations: $ /usr/local/bin/g++ -c 67906.cc 67906.cc:1:7: error: ‘Frame’ does not name a type const Frame& x = stack.top(); ^~~~~ 67906.cc:2:1: error: ‘Value’ does not name a type Value y = std::move(x.value); // x.value - non-mutable ^~~~~ $ Anyways, bug 81159 is related, but that's about a different misuse of std::move, so I'll keep the 2 separate. Oh, and also "-Wno-effect" would probably be a bad name for the option, since "-Wno-" is reserved for negative versions of warnings. i.e., is "-Wno-effect" the negative of "-Weffect"? That seems wrong. Or is it already in the positive, in which case the negative is "-Wno-no-effect"? That would seem redundant.
Sure, struct Value { Value(); Value(const Value&); Value(Value&&); }; struct Frame { Value value; // previously mutable }; Frame top; const Frame& x = top; Value y = std::move(x.value); https://godbolt.org/g/v24FfQ Thank you for looking into it. Yes, there should be better names than -Wno-effect. Maybe -Wignored-move and -Wmove-const are slightly better. Such warning will help to identify places which become ineffecient after changing constness of something.
Sure, struct Value { Value(); Value(const Value&); Value(Value&&); }; struct Frame { Value value; // previously mutable }; Frame top; const Frame& x = top; Value y = std::move(x.value); Thank you for looking into it. Yes, there should be better names than -Wno-effect. Maybe -Wignored-move and -Wmove-const are slightly better. Such warning will help to identify places which become ineffecient after changing constness of something.
Hello Sure, struct Value { Value(); Value(const Value&); Value(Value&&); }; struct Frame { Value value; // previously mutable }; Frame top; const Frame& x = top; Value y = std::move(x.value); https://godbolt.org/g/v24FfQ Thank you for looking into it. Yes, there should be better names than -Wno-effect. Maybe -Wignored-move and -Wmove-const are slightly better. Such warning will help to identify places which become ineffecient after changing constness of something. P.S. By some reason I were not able to leave comment in Bugzilla and got message "User account creation filtered due to spam." though I were logged in. Thank you, Mykola On Thu, Aug 24, 2017 at 7:40 AM egallager at gcc dot gnu.org < gcc-bugzilla@gcc.gnu.org> wrote: > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67906 > > Eric Gallager <egallager at gcc dot gnu.org> changed: > > What |Removed |Added > > ---------------------------------------------------------------------------- > Keywords| |diagnostic > Status|UNCONFIRMED |WAITING > Last reconfirmed| |2017-08-24 > CC| |egallager at gcc dot > gnu.org > Ever confirmed|0 |1 > > --- Comment #1 from Eric Gallager <egallager at gcc dot gnu.org> --- > Could you please provide a complete self-contained example? The snippet you > posted looks like it's missing some declarations: > > $ /usr/local/bin/g++ -c 67906.cc > 67906.cc:1:7: error: ‘Frame’ does not name a type > const Frame& x = stack.top(); > ^~~~~ > 67906.cc:2:1: error: ‘Value’ does not name a type > Value y = std::move(x.value); // x.value - non-mutable > ^~~~~ > $ > > Anyways, bug 81159 is related, but that's about a different misuse of > std::move, so I'll keep the 2 separate. > > Oh, and also "-Wno-effect" would probably be a bad name for the option, > since > "-Wno-" is reserved for negative versions of warnings. i.e., is > "-Wno-effect" > the negative of "-Weffect"? That seems wrong. Or is it already in the > positive, > in which case the negative is "-Wno-no-effect"? That would seem redundant. > > -- > You are receiving this mail because: > You reported the bug.
Thanks. Testcase still needs: #include <utility> at the top, but once I've added that, confirmed that there's no warning about the usage of std::move.
On 8/24/17, Mykola Orliuk <virkony@gmail.com> wrote: > Hello > > Sure, > > struct Value { > Value(); > Value(const Value&); > Value(Value&&); > }; > > struct Frame { > Value value; // previously mutable > }; > > Frame top; > const Frame& x = top; > Value y = std::move(x.value); > > > https://godbolt.org/g/v24FfQ > > Thank you for looking into it. Yes, there should be better names than > -Wno-effect. Maybe -Wignored-move and -Wmove-const are slightly better. > > Such warning will help to identify places which become ineffecient after > changing constness of something. > > P.S. By some reason I were not able to leave comment in Bugzilla and got > message "User account creation filtered due to spam." though I were logged > in. > No, your comment still went through, 3 times in fact. The "User account creation filtered due to spam" message is pinned to the top for display to everyone; it doesn't actually mean that anything is wrong with your account. > Thank you, > Mykola > > On Thu, Aug 24, 2017 at 7:40 AM egallager at gcc dot gnu.org < > gcc-bugzilla@gcc.gnu.org> wrote: > >> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67906 >> >> Eric Gallager <egallager at gcc dot gnu.org> changed: >> >> What |Removed |Added >> >> ---------------------------------------------------------------------------- >> Keywords| |diagnostic >> Status|UNCONFIRMED |WAITING >> Last reconfirmed| |2017-08-24 >> CC| |egallager at gcc dot >> gnu.org >> Ever confirmed|0 |1 >> >> --- Comment #1 from Eric Gallager <egallager at gcc dot gnu.org> --- >> Could you please provide a complete self-contained example? The snippet >> you >> posted looks like it's missing some declarations: >> >> $ /usr/local/bin/g++ -c 67906.cc >> 67906.cc:1:7: error: ‘Frame’ does not name a type >> const Frame& x = stack.top(); >> ^~~~~ >> 67906.cc:2:1: error: ‘Value’ does not name a type >> Value y = std::move(x.value); // x.value - non-mutable >> ^~~~~ >> $ >> >> Anyways, bug 81159 is related, but that's about a different misuse of >> std::move, so I'll keep the 2 separate. >> >> Oh, and also "-Wno-effect" would probably be a bad name for the option, >> since >> "-Wno-" is reserved for negative versions of warnings. i.e., is >> "-Wno-effect" >> the negative of "-Weffect"? That seems wrong. Or is it already in the >> positive, >> in which case the negative is "-Wno-no-effect"? That would seem >> redundant. >> >> -- >> You are receiving this mail because: >> You reported the bug. >
I don't think this is a useful enhancement request as currently written. What are the precise semantics of the new warning that you want? Which cases should warn, and which should not? "std::move with no effect" is not clear enough, because the intended effect of std::move is simply to cast to an rvalue, and it always does that. A type that only has a copy constructor and no move constructor could be used in generic code, where that case "has no effect" but that doesn't mean it should warn e.g. struct A { A { } A(const X&) { } }; template<typename T> struct identity { using type = T; }; template<typename T> bool f(typename identity<T>::type&& t) { T other = std::move(t); /* ... */ return true; } A a; bool b = f<A>(a); This presumably shouldn't warn. Should there be a warning for the following code? struct X { X() { } X(const X&) { } X(const X&&) { } }; const X x; const X y = std::move(x); Although constructors taking a const rvalue reference are rare and not useful in practice, this code has entirely well-defined meaning. Should there be a warning?
Kinda related: PR 86981
Yes. Return value optimization blocked by abusing std::move also close. Somehow I had feeling that I saw something similar from gcc in pre-C++11 times. Regarding sample: struct X { X() { } X(const X&) { } X(const X&&) { } }; const X x; const X y = std::move(x); It shouldn't warn because std::move is effective here and makes choice between default (const X&) and (const X&&) overloads. I.e. we are not ignoring the fact that it is rvalue. As I stated initially I agree that most likely templates will cause false triggering. Like for other warnings (ex. -Wduplicated-branches ). The main point here was to identify places that potentially migrated incompletely. Kinda temporary flag. Trigger rule might be: cast to rvalue for overloads one of which have slightly different qualifiers combinations (e.g. const vs non-const). That should a bit reduce chances of false trigerring compared to "whenever we ignore rvalue". If someone will want more noise they can request adding -Wsuggest-move-overload to supported flags :)
(In reply to Jonathan Wakely from comment #8) > Kinda related: PR 86981 cc-ing Marek since he fixed that
I'm going to add a testcase that ensures that we don't warn for struct X { X() { } X(const X&) { } X(const X&&) { } }; const X x; const X y = std::move(x); but otherwise I don't see any useful extension to our std::move warnings.
The trunk branch has been updated by Marek Polacek <mpolacek@gcc.gnu.org>: https://gcc.gnu.org/g:177e93e95272e9b373203dee5b28d2b284ffa05e commit r13-2099-g177e93e95272e9b373203dee5b28d2b284ffa05e Author: Marek Polacek <polacek@redhat.com> Date: Wed Aug 17 15:43:45 2022 -0400 c++: Add new std::move test [PR67906] As discussed in 67906, let's make sure we don't warn about a std::move when initializing when there's a T(const T&&) ctor. PR c++/67906 gcc/testsuite/ChangeLog: * g++.dg/cpp0x/Wredundant-move11.C: New test.
Test added.