[m-rev.] for review: improve readability of coerce code

Peter Wang novalazy at gmail.com
Mon Jul 14 12:48:36 AEST 2025


On Sat, 12 Jul 2025 14:45:10 +0200 "Zoltan Somogyi" <zoltan.somogyi at runbox.com> wrote:
> Improve readability of coerce checking code.
> 
> compiler/typecheck_coerce.m:
>     Give some predicates, types and variables more descriptive,
>     and in some cases more consistent, names.
> 
>     Simplify some code.
> 
>     Add some comments.
> 
> compiler/modecheck_coerce.m:
>     Give the names of the predicates that this module
>     copies-with-modification from typecheck_coerce.m an _mc suffix.
> 
>     Add some comments.
> 
> compiler/type_util.m:
>     Use more consistent variable names.

> diff --git a/compiler/modecheck_coerce.m b/compiler/modecheck_coerce.m
> index c87c478d9..4b4e7089f 100644
> --- a/compiler/modecheck_coerce.m
> +++ b/compiler/modecheck_coerce.m
> @@ -1019,36 +1019,54 @@ copy_inst_for_coerce_result(LiveX, InstX, InstY) :-
...
> +    % In typecheck_coerce.m, the reason for allowing the Comparison parameter
> +    % to be compare_equal is that types_compare_as_given (no _mc) can modify
> +    % the type assignment to enforce equality. The code here does not do that,
> +    % which means that calling types_compare_as_given_mc with compare_equal
> +    % is equivalent to invoking "TypeA = TypeB".
> +    %
> +    % The only advantage of handling compare_equal is reducing the textual
> +    % difference between the code in typecheck_coerce.m andthe code here.

and the

> +    % However, this is enough, because even though "TypeA = TypeB" would
> +    % be faster, this does not matter, due to the rarity of coercion.
> +    %
> +    % XXX Peter, is the above correct?
> +    %

It took a while to understand what the comment refers to.
Yes, calling types_compare_as_given_mc with compare_equal is (or should be)
the same as comparing two higher-order types using builtin unification.
Performance is not a concern here, and I prefer to similarity to the
same code in typecheck_coerce.m.

>      ;
> -        CtorArgType = higher_order_type(_PredOrFunc, ArgTypes, _HOInstInfo,
> -            _Purity),
> +        RhsType = higher_order_type(_PoF, ArgTypes, _HOInstInfo, _Purity),
> +        % We do not support any subtyping of higher order types.
> +        % Since the higher order components on the right-hand side of a
> +        % type definition must be identical in the from-type and the to-type,
> +        % all type paramaters that occur in the such higher order types
> +        % must be bound to the exact same value in the from-type and to-type.

parameters

I think "Therefore" would be better than "Since" as it's a consequence
of the preceding statement.

>  %---------------------%
>  
> -:- pred check_coerce_type_params(type_table::in, tvarset::in,
> -    invariant_set::in, list(tvar)::in, list(mer_type)::in, list(mer_type)::in,
> +    % are_type_params_as_related_as_needed(TypeTable, TVarSet, InvariantTVars,
> +    %   TypeParams, FromArgTypes, ToArgTypes, !TypeAssign):
> +    %
> +    % FromArgTypes and ToArgTypes are the actual types bound to TypeParams
> +    % in the from-type and to-type of the coercion respectively.
> +    % If a given type parameter is in InvariantTVars, then the types bound
> +    % to that parameter in the from-type and to-type must be identical,
> +    % while for the type parameters that are not in InvariantTVars,
> +    % it is enough that one is a subtypp of the other (in either direction).

subtype

> +    %
> +    % If e.g. neither the first nor second TypeParam is in InvariantTVars,
> +    % we can succeed if the first FromArgType is a subtype of the first
> +    % ToArgType, but the second ToArgType is a subtype of the second
> +    % FromArgType. The direction of which is the subrtype of the other

subtype

> +    % does NOT need to be consistent. This allows us to support coercion
> +    % from any subtype of the base type to any other of its subtypes;
> +    % the from-type and the to-type do not need to be in a subtype-supertype
> +    % relationship.
> +    %
> +    % XXX Peter, is the above correct?
> +    %

Correct.

That looks fine to me.

Peter


More information about the reviews mailing list