Simple callback wrapper for an embedded C++ app

I'm writing a callback wrapper class for an embedded application in C++11.

The basic idea of the class is to be able to use it as a replacement instead of C-style callback functions, with the added benefit of being able to wrap functors and lambdas.

Requirements

• No dynamic memory allocations are allowed at all.

• It only needs to handle small functors (and lambdas).

• Return type of the wrapped functions is always void.

• It should be able to wrap C-style callback functions too.

Some implementation details

Basically, the class holds a function pointer with an optional user-specified parameter that is passed in as the first argument to the function pointer. It also has a small statically-allocated byte array in order to be able to wrap small functors and lambdas.

The wrapping is done in a templated operator= function (and a constructor) overload which simply copies the state of the functor (or lambda), and then sets the function pointer to a function that calls it.

The code

Here is the implementation of the class.

I also wrote some Doxygen-style API docs, hope it's okay to keep them intact here.

```
/**
* Callback is a functor class that helps wrapping callback functions.
*
* It stores a function pointer and an optional user-specified parameter. The
* function pointer has a signature that takes the user-specified parameter first,
* and then the specified argument types.
*
* It also provides a convenient way to wrap C++ lambda functions, even capturing
* lambdas into a callback.
*
* Template parameters of this class are the argument types of the callback.
*/
template
class Callback final {

public:

/**
* The function pointer type that is stored in this class.
*/
typedef void (Function)(void , TArgs ...);

/**
* The function pointer type without the user parameter that is stored in this class.
*/
typedef void (*SimpleFunction)(TArgs ...);

private:

/**
* The function pointer that is execute

A few thoughts about your code:

1. Use of reinterpret_cast

// Create a simple function that calls the functor
    this->func_ = [](void *user, TArgs ... args) -> void {
        TFunctor *functorPtr = reinterpret_cast(user);
        (*functorPtr)(args...);
    };

This always raises a red flag for me. I understand that the user parameter is used to pass the actual functor instance, and it's a common technique to interface with C-style callback API's.

May be it deserves more commenting why exactly it will be used here, and what exactly user is expected to carry.

1. Using a fixed size for the functorData_ member

This makes your class less flexible for generic usage.

You could use an additional template parameter to specify the size

template 
class Callback final {
    // ...
    uint8_t functorData_[MaxFunctorSize];
};

and use that with the static_asserts etc.

At least the callback holder could decide then, how many stack storage should me reserved as maximum.

The downside is, that the maximum size cannot be defaulted to a standard value because of the variadic parameter pack must appear as the last parameter.

1. Wasting memory for sake of making client code easier to write

I'm not a 100% sure that wasting (stack) space is worth that (especially when dealing with a small MCU at the bare metal level).

May be the design could be improved, and less memory intrusive with bit a more complicated SFINAE based class model to handle the different cases

• Simple C-Style function pointer

• Functor object / Lambda expression

1. Potential slicing problems with functor instances

I smell potential slicing problems with virtual polymorphic functor class hierarchies.

The functor interface using functorData_ may fail to work properly with virtual inheritance:

struct IFunctor {
    virtual void operator()(const uint8_t*, uint32_t) = 0;
    virtual ~IFunctor() = default;
};

struct AFunctor : IFunctor {
    virtual void operator()(const uint8_t*, uint32_t) {
        // Do something
    }
};

void doSomething() {
    SerialPort port;
    AFunctor aFunc;
    IFunctor* iFunc = &aFunc;
    port.setReceiveDoneCallback(*iFunc);
 }

Here is a Demo

You can avoid that adding another static_assert:

static_assert(!std::is_polymorphic(),"Functor object must not be polymorphic.");

Demo

Note that this is related to my points 1. and 2.

1. The use of inline is superfluous inside the class declaration body

As mentioned above, you don't even need to use inline inside of the class body declaration. That's merely a hint for the compiler, and will be done (or overridden) regarding the chosen optimization strategy anyways.

The only case you must use the inline keyword is, if you're defining global functions in a multiply included header file. Otherwise you are going to violate the One Definition Rule principle.

Regarding GCC's behavior, it's documented here (emphasis mine):


GCC does not inline any functions when not optimizing unless you specify the ‘always_inline’ attribute for the function, like this:

/* Prototype.  */
inline void foo (const char) __attribute__((always_inline));

Otherwise, it depends on optimization settings.

1. The use of final is superfluous

Since your class doesn't expose any virtual functions that could be potentially overridden by inheriting classes, you simply can omit it.

See also: In C++, when should I use final in virtual method declaration?

StackExchange Code Review Q#152031, answer score: 6