From 152d311af6970b4cfca02fe2daca7efc4dfaa6d4 Mon Sep 17 00:00:00 2001 From: Jean-Louis Leroy Date: Sun, 29 Sep 2024 13:48:08 -0400 Subject: [PATCH] N2216 ambiguity resolution --- examples/CMakeLists.txt | 2 +- include/yorel/yomm2/core.hpp | 93 ++-- include/yorel/yomm2/decode.hpp | 3 +- include/yorel/yomm2/detail/compiler.hpp | 412 +++++++++++------- include/yorel/yomm2/detail/types.hpp | 6 +- include/yorel/yomm2/generator.hpp | 2 - include/yorel/yomm2/macros.hpp | 6 +- .../yomm2/policies/vectored_error_handler.hpp | 2 +- tests/test_blackbox.cpp | 189 ++++++-- tests/test_core.cpp | 26 +- tests/test_generator.cpp | 6 - 11 files changed, 478 insertions(+), 269 deletions(-) diff --git a/examples/CMakeLists.txt b/examples/CMakeLists.txt index 4c2688d8..18e2e65e 100644 --- a/examples/CMakeLists.txt +++ b/examples/CMakeLists.txt @@ -56,5 +56,5 @@ if (NOT (MSVC AND YOMM2_SHARED)) # would need to add the path to the directory containing yomm2.dll to PATH. # Anyway, if it works with static linking, it is very unlikely that it fails # with the runtime in a DLL. - add_subdirectory(generator) + # add_subdirectory(generator) endif() diff --git a/include/yorel/yomm2/core.hpp b/include/yorel/yomm2/core.hpp index e2fd9b57..2b502b29 100644 --- a/include/yorel/yomm2/core.hpp +++ b/include/yorel/yomm2/core.hpp @@ -162,7 +162,9 @@ auto optimal_cast(B&& obj) -> decltype(auto) { // virtual_traits template -struct virtual_traits; +struct virtual_traits { + using polymorphic_type = void; +}; template struct virtual_traits { @@ -178,6 +180,20 @@ struct virtual_traits { } }; +template +struct virtual_traits { + using polymorphic_type = std::remove_cv_t; + + static auto rarg(const T* arg) -> const T& { + return *arg; + } + + template + static auto cast(T* obj) -> D* { + return &detail::optimal_cast(*obj); + } +}; + template struct virtual_traits { using polymorphic_type = std::remove_cv_t; @@ -224,7 +240,7 @@ using polymorphic_types = boost::mp11::mp_transform< remove_virtual, boost::mp11::mp_filter>; template -struct argument_traits { +struct parameter_traits { static auto rarg(const T& arg) -> const T& { return arg; } @@ -236,14 +252,14 @@ struct argument_traits { }; template -struct argument_traits> : virtual_traits {}; +struct parameter_traits> : virtual_traits {}; template -struct argument_traits> +struct parameter_traits> : virtual_traits> {}; template -struct argument_traits&> +struct parameter_traits&> : virtual_traits&> {}; } // namespace detail @@ -463,7 +479,8 @@ class virtual_ptr { virtual_ptr_traits::template cast(obj); } else { result.obj = - &detail::optimal_cast(*obj); + &detail::optimal_cast( + *obj); } return result; @@ -607,8 +624,6 @@ class method : public detail::method_info { static BOOST_NORETURN auto not_implemented_handler(detail::remove_virtual... args) -> Return; - static BOOST_NORETURN auto - ambiguous_handler(detail::remove_virtual... args) -> Return; template struct thunk; @@ -646,7 +661,7 @@ class method : public detail::method_info { detail::types>>; }; - template + template struct override_impl { explicit override_impl(FunctionPointer* next = nullptr); }; @@ -656,19 +671,17 @@ class method : public detail::method_info { template struct override_aux - : override_impl { - }; + : override_impl {}; template< auto Function, class FnClass, typename FnReturnType, typename... FnParameters> - struct override_aux< - Function, FnReturnType (FnClass::*)(FnParameters...)> { + struct override_aux { static auto fn(FnClass* this_, FnParameters&&... args) -> FnReturnType { return (this_->*Function)(std::forward(args)...); } - override_impl impl{&next}; + override_impl impl{&next}; }; public: @@ -692,18 +705,19 @@ constexpr bool is_method = std::is_base_of_v; template method::method() { - this->slots_strides_ptr = slots_strides; + method_info::slots_strides_ptr = slots_strides; using virtual_type_ids = detail::type_id_list< Policy, boost::mp11::mp_transform_q< boost::mp11::mp_bind_front, VirtualParameters>>; - this->vp_begin = virtual_type_ids::begin; - this->vp_end = virtual_type_ids::end; - this->not_implemented = (void*)not_implemented_handler; - this->ambiguous = (void*)ambiguous_handler; - this->method_type = Policy::template static_type(); + method_info::vp_begin = virtual_type_ids::begin; + method_info::vp_end = virtual_type_ids::end; + method_info::not_implemented = (void*)not_implemented_handler; + method_info::method_type = Policy::template static_type(); + method_info::return_type = Policy::template static_type< + typename virtual_traits::polymorphic_type>(); Policy::methods.push_back(*this); } @@ -742,7 +756,7 @@ template BOOST_FORCEINLINE auto method::operator()( detail::remove_virtual... args) const -> Return { using namespace detail; - auto pf = resolve(argument_traits::rarg(args)...); + auto pf = resolve(parameter_traits::rarg(args)...); return pf(std::forward>(args)...); } @@ -926,30 +940,7 @@ method::not_implemented_handler( auto ti_iter = types; (..., (*ti_iter++ = Policy::dynamic_type( - detail::argument_traits::rarg(args)))); - std::copy_n( - types, (std::min)(sizeof...(args), resolution_error::max_types), - &error.types[0]); - Policy::error(error); - } - - abort(); // in case user handler "forgets" to abort -} - -template -BOOST_NORETURN auto -method::ambiguous_handler( - detail::remove_virtual... args) -> Return { - if constexpr (Policy::template has_facet) { - resolution_error error; - error.status = resolution_error::ambiguous; - error.method = Policy::template static_type(); - error.arity = Arity; - type_id types[sizeof...(args)]; - auto ti_iter = types; - (..., - (*ti_iter++ = Policy::dynamic_type( - detail::argument_traits::rarg(args)))); + detail::parameter_traits::rarg(args)))); std::copy_n( types, (std::min)(sizeof...(args), resolution_error::max_types), &error.types[0]); @@ -969,7 +960,7 @@ auto method:: thunk::fn( detail::remove_virtual... arg) -> Return { return Overrider( - detail::argument_traits::template cast< + detail::parameter_traits::template cast< OverriderParameters>(detail::remove_virtual(arg))...); } @@ -977,16 +968,18 @@ auto method:: // overriders template -template +template method::override_impl< - Function>::override_impl(FunctionPointer* p_next) { + Function, FnReturnType>::override_impl(FunctionPointer* p_next) { + using namespace detail; + // Work around MSVC bug: using &next as a default value // for 'next' confuses it about Parameters not being expanded. if (!p_next) { p_next = &next; } - static detail::overrider_info info; + static overrider_info info; if (info.method) { BOOST_ASSERT(info.method == &fn); @@ -994,6 +987,8 @@ method::override_impl< } info.method = &fn; + info.return_type = Policy::template static_type< + typename virtual_traits::polymorphic_type>(); info.type = Policy::template static_type(); info.next = reinterpret_cast(p_next); using Thunk = thunk; diff --git a/include/yorel/yomm2/decode.hpp b/include/yorel/yomm2/decode.hpp index 7646fa14..38513344 100644 --- a/include/yorel/yomm2/decode.hpp +++ b/include/yorel/yomm2/decode.hpp @@ -77,13 +77,12 @@ void decode_dispatch_data(Data& init) { packed_slots_iter += slots_strides_count; auto specs = - (uintptr_t*)alloca((method.specs.size() + 2) * pointer_size); + (uintptr_t*)alloca((method.specs.size() + 1) * pointer_size); *method_defs_iter++ = specs; ++trace << "specs index: " << specs << "\n"; specs = std::transform( method.specs.begin(), method.specs.end(), specs, [](auto& spec) { return (uintptr_t)spec.pf; }); - *specs++ = (uintptr_t)method.ambiguous; *specs++ = (uintptr_t)method.not_implemented; ++method_index; } diff --git a/include/yorel/yomm2/detail/compiler.hpp b/include/yorel/yomm2/detail/compiler.hpp index 71da381f..320d8cd2 100644 --- a/include/yorel/yomm2/detail/compiler.hpp +++ b/include/yorel/yomm2/detail/compiler.hpp @@ -100,6 +100,12 @@ struct generic_compiler { std::vector vtbl; std::uintptr_t** static_vptr; + bool is_base_of(class_* other) const { + return std::find( + covariant_classes.begin(), covariant_classes.end(), + other) != covariant_classes.end(); + } + const std::uintptr_t* vptr() const { return *static_vptr; } @@ -118,8 +124,10 @@ struct generic_compiler { }; struct overrider { - const detail::overrider_info* info; + detail::overrider_info* info = nullptr; + overrider* next = nullptr; std::vector vp; + class_* covariant_return_type = nullptr; std::uintptr_t pf; std::size_t method_index, spec_index; }; @@ -133,20 +141,20 @@ struct generic_compiler { using group_map = std::map; - struct update_method_report { + struct method_report { std::size_t cells = 0; std::size_t not_implemented = 0; std::size_t ambiguous = 0; }; - struct update_report : update_method_report {}; + struct report : method_report {}; - static void - accumulate(const update_method_report& partial, update_report& total); + static void accumulate(const method_report& partial, report& total); struct method { detail::method_info* info; std::vector vp; + class_* covariant_return_type = nullptr; std::vector specs; std::vector slots; std::vector strides; @@ -154,12 +162,11 @@ struct generic_compiler { // following two are dummies, when converting to a function pointer, we will // get the corresponding pointer from method_info overrider not_implemented; - overrider ambiguous; - const std::uintptr_t* gv_dispatch_table{nullptr}; + const std::uintptr_t* gv_dispatch_table = nullptr; auto arity() const { return vp.size(); } - update_method_report report; + method_report report; }; std::deque classes; @@ -208,9 +215,7 @@ struct spec_name { template trace_type& operator<<(trace_type& trace, const spec_name& sn) { - if (sn.def == &sn.method.ambiguous) { - trace << "ambiguous"; - } else if (sn.def == &sn.method.not_implemented) { + if (sn.def == &sn.method.not_implemented) { trace << "not implemented"; } else { trace << type_name(sn.def->info->type); @@ -227,7 +232,7 @@ struct compiler : detail::generic_compiler { using type_index_type = decltype(Policy::type_index(0)); typename detail::aggregate_reports< - detail::types, typename Policy::facets>::type report; + detail::types, typename Policy::facets>::type report; std::unordered_map class_map; @@ -249,10 +254,11 @@ struct compiler : detail::generic_compiler { method& m, std::size_t dim, std::vector::const_iterator group, const bitvec& candidates, bool concrete); - void install_gv(); - void print(const update_method_report& report) const; - static std::vector - best(std::vector& candidates); + void write_global_data(); + void print(const method_report& report) const; + static void select_dominant_overriders( + std::vector& dominants, std::size_t& pick, + std::size_t& remaining); static bool is_more_specific(const overrider* a, const overrider* b); static bool is_base(const overrider* a, const overrider* b); @@ -279,7 +285,7 @@ void compiler::install_global_tables() { abort(); } - install_gv(); + write_global_data(); print(report); ++trace << "Finished\n"; @@ -320,41 +326,37 @@ void compiler::resolve_static_type_ids() { }; if constexpr (std::is_base_of_v) { - if (!Policy::classes.empty()) - for (auto& ci : Policy::classes) { - resolve(&ci.type); - - if (*ci.last_base == 0) { - for (auto& ti : range{ci.first_base, ci.last_base}) { - resolve(&ti); - } + for (auto& ci : Policy::classes) { + resolve(&ci.type); - *ci.last_base = 1; + if (*ci.last_base == 0) { + for (auto& ti : range{ci.first_base, ci.last_base}) { + resolve(&ti); } - } - if (!Policy::methods.empty()) - for (auto& method : Policy::methods) { - for (auto& ti : range{method.vp_begin, method.vp_end}) { - if (*method.vp_end == 0) { - resolve(&ti); - *method.vp_end = 1; - } + *ci.last_base = 1; + } + } - if (!method.specs.empty()) - for (auto& overrider : method.specs) { - if (*overrider.vp_end == 0) { - for (auto& ti : range{ - overrider.vp_begin, - overrider.vp_end}) { - resolve(&ti); - } + for (auto& method : Policy::methods) { + for (auto& ti : range{method.vp_begin, method.vp_end}) { + if (*method.vp_end == 0) { + resolve(&ti); + *method.vp_end = 1; + } - *overrider.vp_end = 1; - } + for (auto& overrider : method.specs) { + if (*overrider.vp_end == 0) { + for (auto& ti : + range{overrider.vp_begin, overrider.vp_end}) { + resolve(&ti); } + + *overrider.vp_end = 1; + } } } + } } } @@ -563,17 +565,25 @@ void compiler::augment_methods() { meth_iter->vp.push_back(class_); } - // initialize the function pointer in the dummy specs + if (Policy::type_index(meth_info.return_type) != + Policy::type_index(Policy::template static_type())) { + auto covariant_return_iter = + class_map.find(Policy::type_index(meth_info.return_type)); + + if (covariant_return_iter != class_map.end()) { + meth_iter->covariant_return_type = + covariant_return_iter->second; + } + } + + // initialize the function pointer in the synthetic not_implemented + // overrider const auto method_index = meth_iter - methods.begin(); - meth_iter->ambiguous.pf = - reinterpret_cast(meth_iter->info->ambiguous); - meth_iter->ambiguous.method_index = method_index; auto spec_size = meth_info.specs.size(); - meth_iter->ambiguous.spec_index = spec_size; meth_iter->not_implemented.pf = reinterpret_cast(meth_iter->info->not_implemented); meth_iter->not_implemented.method_index = method_index; - meth_iter->not_implemented.spec_index = spec_size + 1; + meth_iter->not_implemented.spec_index = spec_size; meth_iter->specs.resize(spec_size); auto spec_iter = meth_iter->specs.begin(); @@ -592,8 +602,9 @@ void compiler::augment_methods() { range{overrider_info.vp_begin, overrider_info.vp_end}) { indent _(trace); auto class_ = class_map[Policy::type_index(type)]; + if (!class_) { - ++trace << "error for *virtual* parameter #" + ++trace << "unknown class error for *virtual* parameter #" << (param_index + 1) << "\n"; unknown_class_error error; error.type = type; @@ -609,6 +620,25 @@ void compiler::augment_methods() { spec_iter->vp.push_back(class_); } + if (meth_iter->covariant_return_type) { + auto covariant_return_iter = class_map.find( + Policy::type_index(overrider_info.return_type)); + + if (covariant_return_iter != class_map.end()) { + spec_iter->covariant_return_type = + covariant_return_iter->second; + } else { + unknown_class_error error; + error.type = overrider_info.return_type; + + if constexpr (has_facet) { + Policy::error(error); + } + + abort(); + } + } + ++spec_iter; } @@ -856,7 +886,7 @@ void compiler::build_dispatch_tables() { } { - ++trace << "assigning specs\n"; + ++trace << "building dispatch table\n"; bitvec all(m.specs.size()); all = ~all; build_dispatch_table(m, dims - 1, groups.end() - 1, all, true); @@ -890,55 +920,6 @@ void compiler::build_dispatch_tables() { print(m.report); accumulate(m.report, report); - ++trace << "assigning next\n"; - - std::vector specs; - std::transform( - m.specs.begin(), m.specs.end(), std::back_inserter(specs), - [](const overrider& spec) { return &spec; }); - - for (auto& spec : m.specs) { - indent _(trace); - ++trace << type_name(spec.info->type) << ":\n"; - std::vector candidates; - std::copy_if( - specs.begin(), specs.end(), std::back_inserter(candidates), - [&spec](const overrider* other) { - return is_base(other, &spec); - }); - - if constexpr (trace_enabled) { - indent _(trace); - ++trace << "for next, select best:\n"; - - for (auto& candidate : candidates) { - indent _(trace); - ++trace << "#" << candidate->spec_index - << type_name(candidate->info->type) << "\n"; - } - } - - auto nexts = best(candidates); - void* next; - - if (nexts.size() == 1) { - const overrider_info* next_info = nexts.front()->info; - next = next_info->pf; - ++trace << "-> " - << "#" << nexts.front()->spec_index - << type_name(next_info->type) << "\n"; - } else if (nexts.empty()) { - ++trace << "-> none\n"; - next = m.info->not_implemented; - } else if (nexts.size() > 1) { - ++trace << "-> ambiguous\n"; - next = m.info->ambiguous; - } - - if (spec.info->next) { - *spec.info->next = next; - } - } } } } @@ -966,12 +947,12 @@ void compiler::build_dispatch_table( } if (dim == 0) { - std::vector applicable; + std::vector candidates; std::size_t i = 0; - for (const auto& spec : m.specs) { + for (auto& spec : m.specs) { if (mask[i]) { - applicable.push_back(&spec); + candidates.push_back(&spec); } ++i; } @@ -980,49 +961,109 @@ void compiler::build_dispatch_table( ++trace << "select best of:\n"; indent _(trace); - for (auto& app : applicable) { + for (auto& app : candidates) { ++trace << "#" << app->spec_index << " " << type_name(app->info->type) << "\n"; } } - auto specs = best(applicable); + std::vector dominants = candidates; + std::size_t pick, remaining; - if (specs.size() > 1) { - indent _(trace); - ++trace << "ambiguous\n"; - m.dispatch_table.push_back(&m.ambiguous); - ++m.report.ambiguous; - } else if (specs.empty()) { + select_dominant_overriders(dominants, pick, remaining); + + if (remaining == 0) { indent _(trace); ++trace << "not implemented\n"; m.dispatch_table.push_back(&m.not_implemented); ++m.report.not_implemented; } else { - auto spec = specs[0]; - m.dispatch_table.push_back(spec); - ++trace << "-> #" << spec->spec_index << " " - << type_name(spec->info->type) - << " pf = " << spec->info->pf << "\n"; + auto overrider = dominants[pick]; + m.dispatch_table.push_back(overrider); + ++trace; + + trace << "-> #" << overrider->spec_index << " " + << type_name(overrider->info->type) + << " pf = " << overrider->info->pf; + + if (remaining > 1) { + trace << " (ambiguous)"; + ++m.report.ambiguous; + } + + trace << "\n"; + + // ------------------------------------------------------------- + // next + + // First remove the dominant overriders from the candidates. + // Note that the dominants appear in the candidates in the same + // relative order. + auto candidate = candidates.begin(); + remaining = 0; + + for (auto dominant : dominants) { + if (*candidate == dominant) { + *candidate = nullptr; + } else { + ++remaining; + } + + ++candidate; + } + + if (remaining == 0) { + ++trace << "no 'next'\n"; + overrider->next = &m.not_implemented; + } else { + if constexpr (trace_enabled) { + ++trace << "for 'next', select best of:\n"; + indent _(trace); + + for (auto& app : candidates) { + if (app) { + ++trace << "#" << app->spec_index << " " + << type_name(app->info->type) << "\n"; + } + } + } + + select_dominant_overriders(candidates, pick, remaining); + + auto next_overrider = candidates[pick]; + overrider->next = next_overrider; + + ++trace << "-> #" << next_overrider->spec_index << " " + << type_name(next_overrider->info->type) + << " pf = " << next_overrider->info->pf; + + if (remaining > 1) { + trace << " (ambiguous)"; + // do not increment m.report.ambiguous, for same reason + } + + trace << "\n"; + } } } else { build_dispatch_table( m, dim - 1, group_iter - 1, mask, concrete && group.has_concrete_classes); } + ++group_index; } } inline void detail::generic_compiler::accumulate( - const update_method_report& partial, update_report& total) { + const method_report& partial, report& total) { total.cells += partial.cells; total.not_implemented += partial.not_implemented != 0; total.ambiguous += partial.ambiguous != 0; } template -void compiler::install_gv() { +void compiler::write_global_data() { using namespace policies; using namespace detail; @@ -1045,32 +1086,52 @@ void compiler::install_gv() { if (m.info->arity() == 1) { // Uni-methods just need an index in the method table. m.info->slots_strides_ptr[0] = m.slots[0]; - continue; + } else { + auto strides_iter = std::copy( + m.slots.begin(), m.slots.end(), m.info->slots_strides_ptr); + std::copy(m.strides.begin(), m.strides.end(), strides_iter); + + if constexpr (trace_enabled) { + ++trace << rflush(4, Policy::dispatch_data.size()) << " " + << " method #" << m.dispatch_table[0]->method_index + << " " << type_name(m.info->method_type) << "\n"; + indent _(trace); + + for (auto& entry : m.dispatch_table) { + ++trace << "spec #" << entry->spec_index << " " + << spec_name(m, entry) << "\n"; + } + } + + m.gv_dispatch_table = gv_iter; + BOOST_ASSERT(gv_iter + m.dispatch_table.size() <= gv_last); + gv_iter = std::transform( + m.dispatch_table.begin(), m.dispatch_table.end(), gv_iter, + [](auto spec) { return spec->pf; }); } + } - // multi-methods only + ++trace << "Setting 'next' pointers\n"; - auto strides_iter = std::copy( - m.slots.begin(), m.slots.end(), m.info->slots_strides_ptr); - std::copy(m.strides.begin(), m.strides.end(), strides_iter); + for (auto& m : methods) { + indent _(trace); + ++trace << "method #" + << " " << type_name(m.info->method_type) << "\n"; - if constexpr (trace_enabled) { - ++trace << rflush(4, Policy::dispatch_data.size()) << " " - << " method #" << m.dispatch_table[0]->method_index << " " - << type_name(m.info->method_type) << "\n"; - indent _(trace); + for (auto& overrider : m.specs) { + if (overrider.next) { + ++trace << "#" << overrider.spec_index << " " + << spec_name(m, &overrider) << " -> "; - for (auto& entry : m.dispatch_table) { - ++trace << "spec #" << entry->spec_index << " " - << spec_name(m, entry) << "\n"; + trace << "#" << overrider.next->spec_index << " " + << spec_name(m, overrider.next); + *overrider.info->next = (void*)overrider.next->pf; + } else { + trace << "none"; } - } - m.gv_dispatch_table = gv_iter; - BOOST_ASSERT(gv_iter + m.dispatch_table.size() <= gv_last); - gv_iter = std::transform( - m.dispatch_table.begin(), m.dispatch_table.end(), gv_iter, - [](auto spec) { return spec->pf; }); + trace << "\n"; + } } ++trace << "Initializing v-tables at " << gv_iter << "\n"; @@ -1129,30 +1190,61 @@ void compiler::install_gv() { } template -std::vector -compiler::best(std::vector& candidates) { - std::vector best; - - for (auto spec : candidates) { - const overrider* candidate = spec; - - for (auto iter = best.begin(); iter != best.end();) { - if (is_more_specific(spec, *iter)) { - iter = best.erase(iter); - } else if (is_more_specific(*iter, spec)) { - candidate = nullptr; - break; - } else { - ++iter; +void compiler::select_dominant_overriders( + std::vector& candidates, std::size_t& pick, + std::size_t& remaining) { + + pick = 0; + remaining = 0; + + for (size_t i = 0; i < candidates.size(); ++i) { + if (candidates[i]) { + for (size_t j = i + 1; j < candidates.size(); ++j) { + if (candidates[j]) { + if (is_more_specific(candidates[i], candidates[j])) { + candidates[j] = nullptr; + } else if (is_more_specific(candidates[j], candidates[i])) { + candidates[i] = nullptr; + break; // this one is dead + } + } } } - if (candidate) { - best.push_back(candidate); + if (candidates[i]) { + pick = i; + ++remaining; } } - return best; + if (remaining <= 1 || !candidates[pick]->covariant_return_type) { + return; + } + + remaining = 0; + + for (size_t i = 0; i < candidates.size(); ++i) { + if (candidates[i]) { + for (size_t j = i + 1; j < candidates.size(); ++j) { + if (candidates[j]) { + BOOST_ASSERT(candidates[i] != candidates[j]); + + if (candidates[i]->covariant_return_type->is_base_of( + candidates[j]->covariant_return_type)) { + candidates[i] = nullptr; + } else if (candidates[j]->covariant_return_type->is_base_of( + candidates[i]->covariant_return_type)) { + candidates[j] = nullptr; + } + } + } + } + + if (candidates[i]) { + pick = i; + ++remaining; + } + } } template @@ -1199,7 +1291,7 @@ bool compiler::is_base(const overrider* a, const overrider* b) { } template -void compiler::print(const update_method_report& report) const { +void compiler::print(const method_report& report) const { ++trace; if (report.cells) { @@ -1211,7 +1303,7 @@ void compiler::print(const update_method_report& report) const { << " ambiguous\n"; } -template +template auto initialize() -> compiler { compiler compiler; compiler.initialize(); @@ -1219,10 +1311,6 @@ auto initialize() -> compiler { return compiler; } -inline auto initialize() -> compiler { - return initialize(); -} - } // namespace yomm2 } // namespace yorel diff --git a/include/yorel/yomm2/detail/types.hpp b/include/yorel/yomm2/detail/types.hpp index a43fb99e..940541d2 100644 --- a/include/yorel/yomm2/detail/types.hpp +++ b/include/yorel/yomm2/detail/types.hpp @@ -72,11 +72,12 @@ struct class_info : static_list::static_link { struct overrider_info; struct method_info : static_list::static_link { - type_id *vp_begin, *vp_end; + type_id* vp_begin; + type_id* vp_end; static_list specs; - void* ambiguous; void* not_implemented; type_id method_type; + type_id return_type; std::size_t* slots_strides_ptr; auto arity() const { @@ -90,6 +91,7 @@ struct overrider_info : static_list::static_link { } method_info* method; // for the destructor, to remove definition + type_id return_type; // for N2216 disambiguation type_id type; // of the function, for trace void** next; type_id *vp_begin, *vp_end; diff --git a/include/yorel/yomm2/generator.hpp b/include/yorel/yomm2/generator.hpp index 5baa1b4e..8a1d6f9e 100644 --- a/include/yorel/yomm2/generator.hpp +++ b/include/yorel/yomm2/generator.hpp @@ -267,8 +267,6 @@ uint16_t generator::encode_group( if (spec == &method->not_implemented) { return method->specs.size(); - } else if (spec == &method->ambiguous) { - return method->specs.size() + 1; } else { return entry.group_index; } diff --git a/include/yorel/yomm2/macros.hpp b/include/yorel/yomm2/macros.hpp index 4abaf07c..ea0c3848 100644 --- a/include/yorel/yomm2/macros.hpp +++ b/include/yorel/yomm2/macros.hpp @@ -59,7 +59,7 @@ YOREL_YOMM2_DETAIL_LOCATE_METHOD(NAME, ARGS); \ static auto fn ARGS->YOREL_YOMM2_DETAIL_RETURN_TYPE(__VA_ARGS__); \ static auto has_next() { \ - return method_type::next != nullptr; \ + return method_type::next != method_type::fn.not_implemented; \ } \ template \ static decltype(auto) next(Args&&... args) { \ @@ -69,8 +69,8 @@ }; \ INLINE YOMM2_REGISTER( \ OVERRIDERS:: \ - method_type::override::fn>); \ + method_type::override::fn>); \ INLINE auto \ OVERRIDERS::fn ARGS \ ->boost::mp11::mp_back> diff --git a/include/yorel/yomm2/policies/vectored_error_handler.hpp b/include/yorel/yomm2/policies/vectored_error_handler.hpp index 563cd835..fb679ec4 100644 --- a/include/yorel/yomm2/policies/vectored_error_handler.hpp +++ b/include/yorel/yomm2/policies/vectored_error_handler.hpp @@ -45,7 +45,7 @@ struct vectored_error_handler : virtual error_handler { if constexpr (Policy::template has_facet) { if (auto error = std::get_if(&error_v)) { const char* explanation[] = { - "no applicable definition", "ambiguous call"}; + "no applicable overrider", "ambiguous call"}; Policy::error_stream << explanation [error->status - resolution_error::no_definition] diff --git a/tests/test_blackbox.cpp b/tests/test_blackbox.cpp index a4716780..043f7fcc 100644 --- a/tests/test_blackbox.cpp +++ b/tests/test_blackbox.cpp @@ -19,7 +19,7 @@ using namespace yorel::yomm2; namespace states { -using test_policy = test_policy_<__COUNTER__>; +using policy = test_policy_<__COUNTER__>; using std::string; struct Animal { @@ -45,9 +45,9 @@ struct Cat : virtual Animal { } }; -YOMM2_CLASSES(Animal, Dog, Cat, test_policy); +YOMM2_CLASSES(Animal, Dog, Cat, policy); -YOMM2_METHOD(name, (virtual_), string, test_policy); +YOMM2_METHOD(name, (virtual_), string, policy); YOMM2_OVERRIDE(name, (const Cat& cat), string) { return "cat " + cat.name; @@ -58,7 +58,7 @@ YOMM2_OVERRIDE(name, (const Dog& dog), string) { } BOOST_AUTO_TEST_CASE(initializing) { - initialize(); + initialize(); const Animal& dog = Dog("spot"); BOOST_TEST("dog spot" == name(dog)); const Animal& cat = Cat("felix"); @@ -69,6 +69,8 @@ BOOST_AUTO_TEST_CASE(initializing) { namespace matrices { +using policy = test_policy_<__COUNTER__>; + struct matrix { virtual ~matrix() { } @@ -88,18 +90,19 @@ enum Subtype { DIAGONAL_DIAGONAL }; -YOMM2_CLASSES(matrix, dense_matrix, diagonal_matrix); +YOMM2_CLASSES(matrix, dense_matrix, diagonal_matrix, policy); YOMM2_METHOD( - times, (virtual_, virtual_), Subtype); -YOMM2_METHOD(times, (double, virtual_), Subtype); -YOMM2_METHOD(times, (virtual_, double), Subtype); + times, (virtual_, virtual_), Subtype, policy); +YOMM2_METHOD(times, (double, virtual_), Subtype, policy); +YOMM2_METHOD(times, (virtual_, double), Subtype, policy); YOMM2_OVERRIDE(times, (const matrix&, const matrix&), Subtype) { return MATRIX_MATRIX; } -YOMM2_OVERRIDE(times, (const diagonal_matrix&, const diagonal_matrix&), Subtype) { +YOMM2_OVERRIDE( + times, (const diagonal_matrix&, const diagonal_matrix&), Subtype) { return DIAGONAL_DIAGONAL; } @@ -119,9 +122,9 @@ YOMM2_OVERRIDE(times, (const matrix& m, double a), Subtype) { return MATRIX_SCALAR; } -YOMM2_METHOD(times, (virtual_, virtual_), int); -YOMM2_METHOD(times, (double, virtual_), int); -YOMM2_METHOD(times, (virtual_, double), int); +YOMM2_METHOD(times, (virtual_, virtual_), int, policy); +YOMM2_METHOD(times, (double, virtual_), int, policy); +YOMM2_METHOD(times, (virtual_, double), int, policy); YOMM2_OVERRIDE(times, (matrix&&, matrix&&), int) { return -MATRIX_MATRIX; @@ -147,20 +150,21 @@ YOMM2_OVERRIDE(times, (matrix && m, double a), int) { return -MATRIX_SCALAR; } -YOMM2_METHOD(zero_ref, (virtual_), Subtype); +YOMM2_METHOD(zero, (virtual_), Subtype, policy); -YOMM2_OVERRIDE(zero_ref, (dense_matrix & m), Subtype) { +YOMM2_OVERRIDE(zero, (dense_matrix & m), Subtype) { return MATRIX; } -YOMM2_OVERRIDE(zero_ref, (diagonal_matrix & m), Subtype) { +YOMM2_OVERRIDE(zero, (diagonal_matrix & m), Subtype) { return DIAGONAL; } BOOST_AUTO_TEST_CASE(simple) { - auto report = initialize(); + auto report = initialize(); { + // pass by const ref const matrix& dense = dense_matrix(); const matrix& diag = diagonal_matrix(); BOOST_TEST(times(dense, dense) == MATRIX_MATRIX); @@ -172,6 +176,7 @@ BOOST_AUTO_TEST_CASE(simple) { } { + // pass by xref BOOST_TEST(times(dense_matrix(), dense_matrix()) == -MATRIX_MATRIX); BOOST_TEST( times(diagonal_matrix(), diagonal_matrix()) == -DIAGONAL_DIAGONAL); @@ -182,15 +187,119 @@ BOOST_AUTO_TEST_CASE(simple) { } { + // pass by ref dense_matrix dense; - BOOST_TEST(zero_ref(dense) == MATRIX); + BOOST_TEST(zero(dense) == MATRIX); diagonal_matrix diagonal; - BOOST_TEST(zero_ref(diagonal) == DIAGONAL); + BOOST_TEST(zero(diagonal) == DIAGONAL); } } } // namespace matrices +namespace ambiguity { + +using policy = test_policy_<__COUNTER__>; + +struct matrix { + virtual ~matrix() { + } +}; + +struct dense_matrix : matrix {}; +struct diagonal_matrix : matrix {}; + +enum Subtype { NONE, MATRIX_MATRIX, MATRIX_DIAGONAL, DIAGONAL_MATRIX }; + +YOMM2_CLASSES(matrix, dense_matrix, diagonal_matrix, policy); + +YOMM2_METHOD( + times, (virtual_, virtual_), + std::pair, policy); + +YOMM2_OVERRIDE( + times, (const matrix&, const matrix&), std::pair) { + BOOST_TEST(!has_next()); + return std::pair(MATRIX_MATRIX, NONE); +} + +YOMM2_OVERRIDE( + times, (const matrix& a, const diagonal_matrix& b), + std::pair) { + BOOST_TEST(has_next()); + return std::pair(MATRIX_DIAGONAL, next(a, b).first); +} + +YOMM2_OVERRIDE( + times, (const diagonal_matrix& a, const matrix& b), + std::pair) { + BOOST_TEST(has_next()); + return std::pair(DIAGONAL_MATRIX, next(a, b).first); +} + +BOOST_AUTO_TEST_CASE(ambiguity) { + auto compiler = initialize(); + BOOST_TEST(compiler.report.ambiguous == 1); + + // N2216: in case of ambiguity, pick one. + diagonal_matrix diag1, diag2; + auto result1 = times(diag1, diag2); + BOOST_TEST(result1.first == DIAGONAL_MATRIX); + // Which overrider is picked is NOT documented! However, I know that it is + // the last in registration order. This is important for the test for + // ambiguity resolution using covariant return types. + BOOST_TEST(result1.second == MATRIX_MATRIX); + + // but always the same + auto result2 = times(diag1, diag2); + BOOST_TEST((result1 == result2)); +} + +} // namespace ambiguity +namespace covariant_return_type { + +using policy = test_policy_<__COUNTER__>; + +enum Subtype { MATRIX_MATRIX, MATRIX_DENSE, DENSE_MATRIX }; + +struct matrix { + virtual ~matrix() { + } + + Subtype type; +}; + +struct dense_matrix : matrix {}; + +YOMM2_CLASSES(matrix, dense_matrix, policy); + +YOMM2_METHOD( + times, (virtual_, virtual_), matrix*, policy); + +YOMM2_OVERRIDE(times, (const matrix&, const dense_matrix&), matrix*) { + auto result = new dense_matrix; + result->type = MATRIX_DENSE; + return result; +} + +YOMM2_OVERRIDE(times, (const dense_matrix&, const matrix&), dense_matrix*) { + auto result = new dense_matrix; + result->type = DENSE_MATRIX; + return result; +} + +BOOST_AUTO_TEST_CASE(covariant_return_type) { + auto compiler = initialize(); + BOOST_TEST(compiler.report.ambiguous == 0); + + // N2216: use covariant return types to resolve ambiguity. + dense_matrix left, right; + std::unique_ptr result(times(left, right)); + BOOST_TEST(result->type == DENSE_MATRIX); +} + +} // namespace covariant_return_type + namespace test_next_fn { struct Animal { @@ -270,26 +379,26 @@ void test_handler(const default_policy::error_variant& error_v) { namespace initialize_error_handling { -using test_policy = test_policy_<__COUNTER__>; +using policy = test_policy_<__COUNTER__>; struct base { virtual ~base() { } }; -YOMM2_METHOD(foo, (virtual_), void, test_policy); +YOMM2_METHOD(foo, (virtual_), void, policy); BOOST_AUTO_TEST_CASE(test_initialize_error_handling) { - auto prev_handler = test_policy::set_error_handler(errors::test_handler); + auto prev_handler = policy::set_error_handler(errors::test_handler); try { - initialize(); + initialize(); } catch (const unknown_class_error& error) { - test_policy::set_error_handler(prev_handler); + policy::set_error_handler(prev_handler); BOOST_TEST(error.type == reinterpret_cast(&typeid(base))); return; } catch (...) { - test_policy::set_error_handler(prev_handler); + policy::set_error_handler(prev_handler); BOOST_FAIL("unexpected exception"); } BOOST_FAIL("did not throw"); @@ -331,7 +440,7 @@ BOOST_AUTO_TEST_CASE(across_namespaces) { namespace report { -using test_policy = test_policy_<__COUNTER__>; +using policy = test_policy_<__COUNTER__>; struct Animal { virtual void foo() = 0; @@ -355,38 +464,38 @@ template void fn(Class&...) { } -YOMM2_CLASSES(Animal, Dog, Cat, test_policy); +YOMM2_CLASSES(Animal, Dog, Cat, policy); -BOOST_AUTO_TEST_CASE(update_report) { - using kick = method), void, test_policy>; - using pet = method), void, test_policy>; +BOOST_AUTO_TEST_CASE(initialize_report) { + using kick = method), void, policy>; + using pet = method), void, policy>; using meet = - method, virtual_), void, test_policy>; + method, virtual_), void, policy>; - auto report = initialize().report; + auto report = initialize().report; BOOST_TEST(report.not_implemented == 3); BOOST_TEST(report.ambiguous == 0); // 'meet' dispatch table is one cell, containing 'not_implemented' BOOST_TEST(report.cells == 1); YOMM2_REGISTER(kick::override>); - report = initialize().report; + report = initialize().report; BOOST_TEST(report.not_implemented == 2); YOMM2_REGISTER(pet::override>); YOMM2_REGISTER(pet::override>); - report = initialize().report; + report = initialize().report; BOOST_TEST(report.not_implemented == 2); // create ambiguity YOMM2_REGISTER(meet::override>); YOMM2_REGISTER(meet::override>); - report = initialize().report; + report = initialize().report; BOOST_TEST(report.cells == 4); BOOST_TEST(report.ambiguous == 1); YOMM2_REGISTER(meet::override>); - report = initialize().report; + report = initialize().report; BOOST_TEST(report.cells == 6); BOOST_TEST(report.ambiguous == 1); @@ -394,7 +503,7 @@ BOOST_AUTO_TEST_CASE(update_report) { YOMM2_REGISTER(meet::override>); YOMM2_REGISTER(meet::override>); YOMM2_REGISTER(meet::override>); - report = initialize().report; + report = initialize().report; BOOST_TEST(report.cells == 9); BOOST_TEST(report.ambiguous == 0); } @@ -403,22 +512,22 @@ BOOST_AUTO_TEST_CASE(update_report) { namespace test_comma_in_return_type { -using test_policy = test_policy_<__COUNTER__>; +using policy = test_policy_<__COUNTER__>; struct Test { virtual ~Test() {}; }; -YOMM2_CLASSES(Test, test_policy); +YOMM2_CLASSES(Test, policy); -YOMM2_METHOD(foo, (virtual_), std::pair, test_policy); +YOMM2_METHOD(foo, (virtual_), std::pair, policy); YOMM2_OVERRIDE(foo, (Test&), std::pair) { return {1, 2}; } BOOST_AUTO_TEST_CASE(comma_in_return_type) { - initialize(); + initialize(); Test test; diff --git a/tests/test_core.cpp b/tests/test_core.cpp index 3ae78e0a..9d6006dc 100644 --- a/tests/test_core.cpp +++ b/tests/test_core.cpp @@ -18,7 +18,7 @@ using namespace yorel::yomm2::detail; // clang-format off -namespace YOMM2_GENSYM { +namespace test_virtual { struct base { virtual ~base() {} @@ -31,6 +31,30 @@ struct d : base {}; struct e : base {}; struct f : base {}; +static_assert( + std::is_same_v< + virtual_traits::polymorphic_type, base>); + +static_assert( + std::is_same_v< + virtual_traits::polymorphic_type, base>); + +static_assert( + std::is_same_v< + virtual_traits::polymorphic_type, base>); + +static_assert( + std::is_same_v< + virtual_traits::polymorphic_type, base>); + +static_assert( + std::is_same_v< + virtual_traits::polymorphic_type, base>); + +static_assert( + std::is_same_v< + virtual_traits::polymorphic_type, void>); + static_assert( std::is_same_v< boost::mp11::mp_filter< diff --git a/tests/test_generator.cpp b/tests/test_generator.cpp index 88092418..9cce67a8 100644 --- a/tests/test_generator.cpp +++ b/tests/test_generator.cpp @@ -59,12 +59,6 @@ BOOST_AUTO_TEST_CASE(test_generator) { } catch (const resolution_error& e) { } - try { - meet(*cat, *dog, os); - BOOST_FAIL("should have thrown"); - } catch (const resolution_error& e) { - } - os.str(""); identify(*cat, os); BOOST_TEST(os.str() == "Alice's cat");