[m-rev.] for review: Fix bug with typechecking of coercions.

[Julien Fischer]

On Tue, 2 Jul 2024 at 14:51, Zoltan Somogyi <zoltan.somogyi at runbox.com> wrote:

> > --- a/compiler/typecheck_clauses.m
> > +++ b/compiler/typecheck_clauses.m
>
> > @@ -2503,8 +2502,8 @@ compare_types_corresponding(TypeTable, TVarSet, Comparison,
> >
> >  %---------------------------------------------------------------------------%
> >
> > -    % Remove satisfied coerce constraints from each type assignment,
> > -    % then drop any type assignments with unsatisfied coerce constraints
> > +    % Check coerce constraints in each type assignment to see if they can be
> > +    % satisfied. Drop any type assignments with unsatisfied coerce constraints
> >      % if there is at least one type assignment that does satisfy coerce
> >      % constraints.
> >      %
>
> "that DOES satisfy" or "that CAN satisfy"? Either way, that comment should
> explain *why* you answer that question the way you do.
>
> > -check_pending_coerce_constraints(_TypeTable, [], [], !TypeAssign).
> > -check_pending_coerce_constraints(TypeTable, [Coercion0 | Coercions0],
> > -        KeepCoercions, !TypeAssign) :-
> > +check_pending_coerce_constraints_to_fixpoint(TypeTable, Coercions0, Coercions,
> > +        !TypeAssign) :-
> > +    check_pending_coerce_constraints_loop(TypeTable, Coercions0,
> > +        KeepCoercions, DelayedCoercions, !TypeAssign),
> > +    ( if
> > +        KeepCoercions = [],
> > +        list.length(Coercions0) = list.length(DelayedCoercions) : int
>
> I would call length/2 twice and unify the vars containing the lengths;
> it is longer, but clearer. Especially if you take the length of Coercions0
> *before* the call to the loop.

Or just call list.same_length/2 (which makes the intent even clearer).

Julien.