From 11fec25112a92f085c92f402acb8ab5e90250b8f Mon Sep 17 00:00:00 2001 From: leftibot Date: Sat, 11 Apr 2026 16:42:40 -0600 Subject: [PATCH] Fix #625: function_less_than comparator violates strict-weak ordering (#654) The function_less_than comparator used by std::stable_sort violated the strict-weak ordering requirement in two ways: (1) functions with different arities but matching overlapping parameters were treated as equivalent, breaking transitivity, and (2) the dynamic_object_type_name comparison silently fell through when one side had an empty type name. Fixed by ordering by arity when overlapping parameters match, and imposing a total order on dynamic object type names. Co-authored-by: leftibot Co-authored-by: Claude Opus 4.6 (1M context) Co-authored-by: Jason Turner --- .../chaiscript/dispatchkit/dispatchkit.hpp | 22 ++++++++++++++----- unittests/compiled_tests.cpp | 19 ++++++++++++++++ 2 files changed, 35 insertions(+), 6 deletions(-) diff --git a/include/chaiscript/dispatchkit/dispatchkit.hpp b/include/chaiscript/dispatchkit/dispatchkit.hpp index 95627dbc..be43905d 100644 --- a/include/chaiscript/dispatchkit/dispatchkit.hpp +++ b/include/chaiscript/dispatchkit/dispatchkit.hpp @@ -1087,13 +1087,21 @@ namespace chaiscript { // overridden methods in derived classes are tried first during dispatch const auto &lhs_dotn = lhs->dynamic_object_type_name(); const auto &rhs_dotn = rhs->dynamic_object_type_name(); - if (!lhs_dotn.empty() && !rhs_dotn.empty() && lhs_dotn != rhs_dotn) { - if (dispatch::Dynamic_Object::type_matches(lhs_dotn, rhs_dotn)) { - return true; // lhs is derived from rhs, so lhs is more specific + if (lhs_dotn != rhs_dotn) { + if (!lhs_dotn.empty() && !rhs_dotn.empty()) { + if (dispatch::Dynamic_Object::type_matches(lhs_dotn, rhs_dotn)) { + return true; // lhs is derived from rhs, so lhs is more specific + } + if (dispatch::Dynamic_Object::type_matches(rhs_dotn, lhs_dotn)) { + return false; // rhs is derived from lhs, so rhs is more specific + } } - if (dispatch::Dynamic_Object::type_matches(rhs_dotn, lhs_dotn)) { - return false; // rhs is derived from lhs, so rhs is more specific + // Impose a total order on type names to maintain strict-weak ordering: + // non-empty names sort before empty names, then lexicographically + if (lhs_dotn.empty() != rhs_dotn.empty()) { + return !lhs_dotn.empty(); } + return lhs_dotn < rhs_dotn; } const auto &lhsparamtypes = lhs->get_param_types(); @@ -1143,7 +1151,9 @@ namespace chaiscript { return lt < rt; } - return false; + // When all overlapping parameters match, order by arity to maintain + // strict-weak ordering (transitivity of equivalence) + return lhssize < rhssize; } /// Implementation detail for adding a function. diff --git a/unittests/compiled_tests.cpp b/unittests/compiled_tests.cpp index 5a3f09d1..c4094da1 100644 --- a/unittests/compiled_tests.cpp +++ b/unittests/compiled_tests.cpp @@ -1437,6 +1437,25 @@ TEST_CASE("Issue #421 - Switch with type_conversion does not compare destroyed o })") == 0); } +// Issue #625: function_less_than comparator must satisfy strict-weak ordering. +// Registering overloaded functions with different arities triggered a +// std::stable_sort assertion on macOS 15.2 (hardened libc++) because the +// comparator violated transitivity of equivalence. +TEST_CASE("Issue 625: function_less_than strict-weak ordering with different arities") { + chaiscript::ChaiScript_Basic chai(create_chaiscript_stdlib(), create_chaiscript_parser()); + + // Register overloaded functions with varying arities under the same name. + // If the comparator doesn't order by arity when overlapping params match, + // std::stable_sort may exhibit undefined behavior. + CHECK_NOTHROW(chai.add(chaiscript::fun([](int x, const chaiscript::Boxed_Value &) { return x; }), "overloaded")); + CHECK_NOTHROW(chai.add(chaiscript::fun([](int x, double y) { return x + static_cast(y); }), "overloaded")); + CHECK_NOTHROW(chai.add(chaiscript::fun([](int x) { return x; }), "overloaded")); + + // Verify dispatch still works correctly + CHECK(chai.eval("overloaded(5)") == 5); + CHECK(chai.eval("overloaded(3, 2.0)") == 5); +} + // Regression test: push_back() on script-created vector has no effect when // vector_conversion is in effect. The bug occurs because dispatch selects // the C++ push_back for the converted type over the built-in one, operating