mirror of
https://github.com/ChaiScript/ChaiScript.git
synced 2026-04-30 19:09:26 +08:00
Fix #514: Avoid exception-based control flow for arithmetic type dispatch
During overloaded function dispatch, when a function's parameter mismatches are all arithmetic-to-arithmetic (e.g., int where float is expected), the dispatcher now performs inline Boxed_Number conversion before calling the function. Previously, the function was called with mismatched types, threw bad_boxed_cast, and fell through to dispatch_with_conversions — a slow path on platforms like MSVC where exceptions are expensive under the debugger. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
d4c5bdb3e4
commit
f1411841a3
@ -733,20 +733,18 @@ namespace chaiscript {
|
||||
InItr matching_func(end);
|
||||
|
||||
while (begin != end) {
|
||||
if (types_match_except_for_arithmetic(begin->second, plist, t_conversions)) {
|
||||
if (types_match_except_for_arithmetic(begin->func, plist, t_conversions)) {
|
||||
if (matching_func == end) {
|
||||
matching_func = begin;
|
||||
} else {
|
||||
// handle const members vs non-const member, which is not really ambiguous
|
||||
const auto &mat_fun_param_types = matching_func->second->get_param_types();
|
||||
const auto &next_fun_param_types = begin->second->get_param_types();
|
||||
const auto &mat_fun_param_types = matching_func->func->get_param_types();
|
||||
const auto &next_fun_param_types = begin->func->get_param_types();
|
||||
|
||||
if (plist[0].is_const() && !mat_fun_param_types[1].is_const() && next_fun_param_types[1].is_const()) {
|
||||
matching_func = begin; // keep the new one, the const/non-const matchup is correct
|
||||
matching_func = begin;
|
||||
} else if (!plist[0].is_const() && !mat_fun_param_types[1].is_const() && next_fun_param_types[1].is_const()) {
|
||||
// keep the old one, it has a better const/non-const matchup
|
||||
} else {
|
||||
// ambiguous function call
|
||||
throw exception::dispatch_error(plist, std::vector<Const_Proxy_Function>(t_funcs.begin(), t_funcs.end()));
|
||||
}
|
||||
}
|
||||
@ -756,14 +754,13 @@ namespace chaiscript {
|
||||
}
|
||||
|
||||
if (matching_func == end) {
|
||||
// no appropriate function to attempt arithmetic type conversion on
|
||||
throw exception::dispatch_error(plist, std::vector<Const_Proxy_Function>(t_funcs.begin(), t_funcs.end()));
|
||||
}
|
||||
|
||||
std::vector<Boxed_Value> newplist;
|
||||
newplist.reserve(plist.size());
|
||||
|
||||
const std::vector<Type_Info> &tis = matching_func->second->get_param_types();
|
||||
const std::vector<Type_Info> &tis = matching_func->func->get_param_types();
|
||||
std::transform(tis.begin() + 1, tis.end(), plist.begin(), std::back_inserter(newplist), [](const Type_Info &ti, const Boxed_Value ¶m) -> Boxed_Value {
|
||||
if (ti.is_arithmetic() && param.get_type_info().is_arithmetic() && param.get_type_info() != ti) {
|
||||
return Boxed_Number(param).get_as(ti).bv;
|
||||
@ -773,13 +770,10 @@ namespace chaiscript {
|
||||
});
|
||||
|
||||
try {
|
||||
return (*(matching_func->second))(chaiscript::Function_Params{newplist}, t_conversions);
|
||||
return (*(matching_func->func))(chaiscript::Function_Params{newplist}, t_conversions);
|
||||
} catch (const exception::bad_boxed_cast &) {
|
||||
// parameter failed to cast
|
||||
} catch (const exception::arity_error &) {
|
||||
// invalid num params
|
||||
} catch (const exception::guard_error &) {
|
||||
// guard failed to allow the function to execute
|
||||
}
|
||||
|
||||
throw exception::dispatch_error(plist, std::vector<Const_Proxy_Function>(t_funcs.begin(), t_funcs.end()));
|
||||
@ -791,22 +785,34 @@ namespace chaiscript {
|
||||
/// function is found or throw dispatch_error if no matching function is found
|
||||
template<typename Funcs>
|
||||
Boxed_Value dispatch(const Funcs &funcs, const Function_Params &plist, const Type_Conversions_State &t_conversions) {
|
||||
std::vector<std::pair<size_t, const Proxy_Function_Base *>> ordered_funcs;
|
||||
struct Dispatch_Candidate {
|
||||
size_t priority;
|
||||
const Proxy_Function_Base *func;
|
||||
bool arithmetic_only_diffs;
|
||||
};
|
||||
|
||||
std::vector<Dispatch_Candidate> ordered_funcs;
|
||||
ordered_funcs.reserve(funcs.size());
|
||||
|
||||
for (const auto &func : funcs) {
|
||||
const auto arity = func->get_arity();
|
||||
|
||||
if (arity == -1) {
|
||||
ordered_funcs.emplace_back(plist.size(), func.get());
|
||||
ordered_funcs.push_back({plist.size(), func.get(), false});
|
||||
} else if (arity == static_cast<int>(plist.size())) {
|
||||
size_t numdiffs = 0;
|
||||
bool all_arithmetic = true;
|
||||
for (size_t i = 0; i < plist.size(); ++i) {
|
||||
if (!func->get_param_types()[i + 1].bare_equal(plist[i].get_type_info())) {
|
||||
++numdiffs;
|
||||
if (!func->get_param_types()[i + 1].is_arithmetic() || !plist[i].get_type_info().is_arithmetic()) {
|
||||
all_arithmetic = false;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
const bool needs_arithmetic_conversion = numdiffs > 0 && all_arithmetic;
|
||||
|
||||
// Deprioritize functions whose first parameter (object/receiver) requires
|
||||
// type conversion: conversions create temporaries, so mutations on the
|
||||
// converted object are silently lost.
|
||||
@ -838,23 +844,38 @@ namespace chaiscript {
|
||||
}
|
||||
}
|
||||
|
||||
ordered_funcs.emplace_back(numdiffs, func.get());
|
||||
ordered_funcs.push_back({numdiffs, func.get(), needs_arithmetic_conversion});
|
||||
}
|
||||
}
|
||||
|
||||
for (size_t i = 0; i <= plist.size(); ++i) {
|
||||
for (const auto &func : ordered_funcs) {
|
||||
try {
|
||||
if (func.first == i && (i == 0 || func.second->filter(plist, t_conversions))) {
|
||||
return (*(func.second))(plist, t_conversions);
|
||||
for (const auto &candidate : ordered_funcs) {
|
||||
if (candidate.priority != i) {
|
||||
continue;
|
||||
}
|
||||
|
||||
if (i == 0 || candidate.func->filter(plist, t_conversions)) {
|
||||
try {
|
||||
if (candidate.arithmetic_only_diffs) {
|
||||
const auto &tis = candidate.func->get_param_types();
|
||||
std::vector<Boxed_Value> converted;
|
||||
converted.reserve(plist.size());
|
||||
for (size_t j = 0; j < plist.size(); ++j) {
|
||||
if (tis[j + 1].is_arithmetic() && plist[j].get_type_info().is_arithmetic()
|
||||
&& plist[j].get_type_info() != tis[j + 1]) {
|
||||
converted.push_back(Boxed_Number(plist[j]).get_as(tis[j + 1]).bv);
|
||||
} else {
|
||||
converted.push_back(plist[j]);
|
||||
}
|
||||
}
|
||||
return (*(candidate.func))(Function_Params{converted}, t_conversions);
|
||||
} else {
|
||||
return (*(candidate.func))(plist, t_conversions);
|
||||
}
|
||||
} catch (const exception::bad_boxed_cast &) {
|
||||
} catch (const exception::arity_error &) {
|
||||
} catch (const exception::guard_error &) {
|
||||
}
|
||||
} catch (const exception::bad_boxed_cast &) {
|
||||
// parameter failed to cast, try again
|
||||
} catch (const exception::arity_error &) {
|
||||
// invalid num params, try again
|
||||
} catch (const exception::guard_error &) {
|
||||
// guard failed to allow the function to execute,
|
||||
// try again
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@ -1970,3 +1970,50 @@ TEST_CASE("Exception from C++ [] operator is catchable in ChaiScript") {
|
||||
caught
|
||||
)") == true);
|
||||
}
|
||||
|
||||
// Issue #514: overloaded function dispatch with arithmetic type mismatch
|
||||
// should not be drastically slower than exact-match dispatch
|
||||
TEST_CASE("Overloaded dispatch with arithmetic type mismatch is not drastically slow") {
|
||||
chaiscript::ChaiScript_Basic chai(create_chaiscript_stdlib(), create_chaiscript_parser());
|
||||
|
||||
int func1_called = 0;
|
||||
int func2_called = 0;
|
||||
|
||||
chai.add(chaiscript::fun([&func1_called](const std::string &, float, float) { ++func1_called; }), "func");
|
||||
chai.add(chaiscript::fun([&func2_called](float, float, float) { ++func2_called; }), "func");
|
||||
|
||||
// Mismatched type: last arg is int (0) instead of float (0.0f)
|
||||
// This should still dispatch correctly via arithmetic conversion
|
||||
CHECK_NOTHROW(chai.eval("func(0.0f, 0.0f, 0)"));
|
||||
CHECK(func2_called == 1);
|
||||
CHECK(func1_called == 0);
|
||||
|
||||
// Exact match: all args are float
|
||||
CHECK_NOTHROW(chai.eval("func(0.0f, 0.0f, 0.0f)"));
|
||||
CHECK(func2_called == 2);
|
||||
|
||||
// Performance: mismatched dispatch should complete in reasonable time
|
||||
// relative to exact-match dispatch (not 50x slower)
|
||||
const int iterations = 100;
|
||||
|
||||
const auto mismatch_start = std::chrono::steady_clock::now();
|
||||
for (int i = 0; i < iterations; ++i) {
|
||||
chai.eval("func(0.0f, 0.0f, 0)");
|
||||
}
|
||||
const auto mismatch_elapsed = std::chrono::steady_clock::now() - mismatch_start;
|
||||
|
||||
const auto exact_start = std::chrono::steady_clock::now();
|
||||
for (int i = 0; i < iterations; ++i) {
|
||||
chai.eval("func(0.0f, 0.0f, 0.0f)");
|
||||
}
|
||||
const auto exact_elapsed = std::chrono::steady_clock::now() - exact_start;
|
||||
|
||||
const auto mismatch_us = std::chrono::duration_cast<std::chrono::microseconds>(mismatch_elapsed).count();
|
||||
const auto exact_us = std::chrono::duration_cast<std::chrono::microseconds>(exact_elapsed).count();
|
||||
|
||||
// Mismatched dispatch should be no more than 5x slower than exact match.
|
||||
// Before the fix, it was 50-100x slower due to exception-based control flow.
|
||||
if (exact_us > 0) {
|
||||
CHECK(mismatch_us < exact_us * 5);
|
||||
}
|
||||
}
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user