[m-rev.] for review: improve `_init.c' file creation

[Fergus Henderson]

On 27-Nov-2001, Simon Taylor <stayl at cs.mu.OZ.AU> wrote:
> 
> Improvements for the code which creates the `_init.o' files.
>
> compiler/mercury_compile.m:
> 	Separate out the code to create the `_init.o' file into
> 	a new predicate.
> 
> 	Pass the grade to c2init.
> 
> 	Use the `--init-c-file' c2init option rather than `-o'.

That bit is fine, although it would be clearer if you explained
in the log message why you pass the grade to c2init.

> compiler/modules.m:
> compiler/mercury_compile.m:
> 	Update the `_init.c' file if the c2init command
> 	line has changed since the last time it was run,
> 	for example if the user has added C2INITFLAGS=--trace.	

In the first line, s/if/only if/ ?

This change does the wrong thing when `-x' or `--extra-inits' option
is included in C2INITFLAGS, or if any `*.init' files are included on
the c2init command line, because in that case, the contents of the
`_init.c' file may change even when the command hasn't.
This is a serious problem that would need to be resolved
before this change could be acceptable.

I also don't think that this behaviour should be unconditional,
and right now I don't think it should even be the default.
I think it should only be enabled when the `--make' option (rebuild
only those things which are not up-to-date) is set; if that option
is not set, then things should be rebuilt unconditionally.
This is needed for consistency with the way we handle all the other
files.  

(Perhaps later we may want to make `--make' be the default,
but even then I would still want to have a `--build' option
that rebuilt everything unconditionally.)

Also, if you're creating new files, as seems to be the case judging
from the code in the diff below, then they should be mentioned in the
log message and documented in the Mercury user's guide.

Also, I'm not yet convinced that this change is worthwhile from an
efficiency perspective, at least in the case when c2init is invoked from
mmake rather than mmc, because I suspect that the overhead of checking
to see whether the *.cmd file has changed may be similar to the cost
of running c2init in the first place.

> Index: mercury_compile.m
...
> +:- pred mercury_compile__make_init_o_file(string, string, module_name,

I suggest s/o_file/obj_file/g

> +		% XXX Why does `io__read_file_as_string' return a separate
> +		% status and result, rather than an `io__res(string)'?

The reason is to ensure that, in the case when some characters are
successfully read and then an I/O error occurs, the part of the file
that was successfully read can be returned.

> Index: modules.m
...
> +	%
> +	% Update `module_init.c.cmd' if the c2init command line
> +	% has changed since the last time it was built.
> +	% The `FORCE-module.c.cmd' dependency forces the commands for
> +	% the `module_init.c.cmd' rule to be run every time the rule
> +	% is considered.

s/module.c.cmd/module_init.c.cmd/

Using "FORCE-*" as a target name goes against the usual
Make convention of having target names be lower case and
variable names upper case.

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