[m-rev.] diff: bug fixes for il grade

[Fergus Henderson]

On 19-Apr-2001, Tyson Dowd <trd at cs.mu.OZ.AU> wrote:
> Branches: main
> 
> Add support for nested classes.
> Fix the implementation of inheritance.
> 
> compiler/ilasm.m:
> 	Add code to allow nested classes in the IL assembler.
> 	
> compiler/mlds_to_il.m:
> 	Add code to generate nested classes.
> 	Fix bugs in the code to handle inheritance -- most of the
> 	inheritance was hand-coded for the case of continutation
> 	environments.

Thanks for that, Tyse.

> +++ compiler/mlds_to_il.m	2001/04/19 13:50:22
> @@ -471,33 +471,21 @@
>  		( 
>  			Entity = mlds__class(ClassDefn) 
>  		->
> +			ClassDefn = mlds__class_defn(_ClassType, _Imports, 
> +				Inherits, _Implements, Defns),
> +			( Inherits = [],
> +				Extends = extends_nothing
> +			; Inherits = [InheritType | _],
> +				Extends = extends(mlds_type_to_ilds_class_name(
> +					InheritType))
> +			),

It would be better for this code to complain rather than
just ignoring the second base class, if there is more than one:

			( Inherits = [],
				Extends = extends_nothing
			; Inherits = [InheritType],
				Extends = extends(mlds_type_to_ilds_class_name(
					InheritType))
			; Inherits = [_, _ | _],
				error("multiple inheritance not supported")
			),

> +			list__map(defn_to_class_decl, Defns, ILDefns),
> +			make_constructor(FullClassName, ClassDefn, 
> +				ConstructorILDefn),
> +			Decls = [comment_term(MLDSDefnTerm),
> +				class([public], TypeName,
> +				Extends, implements([]),
> +				[ConstructorILDefn | ILDefns])]

I think the "[public]" there is not right.
It's pretty harmless, so no need to fix it now, but an XXX comment
would be a good idea.

>  		;
>  			Decls = [comment_term(MLDSDefnTerm),
>  				comment("This type unimplemented.")]
> @@ -663,12 +651,25 @@
>  	mlds__function(_PredProcId, _Params, _MaybeStatements)), ILClassDecl) :-
>  		ILClassDecl = comment("unimplemented: functions in classes").
>  
> -	% XXX this might not need to be implemented (nested classes)
> -	% since it will probably be flattened earlier.
> -defn_to_class_decl(mlds__defn(_Name, _Context, _DeclFlags,
> -		mlds__class(_)), _ILClassDecl) :-
> -	error("nested data definition not expected here").
> -
> +defn_to_class_decl(mlds__defn(EntityName, _Context, _DeclFlags,
> +		mlds__class(ClassDefn)), ILClassDecl) :-
> +	( EntityName = type(TypeName, _Arity) ->
> +		ClassDefn = mlds__class_defn(_ClassType, _Imports, 
> +			Inherits, _Implements, Defns),
> +		FullClassName = structured_name("", [TypeName]),
> +		list__map(defn_to_class_decl, Defns, ILDefns),
> +		make_constructor(FullClassName, ClassDefn, ConstructorILDefn),
> +		( Inherits = [],
> +			Extends = extends_nothing
> +		; Inherits = [InheritType | _],
> +			Extends = extends(mlds_type_to_ilds_class_name(
> +				InheritType))
> +		),
> +		ILClassDecl = nested_class([public], TypeName, Extends,
> +			implements([]), [ConstructorILDefn | ILDefns])
> +	;
> +		error("expected type entity name for a nested class")
> +	).

Same comments apply here too.
There seems to be some duplicated code here, it would be worth extracting
that out into a separate procedure.

>  :- mode make_constructor(in, in, out) is det.
>  make_constructor(ClassName, mlds__class_defn(_,  _Imports, Inherits, 
>  		_Implements, Defns), ILDecl) :-
> -	( Inherits = [] ->
> +	( Inherits = [],
>  		CtorMemberName = il_generic_class_name
> -	;
> -		% XXX this needs to be calculated correctly
> -		% (i.e. according to the value of inherits)
> -		CtorMemberName = il_envptr_class_name
> +	; Inherits = [InheritType | _],
> +		CtorMemberName = mlds_type_to_ilds_class_name(InheritType)
>  	),
>  	list__map(call_field_constructor(ClassName), Defns, 
>  		FieldConstrInstrsLists),

Here too it would be better to report an error for multiple inheritance
rather than silently ignoring it.

Otherwise this change looks fine.

-- 
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-reviews mailing list
post:  mercury-reviews at cs.mu.oz.au
administrative address: owner-mercury-reviews at cs.mu.oz.au
unsubscribe: Address: mercury-reviews-request at cs.mu.oz.au Message: unsubscribe
subscribe:   Address: mercury-reviews-request at cs.mu.oz.au Message: subscribe
--------------------------------------------------------------------------