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

[Fergus Henderson]

On 13-Jun-2000, Peter Ross <peter.ross at miscrit.be> wrote:
> 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.

Oh, I see.

LIB_PREFIX is not the prefix on the name of the
library file, it is the prefix that you need on the link command.
That's OK, but it's definitely somewhat confusing (at least to me),
so I think it ought to be documented.

In addition, I think that on Windows the .dll files ought to be
named just `foo.dll', not `libfoo.dll'.  But that can be a
separate change.

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

ANSI/ISO C does not allow empty arrays or empty structures at all,
and some compilers (e.g. lcc) enforce this.  So really what we
ought to do is to ensure that empty arrays and empty structs
are not generated.

There's two ways of doing that; either we could ensure that the
MLDS code generator never generates empty structs/arrays, or we could
check for empty structs/arrays in mlds_to_c.m and output non-empty
definitions for them.

Note that this is specific to ANSI/ISO C; many other languages do
allow them (e.g. GNU C allows both empty structs and empty arrays,
and ANSI/ISO C++ allows empty structs).

So for now at least I think mlds_to_c.m is probably the easiest
place to handle it.

Anyway, what you have here is OK for now;
I suggest you just put an XXX comment there
(e.g. "XXX ANSI/ISO C does not allow empty arrays or empty structs;
what we do for them here is probably not quite right.")


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

Well, putting `0' would be better than putting `NULL'; that would at
least be guaranteed to work for integer and floating point fields too,
not just pointers.


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