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

[Simon Taylor]

> On 05-Jun-1999, Simon Taylor <stayl at cs.mu.OZ.AU> wrote:
> > Index: compiler/post_typecheck.m
> > +	% Resolve overloading.
> > +:- pred post_typecheck__finish_aditi_builtin(module_info, pred_info,
> > +		list(prog_var), aditi_builtin, aditi_builtin,
> > +		simple_call_id, simple_call_id, list(mode)).
> > +:- mode post_typecheck__finish_aditi_builtin(in, in, in,
> > +		in, out, in, out, out) is det.
> 
> I think it does a bit more than just resolving overloading.
> More comments please ;-)

        % Resolve overloading and fill in the argument modes
        % of a call to an Aditi builtin.
        % Check that a relation modified by one of the Aditi update
        % goals is a base relation.
        %
 
> > +post_typecheck__finish_aditi_builtin(_, _, _, aditi_call(_, _, _, _),

> Some comments here would be helpful too.

OK.


> Hmm... if the aditi__state argument is always going to have mode `unused'
> in this case, why pass it at all?

So that the arguments of the terms in the Aditi update goal match
the arguments of the base relation being modified.

> 
> > +++ prog_io_goal.m	1999/05/24 05:43:29
> > +	% parse_lambda_eval_method/3 extracts the `aditi' or `aditi_top_down'
> > +	% annotation from a pred expression and returns the rest of the term.
> > +:- pred parse_lambda_eval_method(term(T), lambda_eval_method, term(T)).
> > +:- mode parse_lambda_eval_method(in, out, out) is det.
> 
> I suggest addition "(if any)" after "annotation".

OK.

> > --- purity.m	1999/03/05 13:09:31	1.13
> > +++ purity.m	1999/05/27 23:47:31
> > +compute_expr_purity(generic_call(GenericCall0, Args, Modes0, Det),
> > +		generic_call(GenericCall, Args, Modes, Det),
> > +		_GoalInfo, PredInfo, ModuleInfo, _InClosure, pure,
> > +		NumErrors, NumErrors) -->
> > +	(
> 
> For readability, I suggest changing that to
> 
> 	compute_expr_purity(generic_call(...), ..., Purity, ...) -->
> 		{ Purity = pure },
> 		...

Done.

> 
> > +fix_aditi_state_modes(Mode, [Type | Types],
> > +		[ArgMode0 | Modes0], [ArgMode | Modes]) :-
> > +	( type_is_aditi_state(Type) ->
> > +		ArgMode = Mode
> > +	;
> > +		ArgMode = ArgMode0
> > +	),
> > +	fix_aditi_state_modes(Mode, Types, Modes0, Modes).
> 
> I suggest renaming the variable `Mode' as `AditiStateMode'.

Done.
 
> > Index: compiler/quantification.m
> > +	{ Unification = construct(_, _, _, _, CellToReuse, _, _) ->
> > +		( CellToReuse = yes(cell_to_reuse(ReuseVar, _, _)) ->
> > +			set__insert(GoalVars0, ReuseVar, GoalVars)
> > +		;
> > +			GoalVars = GoalVars0
> > +		)
> > +	;
> > +		GoalVars = GoalVars0
> > +	},
> 
> The nested if-then-else here seems overly complicated -- why not just
> use a single if-then-else?

Done.

> > +	( D = construct(_, _, _, _, Reuse, _, _) ->
> > +		( Reuse = yes(cell_to_reuse(ReuseVar, _, _)) ->
> > +			set__insert(Set1, ReuseVar, Set2)
> > +		;
> > +			Set2 = Set1
> > +		)
> > +	;
> > +		Set2 = Set1
> > +	),
> > +	quantification__unify_rhs_vars(B, Set2, LambdaSet0, Set, LambdaSet).
> 
> Likewise here.

Done.
 
> > diff -u -u -r1.7 table_gen.m
> > --- table_gen.m	1999/04/20 11:47:50	1.7
> > +++ table_gen.m	1999/04/23 00:40:20
> > @@ -636,7 +636,8 @@
> >  	UnifyMode = (free -> VarInst) - (VarInst -> VarInst),
> >  	UnifyContext = unify_context(explicit, []),
> >  	GoalExpr = unify(PredTableVar, functor(ConsId, []), UnifyMode,
> > -			construct(PredTableVar, ConsId, [], []), UnifyContext),
> > +	    construct(PredTableVar, ConsId, [], [], no, cell_is_unique, no),
> > +	    UnifyContext),
> 
> The meaning of the `no's here is unclear.

I've changed these to use the `make_*_construction' predicates
in hlds_goal.m.

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