[m-dev.] for review: last call modulo constructors [2/3]

[Fergus Henderson]

On 22-Jun-1998, David Matthew Overton <dmo at students.cs.mu.oz.au> wrote:
>  inst_matches_initial_3(any(UniqA), any(UniqB), _, _, _) :-
>  	unique_matches_initial(UniqA, UniqB).
> -inst_matches_initial_3(any(_), free, _, _, _).
> -inst_matches_initial_3(free, any(_), _, _, _).
> -inst_matches_initial_3(free, free, _, _, _).
> +inst_matches_initial_3(any(_), free(unique), _, _, _).
> +inst_matches_initial_3(free(unique), any(_), _, _, _).
> +inst_matches_initial_3(free(alias), free(alias), _, _, _). % AAA
> +inst_matches_initial_3(free(unique), free(unique), _, _, _). % AAA

What are the "% AAA" comments for?

Why doesn't any(_) match_initial with free(alias), and vice versa?
Why doesn't free(alias) match_initial with free(unique), and vice versa?
(I'm not saying the code is wrong, but I would like you to explain
the reasoning behind it.)

> @@ -400,6 +412,14 @@
>  	%     aliasing in their argument_modes.
>  	pred_inst_argmodes_matches(ModesA, ModesB, InstTable, ModuleInfo, Expansions).
>  
> +pred_inst_matches_2(pred_inst_info(PredOrFunc, ArgModesA, Det),
> +		pred_inst_info(PredOrFunc, ArgModesB, Det),
> +		InstTable, ModuleInfo, Expansions) :-
> +	ArgModesA = argument_modes(_, ModesA),
> +	ArgModesB = argument_modes(_, ModesB),
> +	pred_inst_argmodes_matches(ModesA, ModesB, InstTable, ModuleInfo, 
> +		Expansions).
> +

I didn't understand that change.  Are there two clauses for
pred_inst_matches_2 now?  Is that what you intended?

> @@ -562,13 +582,13 @@
>  
>  inst_matches_final_3(any(UniqA), any(UniqB), _, _, _) :-
>  	unique_matches_final(UniqA, UniqB).
> -inst_matches_final_3(free, any(Uniq), _, _, _) :-
> +inst_matches_final_3(free(unique), any(Uniq), _, _, _) :-

Why doesn't free(alias) match_final with any(_)?

> -inst_matches_final_3(free, free, _, _, _).
> +inst_matches_final_3(free(Aliasing), free(Aliasing), _, _, _).

Why doesn't free(unique) match_final with free(alias)?

>  % Note that `any' is *not* considered to match `any'.
> +inst_matches_binding_3(free(Aliasing), free(Aliasing), _, _, _).

Why doesn't free(alias) match_binding with free(unique)
and vice versa?

> +	% inst_is_free_alias succeeds iff the inst passed is `free(alias)'
> +	% or a user-defined inst which is defined as `free(alias)' or
> +	% `alias(IK)' where `IK' points to a `free(alias)' inst in the IKT.
> +
> +inst_is_free_alias(free(alias), _, _).
> +inst_is_free_alias(free(alias, _), _, _).
> +inst_is_free_alias(inst_var(_), _, _) :-
> +        error("internal error: uninstantiated inst parameter").
> +inst_is_free_alias(defined_inst(InstName), InstTable, ModuleInfo) :-
> +        inst_lookup(InstTable, ModuleInfo, InstName, Inst),
> +        inst_is_free_alias(Inst, InstTable, ModuleInfo).
> +inst_is_free_alias(alias(Key), InstTable, ModuleInfo) :-
> +	inst_table_get_inst_key_table(InstTable, IKT),

Please use tabs rather than spaces for indentation.

> +        % inst_is_higher_order_ground succeeds iff the inst passed is `ground'
> +        % or equivalent and has a pred_inst_info.
> +
> +inst_is_higher_order_ground(ground(_, yes(_PredInstInfo)), _, _).
> +inst_is_higher_order_ground(inst_var(_), _, _) :-
> +        error("internal error: uninstantiated inst parameter").

Ditto.

> @@ -933,7 +1013,7 @@
>  	bound_inst_list_has_property(inst_is_unique_2, List, InstTable,
>  		ModuleInfo, Expansions).
>  inst_is_unique_2(any(unique), _, _, _).
> -inst_is_unique_2(free, _, _, _).
> +inst_is_unique_2(free(unique), _, _, _).

Why doesn't free(alias) qualify as unique?
After all, ground(unique) qualifies as unique even when aliased,
doesn't it?

(Ditto for mostly_unique.)

> +abstractly_unify_inst_functor_2(live, _Real, free(_), ConsId, Args0, ArgLives,
> +			UI0, bound(unique, [functor(ConsId, Args)]), det, UI) :-
> +	unify_inst_info_get_module_info(UI0, M),
> +	unify_inst_info_get_inst_table(UI0, InstTable0),
> +	assoc_list__from_corresponding_lists(Args0, ArgLives, ArgsAndLives),
> +	list__map_foldl(abstractly_unify_bound_inst_arg_with_free(M),
> +		ArgsAndLives, Args, InstTable0, InstTable),
> +	unify_inst_info_set_inst_table(UI0, InstTable, UI).

This is probably a fairly common operation, so it may be worth
manually unfolding the call to list__map_foldl.

> +:- pred abstractly_unify_bound_inst_arg_with_free(module_info, 
> +	pair(inst, is_live), inst, inst_table, inst_table).
> +:- mode abstractly_unify_bound_inst_arg_with_free(in, in, out, in, out) is det.
> +
> +abstractly_unify_bound_inst_arg_with_free(_ModuleInfo, Inst - dead, Inst,
> +		InstTable, InstTable).
> +
> +abstractly_unify_bound_inst_arg_with_free(ModuleInfo, Inst0 - live, Inst,
> +		InstTable0, InstTable) :-
> +	inst_expand_defined_inst(InstTable0, ModuleInfo, Inst0, Inst1),
> +	( inst_is_ground_or_any(Inst1, InstTable0, ModuleInfo) ->
> +		Inst = Inst1,
> +		InstTable = InstTable0
> +	; inst_is_free(Inst1, InstTable0, ModuleInfo) ->
> +		(
> +			Inst0 = alias(_),
> +			inst_is_free_alias(Inst0, InstTable0, ModuleInfo)
> +		->

Why are you referring to Inst0 rather than Inst1 here?

> +			Inst = Inst1,
> +			InstTable = InstTable0
> +		;
> +			inst_table_get_inst_key_table(InstTable0, IKT0),
> +			inst_key_table_add(IKT0, free(alias), IK, IKT),
> +			inst_table_set_inst_key_table(InstTable0, IKT,
> +				InstTable),
> +			Inst = alias(IK)
> +		)
> +	;
> +		Inst = Inst0,
> +		InstTable = InstTable0

Is that the right thing to do for the non-free, non-(ground_or_any) case?
Shouldn't you do the same thing recursively in the case of partially
instantiated insts?

> -make_any_inst(free, IsLive, Uniq0, Real, UI, any(Uniq), det, UI) :-
> +make_any_inst(free(unique), IsLive, Uniq0, Real, UI, any(Uniq), det, UI) :-
>  	unify_uniq(IsLive, Real, det, unique, Uniq0, Uniq).
> -make_any_inst(free(T), IsLive, Uniq, Real, UI,
> +make_any_inst(free(unique, T), IsLive, Uniq, Real, UI,

Why is that restricted to the free(unique) case?

> +inst_merge_3(free(Aliasing), free(Aliasing), InstTable, M, free(Aliasing),
> +		InstTable, M).

Why isn't merging of `free(alias)' and `free(unique)' allowed?

-- 
Fergus Henderson <fjh at cs.mu.oz.au>  |  "I have always known that the pursuit
WWW: <http://www.cs.mu.oz.au/~fjh>  |  of excellence is a lethal habit"
PGP: finger fjh at 128.250.37.3        |     -- the last words of T. S. Garp.