[MLton] The evils of importing as 'extern'
Matthew Fluet
fluet@cs.cornell.edu
Wed, 24 May 2006 21:51:30 -0400 (EDT)
>> 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.)