Update Delegate and Signal for lambdas

[DanRStevens]

Our Delegate code is super old. We should consider updating it.

---

The Delegate code has a lot of macro #if to support really old compilers. Our code relies on C++17, and perhaps soon, C++20. That means we need a reasonably modern compiler. We could gut many of the #if checks for old compiler support from the Delegate code.

---

Our custom Delegate implementation is from about 2005. Source:
https://www.codeproject.com/Articles/11015/The-Impossibly-Fast-C-Delegates

The current implementation far predates the C++11 standard, which introduced std::function. It seems std::function replaces the need to write a custom Delegate class. Maybe we should update to using std::function and get rid of the custom Delegate class.

---

Our current implementation of Delegate and Signal only really supports member function pointers. We should add lambda support when calling Signal::connect and Signal::disconnect.

NAS2D::Signal<> signal;
const auto delegate = [&object]() { object.method(); };

signal.connect(delegate);
signal.emit();
signal.disconnect(delegate);

It may be possible to inline the lambda. Here's how lambdas would compare to the existing way:

signal.connect([&object]() { object.method(); });
signal.connect(&object, &object::method);

If we move to std::function, it may make sense to use lambdas as the primary means of representing Delegate. That might mean using it as the primary storage type internally, or perhaps updating calls to prefer passing lambdas.

[DanRStevens]

One area of concern I wanted to mention regarding the use of std::function, is there are some reports of performance issues with std::function, as compared to using lambas with templated methods. In particular, it adds a virtual function call, and may result in allocation of external memory when storing certain large functions, such as lambdas with a large capture group. These are not likely to be of any concern to our particular use case. In particular, for user input handlers, the tiny amount of overhead is never going to matter. Nevertheless, it might be good to better understand the situation by learning how std::function works.

First off, the performance comparison was relating std::function to template methods taking a lambda, which could inline the lambda call when instantiating the template method. The current implementation of Delegate doesn't do that. It uses pointers internally, and so it's already very similar to the virtual function call dispatch offered by std::function. The main concern then is the potential allocation of external memory.

Second, the potential use of external memory is because std::function takes ownership of the function it holds, and it can hold arbitrarily large functors, such as lambdas with large capture groups. The current implementation of Delegate doesn't provide for capturing anything beyond the this pointer, and a member function pointer, so we effectively already limit capture group size. Further, std::function offers a SBO (Small Buffer Optimization), which is generally sufficient to store such references without allocating external memory. Effectively, the only cases where std::function should need to allocate, are cases not currently supported by the Delegate code.

According to the C++ standard, the std::function object will not allocate external memory when used to store two particular small cases:

• a function pointer
• a std::reference_wrapper to a callable object

Notably absent is a pointer to a member function. This is relevant, as all our current uses are pretty similar to calling std::bind on a this pointer with a member function pointer. As can be demonstrated, using std::bind to group a this pointer with a member function pointer does generally produce a functor that is too large to store in a std::function without externally allocated memory. However, the solution to that is simply to use a lambda rather than std::bind, as the lambda generally does fit without the SBO constraints offered by std::function.

Example:

#include <iostream>
#include <functional>

struct Object{
	void method() {}
};

int main() {
	Object object;

	const auto func1 = std::bind(&Object::method, &object);
	const auto func2 = [&object]{ object.method(); };

	std::cout << sizeof(func1) << std::endl; // 24 - (64-bit g++)
	std::cout << sizeof(func2) << std::endl; // 8 - (64-bit g++)

	std::function<void()> callback;
	callback = func1; // external allocation
	callback = func2; // no external allocation
}

The reason for the difference in size is the lambda stores only a single pointer to object, where as the std::bind call produces an object with a pointer to object, and a somewhat large member function pointer, which consists of a function pointer and an offset value for the this pointer. For the lambda, that member function pointer and this adjustment is effectively compiled into a thunk when the compiler generates code for the lambda's operator(), and so become part of the code rather than extra data fields.

As for verifying the lambda can be stored without extra memory allocation, that can be done with valgrind, or with other test code, such as is presented in one of the ansers to this StackOverflow question:
Avoid memory allocation with std::function and member function

In summary, std::function can replace all our current uses of Delegate with no loss of efficiency, plus support other new uses of lambdas, which may (or may not) incur a slight overhead over the existing code if they are actually used (and only when they are used).

[DanRStevens]

On a related matter, there is a proposal for a non-owning function_ref that never requires external storage. Some reasonably short code (~200 line) can be found on GitHub:

• https://github.com/TartanLlama/function_ref

It may be worth giving some consideration to ownership characteristics. Should Signal store owning references or non-owning references to delegates.

Currently SignalSource stores the Delegate instances internally, and makes calls to them at arbitrary later times. This seems to imply ownership, and so it would make sense for SignalSource to use the characteristics of std::function.

On the other hand, SignalSource offers a disconnect function, which requires passing in a Delegate instance, which can then be searched for and removed from the list of connections. This implies the Delegate instance may exist externally for the duration of it's active lifetime. This would imply that function_ref may have suitable characteristics.

Somewhat complicating the matter is that all Delegates are cleaned up when SignalSource is destructed, and so automatically cleaned up Delegate instances might not have any corresponding external instances. Additionally, the current Delegate class has a trivial destructor, so there really is no cleanup, and no particular relevance to ownership characteristics. In fact, during cleaning, sometimes a new Delegate object is constructed, which is merely compared as having equal value to the stored Delegate instance (corresponding fields are equal), rather than having the same identity (same address).

---

When the delegate is a callback to a long lived object, extra storage can be kept in that long lived object, rather than the delegate. Such a setup makes std::function and std::function_ref somewhat equivalent. Instead of capturing lots of values and copying them into a lambda, a reference can be stored to externally managed data. This is pretty much the example from the earlier posts where the delegates captures &object.

For more local usage, such as for an immediate callback, catpure group parameters could potentially be stored in a struct or std::tuple, which is captured by reference:

auto data = std::tuple(a, b, c);
auto func = [&data]() { doSomething(data); };
someFunctionCallWithACallback(func);

This seems to be more what function_ref is about, since here the lambda doesn't need to live past the end of the function call it was passed to. For such a case, the function could take a function_ref instead of a std::function, which would allow it to bind to a lambda with a larger capture group without external memory allocation:

someFunctionCallWithACallback([&a, &b, &c]() { doSomething(a, b, c); });

This isn't really our use case though.

I think we should probably stick with std::function, rather than try to force function_ref hoping to gain efficiencies in areas we don't actually need or could easily work around.

---

A deeper question is if we even need the Signal abstraction. The point of SignalSource is to distribute an event to multiple listeners. The current code base only appears to ever have a single listener for any event. We could be using Delegate directly, rather than using SignalSource. Though I suppose there is that "what if" scenario.

[DanRStevens]

I spent a fair bit of time doing research into std::function, and background material on "Type Erasure". I thought maybe I should store a list of links for posterity.

Basic research:

• Avoid memory allocation with std::function and member function
• Avoid memory allocation with std::function and member function
• Avoiding The Performance Hazzards of std::function
• C++ type erasure

Andrzej's C++ blog:

• Type erasure — Part I
• Type erasure — Part II
• Type erasure — Part III
• Type erasure — Part IV

The Old New Thing:

• Inside std::function, part 1: The basic idea
• Inside std::function, part 2: Storage optimization

Arthur O'Dwyer's blog:

• What is Type Erasure?
• The space of design choices for std::function

Misc related research:

• Be Mindful with Compiler-Generated Move Constructors
• Why C++ Member Function Pointers Are 16 Bytes Wide
• Pointers to member functions are very strange animals
• Value semantics
• std::ref and std::reference_wrapper: common use cases

Standard library documentation:

• std::function
• std::reference_wrapper
• std::ref, std::cref
• std::bind

Standard library implementations and proposals:

• GCC std::function
• Clang std::function
• MSVC std::function
• function_ref
• function_ref: a non-owning reference to a Callable

[DanRStevens]

So I've discovered a bit of a snag. There is no operator== on std::function, which means you can't search a collection for an instance of one, either to check for duplicates in connect, or to remove an existing entry in disconnect.

There is a target template method, which can convert the std::function to the underlying function type, and which could potentially be used to write an equality comparison. However, if you're converting to a lambda type, the lambda also lacks operator==.

Conceptually it should be fairly simple to compare a lambda instance if you know they're the same type, and all captures are equality comparable. It would essentially be the same as the default equality comparison for any regular struct. Though given the nature of lambdas, I don't know of any way to add such a comparison operator to one.

[DanRStevens]

Bit of a side note, but I've always found the DefaultVoidToVoid template in the Delegate code to be a bit unexpected. It doesn't appear to actually do anything. I assume it was some kind of workaround for an earlier compiler bug, though I've never found any document that describes that in any sort of detail.

Edit: Found this:
https://github.com/dreamcat4/FastDelegate/blob/master/FastDelegate.h

// DefaultVoid - a workaround for 'void' templates in VC6.
//
// (1) VC6 and earlier do not allow 'void' as a default template argument.
// (2) They also doesn't allow you to return 'void' from a function.
//
// Workaround for (1): Declare a dummy type 'DefaultVoid' which we use
// when we'd like to use 'void'. We convert it into 'void' and back
// using the templates DefaultVoidToVoid<> and VoidToDefaultVoid<>.
// Workaround for (2): On VC6, the code for calling a void function is
// identical to the code for calling a non-void function in which the
// return value is never used, provided the return value is returned
// in the EAX register, rather than on the stack.
// This is true for most fundamental types such as int, enum, void *.
// Const void * is the safest option since it doesn't participate
// in any automatic conversions. But on a 16-bit compiler it might
// cause extra code to be generated, so we disable it for all compilers
// except for VC6 (and VC5).
#ifdef FASTDLGT_VC6
// VC6 workaround
typedef const void * DefaultVoid;
#else
// On any other compiler, just use a normal void.
typedef void DefaultVoid;
#endif

So it looks like it's a workaround for VC6. That's pretty old, and rather irrelevant at this point.

---

Edit: Found a lot of comments describing why things are they way they are were removed back in:
7a92544