How do I extend the lifetime of a temporary in a ranged for expression?

Question

I'm getting dangling references while using a ranged-for loop. Consider the following C++14 expression (full example program below):

    for(auto& wheel: Bike().wheels_reference())
        wheel.inflate();

It's output is:

 Wheel()
 Wheel()
 Bike()
~Bike() with 0 inflated wheels.
~Wheel()
~Wheel()
 Wheel::inflate()
 Wheel::inflate()

Obviously something is going very wrong. Wheels are accessed beyond their lifetime and the result is 0, not the expected 2.

An easy fix is to introduce a variable for Bike in main. However, I do not control the code in main or Wheel. I can only change the struct Bike.

Is there any way to fix this example by only changing Bike?

A successful solution solution will either fail at compile time, or count 2 inflated tires and not touch any objects beyond their lifetimes.

Appendix: compile ready source

#include <cstdlib>
#include <iostream>
#include <array>
#include <algorithm>
using std::cout;
using std::endl;

struct Wheel
{
    Wheel() { cout << " Wheel()" << endl; }
    ~Wheel() { cout << "~Wheel()" << endl; }
    void inflate() { inflated = true; cout << " Wheel::inflate()" << endl; }
    bool inflated = false;
};

struct Bike
{
    Bike() { cout << " Bike()" << endl; }
    ~Bike() {
        cout << "~Bike() with " << std::count_if(wheels.begin(),    wheels.end(),
            [](auto& w) { return w.inflated; }) << " inflated wheels." << endl;
    }
    std::array<Wheel, 2>& wheels_reference() { return wheels; }
    std::array<Wheel, 2> wheels{Wheel(), Wheel()};
};

int main()
{
    for(auto& wheel: Bike().wheels_reference())
        wheel.inflate();
    return EXIT_SUCCESS;
}

Klepht

If only in Bike, than function reference should return array by value, not by reference, but I think, it's not what you want. – Lyublin

@Lyublin but auto& couldn't bind to that. – Aeon

@ForEveR: While it solves the use-after destroy, it does not satisfy my conditions for a solution. The printed value will be 0, not 2. – Klepht

@Aeon but binds in both gcc/clang. – Lyublin

@Lyublin Huh, I guess by brain-compiler is broken. – Aeon

This code doesn't compile. – Pensive

Change the loop to for(auto& wheel: Bike().wheels) and it's fine. – Evangelin

@erip: You must have copied it in between two edits where I introduced a small bug. Current code compiles for me with: clang++ -std=c++14 ./code.cpp -o ./code. Please try again. – Klepht

@JonathanPotter: Thanks! That is an interesting solution, but I'm not sure if it is usable in my context, since I can not change main. – Klepht

@Klepht Make Bike() a function returning a reference to a singleton then :) I think you have come across a weakness in the standard relating to lifetime extension of temporary objects. – Evangelin

I disagree that it's duplicate actually, question is not about rules, but about how handle this situation. – Lyublin

Thanks @ForEveR. I'm indeed aware of what is going on and asking for a workaround. – Klepht

Here's a workaround: stop doing this: Bike().wheels_reference(). Stop accessing the members of a temporary. – Farsighted

@NicolBolas: Sounds good. How do I turn it into compile time errors? – Klepht

Answer 1

Delete the rvalue overload of wheels_reference.

std::array<Wheel, 2>& wheels_reference() & { return wheels; }
std::array<Wheel, 2>& wheels_reference() && = delete;

That way you won't return a reference to a member of a temporary.

Your example usage of the Bike class

for(auto& wheel: Bike().wheels_reference())
    wheel.inflate();

Will then refuse to compile with (clang 3.4 output):

test.cpp:31:29: error: call to deleted member function 'wheels_reference'
    for(auto& wheel: Bike().wheels_reference())
                     ~~~~~~~^~~~~~~~~~~~~~~~
test.cpp:24:27: note: candidate function has been explicitly deleted
    std::array<Wheel, 2>& wheels_reference() && = delete;
                          ^
test.cpp:23:27: note: candidate function not viable: no known conversion from 'Bike' to 'Bike' for object argument
    std::array<Wheel, 2>& wheels_reference() & { return wheels; }

If the lifetime of the temporary is manually extended things work.

Bike&& bike = Bike();
for(auto& wheel: bike.wheels_reference())
    wheel.inflate();

Migdaliamigeon

I am puzzled about the non exiting function and "std::array<Wheel, 2>& wheels_reference() && = delete;" – Impasto

@DieterLücking check this en.cppreference.com/w/cpp/language/… – Migdaliamigeon

Why do you need to delete && overloading? You can just define single & overloading. – Sik

@Orient you are right, but the error messages I get when only providing the lvalue-ref overload is not as good: clang gives the most unhelpful error: cannot initialize object parameter of type 'Bike' with an expression of type 'Bike' while g++5 tells me that error: passing ‘Bike’ as ‘this’ argument discards qualifiers. Explicitly deleting an overload is much clearer. – Migdaliamigeon

@Migdaliamigeon Right you are. – Sik

Answer 2

The best solution is to stop getting members of a type through a member function call on temporary.

If wheels_reference were a non-member function, you could simply declare it like this:

wheels_reference(Bike &bike);

Since a non-const lvalue parameter cannot be attached to a temporary, you would be unable to call wheels_reference(Bike()). Since wheels_reference is a member function, you'll just have to use the member function syntax for saying the same thing:

std::array<Wheel, 2>& wheels_reference() & //<--
{ return wheels; }

If the user now tries to call Bike().wheels_reference(), the compiler will complain.

Farsighted

Perfect! Essentially the same as @jeplo's solution. The analogy with non-member functions is insightful. – Klepht

Answer 3

The following horrible contraption seems to satisfy all the conditions:

#include <memory>

struct Bike
{
    // Bike() { cout << " FakeBike()" << endl; }
    // ~Bike() { cout << "~FakeBike()" << endl; }
    struct RealBike;
    struct Wrap {
        std::shared_ptr<RealBike> parent;
        auto begin() { return parent->wheels.begin(); }
        auto end() { return parent->wheels.end(); }
    };
    struct RealBike {
        RealBike() { cout << " Bike()" << endl; }
        ~RealBike() {
            cout << "~Bike() with " << std::count_if(wheels.begin(),    wheels.end(),
                [](auto& w) { return w.inflated; }) << " inflated wheels." << endl;
        }
        std::array<Wheel, 2> wheels;
    };
    std::shared_ptr<RealBike> real = std::make_shared<RealBike>();
    Wrap wheels_reference() { return Wrap{real}; }
};

What I don't like is that it requires wrapping all the API of std::array<Wheel, 2> in Wrap.

Klepht

Answer 4

You can just add a couple of cv-ref-qualified overloadings of wheels_reference member function:

std::array<Wheel, 2>& wheels_reference() & { return wheels; }

std::array<Wheel, 2> const & wheels_reference() const & { return wheels; }

std::array<Wheel, 2> wheels_reference() && { return std::move(wheels); }

std::array<Wheel, 2> wheels_reference() const && { return wheels; }

Note, if object is temporary (&& case), then you should return a value (copy-constructed from data member or even better move-constructed from it), nor reference.

Having all four overloadings you cover all the use-cases possible.

Sik