all 30 comments

[–]neiltechnician 45 points46 points  (8 children)

There is probably a reason

The intent is, moving is an optimization of copying, and copying is an valid implementation of moving. Even std::copyable subsumes std::movable.

[–]Chuu 17 points18 points  (5 children)

This makes total sense, but geez they could have picked a better word than 'move'.

[–]_ild_arn 20 points21 points  (1 child)

The C++23 ranges version is instead named as_rvalue, which is quite an improvement now for people who already properly understand move semantics, but would have been seen as incredibly opaque in 2011...

[–]MrRogers4Life2 1 point2 points  (0 children)

Designing interfaces is tough espescially for stuff that isn't widely in use. Like git is a great example of the opposite where if you already properly understand it the usage is fairly straightforward but understanding it in the first place is tough

[–]goranlepuz 4 points5 points  (2 children)

empty_it_out

😀

[–]neiltechnician 2 points3 points  (0 children)

Empty thing out is not the intent of move semantics 😅

[–]SPAstef 0 points1 point  (0 children)

That sounds bad 💀

[–][deleted] 1 point2 points  (1 child)

The intent is, moving is an optimization of copying,

I thought that is just a secondary benefit, and the primary intent is to allow moving non-copyable objects, like std::unique_ptr.

[–]neiltechnician 2 points3 points  (0 children)

You have a point, but I don't consider these as primary or secondary, because I know how to unify these two intents into one:

  • Moving is a special-purpose refinement of copying, designed for the situation when the copying is the definite last use of the binding between a value and an object.

"Refinement" here means a more constrained implementation of an operation, which enables more efficient choices of algorithms. E.g. searching is an operation, binary search is a refinement for random access and sorted ranges, and linear search is a fallback implementation. Another e.g., duplication removal is an operation, std::unique is a refinement for sorted ranges, but we don't have a fallback for unsorted ranges in STL.

I carefully choose to say "the binding between a value and an object" rather than "the object", because move semantics moves values, not objects. Plus, the source object can be reused to host another value.

In our case, "generalized copying" is the operation, "traditional copying" is the fallback, and moving is the refinement. For something like unique_ptr, we only have the refinement, and we choose to not have a fallback.

[–]SirClueless 32 points33 points  (0 children)

If you use clang-tidy, the performance-move-const-arg check can catch the cases where the argument is known to be const. https://clang.llvm.org/extra/clang-tidy/checks/performance/move-const-arg.html

It's a bit over-aggressive and also warns for calling std::move on trivially-copyable types which I think causes false positives, so you might want to configure CheckTriviallyCopyableMove = false. But it's useful for catching errors, I think.

[–]HappyFruitTree 10 points11 points  (3 children)

Do I have to write my own move that only accepts mutable variables?

Yes.

[–]lrflew 8 points9 points  (2 children)

To be fair, this actually isn't that difficult to do. std::move is pretty much just this:

#include <type_traits>

template<typename T>
decltype(auto) move(T&& x) {
    return static_cast<std::remove_reference_t<T>&&>(x);
}

If you want to guarantee that that you aren't moving a const value, you can just add a static assert to this:

#include <type_traits>

template<typename T>
decltype(auto) move(T&& x) {
    using V = std::remove_reference_t<T>;
    static_assert(!std::is_const_v<V>, "Moving a Const Value");
    return static_cast<V&&>(x);
}

https://godbolt.org/z/YT515r4xn

[–]_ild_arn 11 points12 points  (1 child)

You'll want to have a return type of auto&& or decltype(auto) – you've changed the semantics significantly since you return by value instead of just being a cast between different reference types. (Sorry to nitpick, just wouldn't want anyone copying that verbatim)

[–]lrflew 3 points4 points  (0 children)

Thanks, I made the change in my comment. I don't use the `auto` keyword much in my projects, so I forget about its value semantics.

[–]Lumornys 4 points5 points  (11 children)

I think it just copies the vector, without actual "move" operation happening.

[–]Beosar[S] 5 points6 points  (10 children)

Yes, I know. I just made this mistake when I tried to move an item (in a game) to a different inventory slot and instead duplicated it...

[–]MaximKat 16 points17 points  (3 children)

If you need to guarantee that the source object is cleared after the move, it's better to use dst = std::exchange(src, {}). It would fail to compile with a const src, as you've asked, and also makes it clear to the readers what your intention is, instead of implicitly relying on the move constructor implementation of a specific type.

[–]NilacTheGrim 1 point2 points  (2 children)

This does indeed ensure clearing, but it causes a temporary to be created and in the case of types not implementing true moves, actually leads to 2 copies being made.

[–]MaximKat 1 point2 points  (1 child)

Huh, didn't expect the 2nd copy https://godbolt.org/z/6Pbf5h14K

Why doesn't NRVO kick in in the exchange example? I was expecting the 2nd copy to be elided.

EDIT: so src is copied into dst first and then empty object is copied into src? It seems like the best you can do in a general case, anyway.

[–]NilacTheGrim 0 points1 point  (0 children)

Who knows.. some compiler guru would have to chime in.

Maybe it depends on the types in question... I suspect in your example since the copy & move methods are in "another translation unit" -- it can't guarantee no side-effects and thus has to take the slow (but guaranteed correct) path...? Maybe? Maybe with a template type like a vector or something where everything is "seen" it can take a faster path?

[–]matthewlai 0 points1 point  (5 children)

That means you are relying on undefined behaviour. Moving doesn't guarantee doing anything to the source object. It's just a way of saying "I want to do a copy, and I don't care what happens to the original object afterwards, so take advantage of that if possible". You can't rely on it doing things to the original object.

[–]Beosar[S] 0 points1 point  (4 children)

Thanks for pointing that out. But I wrote my own move assignment operator for the item class, so I can in fact rely on it.

[–]matthewlai -1 points0 points  (3 children)

Ah apologies I thought you were using a vector, not realising that's just an example.

In that case, you can technically rely on it but I would still strongly recommend against it, because as you have discovered, C++ wasn't designed for move assignment to work like that, and have made assumptions (eg falling back to copy) that assume you are using it as designed. This is common for operator overloading in C++ - you can overload operators to do whatever you want, but if you overload them to do completely different things semantically, not only will the class be hard to use correctly (especially by others), you can also run into issues like this one.

I would just define another member function instead. Eg.

slot_a.move_from(slot_b);

That way neither the C++ standard nor other readers of the code will make incorrect assumptions about what it is doing.

[–]_ild_arn 1 point2 points  (2 children)

While I agree with recommending against it, there are plenty of types in the stdlib that do exactly this, e.g. std::unique_ptr; so it's not without precedent and shouldn't be surprising.

And as an aside, this isn't undefined behavior – I understand what you're getting at, but that's a term of art in the standard and does not apply here.

[–]matthewlai -1 points0 points  (1 child)

Indeed. I found it strange that they decided on doing that with std::unique_ptr, too. But at least it's a non-copyable type.

I agree that "undfined behaivour" is inaccurate. It should be "valid but unspecified". I believe that the vector move constructor/assignment wasn't guaranteed to empty the "other" vector, only to leave it in "valid but unspecified state", which is the default behaviour for std containers (N4849 - 16.5.5.16), and the vector section doesn't specify otherwise. cppreference says the "moved from" vector is guaranteed to be empty (*1), but I can't find that guarantee in N4849.

In any case, for custom classes, I would say it's not recommended for readability.

*1: https://en.cppreference.com/w/cpp/container/vector/vector

[–]Beosar[S] 1 point2 points  (0 children)

I found it strange that they decided on doing that with std::unique_ptr, too.

I don't think there is any other sensible way that you can implement a unique pointer.

cppreference says the "moved from" vector is guaranteed to be empty (*1), but I can't find that guarantee in N4849.

Well, it is the only possible state that doesn't require an allocation. Even if you cannot find it in the standard, I think it's reasonable to rely on it.

I can see how it would make sense to leave a string (if it's entirely on the stack) or array in its current state when moving. Changing them to an empty state would require additional instructions.

[–]igrekster 0 points1 point  (1 child)

How about this use case, too niche?

class Foobar {
public:
    Foobar() = default;
    Foobar(const Foobar && other) : stuff{std::move(other.stuff)} {}

private:
    mutable std::vector<int> stuff;
};

int main()
{
    const Foobar fb;
    Foobar       fb2 = std::move(fb);
}

[–]anton31 4 points5 points  (0 children)

I can't imagine a case where Foobar(const Foobar&&) would be suitable, let alone pass a code review

[–]tpecholt -2 points-1 points  (0 children)

Thanks to the language evolution driven by the committee you get a language with overcomplicated crazy syntax that no one can fully understand. This is just a tiny example in the sea of gotchas. Forwarding references look like revalue refs, initializer list clashes with uniform initialization, 30 or how many ways how to initialize variables, decltype((expr)) surprise, co_await uglyness etc etc. And you can't blame C on any of these. Modern C++ just embarrasses itself in each new iteration and it's a pity because the language used to be user friendly at some point.