[m-dev.] for review: compile in hlc.gc using MSVC

[Fergus Henderson]

Hi Pete,

I have a few more review comments for this change
now that I've seen the log message and had a more
careful look at the code.

On 07-Jun-2000, Peter Ross <petdr at cs.mu.OZ.AU> wrote:
> 
> +if test "$ac_microsoft" = "yes" ; then
> +	EXE_SUFFIX=".exe"
> +	OBJ_SUFFIX="obj"
> +	LIB_SUFFIX="lib"
> +	LIB_PREFIX="lib"
> +	LIB_LIBPATH="/LIBPATH:"
> +	LINK_LIB=""
> +	LINK_OPT_SEP="/link"
> +
> +	OBJFILE_OPT="/Fo"
> +	AR="lib"
> +	ARFLAGS=""
> +	AR_LIBFILE_OPT="/OUT:"
> +	BOEHMGC_MAKEFILE="-f NT_MAKEFILE"
> +
> +	AC_DEFINE(MR_WIN32)
> +
> +		# MS doesn't use a ranlib.
> +	RANLIB="echo"
> +	AC_SUBST(RANLIB)
> +else
> +	EXE_SUFFIX=""
> +	OBJ_SUFFIX="o"
> +	LIB_SUFFIX="a"
> +	LIB_PREFIX=""
> +	LIB_LIBPATH="-L"
> +	LINK_LIB="-l"
> +	LINK_OPT_SEP=""
> +
> +	OBJFILE_OPT="-o"
> +	AR="ar"
> +	ARFLAGS="cr"
> +	AR_LIBFILE_OPT=""
> +	BOEHMGC_MAKEFILE=""
> +
> +	AC_PROG_RANLIB
> +fi

The settings for "LIB_PREFIX" here look the wrong way around.

> --- compiler/llds_out.m	2000/05/10 18:06:26	1.143
> +++ compiler/llds_out.m	2000/06/07 16:04:10
> @@ -31,6 +31,9 @@
>  		io__state, io__state).
>  :- mode output_llds(in, in, in, di, uo) is det.
>  
> +:- pred output_c_file_intro_and_grade(string, string, io__state, io__state).
> +:- mode output_c_file_intro_and_grade(in, in, di, uo) is det.
> +
>  	% output_rval_decls(Rval, FirstIndent, LaterIndent, N0, N,
>  	% DeclSet0, DeclSet) outputs the declarations of any static constants,
>  	% etc. that need to be declared before output_rval(Rval) is called.
> @@ -415,9 +418,6 @@
>  	;
>  		io__write_string("#include ""mercury_imp.h""\n")
>  	).
> -
> -:- pred output_c_file_intro_and_grade(string, string, io__state, io__state).
> -:- mode output_c_file_intro_and_grade(in, in, di, uo) is det.
>  
>  output_c_file_intro_and_grade(SourceFileName, Version) -->

If you're exporting that, then you should document it
(what it does, and what the parameters are).

> Index: compiler/mlds_to_c.m
> @@ -753,13 +758,21 @@
>  :- mode mlds_output_initializer(in, in, di, uo) is det.
>  
>  mlds_output_initializer(_Type, Initializer) -->
> -	( { Initializer = no_initializer } ->
> -		[]
> -	;
> +	( { mlds_needs_initialization(Initializer) = yes } ->
>  		io__write_string(" = "),
>  		mlds_output_initializer_body(Initializer)
> +	;
> +		[]
>  	).
>  
> +:- func mlds_needs_initialization(mlds__initializer) = bool.
> +
> +mlds_needs_initialization(no_initializer) = no.
> +mlds_needs_initialization(init_obj(_)) = yes.
> +mlds_needs_initialization(init_struct([])) = no.
> +mlds_needs_initialization(init_struct([_|_])) = yes.
> +mlds_needs_initialization(init_array(_)) = yes.
> +
>  :- pred mlds_output_initializer_body(mlds__initializer, io__state, io__state).
>  :- mode mlds_output_initializer_body(in, di, uo) is det.
>  

That change is not documented in the log message.
I don't see why it is needed.

> @@ -772,7 +785,16 @@
>  	io__write_string("}").
>  mlds_output_initializer_body(init_array(ElementInits)) -->
>  	io__write_string("{\n\t\t"),
> -	io__write_list(ElementInits, ",\n\t\t", mlds_output_initializer_body),
> +	(
> +		{ ElementInits = [] }
> +	->
> +			% The MS VC++ compiler only generates a symbol, if
> +			% the array has a known size.
> +		io__write_string("NULL")

How do you know that `NULL' will be a valid value for the array
element type?

> Index: compiler/passes_aux.m
...
> +invoke_shell_command(Command0, Succeeded) -->
> +	{
> +		use_win32
> +	->
> +		string__append_list(["bash -c '", Command0, " '"], Command)

Why `bash' rather than `sh'?

> +	% Are we compiling in a win32 environment?
> +:- pred use_win32 is semidet.
> +:- pragma c_code(use_win32,
> +	[will_not_call_mercury],

You should declare that as `thread_safe' too.

> Index: scripts/mgnuc.in
> ===================================================================
> RCS file: /home/mercury1/repository/mercury/scripts/mgnuc.in,v
> retrieving revision 1.68
> diff -u -r1.68 mgnuc.in
> --- scripts/mgnuc.in	1999/12/21 09:56:45	1.68
> +++ scripts/mgnuc.in	2000/06/07 16:04:50
> @@ -366,7 +366,11 @@
>  		# about using possibly uninitialized variables;
>  		# there's no easy way to supress them except by
>  		# disabling the warning.
> -		CHECK_OPTS="$CHECK_OPTS -Wno-uninitialized"
> +		case "$CC" in
> +	    		*gcc*)
> +				CHECK_OPTS="$CHECK_OPTS -Wno-uninitialized"
> +			;;
> +		esac

It would be better to change that test to

	case "$COMPILER" in
		gcc)

That way, the (flawed) assumption that you're using GNU C
iff the compiler's name contains the letters "gcc" remains
in just one spot, rather than being duplicated here.

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