From bb733f1ee8433e92435d7e71f70c6c3a8ff9e294 Mon Sep 17 00:00:00 2001 From: Kevin Bracey Date: Tue, 2 Jul 2019 17:27:15 +0300 Subject: [PATCH 1/3] Callback updates * Optimise clearing by adding `nullptr` overload. This overload means `Callback(NULL)` or `Callback(0)` will no longer work; users must use `Callback(nullptr)` or `Callback()`. * Optimise clearing by not clearing storage - increases code size of comparison, but that is extremely rare. * Reduce ROM used by trivial functors - share copy/destroy code. * Config option to force trivial functors - major ROM saving by eliminating the "operations" table. * Config option to eliminate comparison altogether - minor ROM saving by eliminating zero padding. * Conform more to `std::function` API. --- TESTS/mbed_platform/Transaction/main.cpp | 6 +- TESTS/network/emac/emac_util.h | 2 - .../netsocket/NetworkInterface/unittest.cmake | 2 + .../target_h/platform/cxxsupport/mstd_new | 56 ++ .../platform/cxxsupport/mstd_type_traits | 14 - features/netsocket/NetworkInterface.cpp | 2 + features/netsocket/NetworkInterface.h | 2 + platform/Callback.h | 918 ++++++++++++------ platform/mbed_lib.json | 8 + platform/mbed_retarget.h | 5 +- platform/mbed_toolchain.h | 4 + 11 files changed, 702 insertions(+), 317 deletions(-) create mode 100644 UNITTESTS/target_h/platform/cxxsupport/mstd_new diff --git a/TESTS/mbed_platform/Transaction/main.cpp b/TESTS/mbed_platform/Transaction/main.cpp index 77346f5f67..3b327d0143 100644 --- a/TESTS/mbed_platform/Transaction/main.cpp +++ b/TESTS/mbed_platform/Transaction/main.cpp @@ -58,7 +58,11 @@ void test_Transaction_init() TEST_ASSERT_EQUAL(rx_buffer_size, test_transaction.get_transaction()->rx_length); TEST_ASSERT_EQUAL(event_id, test_transaction.get_transaction()->event); TEST_ASSERT_EQUAL(word_width, test_transaction.get_transaction()->width); - TEST_ASSERT_EQUAL(callback, test_transaction.get_transaction()->callback); +#if MBED_CONF_PLATFORM_CALLBACK_COMPARABLE + TEST_ASSERT_TRUE(callback == test_transaction.get_transaction()->callback); +#else + TEST_ASSERT_FALSE(nullptr == test_transaction.get_transaction()->callback) +#endif } /** Test Transaction class - creation without initialisation diff --git a/TESTS/network/emac/emac_util.h b/TESTS/network/emac/emac_util.h index 8b6b272268..9205ca0f76 100644 --- a/TESTS/network/emac/emac_util.h +++ b/TESTS/network/emac/emac_util.h @@ -102,8 +102,6 @@ char emac_if_get_trace_level(); void emac_if_trace_to_ascii_hex_dump(const char *prefix, int len, char *data); -void emac_if_link_state_change_cb(void *data, bool up); - unsigned char *emac_if_get_own_addr(void); int emac_if_get_mtu_size(); diff --git a/UNITTESTS/features/netsocket/NetworkInterface/unittest.cmake b/UNITTESTS/features/netsocket/NetworkInterface/unittest.cmake index 2d2888f193..46a1ec8ef8 100644 --- a/UNITTESTS/features/netsocket/NetworkInterface/unittest.cmake +++ b/UNITTESTS/features/netsocket/NetworkInterface/unittest.cmake @@ -3,6 +3,8 @@ # UNIT TESTS #################### +set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -DMBED_CONF_PLATFORM_CALLBACK_COMPARABLE") + # Source files set(unittest-sources ../features/netsocket/SocketAddress.cpp diff --git a/UNITTESTS/target_h/platform/cxxsupport/mstd_new b/UNITTESTS/target_h/platform/cxxsupport/mstd_new new file mode 100644 index 0000000000..b7e8154860 --- /dev/null +++ b/UNITTESTS/target_h/platform/cxxsupport/mstd_new @@ -0,0 +1,56 @@ +/* mbed Microcontroller Library + * Copyright (c) 2019 ARM Limited + * SPDX-License-Identifier: Apache-2.0 + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +#ifndef MSTD_NEW_ +#define MSTD_NEW_ + +/* + * + * - includes toolchain's + * - For all toolchains, C++17 backports: + * - mstd::launder + */ + +#include +#if __cpp_lib_launder < 201606 +#include +#endif + +namespace mstd +{ +using std::nothrow_t; +using std::nothrow; +using std::new_handler; +using std::set_new_handler; + +#if __cpp_lib_launder >= 201606 +using std::launder; +#else +template +constexpr T *launder(T *p) noexcept +{ + static_assert(!std::is_function::value && !std::is_void::value, "Can only launder complete object types"); +#if defined __clang__ || __GNUC__ >= 9 + return __builtin_launder(p); +#else + return p; +#endif +} +#endif + +} // namespace mstd + +#endif // MSTD_NEW_ diff --git a/UNITTESTS/target_h/platform/cxxsupport/mstd_type_traits b/UNITTESTS/target_h/platform/cxxsupport/mstd_type_traits index f6974ea0e8..b1c069b8f6 100644 --- a/UNITTESTS/target_h/platform/cxxsupport/mstd_type_traits +++ b/UNITTESTS/target_h/platform/cxxsupport/mstd_type_traits @@ -300,20 +300,6 @@ struct remove_cvref : type_identity> template using remove_cvref_t = typename remove_cvref::type; -/* C++20 unwrap_reference */ -template -struct unwrap_reference : type_identity { }; -template -struct unwrap_reference> : type_identity { }; -template -using unwrap_reference_t = typename unwrap_reference::type; - -/* C++20 unwrap_ref_decay */ -template -struct unwrap_ref_decay : unwrap_reference> { }; -template -using unwrap_ref_decay_t = typename unwrap_ref_decay::type; - } #if __cpp_lib_invoke < 201411 diff --git a/features/netsocket/NetworkInterface.cpp b/features/netsocket/NetworkInterface.cpp index 9a682904bb..12e9ce9b05 100644 --- a/features/netsocket/NetworkInterface.cpp +++ b/features/netsocket/NetworkInterface.cpp @@ -141,6 +141,7 @@ void NetworkInterface::add_event_listener(mbed::Callback status_cb) { iface_eventlist_t *event_list = get_interface_event_list_head(); @@ -152,6 +153,7 @@ void NetworkInterface::remove_event_listener(mbed::Callback status_cb); +#if MBED_CONF_PLATFORM_CALLBACK_COMPARABLE /** Remove event listener from interface. * * Remove previously added callback from the handler list. @@ -359,6 +360,7 @@ public: * @param status_cb The callback to unregister. */ void remove_event_listener(mbed::Callback status_cb); +#endif /** Get the connection status. * diff --git a/platform/Callback.h b/platform/Callback.h index 1dd421c9cc..25c8a54f19 100644 --- a/platform/Callback.h +++ b/platform/Callback.h @@ -17,11 +17,18 @@ #ifndef MBED_CALLBACK_H #define MBED_CALLBACK_H -#include +#include +#include #include -#include +#include #include "platform/mbed_assert.h" #include "platform/mbed_toolchain.h" +#include +#include + +// Controlling switches from config: +// MBED_CONF_PLATFORM_CALLBACK_NONTRIVIAL - support storing non-trivial function objects +// MBED_CONF_PLATFORM_CALLBACK_COMPARABLE - support memcmp comparing stored objects (requires zero padding) namespace mbed { /** \addtogroup platform-public-api */ @@ -35,219 +42,463 @@ namespace mbed { * * @note Synchronization level: Not protected */ -template +template class Callback; -// Internal sfinae declarations -// -// These are used to eliminate overloads based on type attributes -// 1. Does a function object have a call operator -// 2. Does a function object fit in the available storage -// -// These eliminations are handled cleanly by the compiler and avoid -// massive and misleading error messages when confronted with an -// invalid type (or worse, runtime failures) namespace detail { -struct nil {}; -template -struct enable_if { - typedef R type; -}; +/* Convert pointer-to-member type to member type */ +template +struct member_type { }; -template -struct enable_if {}; +template +struct member_type : mstd::type_identity { }; -template -struct is_type { - static const bool value = true; -}; +template +using member_type_t = typename member_type::type; + +/* Remove cv-qualifiers and lvalue ref-qualifiers */ +/* Not rvalue - we store the function object, so are always going to call it on an lvalue */ +template +struct unqualify_fn { }; + +// *INDENT-OFF* +template +struct unqualify_fn : mstd::type_identity { }; +template +struct unqualify_fn : mstd::type_identity { }; +template +struct unqualify_fn : mstd::type_identity { }; +template +struct unqualify_fn : mstd::type_identity { }; +template +struct unqualify_fn : mstd::type_identity { }; +template +struct unqualify_fn : mstd::type_identity { }; +template +struct unqualify_fn : mstd::type_identity { }; +template +struct unqualify_fn : mstd::type_identity { }; +#if __cplusplus >=201703 || __cpp_noexcept_function_type >= 201510 +/* We have to spell out all c/v/ref/noexcept versions here, as specialization needs exact type match */ +/* Compare to callback() and deduction guides, where dropping the noexcept is a permitted conversion */ +template +struct unqualify_fn : mstd::type_identity { }; +template +struct unqualify_fn : mstd::type_identity { }; +template +struct unqualify_fn : mstd::type_identity { }; +template +struct unqualify_fn : mstd::type_identity { }; +template +struct unqualify_fn : mstd::type_identity { }; +template +struct unqualify_fn : mstd::type_identity { }; +template +struct unqualify_fn : mstd::type_identity { }; +template +struct unqualify_fn : mstd::type_identity { }; +#endif + +template +using unqualify_fn_t = typename unqualify_fn::type; + +template ::value, int> = 0> +R invoke_r(F&& f, Args&&... args) +{ + return mstd::invoke(std::forward(f), std::forward(args)...); } -#define MBED_ENABLE_IF_CALLBACK_COMPATIBLE(F, M) \ - typename detail::enable_if< \ - detail::is_type::value && \ - sizeof(F) <= sizeof(uintptr_t) \ - >::type = detail::nil() +template ::value, int> = 0> +R invoke_r(F&& f, Args&&... args) +{ + mstd::invoke(std::forward(f), std::forward(args)...); +} + +template +struct can_null_check : + mstd::disjunction< + std::is_function>, + std::is_member_pointer + > { +}; +// *INDENT-ON* + +struct [[gnu::may_alias]] CallbackBase { + // Storage is sufficient to hold at least a pointer to member + // function, and an object pointer. + struct _model_function_object { + struct _class; + void (_class::*_methodfunc)(int); + void *obj; + }; + + /* Notes on the [[gnu::may_alias]] attribute here. + * + * The CallbackBase::Store is subject to aliasing problems if ever copied via a trivial copy. + * This issue is described here: + * + * https://answers.launchpad.net/gcc-arm-embedded/+question/686870/+index + * http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2019/p0593r5.html + * + * The paper p0593 proposes to solve the problem via a language Defect Report, which would make this + * code valid - it would become legal to copy a trivial object into char array storage. (But not + * aligned_storage_t, as of version 5 - I've suggested a revision). + * + * Real-life problems have only been seen in GCC when the code used aligned_storage_t. + * + * The libstdc++ implementation of std::function uses the [[gnu::may_alias]] attribute itself to avoid + * problems when it swaps locally-stored functors; we need it for copy-assignment too. + * + * It appears [[gnu::may_alias]] doesn't work through composition - it's not sufficent to mark just the + * `Store` type, we have to mark the whole `Callback` if we're going to let the compiler + * implicitly define the trivial copy for it. This potentially could lead to an issue if a `Callback` + * was used in a trivially-copyable type itself, but this seems an unlikely use case. The p0593r5 + * change would, if correctly implemented, work in composition. + * + * Although, to further increase the confusion, it appears that using a character array does work + * fine without may_alias, while aligned_storage_t does not. I've seen a suggestion that GCC 8 + * may have implicit "may_alias" on character arrays, rendering the attribute in std::function + * and here redundant on current GCC: + * + * https://gcc.gnu.org/ml/gcc-help/2017-06/msg00102.html + * + * For maximum safety, this version now avoids aligned_storage_t, and also has the possibly-redundant + * attribute at each level. + * + * C++17 says that implementations ignore unrecognized attributes, and IAR+clang comply with this + * even in C++14 mode, so we add [[gnu::may_alias]] unconditionally. + */ + struct alignas(_model_function_object) [[gnu::may_alias]] Store { + char data[sizeof(_model_function_object)]; + }; + Store _storage; + +#if MBED_CONF_PLATFORM_CALLBACK_NONTRIVIAL + // Dynamically dispatched operations + const struct ops { + void (*call)(); // type-erased function pointer + void (*copy)(Store &, const Store &); + void (*dtor)(Store &); + } *_ops; + + // Control + using Control = const ops *; + + // Construct as empty + CallbackBase(std::nullptr_t) noexcept : _ops(nullptr) { } +#else + void (*_call)(); // type-erased function pointer + + using Control = void(*)(); + + // Construct as empty + CallbackBase(std::nullptr_t) noexcept : _call(nullptr) { } +#endif + + // Default constructor - no initialization + CallbackBase() = default; + + Control &control() + { +#if MBED_CONF_PLATFORM_CALLBACK_NONTRIVIAL + return _ops; +#else + return _call; +#endif + } + + const Control &control() const + { +#if MBED_CONF_PLATFORM_CALLBACK_NONTRIVIAL + return _ops; +#else + return _call; +#endif + } + + auto call_fn() const + { +#if MBED_CONF_PLATFORM_CALLBACK_NONTRIVIAL + return _ops->call; +#else + return _call; +#endif + } + + // Clear to empty - does not destroy + void clear() noexcept + { + // For copy efficiency we only zero out the operation pointer + // Therefore storage is undefined when we are empty. + // Callback-to-Callback comparison operator has to deal with this, + // but such comparisons are rare. Comparisons to empty are efficient. + control() = nullptr; + } + +#if MBED_CONF_PLATFORM_CALLBACK_NONTRIVIAL + // Copy from another CallbackBase - assumes we are uninitialised + void copy(const CallbackBase &other) + { + _ops = other._ops; + if (_ops) { + _ops->copy(_storage, other._storage); + } + } +#else + void swap(CallbackBase &other) noexcept + { + std::swap(_storage, other._storage); + std::swap(_call, other._call); + } +#endif + + // Destroy anything we hold - does not reset, so we are in undefined state afterwards. + // Must be followed by copy, move, reset, or destruction of the CallbackBase + void destroy() + { +#if MBED_CONF_PLATFORM_CALLBACK_NONTRIVIAL + if (_ops) { + _ops->dtor(_storage); + } +#endif + } + +#if MBED_CONF_PLATFORM_CALLBACK_NONTRIVIAL + // Copy construct F into storage + template + static void target_copy(Store &d, const Store &p) + { + const F &f = reinterpret_cast(p); + new (&d) F(f); + } + + // Destroy F in storage + template + static void target_dtor(Store &p) + { + F &f = reinterpret_cast(p); + f.~F(); + } + + // Trivial copy construction into storage + static void trivial_target_copy(Store &d, const Store &p) noexcept + { + std::memcpy(&d, &p, sizeof d); + } + + // Trivial destruction in storage + static void trivial_target_dtor(Store &p) noexcept + { + } +#endif +}; + +} /** Callback class based on template specialization * * @note Synchronization level: Not protected */ template -class Callback { +class Callback : private detail::CallbackBase { public: - /** Create a Callback with a static function - * @param func Static function to attach + using result_type = R; + + /** Create an empty Callback */ - Callback(R(*func)(ArgTs...) = 0) + Callback() noexcept : CallbackBase(nullptr) { } + + /** Create an empty Callback + */ + Callback(std::nullptr_t) noexcept : Callback() { } + +#if MBED_CONF_PLATFORM_CALLBACK_NONTRIVIAL + /** Copy a Callback + * @param other The Callback to copy + */ + Callback(const Callback &other) : CallbackBase() { - if (!func) { - memset(this, 0, sizeof(Callback)); + copy(other); + } + + /** Move a Callback + * @param other The Callback to move + */ + Callback(Callback &&other) : CallbackBase() + { + // Move constructor exists to ensure that it gets selected + // in preference to the universal constructor form. + copy(other); + } +#else // MBED_CONF_PLATFORM_CALLBACK_NONTRIVIAL + Callback(const Callback &other) = default; + Callback(Callback &&other) = default; +#endif // MBED_CONF_PLATFORM_CALLBACK_NONTRIVIAL + + /** Create a Callback with a member function + * @param obj Pointer to object to invoke member function on + * @param method Member function to attach + */ + template::value, int> = 0> + Callback(Obj obj, Method method) : CallbackBase() + { + generate([obj, method](ArgTs... args) { + return detail::invoke_r(method, obj, std::forward(args)...); + }); + } + + /** Create a Callback with a static function and bound pointer + * @param func Static function to attach + * @param arg Pointer argument to function + */ + template::value, int> = 0> + Callback(Fn func, BoundArg arg) : CallbackBase() + { + generate([func, arg](ArgTs... args) { + return detail::invoke_r(func, arg, std::forward(args)...); + }); + } + + // *INDENT-OFF* + /** Create a Callback with a function object + * @param f Function object to attach + * @note The function object is limited to a a few words of storage + */ + template ::value && + mstd::is_invocable_r::value, int> = 0> + Callback(F f) : CallbackBase() + { + static_assert(std::is_copy_constructible::value, "Callback F must be CopyConstructible"); + generate(std::move(f)); + } + + /** Create a Callback with a function pointer + * @param f Function pointer to attach + */ + template ::value && + mstd::is_invocable_r::value, int> = 0> + Callback(F f) : CallbackBase() + { + static_assert(std::is_copy_constructible::value, "Callback F must be CopyConstructible"); + if (!f) { + clear(); } else { - generate(func); + generate(std::move(f)); } } - - /** Attach a Callback - * @param func The Callback to attach - */ - Callback(const Callback &func) - { - memset(this, 0, sizeof(Callback)); - if (func._ops) { - func._ops->move(this, &func); - } - _ops = func._ops; - } - - /** Create a Callback with a member function - * @param obj Pointer to object to invoke member function on - * @param method Member function to attach - */ - template - Callback(U *obj, R(T::*method)(ArgTs...)) - { - generate(method_context(obj, method)); - } - - /** Create a Callback with a member function - * @param obj Pointer to object to invoke member function on - * @param method Member function to attach - */ - template - Callback(const U *obj, R(T::*method)(ArgTs...) const) - { - generate(method_context(obj, method)); - } - - /** Create a Callback with a member function - * @param obj Pointer to object to invoke member function on - * @param method Member function to attach - */ - template - Callback(volatile U *obj, R(T::*method)(ArgTs...) volatile) - { - generate(method_context(obj, method)); - } - - /** Create a Callback with a member function - * @param obj Pointer to object to invoke member function on - * @param method Member function to attach - */ - template - Callback(const volatile U *obj, R(T::*method)(ArgTs...) const volatile) - { - generate(method_context(obj, method)); - } - - /** Create a Callback with a static function and bound pointer - * @param func Static function to attach - * @param arg Pointer argument to function - */ - template - Callback(R(*func)(T *, ArgTs...), U *arg) - { - generate(function_context(func, arg)); - } - - /** Create a Callback with a static function and bound pointer - * @param func Static function to attach - * @param arg Pointer argument to function - */ - template - Callback(R(*func)(const T *, ArgTs...), const U *arg) - { - generate(function_context(func, arg)); - } - - /** Create a Callback with a static function and bound pointer - * @param func Static function to attach - * @param arg Pointer argument to function - */ - template - Callback(R(*func)(volatile T *, ArgTs...), volatile U *arg) - { - generate(function_context(func, arg)); - } - - /** Create a Callback with a static function and bound pointer - * @param func Static function to attach - * @param arg Pointer argument to function - */ - template - Callback(R(*func)(const volatile T *, ArgTs...), const volatile U *arg) - { - generate(function_context(func, arg)); - } - - /** Create a Callback with a function object - * @param f Function object to attach - * @note The function object is limited to a single word of storage - */ - template - Callback(F f, MBED_ENABLE_IF_CALLBACK_COMPATIBLE(F, R(F::*)(ArgTs...))) - { - generate(f); - } - - /** Create a Callback with a function object - * @param f Function object to attach - * @note The function object is limited to a single word of storage - */ - template - Callback(const F f, MBED_ENABLE_IF_CALLBACK_COMPATIBLE(F, R(F::*)(ArgTs...) const)) - { - generate(f); - } - - /** Create a Callback with a function object - * @param f Function object to attach - * @note The function object is limited to a single word of storage - */ - template - Callback(volatile F f, MBED_ENABLE_IF_CALLBACK_COMPATIBLE(F, R(F::*)(ArgTs...) volatile)) - { - generate(f); - } - - /** Create a Callback with a function object - * @param f Function object to attach - * @note The function object is limited to a single word of storage - */ - template - Callback(const volatile F f, MBED_ENABLE_IF_CALLBACK_COMPATIBLE(F, R(F::*)(ArgTs...) const volatile)) - { - generate(f); - } + // *INDENT-ON* /** Destroy a callback */ +#if MBED_CONF_PLATFORM_CALLBACK_NONTRIVIAL ~Callback() { - if (_ops) { - _ops->dtor(this); + destroy(); + } +#else + ~Callback() = default; +#endif + + /** Swap a callback + */ + void swap(Callback &that) noexcept + { +#if MBED_CONF_PLATFORM_CALLBACK_NONTRIVIAL + if (this != &that) { + Callback temp(std::move(*this)); + *this = std::move(that); + that = std::move(temp); } +#else + CallbackBase::swap(that); +#endif } +#if MBED_CONF_PLATFORM_CALLBACK_NONTRIVIAL /** Assign a callback */ Callback &operator=(const Callback &that) { + // C++ standard says to use swap, but that's overkill with no exceptions + // Callback(f).swap(*this); if (this != &that) { - this->~Callback(); - new (this) Callback(that); + destroy(); + copy(that); } return *this; } + /** Assign a callback + */ + Callback &operator=(Callback &&that) + { + if (this != &that) { + destroy(); + copy(that); + } + + return *this; + } +#else // MBED_CONF_PLATFORM_CALLBACK_NONTRIVIAL + Callback &operator=(const Callback &that) = default; + Callback &operator=(Callback &&that) = default; +#endif // MBED_CONF_PLATFORM_CALLBACK_NONTRIVIAL + + /** Assign a callback + */ + // C++ std::function lacks the is_same restriction here, which would mean non-const lvalue references hit this, + // rather than the normal copy assignment (`F &&` is a better match for `Callback &` than `const Callback &`). + // Wouldn't matter if both used the swap form, but having cut it down, for code size want to ensure we don't use this + // instead of copy assignment. (If nontrivial disabled, definitely want to use the default copy assignment, and + // if nontrivial enabled, we know this doesn't handle self-assignment). + // *INDENT-OFF* + template ::value && + !mstd::is_same, Callback>::value>> + Callback &operator=(F &&f) + { + // C++ standard says to use swap, but that's overkill with no exceptions + // Callback(std::forward(that)).swap(*this); + this->~Callback(); + new (this) Callback(std::forward(f)); + return *this; + } + // *INDENT-ON* + + template + Callback &operator=(std::reference_wrapper f) noexcept + { + // C++ standard says to use swap, but that's overkill with no exceptions + // Callback(f).swap(*this); + this->~Callback(); + new (this) Callback(f); + return *this; + } + + /** Empty a callback + */ + Callback &operator=(std::nullptr_t) noexcept + { + destroy(); + clear(); + + return *this; + } + /** Call the attached function */ R call(ArgTs... args) const { - MBED_ASSERT(_ops); - return _ops->call(this, args...); + MBED_ASSERT(bool(*this)); + auto op_call = reinterpret_cast(call_fn()); + return op_call(this, args...); } /** Call the attached function @@ -257,26 +508,80 @@ public: return call(args...); } - /** Test if function has been attached + /** Test if function has been assigned */ - operator bool() const + explicit operator bool() const noexcept { - return _ops; + return control(); } +#if MBED_CONF_PLATFORM_CALLBACK_COMPARABLE /** Test for equality + * + * @note This only compares stored objects byte-wise using memcmp + * after checking that they're the same type. It does *not* use + * any equality operator defined for the class. + * + * @note This is an extension compared to std::function, and supporting + * it requires extra code even if the comparison is never used. + * Avoid using this operator if possible, so that the option + * `platform.callback-comparable` can be turned off to save ROM. */ - friend bool operator==(const Callback &l, const Callback &r) + friend bool operator==(const Callback &l, const Callback &r) noexcept { - return memcmp(&l, &r, sizeof(Callback)) == 0; + if (l.control() != r.control()) { + /* Type of stored object differs */ + return false; + } + if (!l) { + /* Both must be empty, as we checked the types match. Do not + * check storage in this case - this simplifies clear(), and + * clears are far more common than callback comparison. + */ + return true; + } + return memcmp(&l._storage, &r._storage, sizeof(Store)) == 0; + } +#endif + + /** Test for emptiness + */ + friend bool operator==(const Callback &f, std::nullptr_t) noexcept + { + return !f; } - /** Test for inequality + /** Test for emptiness */ - friend bool operator!=(const Callback &l, const Callback &r) + friend bool operator==(std::nullptr_t, const Callback &f) noexcept + { + return !f; + } + +#if MBED_CONF_PLATFORM_CALLBACK_COMPARABLE + /** Test for inequality + * + * @see operator==(const Callback &l, const Callback &r) + */ + friend bool operator!=(const Callback &l, const Callback &r) noexcept { return !(l == r); } +#endif + + /** Test for non-emptiness + */ + friend bool operator!=(const Callback &f, std::nullptr_t) noexcept + { + return bool(f); + } + + /** Test for non-emptiness + */ + friend bool operator!=(std::nullptr_t, const Callback &f) noexcept + { + return bool(f); + } /** Static thunk for passing as C-style function * @param func Callback to call passed as void pointer @@ -290,93 +595,72 @@ public: } private: - // Stored as pointer to function and pointer to optional object - // Function pointer is stored as union of possible function types - // to guarantee proper size and alignment - struct _class; - union { - void (*_staticfunc)(ArgTs...); - void (*_boundfunc)(_class *, ArgTs...); - void (_class::*_methodfunc)(ArgTs...); - } _func; - void *_obj; - - // Dynamically dispatched operations - const struct ops { - R(*call)(const void *, ArgTs...); - void (*move)(void *, const void *); - void (*dtor)(void *); - } *_ops; + using call_type = R(const CallbackBase *, ArgTs...); + // *INDENT-OFF* // Generate operations for function object - template - void generate(const F &f) + // Storage assumed to be uninitialised - destructor should have already been called if it was previously used + // When generating, function object should always be moved + template ::value>> + void generate(F &&f) { +#ifndef __ICCARM__ /* This assert fails on IAR for unknown reason */ + static_assert(std::is_same), call_type>::value, "Call type mismatch"); +#endif + static_assert(sizeof(Callback) == sizeof(CallbackBase), "Callback should be same size as CallbackBase"); + static_assert(std::is_trivially_copyable::value, "CallbackBase expected to be TriviallyCopyable"); + + // Set the control pointer +#if MBED_CONF_PLATFORM_CALLBACK_NONTRIVIAL + // Generates one static ops for each tuple + // But the functions used for copy/move/dtor depend only on F, and even then only if non-trivial. + // `call` is type-erased - we cast from our call_type to the void (*)(void) in CallbackBase + // This should be a ROMmed constant table, but formally it can't be constexpr because of the reinterpret_cast :( static const ops ops = { - &Callback::function_call, - &Callback::function_move, - &Callback::function_dtor, + reinterpret_cast(target_call), + std::is_trivially_copy_constructible::value ? trivial_target_copy : target_copy, + std::is_trivially_destructible::value ? trivial_target_dtor : target_dtor, }; - - MBED_STATIC_ASSERT(sizeof(Callback) - sizeof(_ops) >= sizeof(F), - "Type F must not exceed the size of the Callback class"); - memset(this, 0, sizeof(Callback)); - new (this) F(f); _ops = &ops; - } +#else // MBED_CONF_PLATFORM_CALLBACK_NONTRIVIAL + // Avoid the need for the const ops table - just one function pointer in the Callback itself + _call = reinterpret_cast(target_call); + static_assert(std::is_trivially_copyable::value, "F must be TriviallyCopyable. Turn on Mbed configuration option 'platform.callback-nontrivial' to use more complex function objects"); + static_assert(std::is_trivially_copyable::value, "Callback expected to be TriviallyCopyable"); +#endif // MBED_CONF_PLATFORM_CALLBACK_NONTRIVIAL - // Function attributes - template - static R function_call(const void *p, ArgTs... args) - { - return (*(F *)p)(args...); - } + // Move the functor into storage + static_assert(sizeof(F) <= sizeof(Store) && alignof(F) <= alignof(Store), + "Type F must not exceed the size of the Callback class"); + new (&_storage) F(std::move(f)); - template - static void function_move(void *d, const void *p) - { - new (d) F(*(F *)p); - } - - template - static void function_dtor(void *p) - { - ((F *)p)->~F(); - } - - // Wrappers for functions with context - template - struct method_context { - M method; - O *obj; - - method_context(O *obj, M method) - : method(method), obj(obj) {} - - R operator()(ArgTs... args) const - { - return (obj->*method)(args...); +#if MBED_CONF_PLATFORM_CALLBACK_COMPARABLE + // Zero out any padding - required for Callback-to-Callback comparisons. + if (sizeof(F) < sizeof(Store)) { + std::memset(reinterpret_cast(&_storage) + sizeof(F), 0, sizeof(Store) - sizeof(F)); } - }; +#endif + } + // *INDENT-ON* - template - struct function_context { - F func; - A *arg; - - function_context(F func, A *arg) - : func(func), arg(arg) {} - - R operator()(ArgTs... args) const - { - return func(arg, args...); - } - }; + // Target call routine - custom needed for each tuple + template + static R target_call(const CallbackBase *p, ArgTs... args) + { + // Need for const_cast here correlates to a std::function bug - see P0045 and N4159 + F &f = const_cast(reinterpret_cast(p->_storage)); + return detail::invoke_r(f, std::forward(args)...); + } }; // Internally used event type -typedef Callback event_callback_t; +using event_callback_t = Callback; +template +void swap(Callback &lhs, Callback &rhs) noexcept +{ + lhs.swap(rhs); +} /** Create a callback class with type inferred from the arguments * @@ -384,7 +668,7 @@ typedef Callback event_callback_t; * @return Callback with inferred type */ template -Callback callback(R(*func)(ArgTs...) = 0) +Callback callback(R(*func)(ArgTs...) = nullptr) noexcept { return Callback(func); } @@ -402,14 +686,13 @@ Callback callback(const Callback &func) /** Create a callback class with type inferred from the arguments * - * @param obj Optional pointer to object to bind to function - * @param method Member function to attach + * @param func Static function to attach * @return Callback with inferred type */ -template -Callback callback(U *obj, R(T::*method)(ArgTs...)) +template +Callback callback(Callback &&func) noexcept { - return Callback(obj, method); + return Callback(std::move(func)); } /** Create a callback class with type inferred from the arguments @@ -419,31 +702,49 @@ Callback callback(U *obj, R(T::*method)(ArgTs...)) * @return Callback with inferred type */ template -Callback callback(const U *obj, R(T::*method)(ArgTs...) const) +Callback callback(U *obj, R(T::*method)(ArgTs...)) noexcept { return Callback(obj, method); } -/** Create a callback class with type inferred from the arguments - * - * @param obj Optional pointer to object to bind to function - * @param method Member function to attach - * @return Callback with inferred type - */ template -Callback callback(volatile U *obj, R(T::*method)(ArgTs...) volatile) +Callback callback(U *obj, R(T::*method)(ArgTs...) &) noexcept { return Callback(obj, method); } -/** Create a callback class with type inferred from the arguments - * - * @param obj Optional pointer to object to bind to function - * @param method Member function to attach - * @return Callback with inferred type - */ template -Callback callback(const volatile U *obj, R(T::*method)(ArgTs...) const volatile) +Callback callback(const U *obj, R(T::*method)(ArgTs...) const) noexcept +{ + return Callback(obj, method); +} + +template +Callback callback(const U *obj, R(T::*method)(ArgTs...) const &) noexcept +{ + return Callback(obj, method); +} + +template +Callback callback(volatile U *obj, R(T::*method)(ArgTs...) volatile) noexcept +{ + return Callback(obj, method); +} + +template +Callback callback(volatile U *obj, R(T::*method)(ArgTs...) volatile &) noexcept +{ + return Callback(obj, method); +} + +template +Callback callback(const volatile U *obj, R(T::*method)(ArgTs...) const volatile) noexcept +{ + return Callback(obj, method); +} + +template +Callback callback(const volatile U *obj, R(T::*method)(ArgTs...) const volatile &) noexcept { return Callback(obj, method); } @@ -455,47 +756,72 @@ Callback callback(const volatile U *obj, R(T::*method)(ArgTs...) co * @return Callback with inferred type */ template -Callback callback(R(*func)(T *, ArgTs...), U *arg) +Callback callback(R(*func)(T *, ArgTs...), U *arg) noexcept { return Callback(func, arg); } -/** Create a callback class with type inferred from the arguments - * - * @param func Static function to attach - * @param arg Pointer argument to function - * @return Callback with inferred type - */ template -Callback callback(R(*func)(const T *, ArgTs...), const U *arg) +Callback callback(R(*func)(const T *, ArgTs...), const U *arg) noexcept { return Callback(func, arg); } -/** Create a callback class with type inferred from the arguments - * - * @param func Static function to attach - * @param arg Pointer argument to function - * @return Callback with inferred type - */ template -Callback callback(R(*func)(volatile T *, ArgTs...), volatile U *arg) +Callback callback(R(*func)(volatile T *, ArgTs...), volatile U *arg) noexcept { return Callback(func, arg); } -/** Create a callback class with type inferred from the arguments - * - * @param func Static function to attach - * @param arg Pointer argument to function - * @return Callback with inferred type - */ template -Callback callback(R(*func)(const volatile T *, ArgTs...), const volatile U *arg) +Callback callback(R(*func)(const volatile T *, ArgTs...), const volatile U *arg) noexcept { return Callback(func, arg); } +/** Create a Create a callback class with type inferred from the arguments + * @param f Function object to attach + * @note The function object is limited to a single word of storage + */ +template +Callback::operator())>>> +callback(F &&f) +{ + return Callback::operator())>>>(std::forward(f)); +} + +#if __cplusplus >= 201703 || __cpp_deduction_guides >= 201703 +/* Deduction guides that can replace callback() helper */ +template +Callback(R(*)(Args...)) -> Callback; +template +Callback(F) -> Callback>>; +template +Callback(U *obj, R(T::*method)(ArgTs...)) -> Callback; +template +Callback(U *obj, R(T::*method)(ArgTs...) &) -> Callback; +template +Callback(const U *obj, R(T::*method)(ArgTs...) const) -> Callback; +template +Callback(const U *obj, R(T::*method)(ArgTs...) const &) -> Callback; +template +Callback(volatile U *obj, R(T::*method)(ArgTs...) volatile) -> Callback; +template +Callback(volatile U *obj, R(T::*method)(ArgTs...) volatile &) -> Callback; +template +Callback(const volatile U *obj, R(T::*method)(ArgTs...) const volatile) -> Callback; +template +Callback(const volatile U *obj, R(T::*method)(ArgTs...) const volatile &) -> Callback; +template +Callback(R(*func)(T *, ArgTs...), U *arg) -> Callback; +template +Callback(R(*func)(const T *, ArgTs...), const U *arg) -> Callback; +template +Callback(R(*func)(volatile T *, ArgTs...), volatile U *arg) -> Callback; +template +Callback(R(*func)(const volatile T *, ArgTs...), const volatile U *arg) -> Callback; +#endif + /**@}*/ /**@}*/ diff --git a/platform/mbed_lib.json b/platform/mbed_lib.json index 2da4dbea2d..94fc4c7982 100644 --- a/platform/mbed_lib.json +++ b/platform/mbed_lib.json @@ -123,6 +123,14 @@ "help": "The maximum CThunk objects used at the same time. This must be greater than 0 and less 256", "value": 8 }, + "callback-nontrivial": { + "help": "Enables support for non-trivial callable objects in Callback. Can be disabled to save ROM if no-one is using non-trivial types. Changing this value may cause incompatibility with pre-built binaries.", + "value": true + }, + "callback-comparable": { + "help": "Enables support for comparing two Callbacks. See notes on operator== for limitations. Can be disabled to save ROM if not required.", + "value": true + }, "crash-capture-enabled": { "help": "Enables crash context capture when the system enters a fatal error/crash.", "value": false diff --git a/platform/mbed_retarget.h b/platform/mbed_retarget.h index a2be2bdf39..5be9a9daf8 100644 --- a/platform/mbed_retarget.h +++ b/platform/mbed_retarget.h @@ -34,11 +34,8 @@ */ #ifndef __error_t_defined #define __error_t_defined 1 -#include -#undef __error_t_defined -#else -#include #endif +#include /* We can get the following standard types from sys/types for gcc, but we * need to define the types ourselves for the other compilers that normally diff --git a/platform/mbed_toolchain.h b/platform/mbed_toolchain.h index 255ab39fd5..2b1c9f03fc 100644 --- a/platform/mbed_toolchain.h +++ b/platform/mbed_toolchain.h @@ -19,6 +19,10 @@ #include "platform/mbed_preprocessor.h" +/* Workaround to prevent GCC library defining error_t, which can collide */ +#ifndef __error_t_defined +#define __error_t_defined 1 +#endif // Warning for unsupported compilers #if !defined(__GNUC__) /* GCC */ \ From 171a5f80c7e26da9837388814618517d1931ef96 Mon Sep 17 00:00:00 2001 From: Kevin Bracey Date: Thu, 5 Dec 2019 15:02:13 +0200 Subject: [PATCH 2/3] Add more Callback tests --- TESTS/mbed_functional/callback/main.cpp | 212 ++++++++++++++++++++++-- 1 file changed, 199 insertions(+), 13 deletions(-) diff --git a/TESTS/mbed_functional/callback/main.cpp b/TESTS/mbed_functional/callback/main.cpp index ee971817a4..ed4ada8abd 100644 --- a/TESTS/mbed_functional/callback/main.cpp +++ b/TESTS/mbed_functional/callback/main.cpp @@ -18,9 +18,27 @@ #include "greentea-client/test_env.h" #include "unity.h" #include "utest.h" +#include using namespace utest::v1; +template +using func0_type = T(); + +template +using func1_type = T(T); + +template +using func2_type = T(T, T); + +template +using func3_type = T(T, T, T); + +template +using func4_type = T(T, T, T, T); + +template +using func5_type = T(T, T, T, T, T); // static functions template @@ -523,13 +541,17 @@ void test_dispatch0() Verifier::verify0(&const_volatile_void_func0, (const volatile Thing *)&thing); Verifier::verify0(callback(static_func0)); - Callback cb(static_func0); + Callback cb(static_func0); Verifier::verify0(cb); - cb = static_func0; + TEST_ASSERT_TRUE(cb); + func0_type *p = nullptr; + cb = p; + TEST_ASSERT_FALSE(cb); + cb = static_func0; Verifier::verify0(cb); cb = {&bound_func0, &thing}; Verifier::verify0(&cb, &Callback::call); - Verifier::verify0(&Callback::thunk, (void *)&cb); + Verifier::verify0(&Callback::thunk, &cb); } template @@ -554,9 +576,13 @@ void test_dispatch1() Verifier::verify1(&const_volatile_void_func1, (const volatile Thing *)&thing); Verifier::verify1(callback(static_func1)); - Callback cb(static_func1); + Callback cb(static_func1); Verifier::verify1(cb); - cb = static_func1; + TEST_ASSERT_TRUE(cb); + func1_type *p = nullptr; + cb = p; + TEST_ASSERT_FALSE(cb); + cb = static_func1; Verifier::verify1(cb); cb = {&bound_func1, &thing}; Verifier::verify1(&cb, &Callback::call); @@ -585,9 +611,13 @@ void test_dispatch2() Verifier::verify2(&const_volatile_void_func2, (const volatile Thing *)&thing); Verifier::verify2(callback(static_func2)); - Callback cb(static_func2); + Callback cb(static_func2); Verifier::verify2(cb); - cb = static_func2; + TEST_ASSERT_TRUE(cb); + func2_type *p = nullptr; + cb = p; + TEST_ASSERT_FALSE(cb); + cb = static_func2; Verifier::verify2(cb); cb = {&bound_func2, &thing}; Verifier::verify2(&cb, &Callback::call); @@ -616,9 +646,13 @@ void test_dispatch3() Verifier::verify3(&const_volatile_void_func3, (const volatile Thing *)&thing); Verifier::verify3(callback(static_func3)); - Callback cb(static_func3); + Callback cb(static_func3); Verifier::verify3(cb); - cb = static_func3; + TEST_ASSERT_TRUE(cb); + func3_type *p = nullptr; + cb = p; + TEST_ASSERT_FALSE(cb); + cb = static_func3; Verifier::verify3(cb); cb = {&bound_func3, &thing}; Verifier::verify3(&cb, &Callback::call); @@ -647,9 +681,13 @@ void test_dispatch4() Verifier::verify4(&const_volatile_void_func4, (const volatile Thing *)&thing); Verifier::verify4(callback(static_func4)); - Callback cb(static_func4); + Callback cb(static_func4); Verifier::verify4(cb); - cb = static_func4; + TEST_ASSERT_TRUE(cb); + func4_type *p = nullptr; + cb = p; + TEST_ASSERT_FALSE(cb); + cb = static_func4; Verifier::verify4(cb); cb = {&bound_func4, &thing}; Verifier::verify4(&cb, &Callback::call); @@ -678,15 +716,159 @@ void test_dispatch5() Verifier::verify5(&const_volatile_void_func5, (const volatile Thing *)&thing); Verifier::verify5(callback(static_func5)); - Callback cb(static_func5); + Callback cb(static_func5); Verifier::verify5(cb); - cb = static_func5; + TEST_ASSERT_TRUE(cb); + func5_type *p = nullptr; + cb = p; + TEST_ASSERT_FALSE(cb); + cb = static_func5; Verifier::verify5(cb); cb = {&bound_func5, &thing}; Verifier::verify5(&cb, &Callback::call); +#if 0 Verifier::verify5(&Callback::thunk, (void *)&cb); +#endif } +#include + +struct TrivialFunctionObject { + TrivialFunctionObject(int n) : val(n) + { + } + + int operator()(int x) const + { + return x + val; + } +private: + int val; +}; + +/* Exact count of copy, construction and destruction may vary depending on + * copy elision by compiler, but the derived live count can be relied upon. + */ +static int construct_count; +static int destruct_count; +static int copy_count; + +static int live_count() +{ + return construct_count - destruct_count; +} + +struct FunctionObject { + FunctionObject(int n) : val(n) + { + construct_count++; + } + + FunctionObject(const FunctionObject &other) : val(other.val) + { + construct_count++; + copy_count++; + } + + ~FunctionObject() + { + destruct_count++; + destroyed = true; + } + + int operator()(int x) const + { + return destroyed ? -1000 : x + val; + } +private: + const int val; + bool destroyed = false; +}; + +void test_trivial() +{ + TrivialFunctionObject fn(1); + TEST_ASSERT_EQUAL(2, fn(1)); + Callback cb(fn); + TEST_ASSERT_TRUE(cb); + TEST_ASSERT_EQUAL(2, cb(1)); + fn = 5; + TEST_ASSERT_EQUAL(6, fn(1)); + TEST_ASSERT_EQUAL(2, cb(1)); + cb = std::ref(fn); + fn = 10; + TEST_ASSERT_EQUAL(11, fn(1)); + TEST_ASSERT_EQUAL(11, cb(1)); + cb = TrivialFunctionObject(3); + TEST_ASSERT_EQUAL(7, cb(4)); + cb = nullptr; + TEST_ASSERT_FALSE(cb); + cb = TrivialFunctionObject(7); + Callback cb2(cb); + TEST_ASSERT_EQUAL(8, cb(1)); + TEST_ASSERT_EQUAL(9, cb2(2)); + cb2 = cb; + TEST_ASSERT_EQUAL(6, cb2(-1)); + cb = cb; + TEST_ASSERT_EQUAL(8, cb(1)); + cb = std::negate(); + TEST_ASSERT_EQUAL(-4, cb(4)); + cb = [](int x) { + return x - 7; + }; + TEST_ASSERT_EQUAL(1, cb(8)); + cb = cb2 = nullptr; + TEST_ASSERT_FALSE(cb); +} + +#if MBED_CONF_PLATFORM_CALLBACK_NONTRIVIAL +void test_nontrivial() +{ + { + FunctionObject fn(1); + TEST_ASSERT_EQUAL(1, construct_count); + TEST_ASSERT_EQUAL(0, destruct_count); + TEST_ASSERT_EQUAL(2, fn(1)); + Callback cb(fn); + TEST_ASSERT_TRUE(cb); + TEST_ASSERT_EQUAL(2, live_count()); + TEST_ASSERT_EQUAL(2, cb(1)); + cb = std::ref(fn); + TEST_ASSERT_EQUAL(1, live_count()); + TEST_ASSERT_EQUAL(5, cb(4)); + cb = FunctionObject(3); + TEST_ASSERT_EQUAL(2, live_count()); + TEST_ASSERT_EQUAL(7, cb(4)); + cb = nullptr; + TEST_ASSERT_FALSE(cb); + TEST_ASSERT_EQUAL(1, live_count()); + cb = FunctionObject(7); + TEST_ASSERT_EQUAL(2, live_count()); + int old_copy_count = copy_count; + Callback cb2(cb); + TEST_ASSERT_EQUAL(old_copy_count + 1, copy_count); + TEST_ASSERT_EQUAL(3, live_count()); + TEST_ASSERT_EQUAL(8, cb(1)); + TEST_ASSERT_EQUAL(9, cb2(2)); + old_copy_count = copy_count; + cb2 = cb; + TEST_ASSERT_EQUAL(old_copy_count + 1, copy_count); + TEST_ASSERT_EQUAL(3, live_count()); + TEST_ASSERT_EQUAL(6, cb2(-1)); + int old_construct_count = construct_count; + old_copy_count = copy_count; + cb = cb; + TEST_ASSERT_EQUAL(3, live_count()); + TEST_ASSERT_EQUAL(old_construct_count, construct_count); + TEST_ASSERT_EQUAL(old_copy_count, copy_count); + cb = cb2 = nullptr; + TEST_ASSERT_FALSE(cb); + TEST_ASSERT_EQUAL(1, live_count()); + } + TEST_ASSERT_EQUAL(0, live_count()); +} +#endif + // Test setup utest::v1::status_t test_setup(const size_t number_of_cases) @@ -717,6 +899,10 @@ Case cases[] = { Case("Testing callbacks with 3 ints", test_dispatch3), Case("Testing callbacks with 4 ints", test_dispatch4), Case("Testing callbacks with 5 ints", test_dispatch5), + Case("Testing trivial function object", test_trivial), +#if MBED_CONF_PLATFORM_CALLBACK_NONTRIVIAL + Case("Testing non-trivial function object", test_nontrivial), +#endif #endif }; From d09d85431c05d6fd4cf824ef24aa0a2e37c9525a Mon Sep 17 00:00:00 2001 From: Kevin Bracey Date: Fri, 14 Feb 2020 16:34:05 +0200 Subject: [PATCH 3/3] Disable test that breaks IAR --- TESTS/mbed_functional/callback/main.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/TESTS/mbed_functional/callback/main.cpp b/TESTS/mbed_functional/callback/main.cpp index ed4ada8abd..5b594b1fc3 100644 --- a/TESTS/mbed_functional/callback/main.cpp +++ b/TESTS/mbed_functional/callback/main.cpp @@ -884,7 +884,10 @@ Case cases[] = { Case("Testing callbacks with 2 uint64s", test_dispatch2), Case("Testing callbacks with 3 uint64s", test_dispatch3), Case("Testing callbacks with 4 uint64s", test_dispatch4), +// IAR currently crashes at link time with this test - skip it as it's well beyond anything needed by real code +#ifndef __ICCARM__ Case("Testing callbacks with 5 uint64s", test_dispatch5), +#endif #elif DO_SMALL_TEST Case("Testing callbacks with 0 uchars", test_dispatch0), Case("Testing callbacks with 1 uchars", test_dispatch1),