How to resolve dangling const ref

Question

The following short program

#include <vector>
#include <iostream>

std::vector<int> someNums()
{
    return {3, 5, 7, 11};
}

class Woop
{
public:
    Woop(const std::vector<int>& nums) : numbers(nums) {}
    void report()
    {
        for (int i : numbers)
            std::cout << i << ' ';
        std::cout << '\n';
    }
private:
    const std::vector<int>& numbers;
};

int main()
{
    Woop woop(someNums());
    woop.report();
}

has a dangling reference problem, that no compiler seems to warn about. The issue is that temporaries can be bound to const-refs, which you may then keep around. The question then is; Is there a method of avoiding getting into this problem? Preferably one that does not involve sacrificing const correctness, or always making copies of big objects.

Answer 1

One way to make your class less vulnerable could be to add a deleted constructor that takes a right-ref. This would stop your class instance from making bindings to temporaries.

Woop(std::vector<int>&& nums)  =delete;

This deleted constructor would actually make the O/P code not compile, which may be the behaviour you are looking for?

Answer 2

In situation when some method keeps a reference after returning it is a good idea to utilize std::reference_wrapper instead of normal reference:

#include <functional>

class Woop
{
public:
    using NumsRef = ::std::reference_wrapper<const std::vector<int>>;
    Woop(NumsRef nums) : numbers_ref{nums} {}
    void report()
    {
        for (int i : numbers_ref.get())
            std::cout << i << ' ';
        std::cout << '\n';
    }
private:
    NumsRef numbers_ref;
};

1. it already comes with a set of overloads preventing binding of rvalues and unintended passing of temporaries, so there is no need to bother with an extra prohibited overload taking a rvalue Woop (std::vector<int> const &&) = delete; for your method:

Woop woop{someNums()}; // error
woop.report();

2. it allows implicit binding of lvalues so it won't break existing valid invocations:

auto nums{someNums()};
Woop woop{nums}; // ok
woop.report();

3. it allows explicit binding of lvalues which is a good practice to indicate that caller will keep the reference after returning:

auto nums{someNums()};
Woop woop{::std::ref(nums)}; // even better because explicit
woop.report();

Answer 3

I agree with the other answers and comments that you should think carefully if you really need to store a reference inside the class. And that if you do, you'll probably want a non-const pointer to a const vector instead (i.e. std::vector<int> const * numbers_).

However, if that's the case, I find that the other currently posted answers are beside the point. They're all showing you how to make Woop own those values.

If you can assure that the vector you pass in will outlive your Woop instance, then you can explicitly disable constructing a Woop from an rvalue. That's possible using this C++11 syntax:

Woop (std::vector<int> const &&) = delete;

Now your example code won't compile anymore. The compiler with give an error similar to:

prog.cc: In function 'int main()':
prog.cc:29:25: error: use of deleted function 'Woop::Woop(const std::vector<int>&&)'
   29 |     Woop woop(someNums());
      |                         ^
prog.cc:15:5: note: declared here
   15 |     Woop(std::vector<int> const &&) = delete;
      |     ^~~~

P.S.: You probably want an explicit constructor, see e.g. What does the explicit keyword mean?.

Answer 4

To prevent that particular case, you can choose to either take a pointer (since Weep(&std::vector<int>{1,2,3}) isn't allowed) or you could take a non-const reference which will also error on a temporary.

Woop(const std::vector<int> *nums);
Woop(std::vector<int> *nums);
Woop(std::vector<int>& nums);

These still don't guarantee the value remains valid, but stops the easiest mistake at least, doesn't create a copy, and doesn't need nums be created in a special way (e.g. as std::shared_ptr or std::weak_ptr does).

std::scoped_lock taking a reference to the mutex would be an example, and one where unique/shared/weak ptr is not really wanted. Often the std::mutex will just be a basic member or local variable. You still have to be very careful, but in these cases it is generally easy to determine the life span.

std::weak_ptr is another option for non-owning, but then you force the caller to use shared_ptr (and thus also heap allocate), and sometimes that isn't wanted.

If a copy is OK, that just avoids the issue.

If Woop should take ownership either pass as an r-value and move (and avoid pointer/reference issues entirely), or use unique_ptr if you can't move the value itself or want the pointer to remain valid.

// the caller can't continue to use nums, they could however get `numbers` from Woop or such like
// or just let Woop only manipulate numbers directly.
Woop(std::vector<int> &&nums) 
   : numbers(std::move(nums)) {}
std::vector<int> numbers;

// while the caller looses the unique_ptr, they might still use a raw pointer, but be careful.
// Or again access numbers only via Woop as with the move construct above.
Woop(std::unique_ptr<std::vector<int>> &&nums) 
    : numbers(std::move(nums)) {}
std::unique_ptr<std::vector<int>> numbers;

Or if ownership is shared, you can use shared_ptr for everything, and it will be deleted along with the final reference, but this can make keeping track of object life cycles get very confusing if over-used.

Answer 5

You may use template programming and arrays if you want to have an object that holds a const container. Due to the constexpr constructor and constexpr arrays you achieve const correctness and compile time execution.

Here is a post that may be interesting: std::move a const vector

#include <array>
#include <iostream>
#include <vector>


std::array<int,4>  someNums()
{
    return {3, 5, 7, 11};
}


template<typename U, std::size_t size>
class Woop
{
public:

template<typename ...T>
    constexpr Woop(T&&... nums) : numbers{nums...} {};

    template<typename T, std::size_t arr_size>
    constexpr Woop(std::array<T, arr_size>&& arr_nums) : numbers(arr_nums) {};

    void report()
    const {
        for (auto&& i : numbers)
            std::cout << i << ' ';
         std::cout << '\n';
    }



private: 
    const std::array<U, size> numbers;
    //constexpr vector with C++20
};

int main()
{
    Woop<int, 4> wooping1(someNums());
    Woop<int, 7> wooping2{1, 2, 3, 5, 12 ,3 ,51};

    wooping1.report();
    wooping2.report();
    return 0;
}

run code

Output:

3 5 7 11                                                                                                                        
1 2 3 5 12 3 51