CODEWITHSUNDEEP
c++lambdacopy-constructor
cabralpinto
cabralpinto

Reputation: 2098

Copy constructor called endlessly from lambda capture group

I wrote the following class to create values of any type which are either fixed or recalculated everytime the call operator is used on them:

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

I also wrote this function, which takes a DynamicValue<int> and returns another DynamicValue<int> which returns its value plus 1:

DynamicValue<int> plus1(DynamicValue<int> a) {
    return [a] { return a() + 1; };
}

However, when I attempt to do use it, the program crashes:

DynamicValue<int> a = 1;
DynamicValue<int> b = plus1(a);

You can try a live example here.

After some testing, I think the problem lies in the copy constructor, which is being called endlessly, but I'm not sure how to fix it. How can I avoid this behavior?

Upvotes: 0

Views: 129

Answers (1)

JaMiT
JaMiT

Reputation: 16963

Some important pieces of this code:

  • A lambda captures a DynamicValue object by value (copy).
  • The lambda is used to initialize a std::variant as a std::function alternative.
  • There is no explicit move constructor for DynamicValue, so the template for invocable objects is used as the move constructor.

The problematic code path starts with the request to construct a DynamicValue object from the lambda. This invokes the template constructor, which attempts to copy the lambda into the std::function alternative of the variant. So far, so good. Copying (not moving) the lambda copies the captured object without problems.

However, this procedure works when the CopyConstructible named requirement is satisfied. Part of this named requirement is being MoveConstructible. In order for a lambda to satisfy MoveConstructible, all of its captures have to satisfy that named requirement. Is this the case for DynamicValue? What happens when your standard library tries to move the lambda (hence also the captured object), with copying as the fallback? While DynamicValue has no explicit move constructor, it is invocable...

When F is DynamicValue<T>, the template constructor serves as the move constructor. It tries to initialize the variant by converting the source DynamicValue (the captured copy of a in the question's code) into a std::function. This is allowed, a copy of the source is made, and the process continues until the copy needs to be moved, at which point the move constructor is again invoked. This time, it tries to initialize the variant by converting the copy of the source DynamicValue into a std::function. This is allowed, a copy of the copy of the source is made, and the process continues until the copy of the copy needs to be moved, at which point the move constructor is again invoked. Etc.

Instead of moving the DynamicValue into the new object, each "move constructor" tries to move the DynamicValue into the variant of the new object. This would add another layer of overhead with each move, except the recursive calls blow up before construction finishes.

The solution is to make DynamicValue move constructible. There are at least two ways to do this.

1) Explicitly provide a move constructor.

    DynamicValue(DynamicValue&& value) : getter(std::move(value.getter)) {}

2) Exclude DynamicValue from being a template argument to the template constructor.

    template <typename F, typename = std::enable_if_t<std::is_invocable_v<F>>,
                          typename = std::enable_if_t<!std::is_same_v<std::decay_t<F>, DynamicValue>>>
    DynamicValue(F&& function) : getter(function) {}

Note that this excludes only DynamicValue<T> from being a template argument, not DynamicValue<U> when U is not T. That might be another issue to contemplate.

You might want to see if this also fixes whatever problem led you to define a second copy constructor. That may have been a band-aid approach that did not address this underlying issue.

Upvotes: 2

Related Questions