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

Peter Ross peter.ross at miscrit.be
Tue Jul 10 23:20:34 AEST 2001


On Tue, Jul 10, 2001 at 03:21:14AM +1000, Fergus Henderson wrote:
> 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.
> 
I think the correct type is mlds__commit_type, but I am not 100%
certain.

> > > 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 ;-)
> 
I did insert a comment into the code as well.
--------------------------------------------------------------------------
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