Is -Wreturn-std-move clang warning correct in case of objects in the same hierarchy?

Question

Consider the following simple code:

struct Base
{
  Base() = default;      
  Base(const Base&);      
  Base(Base&&);
};

struct Derived : Base { };

Base foo()
{
  Derived derived;
  return derived;
}

clang 8.0.0 gives a warning -Wreturn-std-move on it:

prog.cc:21:10: warning: local variable 'derived' will be copied despite being returned by name [-Wreturn-std-move]
  return derived;
         ^~~~~~~
prog.cc:21:10: note: call 'std::move' explicitly to avoid copying
  return derived;
         ^~~~~~~
         std::move(derived)

But if one called std::move here the behavior of the code might change because the Base subobject of the Derived object would be moved before the calling of the destructor of the Derived object and the code of the last would behave differently.

E.g. look at the code (compiled with the -Wno-return-std-move flag):

#include <iostream>
#include <iomanip>

struct Base
{
  bool flag{false};

  Base()
  {
    std::cout << "Base construction" << std::endl;
  }

  Base(const bool flag) : flag{flag}
  {
  }

  Base(const Base&)
  {
    std::cout << "Base copy" << std::endl;
  }

  Base(Base&& otherBase)
  : flag{otherBase.flag}
  {
    std::cout << "Base move" << std::endl;
    otherBase.flag = false;
  }

  ~Base()
  {
    std::cout << "Base destruction" << std::endl;
  }
};

struct Derived : Base
{
  Derived()
  {
    std::cout << "Derived construction" << std::endl;
  }

  Derived(const bool flag) : Base{flag}
  {
  }

  Derived(const Derived&):Base()
  {
    std::cout << "Derived copy" << std::endl;
  }

  Derived(Derived&&)
  {
    std::cout << "Derived move" << std::endl;
  }

  ~Derived()
  {
    std::cout << "Derived destruction" << std::endl;
    std::cout << "Flag: " << flag << std::endl;
  }
};

Base foo_copy()
{
  std::cout << "foo_copy" << std::endl;
  Derived derived{true};
  return derived;
}

Base foo_move()
{
  std::cout << "foo_move" << std::endl;
  Derived derived{true};
  return std::move(derived);
}

int main()
{
  std::cout << std::boolalpha;
  (void)foo_copy();
  std::cout << std::endl;
  (void)foo_move();
}

Its output:

foo_copy
Base copy
Derived destruction
Flag: true
Base destruction
Base destruction

foo_move
Base move
Derived destruction
Flag: false
Base destruction
Base destruction

Answer 1

Clang's warning certainly is correct. Since derived is of a type that differs from the function's return type, in the statement return derived;, the compiler must treat derived as an lvalue, and a copy will occur. And this copy could be avoided by writing return std::move(derived);, making it into an rvalue explicitly. The warning doesn't tell you whether or not you should do this. It just tells you the consequences of what you're doing, and the consequences of using std::move, and lets you make up your own mind.

Your concern is that the destructor of Derived might access the Base state after it has been moved from, which might cause bugs. If such bugs do occur, it is because the author of Derived has made a mistake, not because the user should not have moved the Base subobject. Such bugs could be discovered in the same ways as other bugs, and reported to the author of Derived.

Why do I say this? Because when the author made Base a public base class of Derived, they were promising to the user that they are entitled to use the full Base interface whenever interacting with a Derived object, which includes moving from it. Thus, all member functions of Derived must be prepared to deal with the fact that the user may have modified the Base subobject in any way that Base's interface allows. If this is not desired, then Base could be made a private base class of Derived, or a private data member, rather than a public base class.

Answer 2

Is -Wreturn-std-move clang warning correct in case of objects in the same hierarchy?

Yes, the warning is correct. The current rules for automatic move only happen if overload resolution finds a constructor that takes, specifically, and rvalue reference to that type. In this snippet:

Base foo()
{
  Derived derived;
  return derived;
}

derived is an automatic storage object that is being returned - it's dying anyway, so it's safe to move from. So we try to do that - we treat it as an rvalue, and we find Base(Base&&). That's a viable constructor, but it takes a Base&& - and we need very specifically a Derived&&. So end up copying.

But the copy is wasteful. Why copy when derived is going out of scope anyway? Why use the expensive operation when you can use the cheap one? That's why the warning is there, to remind you to write:

Base foo()
{
  Derived derived;
  return std::move(derived); // ok, no warning
}

Now, if slicing is wrong for this hierarchy, then even copying is doing the wrong thing anyway and you have other problems. But if slicing is acceptable, then you want to move here, not copy, and the language at the moment arguably does the wrong thing. The warning is there to help make sure you do the right thing.

---

In C++20, the original example would actually perform an implicit move due to P1825 (the relevant part comes from P1155).