source-academy / sicp

XML sources of SICP and SICP JS, and support for generating Interactive SICP JS, PDF, e-book and comparison editions
https://sourceacademy.org/sicpjs
Creative Commons Attribution Share Alike 4.0 International
891 stars 115 forks source link

Possible problem with solution to Exercise 2.82 #833

Open tangjm opened 1 year ago

tangjm commented 1 year ago

The implementation of the solution doesn't terminate in the case when we try to call a generic operator that is undefined with respect to arguments of the same type.

Consider this example.

// Suppose the operator "some_op" is not defined for a triple of rationals <x, y, z>. 
// This is to say that get("some_op", list("rational", "rational", "rational")) returns undefined.
// If so, it will mean that functions like the following won't terminate.
function some_op(x, y, z) {
    return apply_generic("some_op", list(x, y, z));
}

I think we can make a slight modification to handle this.

// Current implementation
function can_coerce_to(type_tags, target_type) {
    return accumulate((type_tag, result) =>
                        result &&
                        (type_tag === target_type ||
                         ! is_undefined(get_coercion(type_tag, target_type)),
                      true,
                      type_tags);
}

// This implementation would handle the counterexample 
// and ensure the program terminates with an error
function can_coerce_to(type_tags, target_type) {
    return accumulate((type_tag, result) => 
                        result && type_tag === target_type,
                      true,
                      type_tags)
           ? false 
           : accumulate((type_tag, result) =>
                          result &&
                          (type_tag === target_type ||
                           ! is_undefined(get_coercion(type_tag, target_type)),
                        true,
                        type_tags);
}

The nub of the issue is that we need to account for when the types are the same but there is no method for them. In terms of the apply_generic function presented on p.171 of sicpjs (see below), I think the solution just needs to handle the part I have commented on below.

function apply_generic(op, args) {
    const type_tags = map(type_tag, args);
    const fun = get(op, type_tags);
    if (!is_undefined(fun)) {
        return apply(fun, map(contents, args));
    } else {
        if (length(args) === 2) {
            const type1 = head(type_tags);
            const type2 = head(tail(type_tags));
            const a1 = head(args);
            const a2 = head(tail(args));
            const t1_to_t2 = get_coercion(type1, type2);
            const t2_to_t1 = get_coercion(type2, type1);
            return !is_undefined(t1_to_t2) ?
                apply_generic(op, list(t1_to_t2(a1), a2)) :
                !is_undefined(t2_to_t1) ?
                apply_generic(op, list(a1, t2_to_t1(a2))) :
               //  currently unhandled case
                error(list(op, type_tags),
                    "no method for these types");
        } else {
            return error(list(op, type_tags),
                "no method for these types");
        }
    }
}
martin-henz commented 4 weeks ago

I'm not confident that this works as expected.

function can_coerce_to(type_tags, target_type) {
    return accumulate((type_tag, result) => 
                        result && type_tag === target_type,
                      true,
                      type_tags)
           ? false 

should this not be

...     ? true

?