[MLton] The evils of importing as 'extern'
Wesley W. Terpstra
wesley@terpstra.ca
Thu, 25 May 2006 02:08:44 +0200
--Apple-Mail-3-557316683
Content-Transfer-Encoding: 7bit
Content-Type: text/plain;
charset=US-ASCII;
delsp=yes;
format=flowed
On May 24, 2006, at 6:14 PM, Matthew Fluet wrote:
> 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.
As for not including platform.h, isn't there a lot more than just the
basis-ffi that is pulled in by this header? Furthermore, you couldn't
have included it before because the type information conflicted with
the c-codegen.
> Possibly, although I'd want to review the discussion we had.
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.
> Another solution would be to have Word-ops.h emit the declaration
> for WordS<N>_{quot,rem} when used with the C-codegen.
Or split the functions and variables of basis-ffi.h into two files. I
don't mean to imply that adding the type is the only solution. It
just seems to me that it's a good thing to emit anyways, and fixes
the problem, so I did it.
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.
On May 25, 2006, at 1:09 AM, Stephen Weeks wrote:
>> 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.
>
> The reason is that we want to keep the namespace pollution to a
> minimum in generated C files, which include c-chunk.h. Generated C
> files include names that come from user code via _import declarations,
> so including platform.h would greatly increase the chance of
> conflicts.
I am assuming that this doesn't extend to primitive names, seeing as
how they must are already used by the c-codegen? ie: including basis-
ffi.h doesn't pose a problem?
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.
--Apple-Mail-3-557316683
Content-Transfer-Encoding: 7bit
Content-Type: application/octet-stream;
x-unix-mode=0644;
name="typed-externs.patch"
Content-Disposition: attachment;
filename=typed-externs.patch
Index: mlton/atoms/prim.sig
===================================================================
--- mlton/atoms/prim.sig (revision 4578)
+++ mlton/atoms/prim.sig (working copy)
@@ -36,7 +36,7 @@
| Exn_setExtendExtra (* implement exceptions *)
| Exn_setInitExtra (* implement exceptions *)
| FFI of 'a CFunction.t (* ssa to rssa *)
- | FFI_Symbol of {name: string} (* codegen *)
+ | FFI_Symbol of {name: string, cty: CType.t option} (* codegen *)
| GC_collect (* ssa to rssa *)
| IntInf_add (* ssa to rssa *)
| IntInf_andb (* ssa to rssa *)
@@ -216,7 +216,7 @@
deWeak: 'b -> 'b,
result: 'b} -> 'b vector
val ffi: 'a CFunction.t -> 'a t
- val ffiSymbol: {name: string} -> 'a t
+ val ffiSymbol: {name: string, cty: CType.t option} -> 'a t
val fromString: string -> 'a t option
val gcCollect: 'a t
val intInfEqual: 'a t
Index: mlton/atoms/prim.fun
===================================================================
--- mlton/atoms/prim.fun (revision 4578)
+++ mlton/atoms/prim.fun (working copy)
@@ -46,7 +46,7 @@
| Exn_setExtendExtra (* implement exceptions *)
| Exn_setInitExtra (* implement exceptions *)
| FFI of 'a CFunction.t (* ssa to rssa *)
- | FFI_Symbol of {name: string} (* codegen *)
+ | FFI_Symbol of {name: string, cty: CType.t option} (* codegen *)
| GC_collect (* ssa to rssa *)
| IntInf_add (* ssa to rssa *)
| IntInf_andb (* ssa to rssa *)
@@ -486,7 +486,7 @@
| Exn_setExtendExtra => Exn_setExtendExtra
| Exn_setInitExtra => Exn_setInitExtra
| FFI func => FFI (CFunction.map (func, f))
- | FFI_Symbol {name} => FFI_Symbol {name = name}
+ | FFI_Symbol {name, cty} => FFI_Symbol {name = name, cty = cty}
| GC_collect => GC_collect
| IntInf_add => IntInf_add
| IntInf_andb => IntInf_andb
Index: mlton/elaborate/elaborate-core.fun
===================================================================
--- mlton/elaborate/elaborate-core.fun (revision 4578)
+++ mlton/elaborate/elaborate-core.fun (working copy)
@@ -940,9 +940,10 @@
Type.defaultWord)
fun mkAddress {expandedPtrTy: Type.t,
- name: string}: Cexp.t =
+ name: string,
+ cty: CType.t option }: Cexp.t =
primApp {args = Vector.new0 (),
- prim = Prim.ffiSymbol {name = name},
+ prim = Prim.ffiSymbol {name = name, cty = cty},
result = expandedPtrTy}
fun mkFetch {ctypeCbTy, isBool,
@@ -1038,7 +1039,8 @@
| _ => (error (); ())
val addrExp =
mkAddress {expandedPtrTy = expandedPtrTy,
- name = name}
+ name = name,
+ cty = NONE}
fun wrap (e, t) = Cexp.make (Cexp.node e, t)
in
wrap (addrExp, elabedTy)
@@ -1099,7 +1101,8 @@
| NONE => (error (); CType.word (WordSize.default, {signed = false}))
val addrExp =
mkAddress {expandedPtrTy = Type.word (WordSize.pointer ()),
- name = name}
+ name = name,
+ cty = SOME ctypeCbTy}
val () =
if List.exists (attributes, fn attr =>
attr = SymbolAttribute.Alloc)
@@ -1220,7 +1223,8 @@
val isBool = Type.isBool expandedCbTy
val addrExp =
mkAddress {expandedPtrTy = Type.word (WordSize.pointer ()),
- name = name}
+ name = name,
+ cty = SOME ctypeCbTy}
fun wrap (e, t) = Cexp.make (Cexp.node e, t)
in
wrap (mkFetch {ctypeCbTy = ctypeCbTy,
Index: mlton/codegen/c-codegen/c-codegen.fun
===================================================================
--- mlton/codegen/c-codegen/c-codegen.fun (revision 4578)
+++ mlton/codegen/c-codegen/c-codegen.fun (working copy)
@@ -476,10 +476,16 @@
case s of
Statement.PrimApp {prim, ...} =>
(case Prim.name prim of
- Prim.Name.FFI_Symbol {name} =>
+ Prim.Name.FFI_Symbol {name, cty} =>
doit
(name, fn () =>
- concat ["extern ", name, ";\n"])
+ concat ["extern ",
+ case cty of
+ SOME x => CType.toString x
+ | NONE => "",
+ " ",
+ name,
+ ";\n"])
| _ => ())
| _ => ())
val _ =
--Apple-Mail-3-557316683
Content-Transfer-Encoding: 7bit
Content-Type: text/plain;
charset=US-ASCII;
delsp=yes;
format=flowed
I'll take a look at ppc/linux tomorrow, assuming the regressions for
ppc/osx continue to pass. I hope that the wrinkles there will all be
gone at this point. ;-)
--Apple-Mail-3-557316683--