feat: Enable Delegates to conveniently use stateful lambdas

[benjaminhuth]

This PR adds functionality to the Acts::Delegate, that enables to construct, assign and connect stateful lambdas like this:

int n = 0;
auto lambda = [&](){ ++n; };

Acts::Delegate<void()> d(lambda);
d = lambda;
d.connect(lambda);

The use of temporary stateful lambdas is forbidden due to deleted overloads with Callable &&:

Acts::Delegate<void()> d([&](){ ++n; }); // compile error

I havn't checked yet if this can simplify code somewhere in Core or Examples, but I ran in situations myself where this would be useful.

[codecov]

Codecov Report

Merging #1410 (a8454f5) into main (ba1e563) will increase coverage by 0.02%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main    #1410      +/-   ##
==========================================
+ Coverage   47.76%   47.79%   +0.02%     
==========================================
  Files         380      380              
  Lines       20164    20173       +9     
  Branches     9387     9387              
==========================================
+ Hits         9632     9641       +9     
  Misses       4083     4083              
  Partials     6449     6449              

Impacted Files	Coverage Δ
Core/include/Acts/Utilities/Delegate.hpp	83.87% <100.00%> (+6.59%)	⬆️

📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more

[paulgessinger]

So this just explicitly tries to grab the operator() function pointer for a lambda? Should be ok I think!

[asalzburger]

@paulgessinger is this still waiting for the sentinel backlog?

[paulgessinger]

Still working on fixing that, sorry. I'll override the merge here.

[timadye]

Can we document the additional functionality in this PR in the comments at the top. I saw the limitations listed there and used a complicated workaround, when I could have just used connect(lambda). Fortunately, I eventually noticed the comments further down.

[paulgessinger]

Thinking about this again, I'm wondering if we should make this more explicit somehow, with a tag type to clearly mark what's going on.

I've seen some cases where this was (ab)used and it can be a bit of a footgun. We use the lvalue-ness to imply it's owned somewhere... but that's not clear to the user at all.

Thoughts?

[andiwand]

ping @benjaminhuth

[timadye]

I assume the issue is that the delegate does not own the lambda - or is there something else I might be missing? My use has the lambda in the same scope as the extensions (and hence delegate), so this is good. It is made clear in the comments at the top of Delegate.h. I'm not sure what else one can do to protect the user.

[benjaminhuth]

@paulgessinger I'm not 100% sure what you mean by "make it more explicit"?

Like Delegate(my_stateful_lambda, Acts::Stateful{}) resp my_delegate.connect(my_stateful_lambda, Acts::Stateful{})? And what kind of abuses you refer to?

So to me it seems like a good proposal to improve the documentation to showcase the construction/connection with stateful lambdas...

[paulgessinger]

I've seen it being used to set a lambda and then not being aware that it immediately dangles when the lambda itself goes out of scope.

This can happen as well of course with member functions but it's less likely to do by mistake I'd say.