[m-dev.] for review: nondet pragma C codes (part 1 of 2)

[Fergus Henderson]

On 09-Jan-1998, Zoltan Somogyi <zs at cs.mu.oz.au> wrote:
> 
> compiler/{mercury_compile.m,modules.m}:
> 	Add a missing period after some progress messages.

That should be committed seperately.

> compiler/Mmakefile:
> 	Incidental change: add a new target "debug" that makes both the
> 	compiled and the SICStus versions of the compiler.

Ditto.

> -code_gen__generate_prolog(CodeModel, FrameInfo, PrologCode) -->
> +code_gen__generate_prolog(CodeModel, Goal, FrameInfo, PrologCode) -->
...
> +		(
> +			{ Goal = pragma_c_code(_,_,_,_,_,_, PragmaCode) - _},
> +			{ PragmaCode = nondet(Fields, FieldsContext,
> +				_,_,_,_,_,_,_) }
> +		->
> +			{ pragma_c_gen__struct_name(ModuleName, PredName,
> +				Arity, ProcId, StructName) },
> +			{ Struct = pragma_struct(StructName,
> +				Fields, FieldsContext) },
> +			{ string__format("#define\tMR_ORDINARY_SLOTS\t%d\n",
> +				[i(TotalSlots)], DefineStr) },
> +			{ DefineComps = [pragma_c_raw_code(DefineStr)] },
> +			{ AllocCode = node([
> +				mkframe(PushMsg, TotalSlots, yes(Struct),
> +					do_fail)
> +					- "Allocate stack frame",
> +				pragma_c([], DefineComps,
> +					will_not_call_mercury, no)
> +					- ""

Could you please use a more informative name than `DefineComps'?
(It took me a fair while to figure out what `Comps' stood for.)

> +	{ FrameInfo = frame(TotalSlots, MaybeSuccipSlot, NondetPragma) },
> +	( { NondetPragma = yes } ->
> +		{ UndefStr = "#undef\tMR_ORDINARY_SLOTS\n" },
> +		{ UndefComps = [pragma_c_raw_code(UndefStr)] },

Ditto here for `UndefComps'.

Also, would it be reasonable to split some of the code in generate_prolog
and generate_epilogue out into subroutines?  They're getting pretty long.

>  	;	pragma_c_code(
> -			string,		% The C code to do the work
>  			may_call_mercury,
>  					% Can the C code recursively
>  					% invoke Mercury code?
>  			pred_id,	% The called predicate
>  			proc_id, 	% The mode of the predicate
>  			list(var),	% The (Mercury) argument variables
> -			list(maybe(string)),
> -					% C variable names for each of the
> +			list(maybe(pair(string, mode))),
> +					% C variable names and the original
> +					% mode declaration for each of the
>  					% arguments. A no for a particular 
>  					% argument means that it is not used
>  					% by the C code.  (In particular, the
> @@ -173,25 +173,13 @@
>  			list(type),	% The original types of the arguments.
>  					% (With inlining, the actual types may
>  					% be instances of the original types.)
> -			extra_pragma_info
> -					% Extra information for model_non
> -					% pragma_c_codes; none for others.
> +			pragma_code	% Info about the code that does the
> +					% actual work.

Hmm, `pragma_code' is a bit too general a name.
I would prefer something like `pragma_c_code_info'
or `pragma_c_code' or `pragma_foreign_code'.

> +++ llds.m	1998/01/06 08:09:03
> @@ -126,9 +126,17 @@
> +	;	mkframe(string, int, maybe(pragma_struct), code_addr)
> +			% mkframe(Comment, SlotCount, MaybePragmaStruct,
> +			% FailureContinuation) creates a nondet stack frame.
> +			% Comment says what predicate creates the frame.
> +			% SlotCount says how many ordinary framevar slots
> +			% it ought to have. If MaybePragmaStruct is yes,
> +			% the argument gives the details of the structure
> +			% which occupies the rest of the framevar slots.
> +			% CodeAddr is the code address to branch to when
> +			% trying to generate the next solution from this
> +			% choice point.

I think `pragma_struct' should be named `pragma_c_struct'
or something like that.

> +:- type pragma_struct
> +	--->	pragma_struct(
> +			string,		% The name of the struct tag.
> +			string,		% The field declarations, supplied
> +					% by the user in the program.
> +			term__context	% Where the field declarations
> +					% originally appeared.
> +		).

Some documentation here about what this type is used for would be helpful.

Also s/program/`pragma c_code' declaration/.

> +output_instruction_decls(mkframe(_, _, MaybeStruct, FailureContinuation),
>  		DeclSet0, DeclSet) -->
> +	(
> +		{ MaybeStruct = yes(pragma_struct(StructName,
> +			StructFields, StructFieldsContext)) }
> +	->
> +		{ set__member(pragma_struct(StructName), DeclSet0) ->
> +			string__append_list(["struct ", StructName, " has been declared already"], Msg),
> +			error(Msg)

Please insert "llds_out.m: " at the start of the error message.

> +:- pred output_pragma_component_list_decls(list(pragma_c_component),
> +	decl_set, decl_set, io__state, io__state).
> +:- mode output_pragma_component_list_decls(in, in, out, di, uo) is det.
> +
> +output_pragma_component_list_decls([], DeclSet, DeclSet) --> [].
> +output_pragma_component_list_decls([Component | Components],
>  		DeclSet0, DeclSet) -->
> -	output_pragma_input_rval_decls(Inputs, DeclSet0, DeclSet1),
> -	output_pragma_output_lval_decls(Outputs, DeclSet1, DeclSet).
> +	output_pragma_component_decls(Component, DeclSet0, DeclSet1),
> +	output_pragma_component_list_decls(Components, DeclSet1, DeclSet).
> +
> +:- pred output_pragma_component_decls(pragma_c_component,
> +	decl_set, decl_set, io__state, io__state).
> +:- mode output_pragma_component_decls(in, in, out, di, uo) is det.
> +
> +output_pragma_component_decls(pragma_c_inputs(Inputs), DeclSet0, DeclSet) -->
> +	output_pragma_input_rval_decls(Inputs, DeclSet0, DeclSet).
> +output_pragma_component_decls(pragma_c_outputs(Outputs), DeclSet0, DeclSet) -->
> +	output_pragma_output_lval_decls(Outputs, DeclSet0, DeclSet).
> +output_pragma_component_decls(pragma_c_raw_code(_), DeclSet, DeclSet) --> [].
> +output_pragma_component_decls(pragma_c_user_code(_, _), DeclSet, DeclSet)
> +		--> [].

I suggest s/pragma/pragma_c/g

> +:- pred output_pragma_components(list(pragma_c_component),
> +	io__state, io__state).
> +:- mode output_pragma_components(in, di, uo) is det.
> +
> +output_pragma_components([]) --> [].
> +output_pragma_components([C | Cs]) -->
> +	output_pragma_component(C),
> +	output_pragma_components(Cs).
> +
> +:- pred output_pragma_component(pragma_c_component, io__state, io__state).
> +:- mode output_pragma_component(in, di, uo) is det.

Ditto.

> +output_pragma_component(pragma_c_inputs(Inputs)) -->
> +	output_pragma_inputs(Inputs).
> +output_pragma_component(pragma_c_outputs(Outputs)) -->
> +	output_pragma_outputs(Outputs).
> +output_pragma_component(pragma_c_user_code(Context0, C_Code)) -->
> +	( { C_Code = "" } ->
> +		[]
> +	;
> +			% We should start the C_Code on a new line,
> +			% just in case it starts with a proprocessor directive.
> +			% We must then account for the effect of the \n
> +			% on the context.
> +		{ Context0 = term__context(File, Line0) },
> +		{ Line is Line0 - 1 },
> +		{ Context = term__context(File, Line) },
> +		output_set_line_num(Context),
> +		io__write_string("{\t\t\n"),
> +		io__write_string(C_Code),

Change the above 10 lines to
		io__write_string("{\n"),
		output_set_line_num(Context),
		io__write_string(C_Code),

> +	% We never insert unifications of the form X = X.
> +	% If ForPragmaC is yes, we process unifications of the form
> +	% X = Y by substituting the var expected by the outside environment
> +	% (the head variable) for the variable inside the goal (which was
> +	% created just for the pragma_c_code goal), while giving the headvar
> +	% the name of the just eliminated variable. The result is will be

"is", "will be" -- please choose one ;-)

>  			% Should this be insert_... rather than append_...?
>  			% No, because that causes efficiency problems
> -			% with type-checking :-(
> +			% with type-checking :-|

?

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