[m-dev.] for review: update state transformation

[Peter Ross]

On Sat, Apr 08, 2000 at 01:02:08AM +1000, Fergus Henderson wrote:
> On 29-Mar-2000, Peter Ross <petdr at cs.mu.OZ.AU> wrote:
> > 
> > :- type sets
> > 	--->	sets(
> > 			set(goal_id),	% before
> > 			set(goal_id),	% assoc
> > 			set(goal_id),	% construct_assoc
> > 			set(goal_id),	% construct
> > 			set(goal_id),	% update
> > 			set(goal_id)	% reject
> > 		).
> 
> That documentation is not clear to me.
> You should explain in more detail what this data type
> represents and what the fields mean.
> 
Done.

> > 	% Stage 1 is responsible for identifying, which goals are
> 
> s/,//
> 
> > 	% associative, which can be moved before the recursive call and
> > 	% so on.
> 
> 
Done.

> > :- pred stage1(goal_id::in, int::in, goal_store::in, bool::in, bool::in,
> > 		module_info::in, sets::out) is semidet.
> > 
> > stage1(N - K, M, GoalStore, DoLCO, FullyStrict, ModuleInfo, Sets) :-
> > 	sets_init(Sets0),
> > 	stage1_2(N - (K+1), K, M, GoalStore, FullyStrict, ModuleInfo,
> > 			Sets0, Sets1),
> > 	Sets1 = sets(Before, Assoc, ConstructAssoc, Construct, Update, Reject),
> > 	Sets = sets(Before `union` set_upto(N, (K-1)), Assoc,
> > 			ConstructAssoc, Construct, Update, Reject),
> 
> I recommend s/union/set__union/
> 

Done.

> > 	->
> > 			% If LCMC is not turned on then there must be no
> > 			% construction unifications after the recursive
> > 			% call.
> > 		set__empty(Construct `union` ConstructAssoc)
> 
> It would be more efficient, and IMHO no less clear,
> to write that as
> 
> 		set__empty(Construct),
> 		set__empty(ConstructAssoc)
> 
Done.

> > :- pred sets_init(sets::out) is det.
> > 
> > sets_init(Sets) :-
> > 	set__init(Set),
> > 	Before = Set,
> > 	Assoc = Set,
> > 	ConstructAssoc = Set,
> > 	Construct = Set,
> > 	Update = Set,
> > 	Reject = Set,
> > 	Sets = sets(Before, Assoc, ConstructAssoc, Construct, Update, Reject).
> 
> I suggest s/Set/EmptySet/g
> 
Done.

> > 	% This function is called repeatedly with the same arguments so
> > 	% is an ideal canditate for tabling.
> > :- pragma memo(set_upto/2).
> 
> Hmm... did you measure whether that declaration improves performance? ;-)
> 
> I think that for now, I would prefer it if the compiler source code does not
> make use of `pragma memo', unless there is a compelling reason to use it.
> `pragma memo' is documented as an implementation-dependent feature, rather
> than part of the official Mercury language, and in general it is a good idea
> to be somewhat conservative in the use of language features in the compiler
> source code, since that make it easier to bootstrap new developments.
> For example, avoiding `pragma memo' in the compiler would make bootstrapping
> the compiler via a new JVM back-end easier.
> 

I just wanted to see if it worked, and it did!  However it isn't really
necessary, so I have removed it.

> > 	%
> > 	% set_upto(N, K) returns the set {(N,1)...(N,K)}
> > 	%
> > :- func set_upto(int, int) = set(goal_id).
> > 
> > set_upto(N, K) = Set :-
> > 	(
> > 		K =< 0
> > 	->
> > 		set__init(Set)
> > 	;
> > 		Set0 = set_upto(N, K-1),
> > 		set__insert(Set0, N-K, Set)
> > 	).
> 
> The use of `-' for both minus and the pair constructor there
> could lead to confusion.  I suggest using the pair/2 function.
> Or alternatively, you could define the goal_id type as
> 
Done.

> 	:- type goal_id ---> goal_id(int, int).
> 
> rather than
> 
> 	:- type goal_id = pair(int, int).
> 
> > :- func insert_before(goal_id, sets) = sets.
> > 
> > insert_before(GoalId, sets(Before, Assoc, ConstructAssoc, Construct,
> > 		Update, Reject))
> > 	= sets(set__insert(Before, GoalId), Assoc, ConstructAssoc, Construct,
> > 			Update, Reject).
> > 
> > :- func insert_assoc(goal_id, sets) = sets.
> > 
> > insert_assoc(GoalId, sets(Before, Assoc, ConstructAssoc, Construct,
> > 		Update, Reject))
> > 	= sets(Before, set__insert(Assoc, GoalId), ConstructAssoc, Construct,
> > 			Update, Reject).
> 
> It would be clearer to use field names:
> 
>     insert_assoc(GoalId, Sets) =
>     	Sets^assoc := set__insert(Sets^assoc, GoalId).
> 
> Indeed these insert_* functions are simple enough, and are used
> infrequently enough, that I think it may be simpler to just
> write the code inline, rather than defining them as separate functions.
> 
Removed, there wasn't a record syntax when I completed this change.

Peter
--------------------------------------------------------------------------
mercury-developers mailing list
Post messages to:       mercury-developers at cs.mu.oz.au
Administrative Queries: owner-mercury-developers at cs.mu.oz.au
Subscriptions:          mercury-developers-request at cs.mu.oz.au
--------------------------------------------------------------------------