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

[Fergus Henderson]

On 16-Oct-1998, Peter Ross <petdr at cs.mu.OZ.AU> wrote:
> Before I address all your comments, just one point.  This diff is meant
> to be one that compiles everything in the samples directory.  It is not
> the complete solution, but it does bootstrap using *gcc* without any
> warnings.
> 
> I would prefer to make the changes in an incremental fashion, and check
> them in.

OK, that is fine with me.

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

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.

> > >     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 will attempt to output const's in the correct place after [...]
> I try to get the compiler to bootstrap using lcc.

Fine.

> > I noticed a comment in llds.m about data_ptr being `const Word *'
> > that would be made obsolete by this change; you should fix that.
> > Also there are some uses of `const Word *' in fact_table.m, as well as
> > a lot in llds_out.m.  There are also some in library/builtin.m and lots
> > in runtime/*.[ch].  Probably you should do a grep for `const Word'
> > and check all those places to see whether they still make sense.
> > 
> 
> I will do this will attempting to bootstrap the compiler using lcc.
> 
> > > Index: compiler/llds_out.m
> > > +	%
> > > +	% output_c_data_def_list outputs all the type definitions of
> > > +	% the module.  This is needed because some compilers need the
> > > +	% definition to appear before its declaration.
> > > +	%
> > 
> > I suggest you change the last line to
> > 
> > 	% data definition to appear before any use of the type
> > 	% in forward declarations of static constants.
> > 
> ok.
> 
> > > +	% 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.

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

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