Return rvalue reference from function

Question

CPP Core guidelines F45 states:

Don't return a T&&.

I want to create some class, and pass the class through a sequence of functions that modify the class. Then I will either evaluate some members of that class, or store the class in some container, like std::vector.

I am trying to do this in a way that the class is constructed exactly once, then destroyed, or moved and the moved-from copy is destroyed, and later the stored copy is destroyed (when the container that I stored it in is destroyed).

In essence, I want to do this:

// some class
class foo{}

// construct a foo, with some parameters, and modify it somehow
auto f1 = modify_foo(foo(x, y, z));
// modify the foo some more
auto f2 = modify_foo(f1);
// modify the foo some more
auto f3 = modify_foo(f2);

// use some element of modified foo
auto v = f3.getx();

// maybe store the modified foo in a vector or some other container
vector<foo> vf;
vf.emplace_back(f3);

It should be possible to construct the foo exactly once, and move the constructed foo through any number of modifying functions, then destroy the foo exactly once.

In the case of storing the foo in a vector, an additional copy/move and destroy will have to occur.

I can achieve this behavior, but I can't figure out any way to do it without using this signature for the modify functions:

foo&& modify_foo(foo&& in);

Here is test code that seems to do what I want:

#include <iostream>
#include <functional>
#include <vector>

// A SIMPLE CLASS WITH INSTRUMENTED CONSTRUCTORS
class foo {

public:
    // default constructor
    foo() {
        std::cout << "default construct\n";
    }

    // copy constructor
    foo(foo const &in) : x{ in.x } {
        std::cout << "copy construct\n";
    };

    // copy assignment
    foo& operator=(foo const& in) {
        x = in.x;
        std::cout << "copy assignment\n";
    }

    // move constructor
    foo(foo&& in) noexcept : x(std::move(in.x)) {
        std::cout << "move constructor\n";
    }

    // move assignment
    foo& operator=(foo&& in) noexcept {
        x = std::move(in.x);
        std::cout << "move assignment\n";
        return *this;
    }

    // destructor
    ~foo() {
        std::cout << "destructor\n";
    }

    void inc() {
        ++x;
    }

    int getx() { return x; };

private:

    int x{ 0 };

};

Now a function that will take foo&&, modify foo, return foo&&:

// A SIMPLE FUNCTION THAT TAKES foo&&, modifies something, returns foo&&
foo&& modify(foo&& in) {
    in.inc();
    return std::move(in);
}

Now using the class and modify function:

int main(){

    // construct a foo, modify it, return it as foo&&
    auto&& foo1 = modify(foo());

    // modify foo some more and return it
    auto&& foo2 = modify(std::move(foo1));

    // modify foo some more and return it
    auto&& foo3 = modify(std::move(foo2));

    // do something with the modified foo:

    std::cout << foo3.getx();
}

This will do exactly what I want. It will call the constructor once, correctly print 3, and call the destructor once.

If I do the same thing, except add this:

std::vector<foo> fv;
fv.emplace_back(std::move(foo3));

It will add one move construct, and another destruct when the vector goes out of scope.

This is exactly the behavior I want, and I haven't figured out any other way to get there without returning foo&& from the modifier, and using auto&& for my intermediate variables, and using std::move() on the parameters being passed to the subsequent calls to modify.

This pattern is very useful to me. It is bothering me that I can't resolve this with CPP core guidelines F.45. The guideline does say:

Returning an rvalue reference is fine when the reference to the temporary is being passed "downward" to a callee; Then the temporary is guaranteed to outlive the function call...

Maybe that is what I am doing?

My questions:

1. Is there anything fundamentally wrong, or undefined, in what I am doing?

2. When I do auto&&, and hover over the foo1, it will show it as a foo&&. I still have to wrap the foo1 with std::move(foo1) to get the modify function to accept it as foo&&. I find this a little strange. What is the reason for requiring this syntax?

Answer 1

As was correctly pointed out by NathanOliver, attempting to use rvalue ref's was leaving a dangling reference to an object that was being destroyed at the end of the function's life.

The piece of the puzzle that I was missing was to use 'auto&', instead of 'auto' when returning a ref from a function:

// function taking lvalue ref, returning lvalue ref
foo& modify(foo& in) {
    in.inc();
    return in;
}

{

    auto f =  foo{}; // constructed here

    auto f1 = modify(f); // <-- BAD!!! copy construct occurs here.

    auto& f2 = modify(f); // <-- BETTER - no copy here

} // destruct, destruct

If I use auto& to capture the lvalue ref returning from 'modify', no copies are made. Then I get my desired behavior. One construct, one destruct.

{
    // construct a foo
    foo foo1{};

    // modify some number of times
    auto& foo2 = modify(std::move(foo1));
    auto& foo3 = modify(std::move(foo2));
    auto& foo4 = modify(std::move(foo3));

    std::cout << foo4.getx();
} // 1 destruct here