Factory pattern with abstract base classes - return by ref or by value? Problems with scoping and slicing

Question

I have a type hierarchy similar to the code sample below, and I'm trying to instantiate them via the factory pattern (or, to be pedantic, rather the builder pattern, as my factory takes input from an XML document... but I digress).

However I try to do this, I'm running into problems which I suspect are due to either slicing, if I return by value, or to scoping, if I return by reference.

The below program, for instance, segfaults on the line a.doA() inside C::doStuff(). If I change the call to value_C_factory<C>() to ref_C_factory<C>() instead, I get a couple of warnings to the effect of "returning reference to temporary", but the program compiles, segfaults instead on b.doB() on the next line (without having printed anything from a.doA()...).

The backtrace from gdb looks like this - the second line is the one in my code referred to above

#0  0x00007ffff7dbddb0 in vtable for std::ctype<char> () from /usr/lib/x86_64-linux-gnu/libstdc++.so.6
#1  0x00000000004010e9 in C::doStuff (this=0x7fffffffdd00) at syntax.cpp:57
#2  0x0000000000400cf2 in main () at syntax.cpp:95

What is causing these segfaults? Is it, as I suspect, slicing/scoping in the value/reference case? If not, what is it? And most importantly, what is a good way to build my instances from the input data?

Code sample

The code below should compile and give the above behavior with e.g. GCC 4.8, using
gcc -g -Wall -std=c++11 -o test test.cpp (that's what I do, anyway).

#include <iostream>
#include <typeinfo>

class IA {
public:
    virtual void doA() const = 0;
    virtual ~IA() { }
};

class A : public IA {
private:
    std::string atask;
public:
    explicit A(const std::string &task) : atask(task) {
        std::cout << "Created A with task " << atask << std::endl;
    }

    void doA() const {
        std::cout << "I did A! " << atask << std::endl;
    }
};

class IB {
public:
    virtual void doB() const = 0;
    virtual ~IB() { }
};

class B : public IB {
private:
    std::string btask;
public:
    explicit B(const std::string &task) : btask(task) {
        std::cout << "Created B with task " << btask << std::endl;
    }

    void doB() const {
        std::cout << "I did B! " << btask << std::endl;
    }
};

class IC {
public:
    void doStuff() const;
    virtual ~IC() { }
};

class C : public IC {
private:
    const IA &a;
    const IB &b;

public:
    C(const IA &a, const IB &b) : a(a), b(b) { }

    void doStuff() const {
        a.doA();    // with value factory method, segfault here
        b.doB();    // with reference factory, segfault here instead
    }
};

template<typename TA>
TA value_A_factory() {
    return TA("a value");
}

template<typename TB>
TB value_B_factory() {
    return TB("b value");
}

template<typename TC>
TC value_C_factory() {
    return TC(value_A_factory<A>(), value_B_factory<B>());
}


template<typename TA>
const TA &ref_A_factory() {
    return TA("a ref");
}

template<typename TB>
const TB &ref_B_factory() {
    return TB("b ref");
}

template<typename TC>
const TC &ref_C_factory() {
    const TC &c(ref_A_factory<A>(), ref_B_factory<B>());
    return c;
}

int main() {
    C c = value_C_factory<C>();
    std::cout << typeid(c).name() << std::endl;
    c.doStuff();
}

Answer 1

You have two problems, both caused by undefined behavior.

The first is that you can't return a reference to a local variable. Once the function returns and the local variable goes out of scope and is destructed, what does the returned reference then reference?

The other problem is that you store references to temporary values. When you create your C class like TC(value_A_factory<A>(), value_B_factory<B>()) the values returned by the value_X_factory functions are temporary, and will be destroyed once the complete expression (TC(...)) is done.

Answer 2

In

template<typename TA>
const TA &ref_A_factory() {
    return TA("a ref");
}

returning a reference to a local variable is undefined behavior.

In

TC(value_A_factory<A>(), ...)

the lifetime of the value returned by value_A_factory will be the end of the expression TC(...). After that your references in C are dangling.

If you really want to use interfaces and factories for polymorphic types, there is no real alternative to dynamic memory allocation and some kind of ownership scheme. The simplest would be for C to simply assume ownership of its members and take care of their deletion.

#include <memory>
#include <cassert>

struct IA { 
  virtual void doA() const = 0;
  virtual ~IA() { }; 
};

struct A : IA { 
  void doA() const override {}
};

struct C {
  /* C assumes ownership of a. a cannot be null. */
  C(IA* a) : a{a} { assert(a && "C(IA* a): a was null"); };
private:
  std::unique_ptr<IA> a;
};

C factory() {
  return C{new A};
}

int main()
{
  C c = factory();
  return 0;
}