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

[Peter Ross]

Fergus wrote:
> 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.
>
Not to me.
With MS you need to explicitly name the library

link xxx.o xxx_init.o libmer_rt.lib

while with gcc

gcc xxx.o xxx_init.o -lmer_rt

so you don't need the prefix.

Note that currently nothing uses these, but I just copied all the values
that were set for the ARGO project, as configure takes a long time to
run under Windows, so I put them all in!

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

How about

Outputs a standard header which contains all the

> > 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.
>
Originally no_initializer was the only case which didn't require
initialization.  I then decided empty structures and empty arrays
also don't require initialization.  So I decided to create a predicate
which explicitly showed all the options, rather then individually
adding them to the if case. I  latter had to change to outputing
something for the case of an array with no initialization, as this
caused undefined symbol problems.

I thought of this change as stylistic, and should have mentioned
that we no longer output '= {}' for empty structures.


> > @@ -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?
>
I don't! However from what I can gather, if an array has no initializer
then it is meant to have 0 size, so no code to ever access it should be
generated. I just needed to put something in there, so that the MSVC
compiler would generate a symbol.

What do you suggest I put here?
I guess the easiest option would be to make sure that an empty
initializer is never generated, by creating a dummy initializer instead.

> > Index: compiler/passes_aux.m
> ...
> > +invoke_shell_command(Command0, Succeeded) -->
> > + {
> > + use_win32
> > + ->
> > + string__append_list(["bash -c '", Command0, " '"], Command)
>
> Why `bash' rather than `sh'?
>
Should be sh, will change it.

> > + % 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.
>
Will do.
> > 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.
>
Done.

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