CODEWITHSUNDEEP
c++c++11perfect-forwarding
Debashish
Debashish

Reputation: 1185

Error due to rvalue to lvalue conversion and not using forward

The following code shows a segmentation fault both in windows & Linux machines. The Parent class stores a pointer, but due to rvalue to lvalue conversion, the Parent ends up storing a reference to a pointer. (This is as per my understanding, please correct it otherwise)

#include <iostream>
#include <vector>

class Root {
public:
    virtual void Print() = 0;
};

template<typename PointerType>
class Parent : public Root {
public:
    Parent(PointerType&& p): ptr(p) {}

    void Print() override {
        std::cout <<"About to deref ptr" << std::endl;
        std::cout << *ptr << std::endl;
    }

    PointerType ptr;
};

class Child {
public:
    Root * root;
    template<typename PointerType>
    Child(PointerType&& p) {
        root = new Parent<PointerType>(std::forward<PointerType>(p));
    }
};



std::vector<Child> v;

template<typename PointerType>
void MyFunction(PointerType&& ptr) {  

    Child ch(ptr); /// ptr is lvalue here

    /// FIX : Child ch(std::forward<PointerType>(ptr));

    v.push_back(std::move(ch));
}

int* getInt() {
    return new int(10);
}

int main() {
    MyFunction(getInt()); /// pass any rvalue pointer /// it could be "this" pointer also
    v[0].root->Print();
}

I convinced myself that, I always need to use std::forward as in when universal references are used (Inside the function MyFunction).

I am finding it difficult to understand the following

Why does reference to ptr becomes invalid inside the parent class? Is it because ptr becomes a local variable once it is used inside MyFunction and Child constructor gets a reference to this local only?

Thanks!

Upvotes: 1

Views: 158

Answers (1)

ildjarn
ildjarn

Reputation: 62975

First, here's what's happening, keeping in mind that getInt returns an rvalue: when you don't forward ptr, you end up invoking (post-reference collapsing)

  • MyFunction<int*>(int*&&)
  • which invokes Child::Child<int*&>(int*&)
  • which constructs and stores a Parent<int*&>.

When you do forward ptr, you end up invoking

  • MyFunction<int*>(int*&&)
  • which invokes Child::Child<int*>(int*&&)
  • which constructs and stores a Parent<int*>.

So the manifestation of the problem is a common dangling reference: Parent<int*&>::ptr is necessarily an int*&, which happens to be bound to the temporary returned by getInt. However, the solution is a little more complicated than just dogmatically applying std::forward..:

It so happens that forwarding ptr fixes the problem here, because you are passing an rvalue into MyFunction in your example; but you should certainly be able to call MyFunction with an lvalue and have it work properly, and forward alone doesn't solve that. The actual problem is instantiating Parent with a reference type to begin with; in C++11 this is generally avoided via application of std::decay.

Once this is fixed, a second problem will become apparent (masked by luck in your current code due to reference collapsing rules): the PointerType&& in parent's constructor is not a forwarding reference, because PointerType is a parameter of the type, not the constructor. As a result, when an lvalue is passed in, the constructor invoked will end up being Parent<int*>::Parent(int*&&), which in turn will not compile. Unfortunately the 'proper' solution to this is complicated in C++11... The short summary is that if you want perfect-forwarding to apply to Parent's constructor, that constructor needs to become a template; but implementing this correctly is complicated and verbose in older standards, so I'll leave researching that as an exercise for the reader and choose the easy solution here: accept by value and move.

With both of these fixed the result looks like:

template<typename PointerType>
class Parent : public Root {
public:
    Parent(PointerType p): ptr(std::move(p)) {}

    void Print() override {
        std::cout << "About to deref ptr\n" << *ptr << '\n';
    }

    PointerType ptr;
};

class Child {
public:
    Root* root;

    template<typename PointerType>
    Child(PointerType&& p):
        root(new Parent<typename std::decay<PointerType>::type>(
            std::forward<PointerType>(p)
        ))
    {}
};

Online Demo

(But then one has to wonder, do 'pointer types' really warrant perfect-forwarding in the first place?)

Upvotes: 1

Related Questions