[m-dev.] diff: Reorganisation of inst types and operations

[Fergus Henderson]

Andrew Bromage, you wrote:
> 
> Fergus, could you please review this one?
> 
> Reorganisation of modules to do with the inst data type.
> ... The rationale for
> this reorganisation is to reduce coupling in the part of the mode
> checker which is _not_ in this change (ie most of it).

> compiler/inst.m:
> 	New module which contains the data types inst, uniqueness,
> 	pred_inst_info, bound_inst.

You need more detailed documentation of the rationale.
Why will moving things into this new module reduce coupling?

It is essential to communicate the design rationale to future maintainers.

> compiler/inst_util.m:
> 	New module which contains predicates which perform mode
> 	checking-like operations on insts.

Ditto.

> compiler/inst_match.m:
> 	Moved out:
> 		inst_merge (to inst_util.m)
> 		make_mostly_uniq_inst (to inst_util.m)
> 		abstractly_unify_inst, abstractly_unify_inst_functor
> 			(to inst_util.m)

It would be clear if you didn't repeat "(to inst_util.m)" three times,
and instead just wrote

 		inst_merge,
		make_mostly_uniq_inst,
 		abstractly_unify_inst, abstractly_unify_inst_functor
 			(to inst_util.m)

> 	Now exported:
> 		unique_matches_initial/2, unique_matches_final/2
> 		inst_contains_instname/3
> 		pred_inst_matches/3

Rationale please.

> compiler/instmap.m:
> 	instmap_delta_lookup_var/3 reincarnated as
> 	instmap_delta_search_var/3.  The reason for this change is
> 	that previously, instmap_delta_lookup_var simply returned
> 	`free' if the searched-for var did not occur in the
> 	instmap_delta.  This is somewhat non-obvious behaviour.

It's not clear from the log message whether you just renamed the
predicate or whether you also changed its behaviour.

> compiler/mode_util.m:
> 	Moved out:
> 		inst_is_*, inst_list_is_*, bound_inst_list_is_*
> 			(to inst_match.m)

Rationale please.

> compiler/modecheck_call.m:
> 	Moved in modecheck_higher_order_func_call/5, from modecheck_unify.m
> 
> compiler/modecheck_unify.m:
> 	Moved out modecheck_higher_order_func_call/5, to modecheck_call.m
> 	where it should have been all along.

The rationale for these two is fine ;-)

> compiler/prog_data.m:
> 	Moved out the types inst, uniqueness, pred_inst_info,
> 	bound_inst (to inst.m).

Rationale please.


When you've fixed the above, can you please send a new log message?

Apart from the log message, the changes look fine, but please
be _very_ careful that you have correctly merged in any
recent changes to the large chunks of code that you've moved.

P.S.  Generally speaking, I hate large diffs just before code-freeze time...
on the other hand, there's really almost no new code in this change, so
hopefully the chance of introducing new bugs is small, and making this
change _after_ the 0.7 release would make it more difficult to
integrate any bug fixes we find after the release back into the 0.7.x
branch, so it's probably better to do it now rather than later.

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