Wrong std::reference_wrapper to reference assignment?

[RicoJ]

I was expecting to get -4, but I ended up getting -3. I thought ret in foo will keep a reference to l.front(). Why did that happen?

struct S
{
    int val;
    S(int i) : val(i) {}
};

std::list<S> l = {-4, -3, -2, -1, 0, 1, 2, 3, 4};
std::vector<std::reference_wrapper<S>> v(l.begin(), l.end());
    
S& foo(){S& ret =  v.front(); v.erase(v.begin()); return ret; }
    
int main()
{
    S& lol = foo();
    lol.val = 1000;
    std::cout << v[0].get().val;
}

[Ganado]

My first guess is that it's undefined behavior, since you are trying to reference an element that has been erased. Edit: After more discussion, this appears to be wrong. See below.

http://www.cplusplus.com/reference/vector/vector/erase/

Iterators, pointers and references pointing to position (or first) and beyond are invalidated, with all iterators, pointers and references to elements before position (or first) are guaranteed to keep referring to the same elements they were referring to before the call.

Anyone else wanna give their thoughts?

[highwayman]

Is it possible that, to prevent some overhead assumedly, the implementation just moved everything to the left once?

[mbozzi]

Looks okay to me. In the case of foo(), the return value ret is the wrapped reference to the element of the list l. The reference_wrapper has been erased, but the wrapped reference remains unaffected.

Edit: whoops, nevermind

[RicoJ]

Guys, Just sorted this out. In short, it's just a vector question. In main(), since i was printing the v[0] value, I will see -3 as -4 has been popped off.

The interesting question, though, is instead of printing v[0], we print the list's first value (l.front() value). In this case, in foo(), ret will store the address of the first element of l, as reference can be generally treated as a constant pointer that implicitly dereferences itself. Then, even if we did v.erase(v.begin()), it only erased the first element of the reference_wrapper vector, not the actual list element. So ret in foo() still stores the right address to the first list element.

hope this makes things clear

[helios]

Guys, Just sorted this out. In short, it's just a vector question. In main(), since i was printing the v[0] value, I will see -3 as -4 has been popped off.

I'm pretty sure, as Ganado said, the behavior is undefined. Any container function that causes iterator invalidation leaves pointers and references into elements in an undefined state. There's no guarantee that the std::vector implementation works by shifting elements. For example, std::vector internally could just be an std::deque. In that case, on line 16 you would be accessing a destructed object.

[doug4]

@helios,

on line 16 you would be accessing a destructed object.

Are you sure?

Vector v had an element erased. There were no pointers or references into v when the element was erased. Line 16 uses operator[] on the current value of vector v.

Where is a destructed object being accessed?

[mbozzi]

The reference is into the list, not the vector.

[helios]

Line 16 uses operator[] on the current value of vector v.

Apologies. Line 15.

[doug4]

@helios,

My understanding of the function foo (line 10) is that ret is assigned the value of v.front(). Because v is a std::vector of std::reference_wrapper<S> and ret is a S&, the contents of v.front() are cast to S&, and the vector element is no longer in play.

When the first element of v is erased, ret still contains a reference to the first element of l, which was not erased.

I haven't worked with reference_wrapper very much, so I may be missing some subtlties. Please correct me if I'm not reading this right.

[Ganado]

Yeah, it was just my initial guess. I don't know if it's true. What you're saying sounds reasonable.

I have implemented it myself below, using the implementation from cppreference.com, if anyone wants to try to parse this mess:

#include <utility>
#include <type_traits>
#include <functional>

namespace my {
template< class T >
struct remove_cvref {
    typedef std::remove_cv_t<std::remove_reference_t<T>> type;
};

template< class T >
using remove_cvref_t = typename my::remove_cvref<T>::type;

namespace detail {
template <class T> constexpr T& FUN(T& t) noexcept { return t; }
template <class T> void FUN(T&&) = delete;
}
 
template <class T>
class reference_wrapper {
public:
  // types
  typedef T type;
 
  // construct/copy/destroy
  template <class U, class = decltype(
    detail::FUN<T>(std::declval<U>()),
    std::enable_if_t<!std::is_same_v<reference_wrapper, my::remove_cvref_t<U>>>()
  )>
  constexpr reference_wrapper(U&& u) noexcept(noexcept(detail::FUN<T>(std::forward<U>(u))))
    : _ptr(std::addressof(detail::FUN<T>(std::forward<U>(u)))) {}
  reference_wrapper(const reference_wrapper&) noexcept = default;
 
  // assignment
  reference_wrapper& operator=(const reference_wrapper& x) noexcept = default;
 
  // access
  constexpr operator T& () const noexcept { return *_ptr; }
  constexpr T& get() const noexcept { return *_ptr; }
 
  template< class... ArgTypes >
  constexpr std::invoke_result_t<T&, ArgTypes...>
    operator() ( ArgTypes&&... args ) const {
    return std::invoke(get(), std::forward<ArgTypes>(args)...);
  }
 
private:
  T* _ptr;
};
 
// deduction guides
template<class T>
reference_wrapper(T&) -> reference_wrapper<T>;

}


///////////////////////////

#include <list>
#include <vector>
#include <iostream>

struct S
{
    int val;
    S(int i) : val(i) {}
};

std::list<S> l = {-4, -3, -2, -1, 0, 1, 2, 3, 4};
std::vector<my::reference_wrapper<S>> v(l.begin(), l.end());
    
S& foo() {
    S& ret =  v.front();
    v.erase(v.begin());
    return ret;
}
    
int main()
{
    S& lol = foo();
    lol.val = 1000;
    std::cout << v[0].get().val;
}

The address of ret is the same as the address of the first element of the list, so I suppose it is just directly referencing the list element itself, regardless of what is erased from the vector. So I suppose it is safe.

In which case, the answer is the original question is straightforward: The first element of the vector was erased, so now the first element points to the -3 element.

[helios]

That's what I get for not reading things properly. I would still not accept this code in a code review. It's too confusing. I'd rather see a vector of naked pointers.