[m-dev.] for review: Java backend

[Fergus Henderson]

On 14-Feb-2001, Julien Fischer <juliensf at students.cs.mu.oz.au> wrote:
> 
> compiler/mlds_to_java.m:
> Replaces the existing file.

s/Replaces/<tab>Replaces/

> +++ compiler/mlds_to_java.m	2001/02/14 07:01:20
> @@ -6,9 +6,32 @@
>  %
>  % mlds_to_java - Convert MLDS to Java code.
>  % Main author: juliensf 
> -% 
> -% XXX Not yet implemented.  
> +%
> +% DONE:
> +%	det and semidet predicates
> +%	multiple output arguments
> +%       boxing and unboxing
> +%       conjunctions
> +%       disjunctions
> +%	if-then-else's
> +%       enumerations
> +%	discriminated unions
> +% TODO: 
> +%	multidet and nondet predicates
> +%       RTTI
> +%	handle foreign code written in Java
> +%       higher order functions
> +%       generate names of classes etc. correctly 
> +% 	generate du constructors instead of directly assigning fields
> +%	generate optimized tailcalls
> +%       handle foreign code written in C 

You should list "handle static ground terms" here.

> +% NOTES: 
> +%       Strings in the generated Java source must be fully qualified 
> +%       as `java.lang.String' to avoid conflicting with the library module
> +%       `mercury.String'.

Hmm, in general it would be best to fully qualify all Java names in the
generated Java source, wouldn't it?

> +mlds_to_java__output_mlds(MLDS) -->
> +	{ ModuleName = mlds__get_module_name(MLDS) },
> +	module_name_to_file_name(ModuleName, ".java", yes, SourceFile),
> +	{ Indent = 0 },
> +	mlds_output_to_file(SourceFile, mlds_output_src_file(Indent, MLDS)).

I suggest s/SOurceFile/JavaSourceFile/
and s/output_src_file/output_java_src_file/

> +qualified_name_is_stdlib(unqualified(_)) :- fail.
> +qualified_name_is_stdlib(qualified(Module, Name)) :-
> +	 mercury_std_library_module(Name), Module = unqualified("mercury") ;
> +	 qualified_name_is_stdlib(Module).

The layout of the disjunction here doesn't match that specified
in the Mercury coding guidelines.

> +	%  Succeeds iff this type is something that 
> +	%  the Java backend will represent as an object 
> +	%  i.e. something created using the new operator.
> +	%
> +:- pred type_is_object(mlds__type).
> +:- mode type_is_object(in) is semidet.
> +
> +type_is_object(Type) :-
> +	Type = mercury_type(_, Builtin),
> +	( Builtin = enum_type 
> +	; Builtin = polymorphic_type
> +	; Builtin = user_type ).

The ')' should be on a new line.

> +	% Succeeds iff the Rval represents an integer constant.
> +	%
> +:- pred rval_is_int_const(mlds__rval).
> +:- mode rval_is_int_const(in) is semidet.
> +
> +rval_is_int_const(Rval) :-
> +	Rval = const(Type),
> +	Type = int_const(_).

`Type' isn't a good name for the intermediate variable here,
since it's not a type.

I suggest just writing that as

	rval_is_int_const(Rval) :-
		Rval = const(int_const(_)).

> +	% Succeeds iff the Rval represents an enumeration
> +	% object in the Java backend.
> +	%
> +:- pred rval_is_enum_var(mlds__rval).
> +:- mode rval_is_enum_var(in) is semidet.
> +
> +rval_is_enum_var(Rval) :-
> +	( Rval = lval(Lval),
> +	  Lval = var(_, VarType),
> +	  type_is_enum(VarType)
> +	 ;
> +	  Rval = unop(_, Rval1),
> +	  rval_is_enum_var(Rval1)
> +	).

The layout of the disjunction here doesn't match that specified
in the Mercury coding guidelines.

The definition of this predicate doesn't match the documentation.

You should document why special handling for unary operators is needed
here.

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

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

> The current implementation of the MLDS makes
> +% this rather harder to achieve than it might initially seem.  The current
> +% position is that coding conventions are only enforced on library modules.
> +% This is need as Java compilers don't take too well to compiling
> +% classes named char, int, float etc.
> +% XXX It might be nice if the name mangling code was taken out of which
> +% ever LLDS module it's hiding in and put in a seperate one.
> +%
> +
> +	% XXX This won't work if we start using the Java
> +	% coding conventions for all names.  At the moment
> +	% it only affects library modules.  
> +
> +:- pred mlds_enforce_java_names(string, string).
> +:- mode mlds_enforce_java_names(in, out) is det.
> +
> +mlds_enforce_java_names(Name, JavaName) :-
> +	reverse_string(Name, RevName),
> +	( string__sub_string_search(RevName, ".", Pos) ->
> +		string__split(RevName, Pos, Head0, Tail0),
> +		reverse_string(Tail0, Tail),
> +		reverse_string(Head0, Head1),
> +		string__capitalize_first(Head1, Head),
> +		string__append(Tail, Head, JavaName)
> +
> +	;
> +		reverse_string(RevName, JavaName)
> +	).

Delete the blank line.

Use `JavaName = Name' rather than `reverse_string(RevName, JavaName)'.

Add a comment explaining what this code is doing,
e.g. 

	mlds_enforce_java_names(Name, JavaName) :-
		% If the name contains a dot (`.'), then
		% capitalize the first letter after the last dot.
		...

> +:- pred mlds_sym_name_mangle_java(sym_name, string).
> +:- mode mlds_sym_name_mangle_java(in, out) is det.
> +
> +mlds_sym_name_mangle_java(unqualified(Name), MangledName) :-
> +	llds_out__name_mangle(Name, MangledName).
> +mlds_sym_name_mangle_java(qualified(ModuleName, PlainName), MangledName) :-
> +	mlds_sym_name_mangle_java(ModuleName, MangledModuleName),
> +	llds_out__name_mangle(PlainName, MangledPlainName),
> +	mlds_qualify_mangled_name(MangledModuleName, MangledPlainName, 
> +		MangledName).
>
> +:- pred mlds_qualify_mangled_name(string, string, string).
> +:- mode mlds_qualify_mangled_name(in, in, out) is det.
> +
> +mlds_qualify_mangled_name(Module0, Name0, Name) :-
> +	string__append_list([Module0,".",Name0], Name).
> +%----------------------------------------------------------------------------

Put a blank line before the line with dashes.

Use whitespace: s/,".",/, ".", /

These predicate names here are not great.

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