From 981b085372e3c9b14f01604dbf4a0f55224872c8 Mon Sep 17 00:00:00 2001 From: Florian Fleissner Date: Tue, 2 Apr 2019 18:39:31 +0200 Subject: [PATCH 1/2] Event handler versioning, deprecation and extended signature checks Recently a proposed change to the firmware required a change to one of the event handlers'/hooks' signatures. Up to now, we only supported a single overload/implementation of a event handler of a specific type. This means that changing handler signatures was only possible by adding a handler method with a similar name and a different signature like EventHandlerResult onKeyswitchEvent(Key &mappedKey, byte row, byte col, uint8_t keyState); and EventHandlerResult onKeyswitchEvent2(Key &mappedKey, KeyAddr key_addr, uint8_t keyState); As this was regarded as unacceptable confusion of the users of our programming interface, this commit introduces some changes to the event handler management. Important changes: * handlers can now be overloaded, * handler signatures can be versioned, * individual versions can be declared as deprecated, * we now abort the compile if a handler reimplementation with a bad signature is found, * or if two or more handlers with correct signatures are detected that are furtheron considered as ambiguous. Impact on resources: The proposed changes only affect the way things are handled at compile time. No changes in terms of PROGMEM or RAM are to be expected. Signed-off-by: Florian Fleissner --- src/kaleidoscope/event_handlers.h | 111 +++++++ src/kaleidoscope/hooks.cpp | 5 +- src/kaleidoscope/hooks.h | 3 +- src/kaleidoscope/plugin.h | 11 - src/kaleidoscope_internal/event_dispatch.h | 60 +++- .../eventhandler_signature_check.h | 271 +++++++++++++----- .../type_traits/has_member.h | 126 ++++++++ 7 files changed, 501 insertions(+), 86 deletions(-) create mode 100644 src/kaleidoscope_internal/type_traits/has_member.h diff --git a/src/kaleidoscope/event_handlers.h b/src/kaleidoscope/event_handlers.h index 39f12a81..6a4cda41 100644 --- a/src/kaleidoscope/event_handlers.h +++ b/src/kaleidoscope/event_handlers.h @@ -35,15 +35,68 @@ #define _ABORTABLE true #define _NOT_ABORTABLE false +#define _NOT_DEPRECATED + +//****************************************************************************** +//****************************************************************************** +// Important +//****************************************************************************** +//****************************************************************************** +// When adding new handlers or handler versions, don't forget +// to add them both to the _FOR_EACH_EVENT_HANDLER and +// the _PROCESS_EVENT_HANDLER_VERSIONS function macros. +//****************************************************************************** +//****************************************************************************** + +// The following properties are passed to each invocation of the OPERATION +// macro of the below definition of macro function _FOR_EACH_EVENT_HANDLER: +// +// handler name: +// This is the name that must be used to implement a handler in a plugin. +// +// handler version: +// Sometimes changes to the firmware's implementation require handler +// call signatures (the list of parameters) to be adapted. To allow +// for a deprecation periode to avoid breaking user code, it is +// desirable to have multiple overloads of a handler with the same +// name at the same time. Every overload must be assigned a version +// number and every individual version must appear in the below definition +// of the macros _FOR_EACH_EVENT_HANDLER and _PROCESS_EVENT_HANDLER_VERSIONS. +// +// deprecation flag: +// If a specific version is meant to be deprecated, pass the DEPRECATED(...) +// macro with a meaningful deprecation message. Pass _NOT_DEPRECATED +// otherwise. +// +// abortable flag: +// Plugins' event handlers are called in the same order as the plugins +// are passed to the KALEIDOSCOPE_INIT_PLUGINS(...) macro within the +// sketch. For some handlers it is desirable to not call subsequent +// plugins' handlers once a plugin's handler returned the value +// kaleidoscope::EventHandlerResult::EVENT_CONSUMED. To enable this +// pass the abortable flag value _ABORTABLE, _NOT_ABORTABLE otherwise. +// +// call signature: +// The type of arguments passed to the handlers as a comma separated +// list in brackets. Every parameter must be named. +// +// call parameters: +// The list of parameters as they would be passed to a call to the handler. +// Parameter names must match the names assigned to the call arguments. + #define _FOR_EACH_EVENT_HANDLER(OPERATION, ...) __NL__ \ __NL__ \ OPERATION(onSetup, __NL__ \ + 1, __NL__ \ + _NOT_DEPRECATED, __NL__ \ _NOT_ABORTABLE, __NL__ \ (),(), ##__VA_ARGS__) __NL__ \ __NL__ \ /* Called at the very start of each cycle, before gathering */ __NL__ \ /* events, before doing anything else. */ __NL__ \ OPERATION(beforeEachCycle, __NL__ \ + 1, __NL__ \ + _NOT_DEPRECATED, __NL__ \ _NOT_ABORTABLE, __NL__ \ (), (), ##__VA_ARGS__) __NL__ \ __NL__ \ @@ -55,6 +108,8 @@ /* to react to the event. If it returns anything else, Kaleidoscope */ __NL__ \ /* will stop processing there. */ __NL__ \ OPERATION(onKeyswitchEvent, __NL__ \ + 1, __NL__ \ + _NOT_DEPRECATED, __NL__ \ _ABORTABLE, __NL__ \ (Key &mappedKey, byte row, byte col, uint8_t keyState), __NL__ \ (mappedKey, row, col, keyState), ##__VA_ARGS__) __NL__ \ @@ -67,6 +122,8 @@ /* EventHandlerResult::EVENT_CONSUMED, in which case no other */ __NL__ \ /* plugin will have a chance to react to the event. */ __NL__ \ OPERATION(onFocusEvent, __NL__ \ + 1, __NL__ \ + _NOT_DEPRECATED, __NL__ \ _ABORTABLE, __NL__ \ (const char *command), __NL__ \ (command), ##__VA_ARGS__) __NL__ \ @@ -75,17 +132,71 @@ /* not passed as arguments. If one needs that info, they should */ __NL__ \ /* track Layer.getState() themselves. */ __NL__ \ OPERATION(onLayerChange, __NL__ \ + 1, __NL__ \ + _NOT_DEPRECATED, __NL__ \ _NOT_ABORTABLE, __NL__ \ (), (), ##__VA_ARGS__) __NL__ \ /* Called before reporting our state to the host. This is the */ __NL__ \ /* last point in a cycle where a plugin can alter what gets */ __NL__ \ /* reported to the host. */ __NL__ \ OPERATION(beforeReportingState, __NL__ \ + 1, __NL__ \ + _NOT_DEPRECATED, __NL__ \ _NOT_ABORTABLE, __NL__ \ (),(),##__VA_ARGS__) __NL__ \ __NL__ \ /* Called at the very end of a cycle, after everything's */ __NL__ \ /* said and done. */ __NL__ \ OPERATION(afterEachCycle, __NL__ \ + 1, __NL__ \ + _NOT_DEPRECATED, __NL__ \ _NOT_ABORTABLE, __NL__ \ (),(),##__VA_ARGS__) + +// The following function macro lists event handler/hook method names and +// available versions. It is used to auto-generate code that is +// related to event handlers. +// +// The operators START and END are invoked with a list of all +// available hook versions while the operator OP is invoked for every +// version individually. +// +// If necessary, add handlers as provided in the following example. +// +// Example: Lets assume that a hook fancyHook has three versions, +// then the operator invokation would be. +// +// START(fancyHook, 1, 2, 3) +// OP(fancyHook, 1) +// OP(fancyHook, 2) +// OP(fancyHook, 3) +// END(fancyHook, 1, 2, 3) +// +#define _PROCESS_EVENT_HANDLER_VERSIONS(START, OP, END, ...) __NL__ \ + START(onSetup, 1) __NL__ \ + OP(onSetup, 1) __NL__ \ + END(onSetup, 1) __NL__ \ + __NL__ \ + START(beforeEachCycle, 1) __NL__ \ + OP(beforeEachCycle, 1) __NL__ \ + END(beforeEachCycle, 1) __NL__ \ + __NL__ \ + START(onKeyswitchEvent, 1) __NL__ \ + OP(onKeyswitchEvent, 1) __NL__ \ + END(onKeyswitchEvent, 1) __NL__ \ + __NL__ \ + START(onFocusEvent, 1) __NL__ \ + OP(onFocusEvent, 1) __NL__ \ + END(onFocusEvent, 1) __NL__ \ + __NL__ \ + START(onLayerChange, 1) __NL__ \ + OP(onLayerChange, 1) __NL__ \ + END(onLayerChange, 1) __NL__ \ + __NL__ \ + START(beforeReportingState, 1) __NL__ \ + OP(beforeReportingState, 1) __NL__ \ + END(beforeReportingState, 1) __NL__ \ + __NL__ \ + START(afterEachCycle, 1) __NL__ \ + OP(afterEachCycle, 1) __NL__ \ + END(afterEachCycle, 1) diff --git a/src/kaleidoscope/hooks.cpp b/src/kaleidoscope/hooks.cpp index c8da95f5..ce6aea90 100644 --- a/src/kaleidoscope/hooks.cpp +++ b/src/kaleidoscope/hooks.cpp @@ -29,10 +29,11 @@ namespace kaleidoscope { // during the transition phase. #define INSTANTIATE_WEAK_HOOK_FUNCTION( \ - HOOK_NAME, SHOULD_ABORT_ON_CONSUMED_EVENT, SIGNATURE, ARGS_LIST) __NL__ \ + HOOK_NAME, HOOK_VERSION, DEPRECATION_TAG, \ + SHOULD_ABORT_ON_CONSUMED_EVENT, SIGNATURE, ARGS_LIST) __NL__ \ __NL__ \ __attribute__((weak)) __NL__ \ - EventHandlerResult Hooks::HOOK_NAME SIGNATURE { __NL__ \ + EventHandlerResult Hooks::HOOK_NAME SIGNATURE { __NL__ \ return EventHandlerResult::OK; __NL__ \ } diff --git a/src/kaleidoscope/hooks.h b/src/kaleidoscope/hooks.h index 9fd57ead..1a56ce2e 100644 --- a/src/kaleidoscope/hooks.h +++ b/src/kaleidoscope/hooks.h @@ -64,7 +64,8 @@ class Hooks { // and functions that are declared as friends above. #define DEFINE_WEAK_HOOK_FUNCTION( \ - HOOK_NAME, SHOULD_ABORT_ON_CONSUMED_EVENT, SIGNATURE, ARGS_LIST) __NL__ \ + HOOK_NAME, HOOK_VERSION, DEPRECATION_TAG, \ + SHOULD_ABORT_ON_CONSUMED_EVENT, SIGNATURE, ARGS_LIST) __NL__ \ __NL__ \ static EventHandlerResult HOOK_NAME SIGNATURE; diff --git a/src/kaleidoscope/plugin.h b/src/kaleidoscope/plugin.h index 0a7c62ac..2c7c1193 100644 --- a/src/kaleidoscope/plugin.h +++ b/src/kaleidoscope/plugin.h @@ -31,17 +31,6 @@ class Plugin { public: // Please see "event_handlers.h" for a list of supported event handlers and // their documentation! - -#define DEFINE_AND_IMPLEMENT_EVENT_HANDLER_METHOD( \ - HOOK_NAME, SHOULD_ABORT_ON_CONSUMED_EVENT, SIGNATURE, ARGS_LIST) __NL__ \ - __NL__ \ - EventHandlerResult HOOK_NAME SIGNATURE { __NL__ \ - return EventHandlerResult::OK; __NL__ \ - } - - _FOR_EACH_EVENT_HANDLER(DEFINE_AND_IMPLEMENT_EVENT_HANDLER_METHOD) - -#undef DEFINE_AND_IMPLEMENT_EVENT_HANDLER_METHOD }; } // namespace kaleidoscope diff --git a/src/kaleidoscope_internal/event_dispatch.h b/src/kaleidoscope_internal/event_dispatch.h index 43db3165..84b271ae 100644 --- a/src/kaleidoscope_internal/event_dispatch.h +++ b/src/kaleidoscope_internal/event_dispatch.h @@ -70,12 +70,43 @@ // the event handler method 'Foo' of each registered plugin with a given // set of arguments. +// The following macros help to generate type or instance names +// by concatenating macro parameter tokens. +// +#define _NAME3(A, B, C) A##B##C +#define _NAME4(A, B, C, D) A##B##C##D +#define _NAME5(A, B, C, D, E) A##B##C##D##E + #define _REGISTER_EVENT_HANDLER( \ - HOOK_NAME, SHOULD_ABORT_ON_CONSUMED_EVENT, SIGNATURE, ARGS_LIST) __NL__ \ + HOOK_NAME, HOOK_VERSION, DEPRECATION_TAG, \ + SHOULD_ABORT_ON_CONSUMED_EVENT, SIGNATURE, ARGS_LIST) \ __NL__ \ namespace kaleidoscope_internal { __NL__ \ __NL__ \ - struct EventHandler_##HOOK_NAME { __NL__ \ + template __NL__ \ + struct _NAME5(EventHandler_, HOOK_NAME, _v, HOOK_VERSION, _caller) { __NL__ \ + static DEPRECATION_TAG kaleidoscope::EventHandlerResult __NL__ \ + call(Plugin__ &plugin, Args__&&... hook_args) { __NL__ \ + return plugin.HOOK_NAME(hook_args...); __NL__ \ + } __NL__ \ + }; __NL__ \ + __NL__ \ + /* This specialization is used for those hooks that a plugin does not __NL__ \ + * implement. __NL__ \ + */ __NL__ \ + template __NL__ \ + struct _NAME5(EventHandler_, HOOK_NAME, _v, HOOK_VERSION, _caller) __NL__ \ + { __NL__ \ + static kaleidoscope::EventHandlerResult __NL__ \ + call(Plugin__ &/*plugin*/, Args__&&... /*hook_args*/) { __NL__ \ + return kaleidoscope::EventHandlerResult::OK; __NL__ \ + } __NL__ \ + }; __NL__ \ + __NL__ \ + struct _NAME4(EventHandler_, HOOK_NAME, _v, HOOK_VERSION) { __NL__ \ __NL__ \ static bool shouldAbortOnConsumedEvent() { __NL__ \ return SHOULD_ABORT_ON_CONSUMED_EVENT; __NL__ \ @@ -84,8 +115,25 @@ template __NL__ \ static kaleidoscope::EventHandlerResult __NL__ \ call(Plugin__ &plugin, Args__&&... hook_args) { __NL__ \ + __NL__ \ _VALIDATE_EVENT_HANDLER_SIGNATURE(HOOK_NAME, Plugin__) __NL__ \ - return plugin.HOOK_NAME(hook_args...); __NL__ \ + __NL__ \ + static constexpr bool derived_implements_hook __NL__ \ + = HookVersionImplemented_##HOOK_NAME< __NL__ \ + Plugin__, HOOK_VERSION>::value; __NL__ \ + __NL__ \ + /* The caller type adds another level of indirection that __NL__ \ + * is required to enable some hooks not to be implemented __NL__ \ + * by plugins. __NL__ \ + */ __NL__ \ + typedef _NAME5(EventHandler_, HOOK_NAME, _v, HOOK_VERSION, _caller) __NL__ \ + < __NL__ \ + derived_implements_hook, __NL__ \ + Plugin__, __NL__ \ + Args__... __NL__ \ + > Caller; __NL__ \ + __NL__ \ + return Caller::call(plugin, hook_args...); __NL__ \ } __NL__ \ }; __NL__ \ __NL__ \ @@ -95,7 +143,8 @@ __NL__ \ EventHandlerResult Hooks::HOOK_NAME SIGNATURE { __NL__ \ return kaleidoscope_internal::EventDispatcher::template __NL__ \ - apply __NL__ \ + apply __NL__ \ ARGS_LIST; __NL__ \ } __NL__ \ __NL__ \ @@ -146,4 +195,7 @@ /* */ __NL__ \ /* TODO(anyone): Move this somewhere else, outside of _internal, once */ __NL__ \ /* the V1 API is removed. */ __NL__ \ + __NL__ \ + _PREPARE_EVENT_HANDLER_SIGNATURE_CHECK __NL__ \ + __NL__ \ _FOR_EACH_EVENT_HANDLER(_REGISTER_EVENT_HANDLER) diff --git a/src/kaleidoscope_internal/eventhandler_signature_check.h b/src/kaleidoscope_internal/eventhandler_signature_check.h index 4cc7e24b..5064a14d 100644 --- a/src/kaleidoscope_internal/eventhandler_signature_check.h +++ b/src/kaleidoscope_internal/eventhandler_signature_check.h @@ -18,6 +18,7 @@ #include "kaleidoscope/macro_helpers.h" #include "kaleidoscope/plugin.h" +#include "kaleidoscope_internal/type_traits/has_member.h" // ************************************************************************* // ************************************************************************* @@ -56,33 +57,8 @@ // == decltype(&childPlugin::fooEventHandler) // // Unfortunately, this is not possible with C++-11, as it does not allow -// comparing function-types for equality. As a workaround, we can use the trait -// class EventHandlerSignaturesMatch to perform the comparison. -// -// It defines the bool constant 'value' as true if both signatures match. -// It checks the return value, argument types and const specification. -// (As of this writing, we don't have any const hook method.) - -template -struct EventHandlerSignaturesMatch { - static constexpr bool value = false; -}; - -// R: The return value, -// T1: Type of the first class (plugin), -// T2: Type of the second class (plugin), -// EventHandlerArgs: Variadic types of plugin event handler arguments. - -template -struct EventHandlerSignaturesMatch { - static constexpr bool value = true; -}; - -// Equivalent to allow for const-eventhandlers, e.g. bool getFooEventHandler() const -template struct -EventHandlerSignaturesMatch { - static constexpr bool value = true; -}; +// comparing function-types for equality. This code relies on several +// template traits classes to check if a function's call signature matches. // This template is instantiated when something goes wrong. // Because it does not define a constant 'value', it triggers a compiler error. @@ -96,45 +72,204 @@ template struct static constexpr bool value = true; }; -// If the pointer types are the same, the signatures match, causing -// the first or second specialization to be instantiated. This makes -// the causes the compile time constant `value` to be defined as `true`. -// Otherwise, the unspecialized version of the template class is instantiated -// and `value` is defined as `false` - -#define _VALIDATE_EVENT_HANDLER_SIGNATURE(EVENTHANDLER, PLUGIN) \ -{ __NL__ \ - /* Check if the signatures match. If not, the plugin has implemented */ __NL__ \ - /* a method with a hook's name, but a different signature. */ __NL__ \ - typedef EventHandlerSignaturesMatch< __NL__ \ - decltype(&::kaleidoscope::Plugin::EVENTHANDLER), __NL__ \ - decltype(&PLUGIN::EVENTHANDLER) __NL__ \ - > Check; __NL__ \ - __NL__ \ - static_assert(Check::value, __NL__ \ - VERBOSE_STATIC_ASSERT_HEADER __NL__ \ - "\nOne of your plugins implemented the \"" #EVENTHANDLER "\"" __NL__ \ - "\nevent handler, but its signature didn't match the base class." __NL__ \ - "\n" __NL__ \ - "\nThe plugin with this issue will be marked in the compiler" __NL__ \ - "\noutput with the string:" __NL__ \ - "\n" __NL__ \ - "\n ___________Culprit_Plugin___________." __NL__ \ - "\n" __NL__ \ - "\nYou should compare the event handlers implemented in this plugin" __NL__ \ - "\nto those in \"kaleidoscope::Plugin\"." __NL__ \ - "\n" __NL__ \ - "\nAll of the event handler's argument types, return values and" __NL__ \ - "\nconst qualifiers need to match." __NL__ \ - "\n" __NL__ \ - VERBOSE_STATIC_ASSERT_FOOTER __NL__ \ - ); __NL__ \ - __NL__ \ - /* The following construct is necessary enable reporting of the */ __NL__ \ - /* type of a plugin that implements an event handler with an */ __NL__ \ - /* incorrect signature, because it's not possible to include any */ __NL__ \ - /* non-literal string constant in a static_assert error message. */ __NL__ \ - __attribute__((unused)) constexpr bool dummy __NL__ \ - = ___________Culprit_Plugin___________ __NL__ \ - ::value; __NL__ \ +#define _NOOP(...) __NL__ \ + +#define _DEFINE_IMPLEMENTATION_CHECK_CLASSES(HOOK_NAME, ...) \ + __NL__ \ + /* Defines a traits class Plugin_HasMember_HOOK_NAME __NL__ \ + * where HOOK_NAME is replaced with the actual hook name __NL__ \ + * that is passed in as a parameter to this macro. __NL__ \ + */ __NL__ \ + DEFINE_HAS_MEMBER_TRAITS(Plugin, HOOK_NAME) __NL__ \ + __NL__ \ + /* Specializations of this helper class check if a plugin __NL__ \ + * implements a handler with the specific signature that is __NL__ \ + * identified by a hook version. __NL__ \ + */ __NL__ \ + template __NL__ \ + struct HookVersionImplemented_##HOOK_NAME {}; + +#define _DEFINE_IMPLEMENTATION_CHECK_CLASS_SPECIALIZATION( \ + HOOK_NAME, HOOK_VERSION, DEPRECATION_TAG, \ + SHOULD_ABORT_ON_CONSUMED_EVENT, SIGNATURE, ARGS_LIST) \ + __NL__ \ + /* This specialization checks if a plugin of type Plugin__ __NL__ \ + * implements a handler with given signature SIGNATURE. __NL__ \ + */ __NL__ \ + template __NL__ \ + struct HookVersionImplemented_##HOOK_NAME __NL__ \ + { __NL__ \ + /* Define a pointer to member function with the correct __NL__ \ + * argument signature. The newly defined type is named __NL__ \ + * HookType__. __NL__ \ + */ __NL__ \ + typedef kaleidoscope::EventHandlerResult __NL__ \ + (Plugin__::*HookType__)SIGNATURE; __NL__ \ + __NL__ \ + /* See the definition of HookIimplemented_##HOOK_NAME above __NL__ \ + * for an explanation of the SFINAE concept that is applied here. __NL__ \ + * The difference to the afforementioned class is that __NL__ \ + * here we check if a specific version of a handler was __NL__ \ + * implemented. This is done by forcing the compiler __NL__ \ + * through a static cast to select the respective method __NL__ \ + * if possible. If the method signature cannot be found, __NL__ \ + * the substitution fails and the first version of method "test" __NL__ \ + * will not be defined. __NL__ \ + */ __NL__ \ + template __NL__ \ + static constexpr __NL__ \ + /* The following decltype-clause defines the function return type __NL__ \ + */ __NL__ \ + decltype( __NL__ \ + /* If &PluginAux__::HOOK_NAME exists and is of type __NL__ \ + * HookType__, the list below evaluates to bool{} whose __NL__ \ + * type can be determined. Otherwise the comma expression __NL__ \ + * cannot be evaluated and the content __NL__ \ + * of decltype is undefined and this function overload __NL__ \ + * is ignored by the compiler __NL__ \ + * (SFINAE = substitution failure is not an error) __NL__ \ + * and the test(...) overload is used instead. __NL__ \ + */ __NL__ \ + static_cast(&PluginAux__::HOOK_NAME), bool{} __NL__ \ + ) __NL__ \ + test(int /* unused */) __NL__ \ + { __NL__ \ + return true; __NL__ \ + } __NL__ \ + __NL__ \ + template __NL__ \ + static constexpr bool test(...) __NL__ \ + { __NL__ \ + return false; __NL__ \ + } __NL__ \ + __NL__ \ + static constexpr bool value = test(int{}); __NL__ \ + }; + +#define _PREPARE_EVENT_HANDLER_SIGNATURE_CHECK_START(HOOK_NAME, ...) \ + __NL__ \ + /* This struct enables checking if a handler of a specific type __NL__ \ + * has been implemented. If so, there must be exactly one overload __NL__ \ + * with correct signature. __NL__ \ + */ __NL__ \ + template __NL__ \ + struct NumberOfImplementationsOf_##HOOK_NAME __NL__ \ + { __NL__ \ + static constexpr int value = __NL__ \ + 0 __NL__ \ + /* What follows is a list of handler implementation __NL__ \ + * checks for different versions of a handler __NL__ \ + * generated by several calls to __NL__ \ + * _PREPARE_EVENT_HANDLER_SIGNATURE_CHECK_OP. __NL__ \ + */ + +// This is invoked for every version of a hook. +// +#define _PREPARE_EVENT_HANDLER_SIGNATURE_CHECK_OP(HOOK_NAME, HOOK_VERSION) \ + + ((HookVersionImplemented_##HOOK_NAME::value __NL__ \ + ) ? 1 : 0) + +#define _PREPARE_EVENT_HANDLER_SIGNATURE_CHECK_END(HOOK_NAME, ...) \ + ; __NL__ \ + }; + +#define _VALIDATE_EVENT_HANDLER_SIGNATURE(HOOK_NAME, PLUGIN_TYPE) \ +{ __NL__ \ + static constexpr bool derived_implements_handler __NL__ \ + = Plugin_HasMember_##HOOK_NAME::value; __NL__ \ + __NL__ \ + static constexpr int n_implementations __NL__ \ + = NumberOfImplementationsOf_##HOOK_NAME::value; __NL__ \ + __NL__ \ + /* A handler is acceptable if it is either not implemented __NL__ \ + * by the derived class or if there is only one implementation __NL__ \ + * with correct signature. __NL__ \ + * Please note that any other methods with different names __NL__ \ + * but other, unrelated signatures are ignored as long __NL__ \ + * as there is one correct implementation. __NL__ \ + * Only if there are several versions supported at a time __NL__ \ + * and more than one of them has been implemented, we __NL__ \ + * treat this as an ambiguity and raise an error. __NL__ \ + */ __NL__ \ + static constexpr bool handler_has_wrong_signature = __NL__ \ + derived_implements_handler && (n_implementations == 0); __NL__ \ + __NL__ \ + static_assert(!handler_has_wrong_signature, __NL__ \ + VERBOSE_STATIC_ASSERT_HEADER __NL__ \ + "\nOne of your plugins implemented the \"" #HOOK_NAME "\"" __NL__ \ + "\nevent handler, but its signature didn't match." __NL__ \ + "\n" __NL__ \ + "\nThe plugin with this issue will be marked in the compiler" __NL__ \ + "\noutput with the string:" __NL__ \ + "\n" __NL__ \ + "\n ___________Culprit_Plugin___________." __NL__ \ + "\n" __NL__ \ + "\nYou should compare the event handlers implemented in this plugin" __NL__ \ + "\nto those listed in the file event_handlers.h." __NL__ \ + "\n" __NL__ \ + "\nPlease note that some of the handlers might have multiple" __NL__ \ + "\nversions. Is it possible that you implemented several versions" __NL__ \ + "\nof the same handler?" __NL__ \ + "\n" __NL__ \ + "\nAll of the event handler's argument types, return values and" __NL__ \ + "\nconst qualifiers need to match." __NL__ \ + "\n" __NL__ \ + VERBOSE_STATIC_ASSERT_FOOTER __NL__ \ + ); __NL__ \ + __NL__ \ + /* The following construct is necessary enable reporting of the __NL__ \ + * type of a plugin that implements an event handler with an __NL__ \ + * incorrect signature, because it's not possible to include any __NL__ \ + * non-literal string constant in a static_assert error message. __NL__ \ + */ __NL__ \ + __attribute__((unused)) constexpr bool dummy1 __NL__ \ + = ___________Culprit_Plugin___________ __NL__ \ + ::value; __NL__ \ + __NL__ \ + static constexpr bool handler_ambiguously_implemented = __NL__ \ + derived_implements_handler && (n_implementations > 1); __NL__ \ + __NL__ \ + static_assert(!handler_ambiguously_implemented, __NL__ \ + VERBOSE_STATIC_ASSERT_HEADER __NL__ \ + "\nOne of your plugins implemented multiple versions of the" __NL__ \ + "\n\"" #HOOK_NAME "\" event handler." __NL__ \ + "\n" __NL__ \ + "\nThe plugin with this issue will be marked in the compiler" __NL__ \ + "\noutput with the string:" __NL__ \ + "\n" __NL__ \ + "\n ___________Culprit_Plugin___________." __NL__ \ + "\n" __NL__ \ + "\nYou should compare the event handlers implemented in this plugin" __NL__ \ + "\nto those listed in the file event_handlers.h." __NL__ \ + "\n" __NL__ \ + "\nPlease make sure that one version is implemented in your plugin." __NL__ \ + "\n" __NL__ \ + VERBOSE_STATIC_ASSERT_FOOTER __NL__ \ + ); __NL__ \ + __NL__ \ + /* The following construct is necessary enable reporting of the __NL__ \ + * type of a plugin that implements an event handler with an __NL__ \ + * incorrect signature, because it's not possible to include any __NL__ \ + * non-literal string constant in a static_assert error message. __NL__ \ + */ __NL__ \ + __attribute__((unused)) constexpr bool dummy2 __NL__ \ + = ___________Culprit_Plugin___________ __NL__ \ + ::value; __NL__ \ } + +#define _PREPARE_EVENT_HANDLER_SIGNATURE_CHECK \ + _PROCESS_EVENT_HANDLER_VERSIONS( __NL__ \ + _DEFINE_IMPLEMENTATION_CHECK_CLASSES, __NL__ \ + _NOOP, __NL__ \ + _NOOP __NL__ \ + ) __NL__ \ + __NL__ \ + _FOR_EACH_EVENT_HANDLER( __NL__ \ + _DEFINE_IMPLEMENTATION_CHECK_CLASS_SPECIALIZATION) __NL__ \ + __NL__ \ + _PROCESS_EVENT_HANDLER_VERSIONS( __NL__ \ + _PREPARE_EVENT_HANDLER_SIGNATURE_CHECK_START, __NL__ \ + _PREPARE_EVENT_HANDLER_SIGNATURE_CHECK_OP, __NL__ \ + _PREPARE_EVENT_HANDLER_SIGNATURE_CHECK_END __NL__ \ + ) diff --git a/src/kaleidoscope_internal/type_traits/has_member.h b/src/kaleidoscope_internal/type_traits/has_member.h new file mode 100644 index 00000000..08fded33 --- /dev/null +++ b/src/kaleidoscope_internal/type_traits/has_member.h @@ -0,0 +1,126 @@ +/* Kaleidoscope - Firmware for computer input devices + * Copyright (C) 2013-2018 Keyboard.io, Inc. + * + * This program is free software: you can redistribute it and/or modify it under + * the terms of the GNU General Public License as published by the Free Software + * Foundation, version 3. + * + * This program is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS + * FOR A PARTICULAR PURPOSE. See the GNU General Public License for more + * details. + * + * You should have received a copy of the GNU General Public License along with + * this program. If not, see . + */ + +#pragma once + +#include "kaleidoscope/macro_helpers.h" + +// The following code has been stolen from +// +// https://cpptalk.wordpress.com/2009/09/12/substitution-failure-is-not-an-error-2/ +// Posted by: rmn on: 12/09/2009 + +// It defines a traits class that enables checking if a class +// defines a member (no matter if function or data member) with a given +// name. So what the class does is essentially checking if a equally +// named member in a helper base class (a mixin) is hidden by a +// reimplementation. The traits can by no means answer the question if +// the member that is implemented in the class being checked is +// a data or a function member. But for many applications that is of +// no great importance, which makes this technique very valuable. + +// The code is highly complex that's why so many words are necessary to +// explain what it does. But the explanations are good enough to actually +// understand what's going on. + +#define DEFINE_HAS_MEMBER_TRAITS(PREFIX, MEMBER_NAME) __NL__ \ + __NL__ \ + /* This defines a templated class PREFIX##_HasMember_##MEMBER_NAME. __NL__ \ + * The double hashes just glue together the value of macro __NL__ \ + * parameters like PREFIX or MEMBER_NAME and other __NL__ \ + * string tokens to form new identifiers (here the struct name). __NL__ \ + */ __NL__ \ + template __NL__ \ + struct PREFIX##_HasMember_##MEMBER_NAME { __NL__ \ + __NL__ \ + /* This code defines an inner class, Fallback, with one __NL__ \ + * member – named MEMBER_NAME (remember that MEMBER_NAME is a __NL__ \ + * macro parameter, so it will be replaced with the name of the __NL__ \ + * actual member. Using proper terminology, this class is __NL__ \ + * essentially a Mixin. __NL__ \ + */ __NL__ \ + struct Fallback { __NL__ \ + int MEMBER_NAME; __NL__ \ + }; __NL__ \ + __NL__ \ + /* Next, we introduce a new class: Derived. It inherits both __NL__ \ + * from the class T we’re templated on, and the previous Fallback __NL__ \ + * class. Note that the Derived class inherits the MEMBER_NAME __NL__ \ + * member from class Fallback and possibly another MEMBER_NAME __NL__ \ + * from T. Keep this (possible) ambiguity in mind. __NL__ \ + */ __NL__ \ + struct Derived : T, Fallback { }; __NL__ \ + __NL__ \ + /* The ChT class is templated on a typename C, and an object __NL__ \ + * of that type. As a side note, only compile time constant __NL__ \ + * fixed types are possible here. We will use this class to __NL__ \ + * generate the ambiguity mentioned above. __NL__ \ + */ __NL__ \ + template __NL__ \ + struct ChT; __NL__ \ + __NL__ \ + /* This is where the fun starts. These two lines form a __NL__ \ + * declaration of a templated function f. That function __NL__ \ + * receives a pointer to the ChT class, instantiated with a __NL__ \ + * pointer-to-member type whose parameter is the __NL__ \ + * address-of-member MEMBER_NAME in class C. Note that __NL__ \ + * if we attempt to instantiate this with __NL__ \ + * the Derived class, one of two things will happen: either we __NL__ \ + * will have a substitution failure due to the aforementioned __NL__ \ + * ambiguity (if T also has a member by that name), or it will __NL__ \ + * be successful (if there is no member called MEMBER_NAME, in T). __NL__ \ + * This function returns a reference to a char array of size 1. __NL__ \ + * __NL__ \ + * How returning arrays by reference works becomes a little __NL__ \ + * clearer by means of a more simple example. The definition __NL__ \ + * of a function that returns a reference to a char-array of __NL__ \ + * size 1 e.g. reads __NL__ \ + * __NL__ \ + * char (&f())[1]; __NL__ \ + */ __NL__ \ + template __NL__ \ + static char (&f(ChT*))[1]; __NL__ \ + __NL__ \ + /* This is what gets instantiated if the previous template __NL__ \ + * can’t be instantiated and SFINAE kicks in (since variadic __NL__ \ + * functions have the lowest possible priority when selecting __NL__ \ + * which overloaded function to call). This variadic function __NL__ \ + * returns a reference to a char array of size 2. Note that __NL__ \ + * instantiation of this function means that the previous __NL__ \ + * one failed, implying that class T has a member __NL__ \ + * named MEMBER_NAME. __NL__ \ + */ __NL__ \ + template __NL__ \ + static char (&f(...))[2]; __NL__ \ + __NL__ \ + /* As you could guess, we will try to check which f function __NL__ \ + * can be instantiated. We will do this by checking the sizeof __NL__ \ + * of the return value of that f. If the first signature did __NL__ \ + * not fit (could not be instantiated), then according to __NL__ \ + * SFINAE the second one will be the chosen one __NL__ \ + * (since it can’t fail) and we will take the sizeof of a __NL__ \ + * char array of size 2, which is 2. Therefore, the value __NL__ \ + * will be evaluated to true – meaning that class T really __NL__ \ + * has a member by the name MEMBER_NAME. Otherwise, the first __NL__ \ + * function can be successfully instantiated, and we will measure __NL__ \ + * the sizeof of a char array of size 1, which is 1. In that case __NL__ \ + * the value is evaluated to false – which is good for us, since __NL__ \ + * that means there was no ambiguity when using __NL__ \ + * Derived::MEMBER_NAME, and that means there was no other __NL__ \ + * MEMBER_NAME but the one in Fallback. __NL__ \ + */ __NL__ \ + static bool const value = sizeof(f(0)) == 2; __NL__ \ +}; From f2426e64e51e5b20c8f2a7137612bca951cabf56 Mon Sep 17 00:00:00 2001 From: Jesse Vincent Date: Wed, 3 Apr 2019 22:18:23 -0700 Subject: [PATCH 2/2] Rename _NOT_DEPRECATED to a "positive" phrasing as "_CURRENT_IMPLEMENTATION" --- src/kaleidoscope/event_handlers.h | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/src/kaleidoscope/event_handlers.h b/src/kaleidoscope/event_handlers.h index 6a4cda41..b27396e2 100644 --- a/src/kaleidoscope/event_handlers.h +++ b/src/kaleidoscope/event_handlers.h @@ -35,7 +35,7 @@ #define _ABORTABLE true #define _NOT_ABORTABLE false -#define _NOT_DEPRECATED +#define _CURRENT_IMPLEMENTATION //****************************************************************************** //****************************************************************************** @@ -65,7 +65,7 @@ // // deprecation flag: // If a specific version is meant to be deprecated, pass the DEPRECATED(...) -// macro with a meaningful deprecation message. Pass _NOT_DEPRECATED +// macro with a meaningful deprecation message. Pass _CURRENT_IMPLEMENTATION // otherwise. // // abortable flag: @@ -88,7 +88,7 @@ __NL__ \ OPERATION(onSetup, __NL__ \ 1, __NL__ \ - _NOT_DEPRECATED, __NL__ \ + _CURRENT_IMPLEMENTATION, __NL__ \ _NOT_ABORTABLE, __NL__ \ (),(), ##__VA_ARGS__) __NL__ \ __NL__ \ @@ -96,7 +96,7 @@ /* events, before doing anything else. */ __NL__ \ OPERATION(beforeEachCycle, __NL__ \ 1, __NL__ \ - _NOT_DEPRECATED, __NL__ \ + _CURRENT_IMPLEMENTATION, __NL__ \ _NOT_ABORTABLE, __NL__ \ (), (), ##__VA_ARGS__) __NL__ \ __NL__ \ @@ -109,7 +109,7 @@ /* will stop processing there. */ __NL__ \ OPERATION(onKeyswitchEvent, __NL__ \ 1, __NL__ \ - _NOT_DEPRECATED, __NL__ \ + _CURRENT_IMPLEMENTATION, __NL__ \ _ABORTABLE, __NL__ \ (Key &mappedKey, byte row, byte col, uint8_t keyState), __NL__ \ (mappedKey, row, col, keyState), ##__VA_ARGS__) __NL__ \ @@ -123,7 +123,7 @@ /* plugin will have a chance to react to the event. */ __NL__ \ OPERATION(onFocusEvent, __NL__ \ 1, __NL__ \ - _NOT_DEPRECATED, __NL__ \ + _CURRENT_IMPLEMENTATION, __NL__ \ _ABORTABLE, __NL__ \ (const char *command), __NL__ \ (command), ##__VA_ARGS__) __NL__ \ @@ -133,7 +133,7 @@ /* track Layer.getState() themselves. */ __NL__ \ OPERATION(onLayerChange, __NL__ \ 1, __NL__ \ - _NOT_DEPRECATED, __NL__ \ + _CURRENT_IMPLEMENTATION, __NL__ \ _NOT_ABORTABLE, __NL__ \ (), (), ##__VA_ARGS__) __NL__ \ /* Called before reporting our state to the host. This is the */ __NL__ \ @@ -141,7 +141,7 @@ /* reported to the host. */ __NL__ \ OPERATION(beforeReportingState, __NL__ \ 1, __NL__ \ - _NOT_DEPRECATED, __NL__ \ + _CURRENT_IMPLEMENTATION, __NL__ \ _NOT_ABORTABLE, __NL__ \ (),(),##__VA_ARGS__) __NL__ \ __NL__ \ @@ -149,7 +149,7 @@ /* said and done. */ __NL__ \ OPERATION(afterEachCycle, __NL__ \ 1, __NL__ \ - _NOT_DEPRECATED, __NL__ \ + _CURRENT_IMPLEMENTATION, __NL__ \ _NOT_ABORTABLE, __NL__ \ (),(),##__VA_ARGS__)