[m-dev.] for review: Java backend

[Fergus Henderson]

On 22-Feb-2001, Julien Fischer <juliensf at students.cs.mu.oz.au> wrote:
> +	% Succeeds iff the Rval represents an enumeration
> +	% object in the Java backend.  We need to check both Rval's
> +	% that are variables and Rval's that are casts.  
> +	% We need to know this in order to append the a field name 
> +	% to to object so we can access the value of the enumeration object.

Typos: "the a" and "to to".

> +	% XXX Perhaps we need to do this for other unop's?
> +	%
> +:- pred rval_is_enum_object(mlds__rval).
> +:- mode rval_is_enum_object(in) is semidet.
> +
> +rval_is_enum_object(Rval) :-
> +	( 
> +		Rval = lval(Lval),
> +		Lval = var(_, VarType),
> +		type_is_enum(VarType)
> +	;
> +		Rval = unop(cast(_), Rval1),
> +		rval_is_enum_object(Rval1)
> +	).

I still don't understand why you need to do that for casts.

> +	% Succeeds iff a given string matches the unqualified
> +	% interface name of a interface in the Java runtime system.
> +	%
> +:- pred interface_is_special(string).
> +:- mode interface_is_special(in) is semidet.
> +
> +interface_is_special("Unify").
> +interface_is_special("Compare").
> +interface_is_special("ProcAddr").

s/the Java runtime system/Mercury's Java runtime system/ ?

> +%-----------------------------------------------------------------------------%
> +%
> +% Code to mangle names, enforce Java code conventions regarding class names
> +% etc.
> +% XXX None of this stuff works as it should. The idea is that class
> +% names should be uppercase, while method names and package specifiers
> +% should be lowercase.  The current implementation of the MLDS makes

See my review comment from last round:

	s/be uppercase/start with an uppercase letter/
	Likewise for lowercase.

> +	% Transform a list of function arguments into a list of local
> +	% variable declarations of the same name and type.  Create
> +	% initializers that initalize each new local variable to the
> +	% correct element in the `args' array.

s/initalize/initialize/
                ^

> +:- pred generate_wrapper_decls(mercury_module_name, mlds__context, 
> +	mlds__arguments, int, mlds__defns).
> +:- mode generate_wrapper_decls(in, in, in, in, out) is det.
> +
> +generate_wrapper_decls(_, _, [], _, []).
> +generate_wrapper_decls(ModuleName, Context, [Arg | Args], 
> +		Count, [Defn | Defns]) :-
> +	Arg = Name - Type,
> +	Flags = ml_gen_local_var_decl_flags,
> +	string__int_to_string(Count, Index),
> +	string__append("args[", Index, UnqualName0),
> +	string__append(UnqualName0, "]", UnqualName),
> +	NewVarName = qual(mercury_module_name_to_mlds(ModuleName), UnqualName),

See my review comment from last time:

	Hmm, here you're using "args[0]" as a _variable name_?

	I think it would be much nicer to use the `array_index' unary_op.

If you disagree, please so say, and preferably explain why.

> +:- pred mlds_output_maybe(maybe(T), pred(T, io__state, io__state),
> +		io__state, io__state).
> +:- mode mlds_output_maybe(in, pred(in, di, uo) is det, di, uo) is det.
> +
> +mlds_output_maybe(MaybeValue, OutputAction) -->
> +	( { MaybeValue = yes(Value) } ->
> +		OutputAction(Value)
> +	;
> +		[]
> +	).

No need for the `mlds_' prefix here.

> +:- func mlds_needs_initialization(mlds__initializer) = bool.
> +:- mode mlds_needs_initialization(in) = out is det.
> +
> +mlds_needs_initialization(no_initializer) = no.
> +mlds_needs_initialization(init_obj(_)) = yes.
> +mlds_needs_initialization(init_struct([])) = no.
> +mlds_needs_initialization(init_struct([_|_])) = yes.
> +mlds_needs_initialization(init_array(_)) = yes.

The `mlds_' prefix here is wrong, and probably should be `java_'
instead.

> +mlds_output_pred_proc_id(proc(PredId, ProcId)) -->
> +	globals__io_lookup_bool_option(auto_comments, AddComments),
> +	( { AddComments = yes } ->
> +		io__write_string("/* pred_id: "),
> +		{ pred_id_to_int(PredId, PredIdNum) },
> +		io__write_int(PredIdNum),
> +		io__write_string(", proc_id: "),
> +		{ proc_id_to_int(ProcId, ProcIdNum) },
> +		io__write_int(ProcIdNum),
> +		io__write_string(" */\n")
> +	;
> +		[]
> +	).

You might want to change that to use // comments (I noticed that you
changed the module header comment in that way, might as well be consistent.)

> +:- pred output_param_types(mlds__arguments, io__state, io__state).
> +:- mode output_param_types(in, di, uo) is det.
> +
> +output_param_types(Parameters) -->
> +	io__write_char('('),
> +	( { Parameters = [] } ->
> +		io__write_string("void")
> +	;
> +		io__write_list(Parameters, ", ", output_param_type)
> +	),
> +	io__write_char(')').
> +
> +:- pred output_param_type(pair(mlds__entity_name, mlds__type),
> +		io__state, io__state).
> +:- mode output_param_type(in, di, uo) is det.
> +
> +output_param_type(_Name - Type) -->
> +	output_type(Type).

Those two routines are unused, I think, and should be deleted.

> +output_fully_qualified_name(QualifiedName) -->
> +	{ QualifiedName = qual(_ModuleName, Name) },
> +	%
> +	% Don't module qualify data names, otherwise all 
> +	% variable declarations will be qualified with the
> +	% module name.
> +	%
> +	(
> +		{ Name = data(_) }
> +	->
> +		output_name(Name)
> +	;
> +		output_fully_qualified(QualifiedName, output_name)
> +	).

You may need to treat data(var("dummy_var")) as a special case here.

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