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.
> % Stage 1 is responsible for identifying, which goals are
s/,//
> % associative, which can be moved before the recursive call and
> % so on.
> :- 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/
> ->
> % 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)
> :- 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
> % 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.
> %
> % 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
:- 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.
[to be continued]
--
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.
--------------------------------------------------------------------------
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
--------------------------------------------------------------------------