[MLton] The evils of importing as 'extern'

[Matthew Fluet]

>> I see the issue with the prototypes for WordS<N>_quot, but I wonder if this 
>> is the right solution.  It seems to me that there must have been some reson 
>> not to include "platform.h" in "c-chunk.h", which is where all the FFI 
>> declarations used to live.  On the other hand, including "basis-ffi.h" 
>> would ensure that MLton emits a declaration that is consistent with the 
>> actual runtime function.
>
> The consistency check seems like a good thing to me.

I tend to agree; and I agree that the names exported by "basis-ffi.h" are 
essentially reserved for MLton's use anyways (that is, they exist in 
libmlton.a).

> If we have the information about the type, I don't see the harm in using it. 
> Declaring it as type int, when it might not be, seems strictly suboptimal if 
> we really have the type. From what I remember of our previous conversation, 
> the focus was about what to do with _address, as we went with no type 
> information in the signature. Fortunately, the basis doesn't use _address.
>
> Using the correct type does break the amusing hack where you import _symbol 
> twice as two different c-types. Now, you would get a type conflict in the 
> c-codegen. However, hopefully with the newly improved C FFI, we wouldn't ever 
> need to do this.

I think I'd argue with you: the C-codegen should emit a type-decorated 
extern for _symbol objects, but should emit an undecorated extern for 
_address objects.  If you want to peek and poke at an object at different 
C-types, then you need to get it via _address an construct your own 
getter/setter functions with MLton.Pointer.* functions.

In the discussion, I had argued that is seemed odd that
   _address "name": cPtrTy, cBaseTy;
would elaborate to cPtrTy and completely ignore the cBaseTy (other than 
saving it to be emitted in the C-codegen).  Later discussion moved the FFI 
syntax so that the annotation was always equal to the elaborated type, so 
this argument becomes even stronger.

Not it seems equally odd that _symbol doesn't save the type info for the 
C-codegen.

> Regarding these warnings:
>>> I do get these warnings now (new):
>>> ../runtime/basis-ffi.h:1005: warning: 'WordS16_quot' used but never 
>>> defined
>>> ../runtime/basis-ffi.h:1006: warning: 'WordS16_rem' used but never defined
>> 
>> This is presumably a consequence of r4564, which allows the C-side 
>> implementation of some primitives to be shared between libmlton.a (from 
>> which they are almost never used, but are there as a fallback), the 
>> C-codegen (which uses them as static inlines) and the bytecode-codegen 
>> (which also uses them as static inlines in the bytecode interpreter).
>
> I still don't know why they happen. I suspect it's just a bug in apple's gcc. 
> They come from compiling interpret.c with -O2. With -O1, there's no warning.

I think I fixed this as part of r4581.  Before that commit, interpret.c 
was pulling in c-chunk.h, which was hiding the static inline definition of 
WordS16_quot.  Because interpret.c includes platform.h, it was also 
getting basis-ffi.h, and hence had a static inline declaration for 
WordS16_quot.  So, gcc was rightly complaining about seeing a declaration 
for a static inline function, but no definition.

> With the attached patch, I've completed a full build of MLton, and am most of 
> the way through the regression suite without event. I'm going to sleep now, 
> but if something goes wrong, I'll post it here tomorrow. Let me know if you 
> think the patch is ok to commit.

I think the patch looks great.

After that patch, we can add "basis-ffi.h" to "c-chunk.h" and revise the 
MLTON_CODEGEN_WORDSQUOTREM macro.  (There won't be any need for the 
C-codegen to generate the declaration.)