Copy constructor with pointers fails when returned by a function

Question

I am a new user here and this is my first question, so please don't judge me hard. I cheked many similar questiontions like this not only on this site, also on others too. But I didn't find the answer.

The problem is to create copy constructor with pointers. For those people who may ask: why do you need pointers?: I have a class stmt, which contains vector<stmt>, it will allow me to construct tree-like data structure. Then I will perform some functions to change values of stmt's parameters. Without pointers they won't change

It compiles, but then gives Exception Unhandled (Runtime error)

My first attemp looks like this:

struct stmt
{
    string lexeme;
    int *size;
    int *lay;
    bool *nl;
    vector<stmt>* follow;
    stmt(string l)
    {
        lexeme = l;
        size = new int;
        *size = l.size()+2;
        lay = new int;
        *lay = 0;
        nl = new bool;
        *nl = false;
        follow = new vector<stmt>;
    }
    stmt(const stmt &s)
    {
        lexeme = s.lexeme;      
        size = new int;         //Crashes here : Unhandled exception:std::length_error at memory location ... 
        *size = *s.size;
        lay = new int;
        nl = new bool;
        follow = new vector<stmt>;
        follow = s.follow;
    }
};

Second time I tried also this:

stmt(const stmt &s)
:lexeme.(s.lexeme), size (s.size), ......and so on
{}

Unfortunately, this also doesn't help.

So, this is my third attemp but didn't help

IMPORTANT: I noticed that it happens when I am trying to create and emplace_back new stmt element in a vector of another stmt using functions which return type is stmt.

Here is a code which represents the key problem:

stmt Program("Program");
stmt ParseRelop(string  p);

void EMP(stmt s)
{
    s.follow->push_back(ParseRelop("token"));
}

stmt ParseRelop(string  p)
{
    stmt s(p);
    return s;
}

int main()
 {
    EMP(Program);
    cout<<Program.follow->at(0).lexeme;
 }

Answer 1

Like this

stmt(const stmt &s)
{
    ...
    follow = new vector<stmt>(*s.follow);
}

This version allocates a new vector by copying the vector from the copied object (*s.follow is the vector in the copied object).

Your version allocated a new pointer but then overwrote it with a copy of the pointer from the copied object.

You want to copy the vector, not the pointer that's pointing at the vector.

You should use the same technique for your other pointers

    size = new int(*s.size);
    lay = new int(*s.lay);

etc.

Answer 2

Let's start by making your stmt class copy correct. Once you do that, then the issues will come from other parts of your code that do not involve stmt leaking memory, or accessing invalid memory:

#include <vector>
#include <string>
#include <algorithm>

struct stmt
{
    std::string lexeme;
    int *size;
    int *lay;
    bool *nl;
    std::vector<stmt>* follow;

    // Constructor.  Note the usage of the member initialization list
    stmt(std::string l) : lexeme(l), size(new int), lay(new int(0)), nl(new bool(false)),
        follow(new std::vector<stmt>())
    {
        *size = l.size() + 2;
    }

    // Copy constructor.  Note that we use the member initialization list, 
    // and copy all the members from s to the current object, not partial copies
    stmt(const stmt &s) : lexeme(s.lexeme), size(new int(*s.size)), nl(new bool(*s.nl)),
        lay(new int(*s.lay)), follow(new std::vector<stmt>(*s.follow))
    {}

    // The assignment operator.  Note that we use copy / swap to swap out
    // all the members, and that a check for self-assignment is done.
    stmt& operator=(const stmt &s)
    {
        if (this != &s)
        {
            stmt temp(s);
            std::swap(lexeme, temp.lexeme);
            std::swap(size, temp.size);
            std::swap(lay, temp.lay);
            std::swap(nl, temp.nl);
            std::swap(follow, temp.follow);
        }
        return *this;
    }

    // The destructor destroys all the memory 
    ~stmt()
    {
        delete size;
        delete lay;
        delete nl;
        delete follow;
    }
};

This class now follows the rule of 3 properly, and should be safely copyable. I put in the comments what each function does, and you should research on why the class should work correctly now.

I didn't want to go too much in depth, because in reality you wouldn't have pointers to int, bool or even vector<stmt>.

References:

Rule of 3

Copy / Swap Idiom