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

Fergus Henderson fjh at cs.mu.OZ.AU
Tue Jul 10 03:21:14 AEST 2001


On 09-Jul-2001, Peter Ross <peter.ross at miscrit.be> wrote:
> On Tue, Jul 10, 2001 at 12:54:34AM +1000, Tyson Dowd wrote:
> > > +++ ml_elim_nested.m	9 Jul 2001 13:08:58 -0000
> > > @@ -373,8 +373,30 @@
> > >  	EnvTypeEntityName = type(EnvClassName, 0),
> > >  	EnvTypeFlags = env_type_decl_flags,
> > >  	Fields = list__map(convert_local_to_field, LocalVars),
> > > +	( Target = il ->
> > > +		ThisPtr = self(mlds__commit_type),
> > > +		FieldType = mlds__commit_type,
> > > +		CtorType = mlds__commit_type,
> > > +		PtrType = mlds__commit_type,	% XXX
> > > +		FieldName = qual(mlds__append_name(ModuleName,
> > > +				EnvClassName ++ "_0"), "commit_1"),
> > 
> > The XXX isn't explained.
> > 
> I wasn't sure what to set it as but it appears that the IL backend never
> looks at this field so it doesn't matter.  Maybe we should add an
> invalid type for these sorts of situations.

Actually we've already got one: mlds__unknown_type.

But this is a very fragile coding style.  What typically happens if you
put bogus information in the MLDS is that someone comes along and
modifies the IL back-end so that it does look at that information,
and then spends a couple of hours debugging it before they figure out
what the problem is.

In this case, there's no major difficulty obtaining the types,
so I think it's really worthwhile to figure out what the correct types
are here and to fill them in properly.

mlds__unknown_type should only be used as a last result.

> > This is commented out but not explained.
>
> To get this working would require that we change the implementation to
> return a list of definitions, so that we could add the comment
> definition.

Don't tell us, put such comments in the code ;-)

(It's OK to tell us *as well*, of course ;-)

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



More information about the reviews mailing list