[m-rev.] for post-commit review: allocate type_info vars separately from initialization

Julien Fischer jfischer at opturion.com
Wed Apr 20 14:44:08 AEST 2022



On Wed, 20 Apr 2022, Zoltan Somogyi wrote:

> Assign to type_info vars separately from allocation.
> 
> compiler/polymorphism_type_info.m:
>     Delete both variants of init_type_info_var, which both allocated
>     a new variable to hold a type_info and also create the code to
>     assign the type_info value to it. Replace it with the service predicate
>     that only the latter task, requiring the former to be done by its callers.

If you haven't yet pushed this:

     that only *performs* the latter task

>     The reason is that the former depends on whether we are using varsets
>     or var_tables, while the latter does not.
>
>     Document the reasons for some design decisions.
>
>     Factor out some common code.
> 
> compiler/size_prof.m:
>     Conform to the change above.

...

> diff --git a/compiler/polymorphism_type_info.m b/compiler/polymorphism_type_info.m
> index f1c91f661..57734aa1f 100644
> --- a/compiler/polymorphism_type_info.m
> +++ b/compiler/polymorphism_type_info.m
> @@ -155,18 +155,13 @@
>      %
>      % The first variable in ArgVars should be bound to the type_ctor_info
>      % for Type's principal type constructor. If that type constructor is
> -    % variable arity, the next variable in ArgVars should be bound to an
> -    % integer giving Type's actual arity. The remaining variables in
> -    % ArgVars should be bound to the type_infos or type_ctor_infos giving
> -    % Type's argument types.
> +    % variable arity, and we are not targeting Java, the next variable

Unrelated to this diff: why is Java a special case here?
(It seems unusual that is is not Java *and* C#, given that those backends
are quite similar in most respects.)

> +    % in ArgVars should be bound to an integer giving Type's actual arity.
> +    % The remaining variables in ArgVars should be bound to the type_infos
> +    % or type_ctor_infos giving Type's argument types.
>      %
> -:- pred init_type_info_var(mer_type::in, list(prog_var)::in,
> -    maybe(prog_var)::in, prog_var::out, hlds_goal::out,
> -    prog_varset::in, prog_varset::out, vartypes::in, vartypes::out,
> -    rtti_varmaps::in, rtti_varmaps::out) is det.
> -:- pred init_type_info_var_vt(mer_type::in, list(prog_var)::in,
> -    maybe(prog_var)::in, prog_var::out, hlds_goal::out,
> -    var_table::in, var_table::out, rtti_varmaps::in, rtti_varmaps::out) is det.
> +:- pred init_type_info_var(mer_type::in, list(prog_var)::in, prog_var::in,
> +    hlds_goal::out, rtti_varmaps::in, rtti_varmaps::out) is det.

That looks fine otherwise.

Julien.


More information about the reviews mailing list