[m-rev.] for review: remove compiler Unix dependencies

Simon Taylor stayl at cs.mu.OZ.AU
Wed Aug 6 19:07:29 AEST 2003


On 05-Aug-2003, Fergus Henderson <fjh at cs.mu.OZ.AU> wrote:
> On 02-Aug-2003, Simon Taylor <stayl at cs.mu.OZ.AU> wrote:
> > 
> > Index: compiler/make.program_target.m
> ...
> > +++ compiler/make.util.m	31 Jul 2003 15:02:50 -0000
> > @@ -330,16 +330,24 @@
> >  		{ Succeeded = no }
> >  	;
> >  		{ OptionsResult = yes(ModuleOptionArgs) }, 
> > +		globals__io_lookup_bool_option(make_implies_use_subdirs,
> > +			MakeImpliesUseSubdirs),
> >  		globals__io_get_globals(Globals),
> >  
> > -		% --invoked-by-mmc-make disables reading DEFAULT_MCFLAGS
> > -		% from the environment (DEFAULT_MCFLAGS is included in
> > -		% OptionArgs) and generation of `.d' files.
> > +		% --invoked-by-mmc-make disables reading the options file.
> > +		% (DEFAULT_MCFLAGS is included in OptionArgs) and generation
> > +		% of `.d' files.
> 
> s/file./file/

Done.

> > Index: compiler/options.m
> > @@ -2384,15 +2394,24 @@
> >  quote_arg_2([Char | Chars0]) = Chars :-
> >  	Chars1 = quote_arg_2(Chars0),
> >  	( quote_char(Char) ->
> > -		Chars = [('\\'), Char | Chars1]
> > +		% We want whitespace characters within an argument to not be
> > +		% treated as whitespace when splitting the command line
> > +		% into words. \newline is still treated as whitespace,
> > +		% and not all shells will understand "\n".
> > +		% Newlines and tabs within a word don't really make
> > +		% sense, so just convert them to spaces.
> > +		QuoteChar = ( char__is_whitespace(Char) -> ' ' ; Char ),
> > +		Chars = [('\\'), QuoteChar | Chars1]
> >  	;
> > -		Chars = [Char | Chars1]	
> > -	).	
> > -
> > +		Chars = [Char | Chars1]
> > +	).

> Ouch.  Converting newlines and tabs to spaces doesn't seem
> like a good idea -- it might not preserve the semantics.
> 
> If the problem is that quoting for a Unix shell doesn't work
> when using a Windows shell, then I think it would be better
> to use different quoting algorithms for the different shells.

Done.
 
> > @@ -3983,7 +4010,13 @@
> >  		"\tExecutables and libraries will be symlinked or copied into",
> >  		"\tthe current directory.",
> >  		"\t`--use-grade-subdirs' does not work with Mmake (it does",
> > -		"\twork with `mmc --make')."
> > +		"\twork with `mmc --make').",
> > +
> > +		"--no-make-implies-use-subdirs",
> > +		"\tNormally, `--make' implies `--use-subdirs'.",
> > +		"\tThis option disables that behaviour.",
> > +		"\tLibrary installation does not work with",
> > +		"\t--no-make-implies-use-subdirs."
> 
> Hmm... would it be better for this option to be the default?
> 
> Or alternatively, perhaps we could make --use-subdirs the default?

I tried that a while ago, but for some reason the performance wasn't
good. I'll look into it a bit more.

> (Incidentally, isn't this a completely separate issue from removing
> the Unix dependencies?  Should this be a separate patch?)

I'll do that.

> > Index: library/Mmakefile
> > ===================================================================
> > RCS file: /home/mercury1/repository/mercury/library/Mmakefile,v
> > retrieving revision 1.117
> > diff -u -u -r1.117 Mmakefile
> > --- library/Mmakefile	21 Jul 2003 14:08:40 -0000	1.117
> > +++ library/Mmakefile	30 Jul 2003 14:27:55 -0000
> > @@ -322,19 +322,12 @@
> >  
> >  ifeq ($(MMAKE_USE_MMC_MAKE),no)
> >  
> > -EXTRA_INIT_COMMAND = \
> > -	for file in $($(STD_LIB_NAME).ms); do \
> > -		grep '^INIT ' $$file; \
> > -		true; \
> > -	done
> > +EXTRA_INIT_COMMAND = print_extra_inits $($(STD_LIB_NAME).ms)
> >  
> >  else
> >  
> > -MCFLAGS += --extra-init-command \
> > -		"for module in %; do \
> > -			grep '^INIT ' $$$${module}.m; \
> > -			true; \
> > -		done"
> > +MCFLAGS += --extra-init-command print_extra_inits
> > +
> >  endif	# 
> 
> Is this relying on "." being in $PATH?

Fixed.
 
> > Index: library/print_extra_inits
> > ===================================================================
> > RCS file: library/print_extra_inits
> > diff -N library/print_extra_inits
> > --- /dev/null	1 Jan 1970 00:00:00 -0000
> > +++ library/print_extra_inits	30 Jul 2003 14:27:55 -0000
> > @@ -0,0 +1,24 @@
> > +#!/bin/sh
> > +#-----------------------------------------------------------------------------#
> > +# Copyright (C) 2003 The University of Melbourne.
> > +# This file may only be copied under the terms of the GNU General
> > +# Public License - see the file COPYING in the Mercury distribution.
> > +#-----------------------------------------------------------------------------#
> > +# library/print_extra_inits - print extra .init file lines to stdout.
> > +#
> > +# Invocation:
> > +#	print_extra_inits <mer_std.ms>
> > +# 		where <mer_std.ms> is the names of all of the source files
> > +#		for the modules	in libmer_std.
> > +#
> > +#-----------------------------------------------------------------------------#
> > +
> > +for file; do
> > +	if [ -f $file ]; then
> > +		grep '^INIT ' $file
> > +	else
> > +		echo "source file $file not found" 1>&2
> > +		exit 1
> 
> s/source/$0: source/

Done.
 
> > Index: scripts/mercury.bat.in
> ...
> > +rem There is no equivalent of "$@" available on all Windows platforms.
> > +rem (Windows XP has `%*').
> > +
> > +set v1=%1
> > +set v2=%2
> > +set v3=%3
> > +set v4=%4
> > +set v5=%5
> > +set v6=%6
> > +set v7=%7
> > +set v8=%8
> > +set v9=%9
> > +shift
> > +shift
> > +shift
> > +shift
> > +shift
> > +shift
> > +shift
> > +shift
> > +shift
> > +shift
> > +set v10=%0
> > +set v11=%1
> > +set v12=%2
> > +set v13=%3
> > +set v14=%4
> > +set v15=%5
> > +set v16=%6
> > +set v17=%7
> > +set v18=%8
> > +set v19=%9
> > +shift
> > +shift
> > +shift
> > +shift
> > +shift
> > +shift
> > +shift
> > +shift
> > +shift
> > +shift
> > +%MERCURY_COMPILER% %v1% %v2% %v3% %v4% %v5% %v6% %v7% %v8% %v9% %v10% %v11% %v12% %v13% %v14% %v15% %v16% %v17% %v18% %v19% %0 %1 %2 %3 %4 %5 %6 %7 %8 %9
> 
> It would be a good idea to check if there are any remaining arguments,
> and if so, to issue an error message.

Done.
 
> > Index: scripts/mmc.in
> > ===================================================================
> That change wasn't mentioned in the log message.

Fixed.
 
> > Index: util/mkinit.c
> > ===================================================================
> > RCS file: /home/mercury1/repository/mercury/util/mkinit.c,v
> > retrieving revision 1.89
> > diff -u -u -r1.89 mkinit.c
> > --- util/mkinit.c	25 Sep 2002 07:53:53 -0000	1.89
> > +++ util/mkinit.c	30 Jul 2003 14:27:55 -0000
> > @@ -876,6 +876,9 @@
> >  	if ((position = strrchr(filename, '/')) != NULL) {
> >  		filename = position + 1;
> >  	}
> > +	if ((position = strrchr(filename, '\\')) != NULL) {
> > +		filename = position + 1;
> > +	}
> 
> Missing "else"?

No.

> What happens with file names that contain both separators, e.g.
> 	C:\foo/bar
> or
> 	C:/foo\bar
> ?

That's why there shouldn't be an else there. I've added a comment.

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