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

[David Matthew Overton]

On Wed, Jun 24, 1998 at 12:58:37AM +1000, Fergus Henderson wrote:
> 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?

"% AAA" is a reminder to me of something that will eventually need to
be changed or re-thought before the alias branch is merged back into
the trunk.  Andrew was already using "% YYY" so I picked "% AAA".
I suppose there should be a more detailed comment there.
> 
> Why doesn't any(_) match_initial with free(alias), and vice versa?

The changes to the inst_matches_* routines currently only allow as
much matching as is necessary for LCO.  Andrew and I have been
planning to discuss with you exactly what should match with what.
Perhaps we could do that soon?

> Why doesn't free(alias) match_initial with free(unique), and vice versa?

free(alias) and free(unique) will eventually match_initial with each
other --- as soon as the resulting implied modes are supported by the
mode checker.

> (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?

I'm not sure how that one got there.  It probably happened when
merging CVS branches.  There should be only one clause.  Take the
formatting from the second clause and the "% XXX" comment from the
first.

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

We need to discuss this.
> 
> Why doesn't free(unique) match_final with free(alias)?

Yes, this should be allowed (and will result in an implied mode).

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

It probably should.

> 
> 
> > @@ -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.)

On re-thinking this (i.e. actually thinking about it at all) I suspect
that free(alias) should qualify as unique and mostly_unique, given
that free(unique) does.  

> 
> > +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.

Ok.

> 
> > +:- 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?

That's a typo.  It should be Inst1.

> 
> > +			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?

My reasoning was that partially instantiated insts should already be
appropriately aliased by previous calls to abstractly_unify_inst_functor.  E.g:

		  % initial inst of X, Y & Z: free(unique)
	X = f(Z), % final inst of X: bound(f(alias(IK0, free(alias))))
	Y = g(X)  % final inst of Y: bound(g(bound(f(alias(IK0, free(alias))))))

However, I suppose this may not always be the case.
The only problem with doing a recursive call when unifying Y and
g(X) is that the liveness information for Z is not available.  If you
assume Z is dead, the recursive call does nothing.  If you assume Z is
live, you may get more inst keys than you need.  Andrew, can you tell
me if this is a problem?

> 
> > -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?

I'm not sure about this.

> 
> > +inst_merge_3(free(Aliasing), free(Aliasing), InstTable, M, free(Aliasing),
> > +		InstTable, M).
> 
> Why isn't merging of `free(alias)' and `free(unique)' allowed?
> 

Merging of free(alias) and free(unique) would require allocating some
memory at the end of the free(unique) branch so that the variable
could become free(alias) in that branch.  This is not impossible, but
does complicate things a bit, so it hasn't been done yet.

-- 
David Overton
MEngSc Student                       Email: dmo at cs.mu.oz.au     
Department of Computer Science       Web: http://www.cs.mu.oz.au/~dmo
The University of Melbourne          Phone: +61 3 9344 9159