From: Jonathan Wakely Date: Wed, 18 Mar 2020 00:23:39 +0000 (+0000) Subject: libstdc++: Fix type-erasure in experimental::net::executor (PR 94203) X-Git-Url: https://git.libre-soc.org/?a=commitdiff_plain;h=98f29f5638f73d8e55590eba8098a537ba746287;p=gcc.git libstdc++: Fix type-erasure in experimental::net::executor (PR 94203) The _Tgt and _TgtImpl types that implement type-erasure didn't agree on the virtual interface, so failed as soon as they were instantiated. With Clang they failed even sooner. The interface was also dependent on whether RTTI was enabled or not. This patch fixes the broken virtual functions and makes the type work without RTTI, by using a pointer to a specialization of a function template (similar to the approaches in std::function and std::any). The changes to the virtual functions would be an ABI change, except that the previous code didn't even compile if instantiated. This is experimental TS material anyway. PR libstdc++/94203 * include/experimental/executor (executor::executor(Executor)): Call make_shared directly instead of _M_create. Create _Tgt1 object. (executor::executor(allocator_arg_t, const ProtoAlloc&, Executor)): Call allocate_shared directly instead of _M_create. Create _Tgt2 object. (executor::target_type): Add cast needed for new _Tgt interface. (executor::target): Define when RTTI is disabled. Use _Tgt::_M_func. (executor::_Tgt): Define the same interface whether RTTI is enabled or not. (executor::_Tgt::target_type, executor::_Tgt::target): Do not use std::type_info in the interface. (executor::_Tgt::_M_func): Add data member. (executor::_TgtImpl): Replace with _Tgt1 and _Tgt2 class templates. (executor::_Tgt1::_S_func): Define function to access target without depending on RTTI. (executor::_M_create): Remove. (operator==, operator!=): Simplify comparisons for executor. * include/experimental/socket (is_error_code_enum): Define specialization before use. * testsuite/experimental/net/executor/1.cc: New test. --- diff --git a/libstdc++-v3/ChangeLog b/libstdc++-v3/ChangeLog index 66acdb08cda..b874bb25b9e 100644 --- a/libstdc++-v3/ChangeLog +++ b/libstdc++-v3/ChangeLog @@ -1,3 +1,27 @@ +2020-03-18 Jonathan Wakely + + PR libstdc++/94203 + * include/experimental/executor (executor::executor(Executor)): Call + make_shared directly instead of _M_create. Create _Tgt1 object. + (executor::executor(allocator_arg_t, const ProtoAlloc&, Executor)): + Call allocate_shared directly instead of _M_create. Create _Tgt2 + object. + (executor::target_type): Add cast needed for new _Tgt interface. + (executor::target): Define when RTTI is disabled. Use _Tgt::_M_func. + (executor::_Tgt): Define the same interface whether RTTI is enabled or + not. + (executor::_Tgt::target_type, executor::_Tgt::target): Do not use + std::type_info in the interface. + (executor::_Tgt::_M_func): Add data member. + (executor::_TgtImpl): Replace with _Tgt1 and _Tgt2 class templates. + (executor::_Tgt1::_S_func): Define function to access target without + depending on RTTI. + (executor::_M_create): Remove. + (operator==, operator!=): Simplify comparisons for executor. + * include/experimental/socket (is_error_code_enum): + Define specialization before use. + * testsuite/experimental/net/executor/1.cc: New test. + 2020-03-16 Jonathan Wakely PR libstdc++/94199 diff --git a/libstdc++-v3/include/experimental/executor b/libstdc++-v3/include/experimental/executor index 6955ead6bf5..b5c6e18a19a 100644 --- a/libstdc++-v3/include/experimental/executor +++ b/libstdc++-v3/include/experimental/executor @@ -295,26 +295,22 @@ inline namespace v1 class executor; bool - operator==(const executor& __a, const executor& __b) noexcept; + operator==(const executor&, const executor&) noexcept; bool - operator==(const executor& __e, nullptr_t) noexcept; + operator==(const executor&, nullptr_t) noexcept; - inline bool - operator==(nullptr_t, const executor& __e) noexcept - { return __e == nullptr; } + bool + operator==(nullptr_t, const executor&) noexcept; - inline bool - operator!=(const executor& __a, const executor& __b) noexcept - { return !(__a == __b); } + bool + operator!=(const executor&, const executor&) noexcept; - inline bool - operator!=(const executor& __e, nullptr_t) noexcept - { return !(__e == nullptr); } + bool + operator!=(const executor&, nullptr_t) noexcept; - inline bool - operator!=(nullptr_t, const executor& __e) noexcept - { return !(__e == nullptr); } + bool + operator!=(nullptr_t, const executor&) noexcept; void swap(executor&, executor&) noexcept; @@ -999,12 +995,13 @@ inline namespace v1 template executor(_Executor __e) - : _M_target(_M_create(std::move(__e))) + : _M_target(make_shared<_Tgt1<_Executor>>(std::move(__e))) { } template executor(allocator_arg_t, const _ProtoAlloc& __a, _Executor __e) - : _M_target(_M_create(std::move(__e), __a)) + : _M_target(allocate_shared<_Tgt2<_Executor, _ProtoAlloc>>(__a, + std::move(__e), __a)) { } executor& operator=(const executor&) noexcept = default; @@ -1101,28 +1098,46 @@ inline namespace v1 #if __cpp_rtti const type_info& target_type() const noexcept - { return _M_target ? _M_target->target_type() : typeid(void); } + { + if (_M_target) + return *static_cast(_M_target->target_type()); + return typeid(void); + } +#endif template _Executor* target() noexcept { + void* __p = nullptr; if (_M_target) - if (const auto* __p = _M_target->target(typeid(_Executor))) - return const_cast<_Executor*>(static_cast(__p)); - return nullptr; + { + if (_M_target->_M_func == &_Tgt1>::_S_func) + __p = _M_target->_M_func(_M_target.get(), nullptr); +#if __cpp_rtti + else + __p = _M_target->target(&typeid(_Executor)); +#endif + } + return static_cast<_Executor*>(__p); } template const _Executor* target() const noexcept { + const void* __p = nullptr; if (_M_target) - if (const auto* __p = _M_target->target(typeid(_Executor))) - return static_cast(__p); - return nullptr; - } + { + if (_M_target->_M_func == &_Tgt1>::_S_func) + return (_Executor*)_M_target->_M_func(_M_target.get(), nullptr); +#if __cpp_rtti + else + __p = _M_target->target(&typeid(_Executor)); #endif + } + return static_cast(__p); + } private: struct _Tgt @@ -1133,85 +1148,134 @@ inline namespace v1 virtual void dispatch(std::function) const = 0; virtual void post(std::function) const = 0; virtual void defer(std::function) const = 0; -#if __cpp_rtti - virtual const type_info& target_type() const = 0; - virtual void* target(const std::type_info&) const = 0; + virtual const void* target_type() const noexcept = 0; + virtual void* target(const void*) noexcept = 0; virtual bool _M_equals(_Tgt*) const noexcept = 0; - virtual const void* _M_get_executor() const noexcept = 0; -#endif + + using _Func = void* (_Tgt*, const _Tgt*); + _Func* _M_func; // Provides access to target without RTTI }; - template - struct _TgtImpl : _Tgt + template + struct _Tgt1 : _Tgt { explicit - _TgtImpl(_Ex&& __ex, const _Alloc& __a) - : _M_impl(std::move(__ex), __a) { } + _Tgt1(_Ex&& __ex) + : _M_ex(std::move(__ex)) + { this->_M_func = &_S_func; } + + void + on_work_started() const noexcept override + { _M_ex.on_work_started(); } - void on_work_started() const noexcept { _M_ex().on_work_started(); } - void on_work_finished() const noexcept { _M_ex().on_work_finished(); } - execution_context& context() const noexcept { return _M_ex().context(); } void - dispatch(std::function __f) const - { _M_ex().dispatch(std::move(__f), _M_alloc()); } + on_work_finished() const noexcept override + { _M_ex.on_work_finished(); } + + execution_context& + context() const noexcept override + { return _M_ex.context(); } + void - post(std::function __f) const - { _M_ex().post(std::move(__f), _M_alloc()); } + dispatch(std::function __f) const override + { _M_ex.dispatch(std::move(__f), allocator()); } + + void + post(std::function __f) const override + { _M_ex.post(std::move(__f), allocator()); } + void - defer(std::function __f) const - { _M_ex().defer(std::move(__f), _M_alloc()); } + defer(std::function __f) const override + { _M_ex.defer(std::move(__f), allocator()); } + const void* + target_type() const noexcept override + { #if __cpp_rtti - virtual const type_info& - target_type() const - { return typeid(_Ex); } + return &typeid(_Ex); +#else + return nullptr; +#endif + } - virtual const void* - target(const std::type_info& __ti) const + void* + target(const void* __ti) noexcept override { - if (__ti == typeid(_Ex)) - return std::addressof(_M_ex()); +#if __cpp_rtti + if (*static_cast(__ti) == typeid(_Ex)) + return std::__addressof(_M_ex); +#endif return nullptr; } - virtual bool - _M_equals(const _Tgt* __tgt) const noexcept + bool + _M_equals(_Tgt* __tgt) const noexcept override { - if (__tgt->target_type() == typeid(_Ex)) - *static_cast(__tgt->_M_get_executor()) == _M_ex(); +#if __cpp_rtti + if (const void* __p = __tgt->target(&typeid(_Ex))) + return *static_cast(__p) == _M_ex; +#endif return false; } - virtual const void* - _M_get_executor() const noexcept - { return std::addressof(_M_ex()); } -#endif + _Ex _M_ex [[__no_unique_address__]]; - _Ex& _M_ex() { return std::get<0>(_M_impl); } - _Alloc& _M_alloc() { return std::get<1>(_M_impl); } - std::tuple<_Ex, _Alloc> _M_impl; + static void* + _S_func(_Tgt* __p, const _Tgt* __q) noexcept + { + auto& __ex = static_cast<_Tgt1*>(__p)->_M_ex; + if (__q) + { + if (__ex == static_cast(__q)->_M_ex) + return __p; + else + return nullptr; + } + else + return std::__addressof(__ex); + } }; - template> - shared_ptr<_Tgt> - _M_create(_Ex&& __ex, const _Alloc& __a = _Alloc()) + template + struct _Tgt2 : _Tgt1<_Ex> { - return allocate_shared<_TgtImpl<_Ex, _Alloc>>(__a, std::move(__ex), - __a); - } + explicit + _Tgt2(_Ex&& __ex, const _Alloc& __a) + : _Tgt1<_Ex>(std::move(__ex)), _M_alloc(__a) { } + + void + dispatch(std::function __f) const override + { this->_M_ex.dispatch(std::move(__f), _M_alloc); } + + void + post(std::function __f) const override + { this->_M_ex.post(std::move(__f), _M_alloc); } + + void + defer(std::function __f) const override + { this->_M_ex.defer(std::move(__f), _M_alloc); } + + _Alloc _M_alloc [[__no_unique_address__]]; + }; + + // Partial specialization for std::allocator. + // Don't store the allocator. + template + struct _Tgt2<_Ex, std::allocator<_Tp>> : _Tgt1<_Ex> + { }; friend bool operator==(const executor& __a, const executor& __b) noexcept { - if (__a._M_target == __b._M_target) + _Tgt* __ta = __a._M_target.get(); + _Tgt* __tb = __b._M_target.get(); + if (__ta == __tb) return true; - if (!__a._M_target || !__b._M_target) + if (!__ta || !__tb) return false; -#if __cpp_rtti - return __a._M_target->_M_equals(__b._M_target.get()); -#else - return false; // XXX can we do better? -#endif + if (__ta->_M_func == __tb->_M_func) + return __ta->_M_func(__ta, __tb); + return __ta->_M_equals(__tb); } shared_ptr<_Tgt> _M_target; @@ -1224,6 +1288,22 @@ inline namespace v1 operator==(const executor& __e, nullptr_t) noexcept { return !__e; } + inline bool + operator==(nullptr_t, const executor& __e) noexcept + { return !__e; } + + inline bool + operator!=(const executor& __a, const executor& __b) noexcept + { return !(__a == __b); } + + inline bool + operator!=(const executor& __e, nullptr_t) noexcept + { return (bool)__e; } + + inline bool + operator!=(nullptr_t, const executor& __e) noexcept + { return (bool)__e; } + /// Swap two executor objects. inline void swap(executor& __a, executor& __b) noexcept { __a.swap(__b); } diff --git a/libstdc++-v3/include/experimental/socket b/libstdc++-v3/include/experimental/socket index 392db506674..81b1d15b654 100644 --- a/libstdc++-v3/include/experimental/socket +++ b/libstdc++-v3/include/experimental/socket @@ -79,6 +79,20 @@ inline namespace v1 not_found = 4 }; +} // namespace v1 +} // namespace net +} // namespace experimental + + template<> + struct is_error_code_enum + : public true_type {}; + +namespace experimental +{ +namespace net +{ +inline namespace v1 +{ const error_category& socket_category() noexcept { struct __cat : error_category @@ -2602,10 +2616,6 @@ inline namespace v1 } // namespace net } // namespace experimental - template<> - struct is_error_code_enum - : public true_type {}; - _GLIBCXX_END_NAMESPACE_VERSION } // namespace std diff --git a/libstdc++-v3/testsuite/experimental/net/executor/1.cc b/libstdc++-v3/testsuite/experimental/net/executor/1.cc new file mode 100644 index 00000000000..456d620e193 --- /dev/null +++ b/libstdc++-v3/testsuite/experimental/net/executor/1.cc @@ -0,0 +1,93 @@ +// Copyright (C) 2020 Free Software Foundation, Inc. +// +// This file is part of the GNU ISO C++ Library. This library 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; either version 3, or (at your option) +// any later version. + +// This library 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 library; see the file COPYING3. If not see +// . + +// { dg-do run { target c++14 } } + +#include +#include + +namespace net = std::experimental::net; + +void +test01() +{ + net::executor e; + VERIFY( !e ); + VERIFY( e == nullptr ); + VERIFY( nullptr == e ); + VERIFY( e == e ); + VERIFY( e == e ); + net::executor e2; + VERIFY( e == e2 ); + swap(e, e2); + VERIFY( e == e2 ); + e = e2; + VERIFY( e == e2 ); +} + +void +test02() +{ + struct E + { + void on_work_started() const noexcept { } + void on_work_finished() const noexcept { } + net::execution_context& context() const noexcept { return c; } + void dispatch(std::function, std::allocator) const { } + void post(std::function, std::allocator) const { } + void defer(std::function, std::allocator) const { } + + net::execution_context& c; + + bool operator==(const E& rhs) const noexcept + { return &c == &rhs.c; } + }; + + net::execution_context c; + E d{c}; + net::executor e(d); + VERIFY( e == e ); + VERIFY( e != nullptr ); + VERIFY( nullptr != e ); + + VERIFY( &e.context() == &c ); +#if __cpp_rtti + VERIFY( e.target_type() == typeid(E) ); +#endif + VERIFY( *e.target() == d ); + VERIFY( *e.target() == d ); + VERIFY( *const_cast(e).target() == d ); + VERIFY( *const_cast(e).target() == d ); + + net::executor f = e; + VERIFY( f == e ); + e = nullptr; + VERIFY( f != e ); + swap(e, f); + VERIFY( f == nullptr ); + VERIFY( nullptr != e ); + + net::executor g(E{c}); + VERIFY( e == g ); +} + +int +main() +{ + test01(); + test02(); +}