[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