8

I want to know if there is a safe programming practice that would alert a coder to this subtle behavior when it takes place or, even better, avoid it in the first place.

A user of struct A might not realize there is no move constructor. In their attempt to call the absent ctor they get neither a compiler warning or any run-time indication that a copy ctor was called instead.

An answer below explains the conversion that takes place but I don't see any rationale for this being a good thing. If the constructor taking a const reference as a parameter were missing there would be a compile time error rather than just resolving to the non-const reference version. So why wouldn't an attempt at using move semantics result in a compile time error when there is no move semantics implemented in the class?

Is there a way to avoid this behavior with some compile time option or at least a way to detect it during run time?

One could assert(source is null) after the move if they were anticipating the problem but that is true of so many problems.

Example, when:

struct A {
    A() {...}
    A(A &a) {...}
    A(A const & A) {...}
};

is constructed as in:

A a1;
A a2 = std::move(a1);  //calls const copy (but how would I know?)

this results in the const version of the copy ctor being called. Now two objects might have a pointer to a single resource while one of them is likely to be calling it's destructor soon.

Arbalest
  • 1,095
  • 11
  • 23
  • 6
    "Now two objects might have a pointer to a single resource while one of them is likely to be calling it's destructor soon." Then this is a bug in your program. Fix it. – GManNickG Aug 16 '13 at 18:10
  • create the default one if your class is designed well. `A(A &&) = default` – aaronman Aug 16 '13 at 18:50
  • 1
    @GManNickG - This is a theoretical question and I don't have such bug. The question asks how to detect/avoid the bug. – Arbalest Aug 16 '13 at 20:17
  • @aaronman - I think the question is clear that the "theoretical" programer neither "controls" the source code for `struct A` nor notices the problem. – Arbalest Aug 16 '13 at 20:19
  • 2
    @Arbalest: Your entire question is predicated on copying instead of moving being a bug. You detect it by copying your class and testing it, and obeying the rule of three. Moving has nothing to do with it. – GManNickG Aug 16 '13 at 20:31
  • First line - `I want to know if there is a safe programming practice that would alert a coder to this subtle behavior when it takes place or, even better, avoid it in the first place.` In other words, the compiler ought to warn that _there is no matching ctor_ rather than just resolving to a different one. – Arbalest Aug 16 '13 at 20:46
  • 2
    @Arbalest I guess the short answer is is that you can't. What the other commenters are having trouble with is why you see this as a problem. This is how the struct A was designed. It's saying "I can be copied, but not moved". – zdan Aug 16 '13 at 20:58
  • 1
    @zdan - if someone attempts to copy construct an object with an existing CONST object and there is no copy constructor taking a CONST reference then the compiler will say there is no appropriate constructor in the structure - period. That is logical and safe. Why then, if someone attempts to move-construct an object when there is no move-constructor, does the compiler think that it is OK to resolve the call to a DIFFERENT constructor rather than producing the same error? – Arbalest Aug 16 '13 at 21:13
  • 1
    Just think of moving as an optimization of copying. You don't get warnings when the compiler skips other optimizations; why would you expect one here? – Bill Aug 16 '13 at 21:29

4 Answers4

4

Since std::move returns an rvalue, which is convertible to a const ref that is why the copy ctor is being silently called. You can fix this several ways.

  1. Easy way, if your class has no dynamic allocation just use the default mctor like this.
    A(A &&) = default;

  2. get rid of the const in the cctor, prolly not a good idea but it won't compile than

  3. Just implement your own move constructor, why are you moving to an object if it has no mctor

This is slightly off topic but you can also qualify a member function to only work with a modifiable lvalue like this.

int func()&;

If you are dealing with legacy code here is compile time check for move constructable

aaronman
  • 18,343
  • 7
  • 63
  • 78
  • 1
    the question does not ask how to fix the problem. It asks how to detect/avoid the problem as there is nether compile-time or run-time indications that there is a problem. All three of your suggestions assumes the theoretical programmer "controls" the source code for`struct A` and notices the problem. Since I state that the expected function is missing and the result is that an another is called I know I can provide the missing function. – Arbalest Aug 16 '13 at 20:27
  • 3
    @Arbalest you dont know the rule of 3/5 then, it is **your responsibilty** to implement these things. Do u want the compiler to warn you if you have no move constructor because that is possible – aaronman Aug 16 '13 at 20:31
  • Let me try to be more clear then. Imagine that a programmer is using a legacy API he/she does-not-control and mistakenly attempts to use _move semantics_ that are not supported by the API. The code compiles and runs without an exception at the point of the call. So yes, how do you make the compiler warn you that there is no move constructor. – Arbalest Aug 16 '13 at 20:34
  • @Arbalest i can give u a compile time check for the existence of a mctor would that be acceptable – aaronman Aug 16 '13 at 20:35
  • 3
    @Arbalest: Like I said above, moving has *nothing to do with the bug*. The bug in the legacy API is that its copy-constructor doesn't work. Moves turning into copies is perfectly normal (consider: `T x = get_T();`). So you found a bug in a legacy API: report it and work around it. Wrap it into your own class that bans copy construction and assignment. – GManNickG Aug 16 '13 at 20:44
  • @Arbalest there this is how u can check for the existance of an mctor beyond that i cannot help u – aaronman Aug 16 '13 at 20:52
  • @GManNickG - I did NOT find a bug in a legacy API and I am not asking how to work around it. This is a theoretical question. Answers like: test it, fix it and work around it are not constructive. – Arbalest Aug 16 '13 at 20:55
  • @Arbalest actually these answers are constructive, I suggested you implement the mctor or check for its existence, and GManNickG suggested you write a wrapper which is also a good idea, why should the compiler warn you about using a feature of the language – aaronman Aug 16 '13 at 21:06
  • @aaronman - thanks to the link on how to check if something is move constructable. To answer your question - I am asking the compiler to report that I am calling a function that does not exist rather than silently linking my call to different function. I think you are both misunderstanding the original question. – Arbalest Aug 16 '13 at 21:22
  • @Arbalest: The problem is, like 50% of the time, rvalues are copied, not moved. You'd get a lot of false positives you don't care about. – Mooing Duck Aug 16 '13 at 23:32
3

There isn't a safe programming practice that would alert to such a thing, because doing a copy after std::move is incredibly common and normal. Adding a warning would cause pretty much everything in <algorithm> to start firing warnings, and we'd need a whole new function that we'd use that works exactly like std::move does now. A lot of algorithms rely on copying if there is no move constructor, or moving otherwise, and that is what std::move is for. At best you should be arguing for the existence of a std::force_move.

Secondly, it's completely unnecessary. Take this modified version of the code you showed.

void legacy_api(A a1) {
   A a2 = std::move(a1);
   ...

You say it's a problem that it subtly used an expensive copy rather than a subtle move. I disagree, what it needed was a new instance of the object, potentially destroying the previous. If the code needs another instance, then it needs another instance. Whether or not it destroyed the previous should be completely irrelevant. If the code is moving when it didn't need another instance, well then the lagacy API is clearly pooly written and the aformentioned warning won't help that. I can't think of any reason a function would require a move but no copy, there's no purpose for such a thing.

Finally, "Now two objects might have a pointer to a single resource while one of them is likely to be calling it's destructor soon." No, if that happens then that means that A's copy constructor has a bug, period. Fix the bugs in your code, and the problems go away. Like magic!

Mooing Duck
  • 64,318
  • 19
  • 100
  • 158
  • 1
    Your idea of `std::move` not being the same as a non-existent `std::force_move` gets the crux of the question and pretty much answers the question. As it is then is `std::move` is more like `std::move_if_you_can`. I don't have a bug to resolve or an optimization to make. What I wanted to know (and you answered) is (for what ever scenario - and I don't care what) if there is something like a compiler switch that warned that requests for a move resulted in copies instead. If nothing else it could inform you which of your classes you ought to implement move semantics for later. – Arbalest Aug 16 '13 at 22:09
  • 2
    @Arbalest: Such a thing would create hundreds of warnings for `std::vector`, so would be pointless. Realistically, your best bet is to move anything with an owning pointer member. If you're really lost, use a profiler. – Mooing Duck Aug 16 '13 at 22:13
  • 1
    @Arbalest From [What is move semantics?](http://stackoverflow.com/questions/3106110/what-is-move-semantics), about [`std::move`](http://en.cppreference.com/w/cpp/utility/move) (**emphasis** mine): "This name is a bit unfortunate, because `std::move` simply casts an lvalue to an rvalue; **it does _not_ move anything by itself**. It merely _enables_ moving. Maybe it should have been named `std::cast_to_rvalue` or `std::enable_move`, but we are stuck with the name by now." So I think it doesn't really make sense to want a "force_move" or to speak of "move_if_you_can". [...] – gx_ Aug 17 '13 at 16:55
  • 1
    [...] Now for your question, you would thus seem to want a warning on the implicit conversion of an rvalue reference to an lvalue reference-to-const (aka "const lvalue reference"), e.g. "`warning: implicit conversion from 'A&&' to 'const A&'`". Unfortunately, as Mooing Duck said, it would warn on all the STL. Moreover, given `A a;`, I think that in `const A& r = std::move(a);` there is no actual conversion from `A&&` to `const A&`, but the initialization of `r` with the expression `std::move(a)` (which yields an rvalue of type `A`), binding `r` to `a` (see 8.5.3 [dcl.init.ref] in N3337). – gx_ Aug 17 '13 at 16:56
  • 1
    (Besides, a function taking an rvalue reference parameter (including a move constructor, but also a `void f(A&& a)`) _can_ "steal the guts" of its argument, but is not _required_ to. One could certainly argue about this, but the fact is that you can't "force" an object to be actually moved.) – gx_ Aug 17 '13 at 17:00
  • 2
    @gx_ thanks for your input. I did come across that same post (and other materials) later. I can see that it is essentially `rvalue_cast<>()` and I can also see the point of not calling it that. I also see that move means "move if you can". I guess I was stuck on the lack of symmetry with a common use of const_cast. A calling program/programmer making use of a lot of const-correctness knows right away that some API does not have an appropriate constructor - does not support the intent. Then they put in the const_cast so it works, and future maintainers see it too. – Arbalest Aug 17 '13 at 23:50
  • In the above, never mind about the "appropriate constructor". I mean any API function taking an non-const where further investigation shows that it doesn't modify it's parameter. – Arbalest Aug 18 '13 at 02:20
1

Because std::move returns reference to an rvalue (as in A&&) which is not convertible to a reference to an lvalue(A&), but is convertible to a const reference to an lvalue (const A&).

int x = 5. Here 5 is an rvalue and can never be bound to an lvalue, and that is also the case in your example because you used std::move(a1)

adrian.budau
  • 349
  • 1
  • 6
1

If you are the owner of the class that you want to move, but that class doesn't have a move constructor because you don't want to move it, you can write:

class A
{
public:
   // Other constuctors

   A(A&&) = delete;
};

So, when you try to move A, if I'm not wrong, a compiler error is thrown. If you want the object is always moved, you should write your own move constructor of course, or to enable the default move constructor:

 A(A&&) = default;

If you are not the owner of the class, I think there is no direct ways to avoid calling the copy constructor. Perhaps a possible idiom to force it is the following:

template<bool b, typename T>
using enabling = typename std::enable_if<b, T>::type;

template<typename T>
constexpr bool movable()
{
   return std::is_move_constructible<T>::value;
}

template<typename T>
enabling<movable<T>(), T&&> movify(T&& t)
{
    return std::move(t);
}

A a1;
A a2 = movify(a1);

That should work. I haven't test that, but I thing you have caught the idea.

ABu
  • 10,423
  • 6
  • 52
  • 103