Address review: use implicit derived-to-base matching instead of copying base class functions

Instead of copying all base class methods/attributes into derived classes,
make the type matching system recognize inheritance relationships. Base class
methods now naturally match derived objects through dynamic_object_typename_match,
and dispatch ordering ensures derived overrides are preferred over base methods.

This is simpler (net -25 lines) and avoids duplicating function registrations.

Requested by @lefticus in PR #641 review.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
leftibot 2026-04-10 11:55:39 -06:00
parent 1334e02900
commit 15f04d4fad
4 changed files with 25 additions and 51 deletions

View File

@ -1060,6 +1060,19 @@ namespace chaiscript {
return true;
}
// Sort more-derived Dynamic_Object types before base types so that
// 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 (dispatch::Dynamic_Object::type_matches(rhs_dotn, lhs_dotn)) {
return false; // rhs is derived from lhs, so rhs is more specific
}
}
const auto &lhsparamtypes = lhs->get_param_types();
const auto &rhsparamtypes = rhs->get_param_types();

View File

@ -72,7 +72,7 @@ namespace chaiscript {
bool is_attribute_function() const noexcept override { return m_is_attribute; }
const std::string &get_dynamic_object_type_name() const noexcept { return m_type_name; }
const std::string &dynamic_object_type_name() const noexcept override { return m_type_name; }
bool call_match(const chaiscript::Function_Params &vals, const Type_Conversions_State &t_conversions) const noexcept override {
if (dynamic_object_typename_match(vals, m_type_name, m_ti, t_conversions)) {
@ -114,7 +114,7 @@ namespace chaiscript {
if (bv.get_type_info().bare_equal(m_doti)) {
try {
const Dynamic_Object &d = boxed_cast<const Dynamic_Object &>(bv, &t_conversions);
return name == "Dynamic_Object" || d.get_type_name() == name;
return name == "Dynamic_Object" || Dynamic_Object::type_matches(d.get_type_name(), name);
} catch (const std::bad_cast &) {
return false;
}

View File

@ -233,6 +233,12 @@ namespace chaiscript {
}
}
/// Returns the Dynamic_Object type name this function is bound to, or empty string if not a Dynamic_Object function
virtual const std::string &dynamic_object_type_name() const noexcept {
static const std::string empty;
return empty;
}
virtual bool compare_first_type(const Boxed_Value &bv, const Type_Conversions_State &t_conversions) const noexcept {
/// TODO is m_types guaranteed to be at least 2??
return compare_type_to_param(m_types[1], bv, t_conversions);

View File

@ -16,7 +16,6 @@
#include <map>
#include <memory>
#include <ostream>
#include <set>
#include <stdexcept>
#include <string>
#include <vector>
@ -837,61 +836,17 @@ namespace chaiscript {
const bool has_base_class = (this->children.size() == 3);
const auto &block = has_base_class ? this->children[2] : this->children[1];
// Evaluate the class block first (registers derived-specific methods/attrs)
block->eval(t_ss);
// Register inheritance before evaluating the class body so that
// function dispatch ordering can account for the relationship
if (has_base_class) {
const auto &base_name = this->children[1]->text;
dispatch::Dynamic_Object::register_inheritance(class_name, base_name);
copy_base_functions(*t_ss, base_name, class_name);
}
block->eval(t_ss);
return void_var();
}
private:
/// Collect the set of method/attr names already defined for a given dynamic object type
static std::set<std::string> get_defined_method_names(chaiscript::detail::Dispatch_Engine &t_engine,
const std::string &type_name) {
std::set<std::string> names;
for (const auto &[func_name, func] : t_engine.get_functions()) {
const auto *dof = dynamic_cast<const dispatch::detail::Dynamic_Object_Function *>(func.get());
if (dof && dof->get_dynamic_object_type_name() == type_name) {
names.insert(func_name);
}
}
return names;
}
/// Copy base class methods and attributes to derived class, skipping overrides
static void copy_base_functions(chaiscript::detail::Dispatch_Engine &t_engine,
const std::string &base_name,
const std::string &derived_name) {
const auto derived_methods = get_defined_method_names(t_engine, derived_name);
const auto functions = t_engine.get_functions();
for (const auto &[func_name, func] : functions) {
const auto *dof = dynamic_cast<const dispatch::detail::Dynamic_Object_Function *>(func.get());
if (dof && dof->get_dynamic_object_type_name() == base_name) {
// Skip if derived class already defines this method/attr
if (derived_methods.count(func_name) > 0) {
continue;
}
auto contained = dof->get_contained_functions();
if (!contained.empty()) {
try {
t_engine.add(std::make_shared<dispatch::detail::Dynamic_Object_Function>(
derived_name,
std::const_pointer_cast<dispatch::Proxy_Function_Base>(contained[0]),
dof->is_attribute_function()),
func_name);
} catch (const chaiscript::exception::name_conflict_error &) {
// already registered, skip
}
}
}
}
}
};
template<typename T>