[m-rev.] for review: compare du types separately

Peter Wang novalazy at gmail.com
Mon Jul 27 15:41:24 AEST 2026


On Mon, 27 Jul 2026 00:34:29 +0200 "Zoltan Somogyi" <zoltan.somogyi at runbox.com> wrote:
> For review by anyone.
> 
> I would like to build on this, so in the absence of a review by then,
> I intend to commit this in about 14-18 hours, and deal with any
> review post-commit.
> 
> Zoltan.

> Compare du types separately from other defined types.
> 
> compiler/typecheck_coerce.m:
>     As above.
> 
>     Document where comparing the argument types of non-du defined
>     types definitely makes sense, and where it is dubious.
> 
>     We test whether the values of type_ctor parameters that do not
>     have to be invariant are coercable in either direction. We used
>     to report the coerce_fails from only one of those directions.
>     This was ok, because all the coerce_fails we reported were symmetric.
> 
>     The new coerce_fail is not symmetric. This means that the diagnostics
>     from both directions may be useful to the programmer. Ideally,
>     we would prefer to report just the coerce_fail that corresponds
>     to the programmer's intended use of the coerce, but since we cannot
>     know this in the presence of errors, reporting both versions
>     is better than reporting just one, with the risk of guessing wrong.
>     Document how typecheck_errors.m makes this ok.
> 
> compiler/type_assign.m:
>     Add du_type_is_not_a_subtype as a coerce_fail.
> 
> compiler/typecheck_errors.m:
>     Generate a specific message for the new coerce_fail.
> 
>     If a list of coerce_fails contains both a specific coerce_fail
>     and its mirror image, print only the one whose arguments
>     fall into a simple standard order.
> 
> compiler/typecheck_util.m:
>     Split an existing predicate, and export the split-out part
>     to typecheck_coerce.m.
> 
> tests/invalid/coerce_type_error.{m,exp}:
>     Extend this test case with a test of the new coerce_fail. (This
>     motivated the need for processing mirror-image coerce_fails above.)

> diff --git a/compiler/typecheck_coerce.m b/compiler/typecheck_coerce.m
> index 3d656e2a6..4c0a9ab5c 100644
> --- a/compiler/typecheck_coerce.m
> +++ b/compiler/typecheck_coerce.m
> @@ -757,6 +789,8 @@ types_compare_as_given(TypeTable, TVarSet, Comparison, TypeA, TypeB,
>  
>  types_compare_as_given_nonvar(TypeTable, TVarSet, Comparison,
>          TypeA, TypeB, !TypeAssign, !CoerceFails) :-
> +    % Several of the kinds of coerce_fails that the code below can generate
> +    % are NOT TESTED by any test case in the test suite.
>      require_complete_switch [TypeA]
>      (
>          TypeA = builtin_type(BuiltinTypeA),
> @@ -780,31 +814,68 @@ types_compare_as_given_nonvar(TypeTable, TVarSet, Comparison,
>              defined_type_to_ctor_and_args(TypeA, TypeCtorA, ArgTypesA),
>              defined_type_to_ctor_and_args(TypeB, TypeCtorB, ArgTypesB),
>              ( if TypeCtorA = TypeCtorB then
> +                % Checking for TypeCtorA = TypeCtorB before checking whether
> +                % TypeA and TypeB are du types allows this code to succeed for
> +                %
> +                % - equivalence type
> +                % - foreign types
> +                % - solver types
> +                % - abstract types
> +                % - undefned type_ctors (ones that are not in the type table)

undefined

> +                %
> +                % Equivalence types should have been expanded out by now,
> +                % so they pose no problem. (If they did appear here, we
> +                % would have to expand them out, because without that,
> +                % we cannot check for co- versus contra-variance.)
> +                %
> +                % The other kinds of types can all occur in the input
> +                % of ths code. Most of the time, their argument lists

this

> +                % are the empty list, but they can contain type parameters,
> +                % such as the ones we use to distinguish e.g. prog_vars
> +                % from tvars. XXX If anyone knows the technical name
> +                % of such "ghost" type parameters, please write it here.

Just "phantom type parameter" seems to be it, e.g.
https://doc.rust-lang.org/rust-by-example/generics/phantom.html


> diff --git a/tests/invalid/coerce_type_error.err_exp b/tests/invalid/coerce_type_error.err_exp
> index 0e4ee85e7..5c9604566 100644
> --- a/tests/invalid/coerce_type_error.err_exp
> +++ b/tests/invalid/coerce_type_error.err_exp
> @@ -26,3 +26,10 @@ coerce_type_error.m:118:   error: cannot coerce `X' from
>  coerce_type_error.m:118:   `coerce_type_error.wrap_abs(coerce_type_error.citrus)'
>  coerce_type_error.m:118:   to
>  coerce_type_error.m:118:   `coerce_type_error.wrap_abs(coerce_type_error.fruit)'.
> +coerce_type_error.m:125: In clause for predicate `non_subtypes'/2:
> +coerce_type_error.m:125:   error: cannot coerce `Fruits' from
> +coerce_type_error.m:125:   `list.list(coerce_type_error.fruit)' to
> +coerce_type_error.m:125:   `list.list(coerce_type_error.orange_non_fruit)'.
> +coerce_type_error.m:125:   `coerce_type_error.fruit'/0 is not a subtype.
> +coerce_type_error.m:125:   `coerce_type_error.orange_non_fruit'/0 is not a
> +coerce_type_error.m:125:   subtype.

I think the last two sentences could mislead as to why the coercion is
invalid. Ideally, it would say something like: Neither `fruit' nor
`orange_non_fruit' is a subtype of the other.

The other changes look fine.

Peter


More information about the reviews mailing list