[m-dev.] for review: retry across I/O

[Fergus Henderson]

On 06-Nov-2000, Zoltan Somogyi <zs at cs.mu.OZ.AU> wrote:
> Add tabling of I/O actions for the debugger.
> 
> compiler/options.m:
> 	Add a new option, --trace-table-io, that enables the tabling of I/O
> 	actions, and another, --trace-table-io-states, that governs whether the
> 	tabling includes the I/O state variables themselves. (You want to table
> 	these variables iff they contain meaningful information that is not
> 	stored in global variables.) These options are for developers only
> 	for now.
...
> compiler/table_gen.m:
> 	If a procedure has a pair of I/O state args and is defined using pragma
> 	C code that has the tabled_for_io marker, then perform I/O tabling on
> 	it and mark it as tabled.

Shouldn't that be done only if the --trace-table-io option is specified?
(I haven't looked at the code yet.  Maybe it's just the log message that
is incomplete.)

> library/io.m:
> 	Mark the relevant procedures with the tabled_for_io feature.
> 
> 	Standardize the formatting of predicates defined by pragma C codes.
> 
> library/table_builtin.m:
> 	Define the predicates that perform I/O tabling, to which calls are
> 	inserted in I/O tabled predicates. These depend on knowing what the
> 	maximum MR_Unsigned value is.
>
> configure.in:
> runtime/mercury_conf.h.in:
> 	Detect the proper value of two new macros, MR_{INTEGER,UNSIGNED}_MAX.

The maximum MR_Unsigned value is `((MR_Unsigned) -1)' or `(~ (MR_Unsigned) 0)'.
For any unsigned type T, the C standard guarantees that `((T) -1)'
and `(~ (T) 0)' both evaluate to the maximum value of that type.
So you don't need to autoconf that.

It's not worth autoconfing MR_INTEGER_MAX if we don't use it.

> +++ compiler/hlds_out.m	2000/11/06 01:49:32
> @@ -2917,6 +2922,10 @@
>  		io__write_string("% address is not taken\n")
>  	),
>  
> +	io__write_string("% eval method: "),
> +	hlds_out__write_eval_method(EvalMethod),
> +	io__write_string("\n"),

I think it would be better to write out the eval method iff the eval
method is not `normal'.  This would reduce the verbosity of the HLDS
dump in the usual case, and is similar to what we currently do for the
head_type_params, type class constraint proofs, and Aditi indexes.

> +++ compiler/hlds_pred.m	2000/11/06 01:49:32
> @@ -1505,6 +1505,18 @@
> +	% If the procedure has a pair of io:state arguments, return the index
> +	% of those arguments.
...
> +:- pred proc_info_has_io_state_args(module_info::in, proc_info::in,
> +	int::out, int::out) is semidet.

What do you mean by "the index"?  Does the first argument have index
zero or one?  If there are inserted type_info and typeclass_info args,
do they get counted?  The documentation should explain this in more detail.

I suggest s/io:state/io__state/g

> +     % The first output argument gives the index of the
> +	% input state, the second the output state.
> +	% Note that the automatically constructed unify, index and compare
> +	% procedures for the io:state type are not counted as having io:state
> +	% args, since they do not fall into the scheme of one input and one
> +	% output arg. Since they should never be called, this should not
> +	% matter.

If the idea is that this pred is supposed to only succeed if there is
an input mode io__state argument and an output mode io__state argument,
then you should say so in the first sentence:

	% If the procedure has a pair of io:state arguments, one with
	% an input mode and one with an output mode, return the index of
	% those arguments.

> +proc_info_has_io_state_args(ModuleInfo, ProcInfo, InArgNum, OutArgNum) :-
> +	proc_info_headvars(ProcInfo, HeadVars),
> +	proc_info_argmodes(ProcInfo, ArgModes),
> +	assoc_list__from_corresponding_lists(HeadVars, ArgModes,
> +		HeadVarsModes),
> +	proc_info_vartypes(ProcInfo, VarTypes),
> +	proc_info_has_io_state_args_2(HeadVarsModes, ModuleInfo, VarTypes,
> +		1, no, no, MaybeIn, MaybeOut),
> +	( MaybeIn = yes(In), MaybeOut = yes(Out) ->
> +		InArgNum = In,
> +		OutArgNum = Out
> +	; MaybeIn = no, MaybeOut = no ->
> +		fail
> +	;
> +		% We must be processing the automatically generated index
> +		% procedure for the io_state type. Since this predicate
> +		% should never be called, any transformation meant for
> +		% procedures with io:states would be inappropriate.
> +		fail
> +	).

The comment there is wrong.  It's quite possible to write procedures
that have only one `io__state' argument, and such procedures can
be called.  There's even one in the Mercury compiler itself:
`typecheck_info_init' in typecheck.m.

It's also possible to write predicates that have two io__state
arguments but both have mode `out'.  E.g.

	main(IO0, _IO) -->
		nasty_fail(IO0, IO0).

	:- mode nasty_fail(in, in) is erroneous.
	nasty_fail(_, _) :- throw("ouch").

> +:- pred proc_info_has_io_state_args_2(assoc_list(prog_var, mode)::in,
> +	module_info::in, map(prog_var, type)::in,
> +	int::in, maybe(int)::in, maybe(int)::in,
> +	maybe(int)::out, maybe(int)::out) is det.
> +
> +proc_info_has_io_state_args_2([], _, _, _,
> +		MaybeIn, MaybeOut, MaybeIn, MaybeOut).
> +proc_info_has_io_state_args_2([Var - Mode | VarModes], ModuleInfo, VarTypes,
> +		ArgNum, MaybeIn0, MaybeOut0, MaybeIn, MaybeOut) :-
> +	(
> +		map__lookup(VarTypes, Var, VarType),
> +		type_to_type_id(VarType, TypeCtor, []),
> +		type_util__type_id_module(ModuleInfo, TypeCtor, 
> +			unqualified("io")),
> +		type_util__type_id_name(ModuleInfo, TypeCtor, "state"),
> +		type_util__type_id_arity(ModuleInfo, TypeCtor, 0)

You should use `type_util__type_is_io_state' instead of the five lines
above.

> +	->
> +		( mode_is_fully_input(ModuleInfo, Mode) ->
> +			( MaybeIn0 = no ->
> +				MaybeIn1 = yes(ArgNum),
> +				MaybeOut1 = MaybeOut0
> +			;
> +				% We must be processing the automatically
> +				% generated unification or comparison procedure
> +				% for the io_state type. Since these predicates
> +				% should never be called, any transformation
> +				% meant for procedures with io:states would
> +				% be inappropriate. We therefore effectively
> +				% forget both io:state arguments.

The comment there is wrong too, see above.

> +				MaybeIn1 = no,
> +				MaybeOut1 = MaybeOut0
> +			)
> +		; mode_is_fully_output(ModuleInfo, Mode) ->
> +			require(unify(MaybeOut0, no),
> +				"proc_info_has_io_state_args_2: two io:state outputs"),

That will call error/1 for the test case I gave above.

> +			MaybeOut1 = yes(ArgNum),
> +			MaybeIn1 = MaybeIn0
> +		;
> +			error("proc_info_has_io_state_args_2: bad io:state mode")

It should not be an error to write code which declares an io__state argument
to have mode `any'.  For example, the following should be legal:

	main(IO0, _IO) -->
		another_nasty_fail(IO0, IO0).

	:- mode another_nasty_fail(in(any), in(any)) is erroneous.
	another_nasty_fail(_, _) :- throw("ouch").

[... to be continued]


-- 
Fergus Henderson <fjh at cs.mu.oz.au>  |  "I have always known that the pursuit
                                    |  of excellence is a lethal habit"
WWW: <http://www.cs.mu.oz.au/~fjh>  |     -- 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
--------------------------------------------------------------------------