Search code examples
c++copy-constructorunionsstd-function

Assigning std::function inside union crashes program


I wrote the following class which stores either a fixed value (constant) or a function to get a value (get).

template <typename T>
class DynamicValue {
    private:
    bool isConstant;
    union {
        T constant;
        std::function<T()> get;
    };
    void copy(const DynamicValue& value) {
        isConstant = value.isConstant;
        if (isConstant) {
            constant = value.constant;
        } else {
            std::cout << "won't overcome next line" << std::endl;
            get = value.get;
            std::cout << "why!" << std::endl;
        }
    }

    public:
    DynamicValue(const T& constant) : isConstant(true), constant(constant){};
    template <typename F, typename = std::enable_if_t<std::is_invocable_v<F>>>
    DynamicValue(F&& get) : isConstant(false), get(std::forward<F>(get)) {}
    DynamicValue(const T* pointer) : DynamicValue([pointer]() { return *pointer; }) {}
    DynamicValue(const DynamicValue& value) { copy(value); }
    ~DynamicValue() {}
    DynamicValue& operator=(const DynamicValue& value) {
        copy(value);
        return *this;
    }
    operator T() { return isConstant ? constant : get(); }
};

I also wrote the following dummy class to showcase the issue I've encountered:

class Object {
    private:
    DynamicValue<int> num;
    
    public:
    Object(DynamicValue<int> num) : num(num) {}
};

The problem is when I try to initialize an Object with a function (by doing Object b([] { return 1; }); for example) the program crashes. I've narrowed down the crash to the get = value.get; line inside the copy function. I've also noticed that the crash no longer happens if I move get outside the union.

You can try a live example here.

Why is this happening and how can I fix it (keeping get inside the union)?


Solution

  • Your copy constructor for DynamicValue does not initialize the get member, causing the assignment in copy to fail. Adding an initialization for get

    DynamicValue(const DynamicValue& value) : get() { copy(value); }
    

    resolves the crash but has other issues. See version here

    Going in this direction is problematic, so you should consider changing your approach to using std::variant.

    template <typename T>
    class DynamicValue {
        private:
        std::variant<T, std::function<T()>> value;
        void copy(const DynamicValue& other) {
            value = other.value;
        }
    
        public:
        DynamicValue(const T& constant) : value(constant){};
        template <typename F, typename = std::enable_if_t<std::is_invocable_v<F>>>
        DynamicValue(F&& get) : value(std::forward<F>(get)) {}
        DynamicValue(const T* pointer) : DynamicValue([pointer]() { return *pointer; }) {}
        DynamicValue(const DynamicValue& value) { copy(value); }
        ~DynamicValue() {}
        DynamicValue& operator=(const DynamicValue& value) {
            copy(value);
            return *this;
        }
        operator T() { return value.index() == 0 ? std::get<0>(value) : std::get<1>(value)(); }
    };
    

    See working version here