[m-dev.] for review: The .NET MSIL backend.

[Fergus Henderson]

On 22-Sep-2000, Tyson Dowd <trd at cs.mu.OZ.AU> wrote:
> +	% XXX Should we do our own escaping?
> +:- pred output_string_constant(string::in, io__state::di, io__state::uo)
> +	is det.
> +output_string_constant(String) -->
> +	io__write_string(""""),
> +	term_io__write_escaped_string(String),  
> +	io__write_string("""").

The answer to the question in that XXX is yes.
term_io__write_escaped_string writes the string using Mercury syntax,
which is not the same as IL assembler syntax.
This one really ought to be fixed before it ends up biting us...

> +:- pred output_field_init(field_init::in, io__state::di, io__state::uo) is det.
> +output_field_init(binary_float64(Int)) -->
> +	io__write_string("float64("),
> +	io__write_int(Int),
> +	io__write_string(")").
> +output_field_init(binary_float32(Int)) -->
> +	io__write_string("float32("),
> +	io__write_int(Int),
> +	io__write_string(")").
> +output_field_init(wchar_ptr(String)) -->
> +	io__write_string("wchar *("),
> +	io__write(String), 
> +	io__write_string(")").

Hmm, are the arguments here really supposed to be signed ints?
For float64, that looks particularly unappealing, since Mercury ints
need not have 64 bits...

Oh, I see, those arguments are of type int32, int64, etc.
but currently int32, int64 etc. are typedef'd as just `int'.
Well, at very least there should be an XXX comment here.

But it would be nice to fix it, at least for int64, e.g.
defining `:- type int64 == integer' and then doing
s/io__write_int(Int)/io__write_string(integer__to_string(Int))/
here.

Also perhaps the variable names should reflect their types (s/Int/Int64/
or s/Int/Int32/).

> +:- pred output_data_item(data_item::in, io__state::di, io__state::uo) is det.
> +output_data_item(float64(Float)) -->
> +	io__write_string("float64("),
> +	io__write_float(Float),
> +	io__write_string(")").
> +output_data_item(float32(Float)) -->
> +	io__write_string("float32("),
> +	io__write_float(Float),
> +	io__write_string(")").
> +output_data_item(int64(Int)) -->
> +	io__write_string("int64("),
> +	io__write_int(Int),
> +	io__write_string(")").

Likewise here.

> +	% We need to escape all the IDs we output to avoid bumping into
> +	% keywords that assembler uses (there are a lot of them, and
> +	% there is no list available).
> +:- pred escape_id(ilds__id::in, string::out) is det.
> +escape_id(Id, EscapedId) :-
> +	string__append_list(["'", Id, "'"], EscapedId).

What happens if the Mercury identifier contains a single quote,
e.g. for code like
	
	:- func 'why don''t we allow quotes in identifiers' = int.
	'why don''t we allow quotes in identifiers' = 42.

> Index: compiler/ilds.m
...
> +% ilds - The IL instruction set.
> +% Main author: trd.
> +%
> +% This code is a little messy.  Much of it was lifted straight from the
> +% assembler grammar and could be generalized some more.  Some of the
> +% naming conventions are a bit screwy.
> +%
> +% To do:
> +%	[ ] Use uniform naming.
> +% 	[ ] Generalize and eliminate unnecessary types.

Are these comments all still true?

> +	% A method reference.
> +:- type methodref
> +	---> methoddef(call_conv, ret_type, member_name,
> +		list(ilds__type))
> +			% XXX not sure whether this is used.
> +	;    methodref(call_conv, ret_type, structured_name,
> +		list(ilds__type))
> +	;    local_method(call_conv, ret_type, method_name,
> +		list(ilds__type)).

It's not clear what "this" refers to in the comment.

> +:- type structured_name == list(ilds__id).
>
> +:- type member_name 
> +	---> member_name(structured_name, method_name).

What's the structured_name here for?
Some more comments might help.

> +:- type method_name 
> +	--->	ctor
> +	; 	cctor
> +	;	id(ilds__id).

Comments here explaining what `ctor' and `cctor' stand for would
be helpful.

> +	% calling conventions.
> +:- type call_conv   
> +	---> call_conv(
> +			bool,		% is this an instance method call?
> +			call_kind	% what kind of call is it
> +	).

s/ call_conv/<TAB>call_conv/
  ^          ^^^^^

> +	% return types
> +:- type ret_type
> +	--->	void
> +	;	simple_type(simple_type).
> +
> +:- type ilds__type
> +	--->	ilds__type(list(ilds__type_modifier), simple_type).
> +
> +:- type ilds__type_modifier
> +	--->	const
> +	;	readonly
> +	;	volatile.

Is it really true that a return type can't be const or readonly?

> +:- type simple_type
> +	---> 	int8
> +	;	int16
> +	;	int32
> +	;	int64	
> +	;	uint8
> +	;	uint16
> +	;	uint32
> +	;	uint64
> +	;	native_int
> +	;	native_uint
> +	;	float32
> +	;	float64
> +	;	native_float
> +	;	bool
> +	;	char
> +	;	refany
> +	; 	class(structured_name)
> +	;	value_class(structured_name)
> +	;	interface(structured_name)
> +	;	'[]'(ilds__type, bounds)
> +	;	'&'(ilds__type)
> +	;	'*'(ilds__type).

Some brief comments here saying what refany, '&', '[]', and '*' represent
would be helpful.  The others are clear enough, except for `char' --
is that 8-bit or Unicode?

> +:- type bounds == list(bound).
> +
> +:- type bound 
> +	---> 	is(int)
> +	;    	between(int, int).

What does 'is(int)' represent?

And is `between(X, Y)' an open interval or a closed interval?

[to be continued.]

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