From 660b27eedce36c6a0610bac5e791f41cc447cf7c Mon Sep 17 00:00:00 2001 From: Jean-Louis Leroy Date: Tue, 1 Oct 2024 08:42:33 -0400 Subject: [PATCH] N2216 ambiguity resolution --- examples/CMakeLists.txt | 2 +- include/yorel/yomm2/core.hpp | 48 +++++++------------------ include/yorel/yomm2/decode.hpp | 3 +- include/yorel/yomm2/detail/compiler.hpp | 29 ++++++++++----- include/yorel/yomm2/detail/types.hpp | 2 +- include/yorel/yomm2/macros.hpp | 6 ++-- tests/test_blackbox.cpp | 6 ++-- tests/test_generator.cpp | 6 ---- 8 files changed, 41 insertions(+), 61 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..ae0db7c1 100644 --- a/include/yorel/yomm2/core.hpp +++ b/include/yorel/yomm2/core.hpp @@ -463,7 +463,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 +608,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 +645,7 @@ class method : public detail::method_info { detail::types>>; }; - template + template struct override_impl { explicit override_impl(FunctionPointer* next = nullptr); }; @@ -656,19 +655,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: @@ -702,7 +699,6 @@ method::method() { 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(); Policy::methods.push_back(*this); } @@ -936,29 +932,6 @@ method::not_implemented_handler( 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)))); - 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 -} - // ----------------------------------------------------------------------------- // thunk @@ -977,16 +950,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 +969,7 @@ method::override_impl< } info.method = &fn; + info.return_type = Policy::template static_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 ed505aa6..671e061a 100644 --- a/include/yorel/yomm2/detail/compiler.hpp +++ b/include/yorel/yomm2/detail/compiler.hpp @@ -560,13 +560,14 @@ void compiler::augment_methods() { meth_iter->vp.push_back(class_); } - // initialize the function pointer in the dummy specs + // initialize the function pointer in the synthetic not_implemented + // overrider const auto method_index = meth_iter - methods.begin(); auto spec_size = meth_info.specs.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(); @@ -953,6 +954,7 @@ void compiler::build_dispatch_table( trace << "\n"; + // ------------------------------------------------------------- // next // First remove the dominant overriders from the applicable @@ -986,7 +988,8 @@ void compiler::build_dispatch_table( if (dominants.empty()) { ++trace << "not implemented\n"; - overrider->next = &m.not_implemented; + // do not increment m.not_implemented: 'next'is not + // necessarily called } else { auto next_overrider = dominants[0]; overrider->next = next_overrider; @@ -997,7 +1000,7 @@ void compiler::build_dispatch_table( if (dominants.size() > 1) { trace << " (ambiguous)"; - ++m.report.ambiguous; + // do not increment m.report.ambiguous, for same reason } trace << "\n"; @@ -1076,11 +1079,18 @@ void compiler::write_global_data() { ++trace << "method #" << " " << type_name(m.info->method_type) << "\n"; for (auto& overrider : m.specs) { - ++trace << "#" << overrider.spec_index << " " - << spec_name(m, &overrider) << " -> " - << "#" << overrider.next->spec_index << " " - << spec_name(m, overrider.next) << "\n"; - *overrider.info->next = (void*)overrider.next->pf; + if (overrider.next) { + ++trace << "#" << overrider.spec_index << " " + << spec_name(m, &overrider) << " -> "; + + trace << "#" << overrider.next->spec_index << " " + << spec_name(m, overrider.next); + *overrider.info->next = (void*)overrider.next->pf; + } else { + trace << "none"; + } + + trace << "\n"; } } @@ -1175,6 +1185,7 @@ bool compiler::is_more_specific( auto a_iter = a->vp.begin(), a_last = a->vp.end(), b_iter = b->vp.begin(); for (; a_iter != a_last; ++a_iter, ++b_iter) { + //BOOST_ASSERT(*a_iter != *b_iter); if (*a_iter != *b_iter) { if ((*b_iter)->covariant_classes.find(*a_iter) != (*b_iter)->covariant_classes.end()) { diff --git a/include/yorel/yomm2/detail/types.hpp b/include/yorel/yomm2/detail/types.hpp index a43fb99e..0475c57b 100644 --- a/include/yorel/yomm2/detail/types.hpp +++ b/include/yorel/yomm2/detail/types.hpp @@ -74,7 +74,6 @@ struct overrider_info; struct method_info : static_list::static_link { type_id *vp_begin, *vp_end; static_list specs; - void* ambiguous; void* not_implemented; type_id method_type; std::size_t* slots_strides_ptr; @@ -90,6 +89,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/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/tests/test_blackbox.cpp b/tests/test_blackbox.cpp index 36eae29f..2cf75ee3 100644 --- a/tests/test_blackbox.cpp +++ b/tests/test_blackbox.cpp @@ -219,21 +219,21 @@ YOMM2_METHOD( YOMM2_OVERRIDE( times, (const matrix&, const matrix&), std::pair) { - auto next_ptr = method_type::next; + BOOST_TEST(!has_next()); return std::pair(MATRIX_MATRIX, NONE); } YOMM2_OVERRIDE( times, (const matrix& a, const diagonal_matrix& b), std::pair) { - auto next_ptr = method_type::next; + 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) { - auto next_ptr = method_type::next; + BOOST_TEST(has_next()); return std::pair(DIAGONAL_MATRIX, next(a, b).first); } 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");