[m-dev.] for review: Aditi updates[1]

[Fergus Henderson]

On 05-Jun-1999, Simon Taylor <stayl at cs.mu.OZ.AU> wrote:
> 
> Index: compiler/hlds_goal.m
> +	% Builtin Aditi operations. 
> +:- type aditi_builtin
> +	--->
...
> +		% Modify the tuples in a relation.
> +		% Arguments:
> +		%   `det' or `semidet' `aditi_top_down' closure to
> +		%       construct a new tuple from an old. This is
> + 		%   aditi__state::di, aditi__state::uo.
> +	;	aditi_modify(
> +			pred_id,	% base relation to modify
> +			aditi_builtin_syntax
> +		)

That comment looks incomplete.

Also, if the argument closure can be either `det' or `semidet',
how do you distinguish those two cases?

> +	.
> +
> +	% Which syntax was used for an aditi_delete or aditi_modify
> +	% call. The first syntax is prettier, the second is used
> +	% where the closure to be passed in is not known at the call site.
> +:- type aditi_builtin_syntax
> +	--->	pred_term		% e.g.	aditi_delete(p(_, X) :- X = 1).
> +	;	sym_name_and_closure	% e.g.	aditi_delete(p/2,
> +					%	    (pred(X::in) is semidet :-
> +					%		X = 1)
> +					%	)
> +	.

It would be helpful if you could either document the semantics of these
two syntaxes here, or give a pointer to the place where they are documented.

The second example looks wrong -- how is the system supposed
to know that "X" is the second argument, rather than the first?
Shouldn't that be `pred(_::in, X::in) is semidet'?

> +	% Shared constant cells can be allocated in static data.
> +	% Others must be created on the heap.
> +	% `cell_is_unique' is always a safe approximation.
> +:- type cell_is_unique
> +	--->	cell_is_unique		% a unique copy of the cell must
> +					% be created by the construction
> +	;	cell_is_shared		% the construction may use a shared
> +					% copy of the cell
> +	.

An important point which the comments here don't make is that if
cell_is_unique, then the cell must be writable, whereas if cell_is_shared,
then it's OK to allocate the cell in read-only memory.

> @@ -1169,7 +1332,7 @@
>  	RHS = functor(ConsId, []),
>  	Inst = bound(unique, [functor(ConsId, [])]),
>  	Mode = (free -> Inst) - (Inst -> Inst),
> -	Unification = construct(Var, ConsId, [], []),
> +	Unification = construct(Var, ConsId, [], [], no, cell_is_unique, no),

It would be helpful to use variable names here rather than just `no',
e.g.

	MaybeCellToReuse = no,
	XXXWhatIsThisXXX = no,
	Unification = construct(Var, ConsId, [], [], 
		MaybeCellToReuse, cell_is_unique, XXXWhatIsThisXXX),

Also, looking back at the definition of `construct', I see that the last
field is

> +                       maybe(rl_exprn_id)
> +                                       % Index of the RL expression used to
> +                                       % extract the relevant tuples
> +                                       % from a relation using an index
> +                                       % in the list of expressions for
> +                                       % this module.

I don't understand why this field is needed; for what kind of constructions
would this field have a `yes(_)' value?

It would also be helpful if the comment here used the word "Aditi", so that
people who are not concerned with Aditi stuff know that they can ignore it.

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