[m-rev.] for review: refactor mlds_to_il.m

[Fergus Henderson]

On 10-Jul-2001, Peter Ross <peter.ross at miscrit.be> wrote:
> On Tue, Jul 10, 2001 at 02:46:18AM +1000, Fergus Henderson wrote:
> > On 09-Jul-2001, Peter Ross <peter.ross at miscrit.be> wrote:
> > > Index: ml_elim_nested.m
> > > ===================================================================
> > > RCS file: /home/mercury1/repository/mercury/compiler/ml_elim_nested.m,v
> > > retrieving revision 1.29
> > > diff -u -r1.29 ml_elim_nested.m
> > > --- ml_elim_nested.m	27 Jun 2001 13:41:30 -0000	1.29
> > > +++ ml_elim_nested.m	9 Jul 2001 13:08:58 -0000
> 
> > > +		Rval = new_object(Lval, no, FieldType, no, no, [], []),
> > > +
> > > +		Stmt = mlds__statement(atomic(Rval), Context),
> > > +		Ctor = mlds__function(no, func_params([], []), yes(Stmt)),
> > > +		CtorFlags = init_decl_flags(public, per_instance, non_virtual,
> > > +				overridable, modifiable, concrete),
> > > +		CtorDefn = mlds__defn(export("unused"), Context, CtorFlags,
> > > +				Ctor),
> > > +		Ctors = [CtorDefn]
> > > +	;
> > > +		Ctors = []
> > > +	),
> > 
> > Why do you use `export("unused")' for the constructor name?
> > 
> > This whole section of code is a mystery to me.
> > At very least it needs to be much better documented.
> > 
> The constructor has to be given an entity name, however the name of a
> constructor is determined by the backend (ctor on the ILDS backend,
> class name on the Java backend).  Maybe I should add an alternative
> which to entity_name which is no_name.

Well, at least put a comment there explaining it.

Also "<constructor>" would be better than "unused", IMHO.

> > > +++ mlds_to_csharp.m	9 Jul 2001 13:09:00 -0000
> > > @@ -90,8 +90,8 @@
> > >  generate_csharp_code(MLDS) -->
> > >  
> > >  	{ MLDS = mlds(ModuleName, ForeignCode, _Imports, Defns) },
> > > -	{ ClassName = mlds_module_name_to_class_name(
> > > -		mercury_module_name_to_mlds(ModuleName), yes) },
> > > +	{ ClassName = class_name(mercury_module_name_to_mlds(ModuleName), 
> > > +			"mercury_code") },
> > 
> > What's the rationale for that change?
> > 
> > Increasing the number of places where we hard-code "mercury_code"
> > doesn't seem like an improvement.
>
> You have to do this here, formerly the yes argument of
> mercury_module_name_to_mlds did this for you.  It doesn't any longer so
> you need to do it explicitly.

You don't need to explicitly hard-code the string "mercury-code"
in a zillion places.  Abstract it out.

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