N2216 ambiguity resolution

jll63 · Oct 1, 2024 · a347c9a · a347c9a
1 parent cb5aef9
commit a347c9a
Show file tree

Hide file tree

Showing 8 changed files with 40 additions and 61 deletions.
diff --git 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
@@ -463,7 +463,8 @@ class virtual_ptr {
                 virtual_ptr_traits<Class, Policy>::template cast<Other>(obj);
         } else {
             result.obj =
-                &detail::optimal_cast<Policy, typename Other::element_type&>(*obj);
+                &detail::optimal_cast<Policy, typename Other::element_type&>(
+                    *obj);
         }
 
         return result;
@@ -607,8 +608,6 @@ class method<Name(Parameters...), Return, Policy> : public detail::method_info {
     static BOOST_NORETURN auto
     not_implemented_handler(detail::remove_virtual<Parameters>... args)
         -> Return;
-    static BOOST_NORETURN auto
-    ambiguous_handler(detail::remove_virtual<Parameters>... args) -> Return;
 
     template<auto, typename>
     struct thunk;
@@ -646,7 +645,7 @@ class method<Name(Parameters...), Return, Policy> : public detail::method_info {
                 detail::types<OverriderParameters...>>>;
     };
 
-    template<auto Function>
+    template<auto Function, typename FnReturnType>
     struct override_impl {
         explicit override_impl(FunctionPointer* next = nullptr);
     };
@@ -656,19 +655,17 @@ class method<Name(Parameters...), Return, Policy> : public detail::method_info {
 
     template<auto Function, typename FnReturnType, typename... FnParameters>
     struct override_aux<Function, FnReturnType (*)(FnParameters...)>
-        : override_impl<Function> {
-    };
+        : override_impl<Function, FnReturnType> {};
 
     template<
         auto Function, class FnClass, typename FnReturnType,
         typename... FnParameters>
-    struct override_aux<
-        Function, FnReturnType (FnClass::*)(FnParameters...)> {
+    struct override_aux<Function, FnReturnType (FnClass::*)(FnParameters...)> {
         static auto fn(FnClass* this_, FnParameters&&... args) -> FnReturnType {
             return (this_->*Function)(std::forward<FnParameters>(args)...);
         }
 
-        override_impl<fn> impl{&next<Function>};
+        override_impl<fn, FnReturnType> impl{&next<Function>};
     };
 
   public:
@@ -702,7 +699,6 @@ method<Name(Parameters...), Return, Policy>::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<method>();
     Policy::methods.push_back(*this);
 }
@@ -936,29 +932,6 @@ method<Name(Parameters...), Return, Policy>::not_implemented_handler(
     abort(); // in case user handler "forgets" to abort
 }
 
-template<typename Name, typename Return, typename... Parameters, class Policy>
-BOOST_NORETURN auto
-method<Name(Parameters...), Return, Policy>::ambiguous_handler(
-    detail::remove_virtual<Parameters>... args) -> Return {
-    if constexpr (Policy::template has_facet<policies::error_handler>) {
-        resolution_error error;
-        error.status = resolution_error::ambiguous;
-        error.method = Policy::template static_type<method>();
-        error.arity = Arity;
-        type_id types[sizeof...(args)];
-        auto ti_iter = types;
-        (...,
-         (*ti_iter++ = Policy::dynamic_type(
-              detail::argument_traits<Policy, Parameters>::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,23 +950,26 @@ auto method<Name(Parameters...), Return, Policy>::
 // overriders
 
 template<typename Name, typename Return, typename... Parameters, class Policy>
-template<auto Function>
+template<auto Function, typename FnReturnType>
 method<Name(Parameters...), Return, Policy>::override_impl<
-    Function>::override_impl(FunctionPointer* p_next) {
+    Function, FnReturnType>::override_impl(FunctionPointer* p_next) {
+    using namespace detail;
+
     // Work around MSVC bug: using &next<Function> as a default value
     // for 'next' confuses it about Parameters not being expanded.
     if (!p_next) {
         p_next = &next<Function>;
     }
 
-    static detail::overrider_info info;
+    static overrider_info info;
 
     if (info.method) {
         BOOST_ASSERT(info.method == &fn);
         return;
     }
 
     info.method = &fn;
+    info.return_type = Policy::template static_type<FnReturnType>();
     info.type = Policy::template static_type<decltype(Function)>();
     info.next = reinterpret_cast<void**>(p_next);
     using Thunk = thunk<Function, decltype(Function)>;

diff --git 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
@@ -560,13 +560,14 @@ void compiler<Policy>::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<uintptr_t>(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<Policy>::build_dispatch_table(
 
                 trace << "\n";
 
+                // -------------------------------------------------------------
                 // next
 
                 // First remove the dominant overriders from the applicable
@@ -986,7 +988,8 @@ void compiler<Policy>::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<Policy>::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<Policy>::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";
         }
     }
 

diff --git 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<method_info>::static_link {
     type_id *vp_begin, *vp_end;
     static_list<overrider_info> 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<overrider_info>::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
@@ -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<fn> != nullptr;                           \
+            return method_type::next<fn> != method_type::fn.not_implemented;   \
         }                                                                      \
         template<typename... Args>                                             \
         static decltype(auto) next(Args&&... args) {                           \
@@ -69,8 +69,8 @@
     };                                                                         \
     INLINE YOMM2_REGISTER(                                                     \
         OVERRIDERS<YOREL_YOMM2_DETAIL_RETURN_TYPE(__VA_ARGS__) ARGS>::         \
-            method_type::override<OVERRIDERS<                               \
-                YOREL_YOMM2_DETAIL_RETURN_TYPE(__VA_ARGS__) ARGS>::fn>);       \
+            method_type::override<OVERRIDERS<YOREL_YOMM2_DETAIL_RETURN_TYPE(   \
+                __VA_ARGS__) ARGS>::fn>);                                      \
     INLINE auto                                                                \
         OVERRIDERS<YOREL_YOMM2_DETAIL_RETURN_TYPE(__VA_ARGS__) ARGS>::fn ARGS  \
             ->boost::mp11::mp_back<boost::mp11::mp_list<__VA_ARGS__>>

diff --git 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<Subtype, Subtype>) {
-    auto next_ptr = method_type::next<fn>;
+    BOOST_TEST(!has_next());
     return std::pair(MATRIX_MATRIX, NONE);
 }
 
 YOMM2_OVERRIDE(
     times, (const matrix& a, const diagonal_matrix& b),
     std::pair<Subtype, Subtype>) {
-    auto next_ptr = method_type::next<fn>;
+    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<Subtype, Subtype>) {
-    auto next_ptr = method_type::next<fn>;
+    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
@@ -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");