[MLton] Re: [MLton-user] SVN r6941 MLton/MinGW32 and FFI

[Wesley W. Terpstra]

On Tue, Nov 11, 2008 at 7:04 PM, Matthew Fluet <fluet at tti-c.org> wrote:
> With regards to whether users should actually use GC_getArrayLength, we
> don't really advertise it, but neither is it a particularly subtle
> operation.  And, it is genuinely useful (though, getting the length and
> passing it through the FFI along with the array pointer wouldn't been
> terribly difficult either).  I don't think there is any way to really
> distinguish between visible functions in libmlton.a that are available for
> use in mlton generated .c and .s files, but not available for user .c code.

Well, I personally have always passed the length of arrays with the
array through the FFI. This is the usual way C functions work in my
experience. I was genuinely surprised to see GC_arrayLength used as I
thought GC_* was supposed to be MLton internal. There are some damn
scary functions available with the GC_ prefix! It sounds from your
later comments that you agree with me, so let's just phase
GC_arrayLength out of the examples and FFI documentation. It still
works within a DSO since it is a PRIVATE symbol at the moment, so
there's no backwards compatibility problem.

>> 2. iimport.c uses dlopen/dlclose/dlsym. platform/mingw.c defines
>> these... which is a problem since the runtime is statically linked,
>> but libdl is dynamically linked. Perhaps these should be public
>> libname_ symbols, private MLton_dlopen/close/sym, or both?
>
> To clarify: the issue is that on *NIX platforms (including MacOSX),
> dl{open,close,sym,error,...} are pulled from dynamically linked shared
> libraries.  That is, they are imported from another DSO.  Thus, for these
> platforms, the 'external' attribute should be used on the _import.
>
> MinGW doesn't provide dl{open,close,...} in a (standard) dynamically linked
> shared library.  Vesa added rudimentary emulation in
> <src>/runtime/platform/mingw.c (r4862, 20061127); thus dl{open,close,...}
> are pulled from libmlton.a.  That is, they are imported from the same DSO.
> Thus, for this platform, the 'private' attribute should be used on the
> _import.

Yes, that's the problem (though they are public in mingw.c, not private).

> Personally, I think that MLton should not be attempting to provide
> non-essential functionality that is missing on a platform.

I agree, with the added provision that if MLton needs these functions
itself it implement them, but not export them. A good example being
the mingw.{ch} which adds missing functionality as MLton_* (b/c of
#define's in mingw.h). Several existing files in platform/*.c don't do
this, which IMO is bad (aix.c, float-math.c, hpux.c, mkdir2.c,
setenv.putenv.c, and solaris.c).

> MLton doesn't
> default to including those link options, because they are not essential
> functionality (e.g., required by some of the Basis Library implementation).)

So why were the dl* functions added to mingw.c in the first place? The
only reason I suggested keeping them (and the GC_arrayLength stuff)
around was because I thought there was a reason for them that I didn't
know.

> - MLton shouldn't be providing dl{open,close,...} in any form.

If the basis doesn't use them, then I agree and further suggest they
be removed from mingw.c.

> - GC_getArrayLength isn't advertised, so there is no need to 'bless' it
>  with a MLton_getArrayLength name.  (We should eliminate its use from the
>  <src>/doc/examples/ffi examples.)

Be aware that even the wiki advertises this function at the momeny.
Thus, I thought this was (shockingly) supposed to be part of the
documented FFI. Phasing it out seems a great solution to me.

> It might be a good idea to clone the page (say, as
> NextRelease/ForeignFunctionInterfaceSyntax) to make current documentation
> available somewhere.

Sounds like a plan.

Regarding public/private, I can no longer recommend changing the
default to public. I recall my reasoning now: external is the safe
default on all platforms except MinGW. On all these other platforms it
works, and works correctly. If public were the default, it might be
more convenient for MinGW, but would in some corner cases break
(variables and pointers). Setting the default to external makes it
absolutely clear that the code is in error (and dangerous) on MinGW as
it fails loudly with a link error.