[m-rev.] for review: add source package to mlds names

Tyson Dowd trd at cs.mu.OZ.AU
Fri Jun 8 09:06:10 AEST 2001


On 07-Jun-2001, Peter Ross <peter.ross at miscrit.be> wrote:
> Hi,
> 
> For Tyson to review.
> 
> This change bootchecks in hlc.gc grade, and I have used it to build
> various .NET executables from the samples directory.
> 
> ===================================================================
> 
> 
> Estimated hours taken: 16
> Branches: main
> 
> The .NET backend requires that names are not only qualified with their
> namespace but the source package the name comes from.  In this change we
> add to the name type the name of the source package which this name is
> defined in.  This change will be needed for implementing foreign_class
> in the .NET backend.

A little more rationale would be good.

Why does the .NET backend need the source package name?  What does it
use that for?  Why can't we assume that the package name is the first
part of the namespace name (or maybe the second)? 

Also, a note that this change doesn't actually do anything with this
information would be nice.


I have a few general comments about this change: 

I guess you are planning on using foreign_type to genereate cases where
the package name is different to the module name.
If so, this should be made clear.

If the package name is the same as the module name, is there any sense
in repeating it?  Would a maybe type be more appropriate?

> compiler/mlds.m:
>     Add the new field to the mlds__fully_qualified_name type.
> 
> compiler/ml_call_gen.m:
> compiler/ml_code_gen.m:
> compiler/ml_code_util.m:
> compiler/ml_elim_nested.m:
> compiler/ml_optimize.m:
> compiler/ml_tailcall.m:
> compiler/ml_type_gen.m:
> compiler/ml_unify_gen.m:
> compiler/ml_util.m:
> compiler/mlds_to_c.m:
> compiler/mlds_to_csharp.m:
> compiler/mlds_to_il.m:
> compiler/mlds_to_java.m:
> compiler/mlds_to_mcpp.m:
> compiler/rtti_to_mlds.m:
>     Propogate the changes around.
> 
> Index: compiler/ml_call_gen.m
> ===================================================================
> RCS file: /home/mercury1/repository/mercury/compiler/ml_call_gen.m,v
> retrieving revision 1.22
> diff -u -r1.22 ml_call_gen.m
> --- compiler/ml_call_gen.m	2001/05/27 09:59:47	1.22
> +++ compiler/ml_call_gen.m	2001/06/07 09:17:55
> @@ -556,7 +556,8 @@
>  		PredLabel, PredModule) },
>  	{ Params = ml_gen_proc_params(ModuleInfo, PredId, ProcId) },
>  	{ Signature = mlds__get_func_signature(Params) },
> -	{ QualifiedProcLabel = qual(PredModule, PredLabel - ProcId) },
> +	{ QualifiedProcLabel = qual(PredModule,
> +			PredModule, PredLabel - ProcId) },
>  	{ CodeAddrRval = const(code_addr_const(proc(QualifiedProcLabel,
>  		Signature))) }.

It seems inelegant and possibly error prone to construct and deconstruct
qual(....) each time, repeating PredModule twice.  

A helper function which does the default case would be much better.


Apart from this the change is OK, but I'd much prefer the helper
function was added.

-- 
       Tyson Dowd           # 
                            #  Surreal humour isn't everyone's cup of fur.
     trd at cs.mu.oz.au        # 
http://www.cs.mu.oz.au/~trd #
--------------------------------------------------------------------------
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
--------------------------------------------------------------------------



More information about the reviews mailing list