[m-dev.] for review: changes required for lcc

[Peter Ross]

On 16-Oct-1998, Fergus Henderson <fjh at cs.mu.OZ.AU> wrote:
> > On 16-Oct-1998, Fergus Henderson <fjh at cs.mu.OZ.AU> wrote:
> > > On 16-Oct-1998, Peter Ross <petdr at cs.mu.OZ.AU> wrote:
> > > > compiler/llds_out.m:
> > > >     Add a pass to make sure that the variables are all declared before
> > > >     they are initialised.
> > > 
> > > Hmm, that log message doesn't seem quite right.  How about
> > > 
> > >      Add a pass to make sure that the struct types are all defined
> > >      before they are used.  This is necessary because ANSI C doesn't
> > >      provide any way of forward declaring static variables with
> > >      incomplete type.
> > > 
> > > instead?
> > 
> > It is actually that they must be defined and declared before they are
> > initialised.
> > ie.
> > 
> > struct s {			// 1
> >     int *x;			// 2
> > } y;				// 3
> > struct s y = { &z };		// 4
> 
> Lines 1-3 contain a struct definition (for `struct s')
> and a variable declaration (for `y').
> Line 4 is a variable definition (for `y'), with an initialiser.
> 
Thanks, I understand now.

> How about the following?
> 
>      Add a pass to make sure that the constants are all declared (with
>      a complete type) before they are initialised.  We can't just
>      forward declare each constant immediately before it is used in an
>      initializer, as was done previously, because at that point we don't
>      know the type of the constant, and ANSI C doesn't allow forward
>      declarations of static variables with incomplete types.
> 
Sounds good.

> > > >     Also change data_ptr to type (Word *).  This is because lcc doesn't
> > > >     like casts between const and non-const, when doing certain
> > > >     initialisations.
> > > 
> > > Is that really true?  Or do you mean to say that lcc doesn't like implicit
> > > conversions between (Word *) and (const Word *)?  I thought that lcc
> > > will allow it if you use an explicit cast.
> 
> You didn't answer this question.

I have deleted this line and instead used the rationale about it being a
subset.  It was one of the first changes I made and I can't remember
exactly what caused the problems.  This will be revisited during the
next phase.

> 
> > 
> > > > +	% Also we now conditionally output some parts according to
> > > > +	% whether we are outputting the definition, declaration or both.
> > > >  
> > > > +:- pred output_const_term_decl(list(maybe(rval)), decl_id, bool, bool, bool, 
> > > > +		bool, string, string, int, int, io__state, io__state).
> > > > +:- mode output_const_term_decl(in, in, in, in, in,
> > > > +		in, in, in, in, out, di, uo) is det.
> > > 
> > > The new comment explains two of the new `bool' parameters, but what
> > > is the meaning of the third?
> > 
> > The third parameter is whether the initialisation code is output for
> > the variable.  ie the '= { &z };'
> 
> OK, but the comment is not clear.  Please make it clear.
> 

I have changed to comment so that the example highlights which bit of
code is output.

> > > Also it's not clear here whether `definition' and `declaration' 
> > > refer to the declaration/definition of the struct type or the
> > > declaration/definition of the constant which has that type.
> > 
> > I am not sure what you are trying to say.  Each constant has a unique
> > type.  So I think the answer is it is for both.
> 
> My point is that the *definition* of the struct type is output
> at the same time as the *declaration* of the constant (e.g.
> in lines 1-3 in the above example).  So if you say "declaration"
> or "definition" without saying what is being defined or declared
> then it is ambiguous.
> 
> > > > +output_const_term_decl(ArgVals, DeclId, Exported, Def, Decl, Init, FirstIndent, 
> > > > +		LaterIndent, N1, N) -->
> > > ....
> > > > +		globals__io_get_globals(Globals),
> > > > +		{ globals__have_static_code_addresses(Globals, StaticCode) },
> > > > +		(
> > > > +			{ StaticCode = no },
> > > > +			{ DeclId = data_addr(
> > > > +					data_addr(_, base_type(info, _, _))) }
> > > > +		->
> > > > +			[]
> > > > +		;
> > > > +			io__write_string("const ")
> > > > +		)
> > > 
> > > You should put a comment there explaining why base_type_infos are treated
> > > specially.
> > 
> > I have no idea.  I just put the if-then-else bits around the code.
> 
> Sorry, I thought you had added that.  It wasn't clear from the diff.
> 
> There's another piece of code in llds_out.m similar to that which has
> the comment
> 
>                 % Don't make decls of base_type_infos `const' if we
> 		% don't have static code addresses.
> 
> It would be helpful to copy that comment to here.
> That comment doesn't really explain _why_, but it's better than nothing...
> 
> > > > @@ -3033,7 +3129,7 @@
> > > >  output_rval_const(string_const(String)) -->
> > > >  		% XXX we should change the definition of `string_const'
> > > >  		% so that this cast is not necessary
> > > > -	io__write_string("(const Word *) string_const("""),
> > > > +	io__write_string("(Word *) string_const("""),
> > > 
> > > You changed the definition of `string_const' so that there was no cast.
> > > I think you should either instead change the definition of `string_const'
> > > so that it casts to `Word *', or delete the above `XXX' comment --
> > > probably the former would be better.
> > 
> > I will delete the XXX.  I left it in because someone might remember why
> > the XXX was there and tell me I had done the wrong thing.
> 
> Oh, it was just that the generated C code is smaller and a bit less
> ugly if the cast is done inside the string_const() macro rather than
> being duplicated at every occurrence of it.
> 

I have deleted the XXX and moved the cast into the string const.

> > The rationale is above, and the rest of it is really delete a few lines,
> > and change some of the comments.
> > 
> > Do you still want to see a diff?
> 
> Yes please, although feel free to commit it without waiting for comments
> on the diff.
> 
> -- 
> Fergus Henderson <fjh at cs.mu.oz.au>  |  "I have always known that the pursuit
> WWW: <http://www.cs.mu.oz.au/~fjh>  |  of excellence is a lethal habit"
> PGP: finger fjh at 128.250.37.3        |     -- the last words of T. S. Garp.
> --------------------------------------------------------------------------
> mercury-developers mailing list
> Post messages to:       mercury-developers at cs.mu.oz.au
> Administrative Queries: owner-mercury-developers at cs.mu.oz.au
> Subscriptions:          mercury-developers-request at cs.mu.oz.au
> --------------------------------------------------------------------------
----
 +----------------------------------------------------------------------+
 | Peter Ross      M Sci/Eng Melbourne Uni                              |
 | petdr at cs.mu.oz.au  WWW: www.cs.mu.oz.au/~petdr/ ph: +61 3 9344 9158  |
 +----------------------------------------------------------------------+