0

Consider the following situation:

struct A {
    // `unique_function` is like std::function but does not
    // require its target to be Copyable, and is in turn
    // itself not Copyable either.
    unique_function<void()> callback = []{};

    void set_callback(unique_function<void()> new_callback) {
        callback = std::move(new_callback);
        callback();
    }
};

int main() {
    A a;
    a.set_callback([&, x = std::make_unique<int>(42)]{
        std::cout << "Ok at this point, *x = " << *x << ".\n";
        a.set_callback([]{}); // This destroys enclosing lambda
        // while is is being executed, thus messing things up.
        // Captures are not legal to access here.
    });
}

The intended semantics in this situation is to have all callbacks called when they are set, and have the last one set be stored within callback when the top set_callback returns.

Solutions I can see:

  1. Replace unique_function<void()> with something Copyable, like std::shared_ptr<unique_function<void()>> or specialized shared_function<void()> and then call the copy:

    shared_function<void()> callback = []{};
    
    void set_callback(shared_function<void()> new_callback) {
        callback = std::move(new_callback);
        auto callback_copy = callback;
        callback_copy();
    }
    

    Drawbacks:

    1. Overhead or reference counting.
  2. Have a list, and append callbacks to it before executing them. When top set_callback returns, callbacks.back() is the last callback set, and the rest can be erased. This requires an additional counter tracking current depth of set_callback calls.

    std::deque<unique_function<void()>> callbacks;
    std::size_t depth = 0;
    
    struct depth_controller {
        std::size_t& depth;
        depth_controller(std::size_t& d) : depth(d) { ++depth; }
        ~depth_controller() { --depth; }
    };
    
    void set_callback(shared_function<void()> new_callback) {
        callbacks.emplace_back(std::move(new_callback));
        {
            depth_controller dctl{depth};
            callbacks.back()();
        }
        if (depth > 0) { return; }
        callbacks.erase(callbacks.begin(),
                        std::prev(callbacks.end()));
    }
    

    Drawbacks:

    1. Overhead of callback list and depth counter.

Can we do better?

yuri kilochek
  • 12,709
  • 2
  • 32
  • 59
  • 1
    Can't you just keep the old callback around until the new one finishes executing? `auto old = std::move(callback); callback = std::move(new_callback); callback();` or simply: `swap(callback, new_callback); callback();` – Jonathan Wakely Feb 24 '17 at 14:53
  • @JonathanWakely it seems that I can, yes. Now I feel stupid. – yuri kilochek Feb 24 '17 at 15:10
  • Well it's not that obvious, but I've dealt with problems like this before. – Jonathan Wakely Feb 24 '17 at 15:13
  • @JonathanWakely, actually wait, no. If I do that then `a.set_callback(/* foo: */[&]{ a.set_callback(/* bar: */[]{}); /* at this point foo has been moved to local variable 'old' within preceding 'set_callback' call, and subsequently destroyed when 'set_callback' returned. */})` – yuri kilochek Feb 24 '17 at 15:31
  • Yes. You could return the old callback to the caller so the caller can keep it alive longer. But at some point you need to limit what you expect to work. – Jonathan Wakely Feb 24 '17 at 15:37

0 Answers0