Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Automatically detect C++14 "return should use std::move" situation

My understanding is that in C++17, the following snippet is intended to Do The Right Thing:

struct Instrument;  // instrumented (non-trivial) move and copy operations

struct Base {
    Instrument i;
};

struct Derived : public Base {};

struct Unrelated {
    Instrument i;
    Unrelated(const Derived& d): i(d.i) {}
    Unrelated(Derived&& d): i(std::move(d.i)) {}
};

Unrelated test1() {
    Derived d1;
    return d1;
}

Base test2() {
    Derived d2;
    return d2;  // yes, this is slicing!
}

That is, in C++17, the compiler is supposed to treat both d1 and d2 as rvalues for the purposes of overload resolution in those two return statements. However, in C++14 and earlier, this was not the case; the lvalue-to-rvalue transformation in return operands was supposed to apply only when the operand was exactly the correct return type.

Furthermore, GCC and Clang both appear to have confusing and possibly buggy behavior in this area. Trying the above code on Wandbox, I see these outputs:

GCC 4.9.3 and earlier: copy/copy (regardless of -std=)
Clang 3.8.1 and earlier: copy/copy (regardless of -std=)
Clang 3.9.1 and later: move/copy (regardless of -std=)
GCC 5.1.0 through 7.1.0: move/copy (regardless of -std=)
GCC 8.0.1 (HEAD): move/move (regardless of -std=)

So this started out as a tooling question and ended up with a side order of "what the heck is the correct behavior for a C++ compiler?"

My tooling question is: In our codebase, we have several places that say return x; but that accidentally produces a copy instead of a move because our toolchain is GCC 4.9.x and/or Clang. We'd like to detect this situation automatically and insert std::move() as needed. Is there any easy way to detect this issue? Maybe a clang-tidy check or a -Wfoo flag we could enable?

But of course now I'd also like to know what is the correct behavior of a C++ compiler on this code. Are these outputs indicative of GCC/Clang bugs? Are they being worked on? And is the language version (-std=) supposed to matter? (I'd think that it is supposed to matter, unless the correct behavior has been updated via Defect Reports going all the way back to C++11.)

Here is a more complete test inspired by Barry's answer. We test six different cases where lvalue-to-rvalue conversion would be desirable.

GCC 4.9.3 and earlier:   elided/copy/copy/copy/copy/copy
Clang 3.8.1 and earlier: elided/copy/copy/copy/copy/copy
Clang 3.9.1 and later:   elided/copy/move/copy/copy/copy
GCC 5.1.0 through 7.1.0: elided/copy/move/move/move/move
GCC 8.0.1 (HEAD):        elided/move/move/move/move/move

ICC 17:                  elided/copy/copy/copy/copy/copy  
ICC 18:                  elided/move/move/move/copy/copy
MSVC 2017 (wow):         elided/copy/move/copy/copymove/copymove

After Barry's answer, it seems to me that Clang 3.9+ does the technically correct thing in all cases; GCC 8+ does the desirable thing in all cases; and in general I ought to stop teaching that people "just return x and let the compiler DTRT" (or at least teach it with a huge flashing caveat) because in practice the compiler will not DTRT unless you are using a bleeding-edge (and technically non-conforming) GCC.

like image 845
Quuxplusone Avatar asked Feb 11 '18 02:02

Quuxplusone


People also ask

Do I need STD move on return?

std::move is totally unnecessary when returning from a function, and really gets into the realm of you -- the programmer -- trying to babysit things that you should leave to the compiler.

What does move () do in C ++?

std::move in C++Moves the elements in the range [first,last] into the range beginning at result. The value of the elements in the [first,last] is transferred to the elements pointed by result. After the call, the elements in the range [first,last] are left in an unspecified but valid state.


1 Answers

The correct behavior is move/copy. You probably want to just write a clang-tidy check.


The wording in C++17 is [class.copy.elision]/3 and in C++14 is [class.copy]/32. The specific words and formatting is different but the rule is the same.

In C++11, the rule wording was [class.copy]/32 and was tied to the copy elision rules, the exception for automatic storage local variables was added in CWG 1579 as a defect report. Compilers predating this defect report would behave as copy/copy. But as the defect report is against C++11, compilers implementing the wording change would implement it across all standard versions.

Using the C++17 wording:

In the following copy-initialization contexts, a move operation might be used instead of a copy operation:

  • If the expression in a return statement is a (possibly parenthesized) id-expression that names an object with automatic storage duration declared in the body or parameter-declaration-clause of the innermost enclosing function or lambda-expression, or
  • [ ... ]

overload resolution to select the constructor for the copy is first performed as if the object were designated by an rvalue. If the first overload resolution fails or was not performed, or if the type of the first parameter of the selected constructor is not an rvalue reference to the object's type (possibly cv-qualified), overload resolution is performed again, considering the object as an lvalue.

In:

Unrelated test1() {
    Derived d1;
    return d1;
}

We meet the first bullet, so we try to copy-initialize an Unrelated with an rvalue of type Derived, which gives us Unrelated(Derived&& ). That meets the highlighted criteria so we use it, and the result is a move.

In:

Base test2() {
    Derived d2;
    return d2;  // yes, this is slicing!
}

We again meet the first bullet, but overload resolution will find Base(Base&& ). The first parameter of the selected constructor is not an rvalue reference to Derived (possibly cv-qualified), so we perform overload resolution again - and end up copying.

like image 105
Barry Avatar answered Sep 19 '22 17:09

Barry