[m-rev.] for review: implement solutions on the IL backend

[Fergus Henderson]

Pete, you seem to have ignored these review comments and committed your
code anyway.

This is really not acceptable.
Please address the review comments ASAP or back out your change.

On 15-Jul-2001, Fergus Henderson <fjh at cs.mu.OZ.AU> wrote:
> On 14-Jul-2001, Peter Ross <peter.ross at miscrit.be> wrote:
> > compiler/ml_code_gen.m:
> >     Avoid generating casts and assignments from MR_Box to MR_Word as
> >     these operations aren't type-safe on the IL backend.
> 
> What do you mean by "aren't type-safe"?
> A little more explanation here might help.
> 
> > Index: compiler/ml_code_gen.m
> > +ml_gen_pragma_c_decl(ml_c_arg(_Var, MaybeNameAndMode, Type), Decl) -->
> > +	=(MLDSGenInfo),
> > +	{ ml_gen_info_get_module_info(MLDSGenInfo, ModuleInfo) },
> > +	{ module_info_globals(ModuleInfo, Globals) },
> > +	{ globals__get_target(Globals, Target) },
> > +	{
> >  		MaybeNameAndMode = yes(ArgName - _Mode),
> >  		\+ var_is_singleton(ArgName)
> >  	->
> > -		export__type_to_type_string(Type, TypeString),
> > +		( 
> > +			type_util__var(Type, _),
> > +			Target = il
> > +		->
> > +			TypeString = "MR_Box"
> > +		;
> > +			export__type_to_type_string(Type, TypeString)
> > +		),
> 
> I think "Target = il" is not the right test here, or at least not the best test.
> This is really a property of the foreign language interface, rather than of
> the target language, so you should be switching on
> `Lang = managedcplusplus' rather than `Target = il',
> where `Lang' is obtained via `foreign_language(Attributes, Lang)'
> (you'll need to pass it down to ml_gen_pragma_c_decl).
> 
> This difference between the C interface and the MC++ interface really
> ought to be documented.  Unfortunately the MC++ foreign code interface
> is not documented at all, so for now I just suggest you put something
> commented out (with @c) in doc/reference_manual.texi, e.g. see patch below.
> 
> Also, this test should be done in all of the calls to
> export__type_to_type_string from this file.  (Make a new predicate to
> abstract the common code.)  It might even be better in the long run to
> do this inside export__type_to_type_string itself (i.e. to make the
> foreign_language a parameter to that routine), but that can probably
> wait for another day.
> 
> > @@ -2647,7 +2661,12 @@
> >  			% a cast is for polymorphic types, which are
> >  			% `Word' in the C interface but `MR_Box' in the
> >  			% MLDS back-end.
> > -			( type_util__var(OrigType, _) ->
> > +			% Except for --grade ilc, where polymorphic types
> > +			% are MR_Box.
> > +			( 
> > +				type_util__var(OrigType, _),
> > +				Target \= il
> > +			->
> 
> Likewise here you should be testing Lang \= managed_cplusplus.
> Also the comment is wrong when it talks about `--grade ilc' --
> that's not the only grade which has Target = il.
> 
> -- 
> Fergus Henderson <fjh at cs.mu.oz.au>  |  "I have always known that the pursuit
> The University of Melbourne         |  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
> --------------------------------------------------------------------------

-- 
Fergus Henderson <fjh at cs.mu.oz.au>  |  "I have always known that the pursuit
The University of Melbourne         |  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
--------------------------------------------------------------------------