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

[Fergus Henderson]

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

> +post_typecheck__finish_aditi_builtin(_, _, _, aditi_call(_, _, _, _),
> +		_, _, _, _) :-
> +	error("post_typecheck__finish_aditi_builtin: aditi_call").
> +post_typecheck__finish_aditi_builtin(ModuleInfo, CallerPredInfo, Args,
> +		aditi_insert(PredId0), aditi_insert(PredId),
> +		PredOrFunc - SymName0/Arity, PredOrFunc - SymName/Arity,
> +		Modes) :-
> +	get_state_args_det(Args, OtherArgs, _, _),
> +	post_typecheck__resolve_pred_overloading(PredId0, OtherArgs,
> +		CallerPredInfo, ModuleInfo, SymName0, SymName, PredId),
> +
> +	module_info_pred_info(ModuleInfo, PredId, CalledPredInfo),
> +	pred_info_arg_types(CalledPredInfo, ArgTypes),
> +	in_mode(InMode),
> +	aditi_builtin_modes(InMode, (aditi_top_down),
> +		ArgTypes, InsertArgModes),
> +	list__append(InsertArgModes, [aditi_di_mode, aditi_uo_mode], Modes).
> +
> +post_typecheck__finish_aditi_builtin(ModuleInfo, CallerPredInfo, Args,
> +		aditi_delete(PredId0, Syntax), aditi_delete(PredId, Syntax),
> +		PredOrFunc - SymName0/Arity, PredOrFunc - SymName/Arity,
> +		Modes) :-
> +	AdjustArgTypes = lambda([X::in, X::out] is det, true),
> +	resolve_aditi_builtin_overloading(ModuleInfo, CallerPredInfo, Args,
> +		AdjustArgTypes, PredId0, PredId, SymName0, SymName),
> +
> +	module_info_pred_info(ModuleInfo, PredId, CalledPredInfo),
> +	pred_info_arg_types(CalledPredInfo, ArgTypes),
> +	in_mode(InMode),
> +	aditi_builtin_modes(InMode, (aditi_top_down),
> +		ArgTypes, DeleteArgModes),
> +	Inst = ground(shared, yes(pred_inst_info(PredOrFunc,
> +		DeleteArgModes, semidet))),
> +	Modes = [(Inst -> Inst), aditi_di_mode, aditi_uo_mode].
> +
> +post_typecheck__finish_aditi_builtin(ModuleInfo, CallerPredInfo, Args,
> +		aditi_bulk_operation(Op, PredId0),
> +		aditi_bulk_operation(Op, PredId),
> +		PredOrFunc - SymName0/Arity, PredOrFunc - SymName/Arity,
> +		Modes) :-
> +	AdjustArgTypes = lambda([X::in, X::out] is det, true),
> +	resolve_aditi_builtin_overloading(ModuleInfo, CallerPredInfo, Args,
> +		AdjustArgTypes, PredId0, PredId, SymName0, SymName),
> +
> +	module_info_pred_info(ModuleInfo, PredId, CalledPredInfo),
> +	pred_info_arg_types(CalledPredInfo, ArgTypes),
> +	out_mode(OutMode),
> +	aditi_builtin_modes(OutMode, (aditi_bottom_up), ArgTypes, OpArgModes),
> +	Inst = ground(shared, yes(pred_inst_info(PredOrFunc,
> +		OpArgModes, nondet))),
> +	Modes = [(Inst -> Inst), aditi_di_mode, aditi_uo_mode].

Some comments here would be helpful too.

> +:- pred aditi_builtin_modes((mode), lambda_eval_method,
> +		list(type), list(mode)).
> +:- mode aditi_builtin_modes(in, in, in, out) is det.
> +
> +aditi_builtin_modes(_, _, [], []).
> +aditi_builtin_modes(Mode, EvalMethod, [ArgType | ArgTypes],
> +		[ArgMode | ArgModes]) :-
> +	( type_is_aditi_state(ArgType) ->
> +		( EvalMethod = (aditi_top_down) ->
> +			% The top-down Aditi closures are not allowed
> +			% to call database predicates, so their aditi__state
> +			% arguments must have mode `unused'
> +			ArgMode = (free -> free)
> +		;
> +			ArgMode = aditi_ui_mode
> +		)
> +	;
> +		ArgMode = Mode
> +	),
> +	aditi_builtin_modes(Mode, EvalMethod, ArgTypes, ArgModes).

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

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

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

> +	% Make sure lambda expressions introduced by the compiler
> +	% have the correct mode for their `aditi__state' arguments.
> +:- pred fix_aditi_state_modes((mode), list(type), list(mode), list(mode)).
> +:- mode fix_aditi_state_modes(in, in, in, out) is det.
> +
> +fix_aditi_state_modes(_, [], [], []).
> +fix_aditi_state_modes(_, [_|_], [], []) :-
> +	error("purity:fix_aditi_state_modes").
> +fix_aditi_state_modes(_, [], [_|_], []) :-
> +	error("purity:fix_aditi_state_modes").
> +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'.

> Index: compiler/quantification.m
> @@ -321,7 +320,16 @@
>  	implicitly_quantify_unify_rhs(UnifyRHS0, Unification0, Context,
>  		UnifyRHS, Unification),
>  	quantification__get_nonlocals(VarsUnifyRHS),
> -	{ set__insert(VarsUnifyRHS, Var, GoalVars) },
> +	{ set__insert(VarsUnifyRHS, Var, GoalVars0) },
> +	{ 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?

> +quantification__goal_vars_2(unify(A, B, _, D, _), Set0, LambdaSet0,
>  		Set, LambdaSet) :-
>  	set__insert(Set0, A, Set1),
> -	quantification__unify_rhs_vars(B, Set1, LambdaSet0, Set, LambdaSet).
> -
> -quantification__goal_vars_2(higher_order_call(PredVar, ArgVars, _, _, _, _),
> -		Set0, LambdaSet, Set, LambdaSet) :-
> -	set__insert_list(Set0, [PredVar | ArgVars], Set).
> +	( 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.

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

> @@ -1173,7 +1174,8 @@
>  	Inst = bound(unique, [functor(int_const(VarValue), [])]),
>  	VarUnify = unify(Var, functor(int_const(VarValue), []),
>  		(free -> Inst) - (Inst -> Inst),
> -		construct(Var, int_const(VarValue), [], []),
> +		construct(Var, int_const(VarValue), [], [],
> +			no, cell_is_unique, no),

Likewise here.

> @@ -1198,7 +1200,8 @@
>  	Inst = bound(unique, [functor(string_const(VarValue), [])]),
>  	VarUnify = unify(Var, functor(string_const(VarValue), []),
>  		(free -> Inst) - (Inst -> Inst),
> -		construct(Var, string_const(VarValue), [], []),
> +		construct(Var, string_const(VarValue), [], [],
> +			no, cell_is_unique, no),

And here.

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