[m-rev.] for review: bespoke types for typeclass and instance statuses

Peter Wang novalazy at gmail.com
Fri Aug 7 17:46:29 AEST 2026


On Thu, 06 Aug 2026 23:43:32 +0200 "Zoltan Somogyi" <zoltan.somogyi at runbox.com> wrote:
> For review by anyone.
> 
> Zoltan.

> Use bespoke status types for typeclasses and instances.
> 
> compiler/status.m:
>     As above. The old_import_status type we used to use for them
>     is way overkill, since it contains alternatives that are needed
>     only to handle e.g. type-constructor-specific unification predicates.
> 
>     For *most* operations on the new status types, we perform them twice:
>     once using the new type, once its old equivalent, and throw an
>     exception if the results disagree. This is a temporary measure.
>     After a few weeks of regular usage without problems, the extra
>     checks can be deleted.
> 
>     The exceptions are two operations (defined in this module, or not)
>     for which the new definition is self-evidently correct, and
>     one operation on instance types for which the one-to-many nature
>     of the mapping between the new and old representations
>     makes that test impossible to perform properly.
> 
>     Delete the operation to make an instance status abstract.
>     This is because this op was only ever used when adding instances
>     to the HLDS, but in that context, instances are NEVER marked
>     to have their concrete definition exported ANYWHERE.
> 
>     For instances, the new status type is as tight as possible;
>     for typeclasses, some further simplification is probably still possible.
> 
>     Impose a consistent order of the predicates and functions we use
>     to implement each family of exported operations.
> 
> compiler/add_class.m:
>     Treat imports of instances in the implementation sections of .intN
>     files as an abstract import, since these files never contain
>     *concrete* instance definitions.
> 
>     Stop trying to restrict any (non-intermod-opt) export of instances
>     as sbstract, because with the new status type, they all *start out*
>     abstract.

abstract

> 
> compiler/intermod_status.m:
>     When making a decision to opt-export a typeclass or instance,
>     return the export-specific part of the new status. This prevents
>     part of the logic of the decision from having to be replicated
>     by the caller.
> 
> compiler/check_typeclass.m:
> compiler/hlds_out_util.m:
> compiler/instance_method_clauses.m:
> compiler/intermod.m:
> compiler/intermod_mark_exported.m:
> compiler/xml_documentation.m:
>     Conform to the changes above.
> 
> compiler/make_hlds_passes.m:

This line had no text below it.

> +
> +:- func instance_status_to_string(new_instance_status) = string.
> +
> +instance_status_to_string(InstanceStatus) = Str :-
> +    (
> +        InstanceStatus = instance_defined_in_this_module(InstanceExport),
> +        (
> +            InstanceExport = instance_export_gen_none_sub_none,
> +            Str = "this_module(gen_none_sub_none)"
> +        ;
> +            InstanceExport = instance_export_gen_none_sub_abs,
> +            Str = "this_module(gen_none_sub_ans)"

Typo: ans

> diff --git a/compiler/intermod_status.m b/compiler/intermod_status.m
> index 671d7e55e..257b17d19 100644
> --- a/compiler/intermod_status.m
> +++ b/compiler/intermod_status.m
...
> +:- func new_instance_status_to_write(new_instance_status)
> +    = maybe(instance_export).
> +
> +new_instance_status_to_write(Status) = ToWrite :-
> +    (
> +        Status = instance_defined_in_this_module(Export),
> +        (
> +            ( Export = instance_export_gen_none_sub_none
> +            ; Export = instance_export_gen_none_sub_abs
> +            ; Export = instance_export_gen_abs_sub_abs
> +            ),
> +            ToWrite = yes(instance_export_full_opt)
> +        ;
> +            Export = instance_export_full_opt,
> +            % XXX INSTANCE_STATUS This seems strange, but
> +            % it preserves old bevhavior.
> +            ToWrite = no

Typo: behavior

> diff --git a/compiler/status.m b/compiler/status.m
> index ff887cf37..c9c4d30bd 100644
> --- a/compiler/status.m
> +++ b/compiler/status.m
...
> @@ -422,17 +619,33 @@ mode_status_is_imported(ModeStatus) = IsImported :-
>  
>  pred_status_is_imported(pred_status(OldStatus)) =
>      old_status_is_imported(OldStatus).
> -typeclass_status_is_imported(typeclass_status(OldStatus)) =
> -    old_status_is_imported(OldStatus).
> -instance_status_is_imported(instance_status(OldStatus)) =
> -    old_status_is_imported(OldStatus).
> +
> +typeclass_status_is_imported(Status) = IsDefnThisModule :-
> +    (
> +        Status = typeclass_defined_in_this_module(_Export),
> +        IsDefnThisModule = no
> +    ;
> +        Status = typeclass_defined_in_other_module(_Import),
> +        IsDefnThisModule = yes
> +    ).

Rename the output variable.

> +instance_status_is_imported(Status) = IsDefnThisModule :-
> +    (
> +        Status = instance_defined_in_this_module(_Export),
> +        IsDefnThisModule = no
> +    ;
> +        Status = instance_defined_in_other_module(_Import),
> +        IsDefnThisModule = yes
> +    ).
> +

Rename the output variable.

Peter


More information about the reviews mailing list