Extern/native function reform #3678

Closed
nikomatsakis opened this Issue Oct 6, 2012 · 14 comments

Projects

None yet

8 participants

@nikomatsakis
Contributor

Per some meeting or other, we discussed a plan to reform how how C functions work. I have been laying some of the groundwork for this but I think there is no meta-bug discussing it and bringing together the various things that need to be done.

The plan is:

  • Add a type extern "abi" fn(S) -> T where abi is one of the Rust-supported ABIs, and defaults to C. (#3321)
  • Remove the current extern fn type (#3320) and extern fn (crust fn) declarations
    • Allow bare fns to be inferred to extern fn type; in trans, we generate a wrapper at that time
  • Remove the wrappers in trans that we currently use to adapt C functions to Rust ABI
    • When calling a function of extern fn type, we will generate inline the C-abi style invocation, perhaps generating wrappers
@brson brson was assigned Oct 30, 2012
@nikomatsakis nikomatsakis added a commit to nikomatsakis/rust that referenced this issue Feb 7, 2013
@nikomatsakis nikomatsakis Make ~fn non-copyable, make &fn copyable, split barefn/closure types,
correct handling of moves for struct-record update.

Part of #3678.  Fixes #2828, #3904, #4719.
a32498d
@bors bors added a commit that referenced this issue Feb 7, 2013
@bors bors auto merge of #4810 : nikomatsakis/rust/owned-fn-noncopyable, r=nikom…
…atsakis

Part of #3678.  Fixes #2828, #3904.

r? @brson
3764cfb
Contributor
brson commented Feb 14, 2013

There have been more recent discussions on this topic: https://mail.mozilla.org/pipermail/rust-dev/2013-January/003013.html

Contributor
brson commented Mar 1, 2013

Completing the crust function change likely requires implementing #4479 - when making the foreign call we need to set up the stack pointer correctly so that, when foreign code calls the crust function, it can resume calculating stack boundaries correctly.

Contributor

Some notes on the plan of attack:

[22:19:20] <nmatsakis> so it seems to me that we can:
[22:19:38] <nmatsakis> (1) implement a prepare_extern_call which is a no-op
[22:19:41] <nmatsakis> (2) implement the lint mode
[22:19:51] <nmatsakis> (3) insert the calls to prepare_extern_call
[22:20:02] <nmatsakis> (4) change prepare_extern_call to do the stack switch and change the call itself to do nothing
[22:20:23] <nmatsakis> -- or rather, remove the wrappers etc
[22:20:37] <nmatsakis> (5) change prepare_extern_call to just reserve a chunk of memory
[22:20:49] <nmatsakis> (6) have a beer
[22:20:59] <nmatsakis> by which I mean: implement a painful and annoying deriving mode
[22:21:03] <nmatsakis> does that sound about right?
[22:23:05] <brson> about, but you may hit problems with those reentrant crust functions between 4 & 5 when prepare_extern_call tries to switch stacks again
[22:23:28] <nmatsakis> right, so prepare_extern_call should reproduce the current smart behavior
[22:23:30] <nmatsakis> in its first iteration
[22:24:10] <brson> that behavior probably depends in some way on the assumption that crust functions do a stack switch, which will no longer be true, so it may just need to change
[22:24:32] <brson> or it may just work :)
[22:26:33] <nmatsakis> ok, this sounds like a plan.
[22:26:39] <nmatsakis> we'll see how it goes :)
@bors bors added a commit that referenced this issue Mar 20, 2013
@bors bors auto merge of #5445 : nikomatsakis/rust/issue-3678-refactor-trans_cal…
…l, r=graydon

Refactor trans_call to separate out the translation of the arguments, environment, and return pointer.  Towards #3678.  r? @brson
99ac243
@nikomatsakis nikomatsakis added a commit to nikomatsakis/rust that referenced this issue Mar 25, 2013
@nikomatsakis nikomatsakis Add AbiSet and integrate it into the AST.
I believe this patch incorporates all expected syntax changes from extern
function reform (#3678). You can now write things like:

    extern "<abi>" fn foo(s: S) -> T { ... }
    extern "<abi>" mod { ... }
    extern "<abi>" fn(S) -> T

The ABI for foreign functions is taken from this syntax (rather than from an
annotation).  We support the full ABI specification I described on the mailing
list.  The correct ABI is chosen based on the target architecture.

Calls by pointer to C functions are not yet supported, and the Rust type of
crust fns is still *u8.
063acf7
@brson brson added a commit to brson/rust that referenced this issue Mar 30, 2013
@nikomatsakis @brson nikomatsakis + brson Add AbiSet and integrate it into the AST.
I believe this patch incorporates all expected syntax changes from extern
function reform (#3678). You can now write things like:

    extern "<abi>" fn foo(s: S) -> T { ... }
    extern "<abi>" mod { ... }
    extern "<abi>" fn(S) -> T

The ABI for foreign functions is taken from this syntax (rather than from an
annotation).  We support the full ABI specification I described on the mailing
list.  The correct ABI is chosen based on the target architecture.

Calls by pointer to C functions are not yet supported, and the Rust type of
crust fns is still *u8.
6965fe4
Contributor
brson commented Mar 31, 2013

I think we're as far as we are going to get for 0.6. De-milestoning.

Contributor
erickt commented Apr 9, 2013

Will this allow extern fn functions to be inferred with the |foo| { ... } syntax? fread2281 and I were talking about functions on irc, and he mentioned he wanted to write a function like this:

fn do_spawn_stuff(f: ~fn(&str)) {
    for 5.times {
        do task::spawn {
            f("Stuff");
        }
    }
}
fn main() {
    do_spawn_stuff(|_s| { });
}

Rustc properly errors out because that unique ~fn could be capturing variables, so it can't be shared between multiple tasks. Instead, this works:

fn do_spawn_stuff(f: extern fn(&str)) {
    for 5.times {
        do task::spawn {
            f("Stuff");
        }
    }
}
fn foo(_s: &str) { }
fn main() {
    do_spawn_stuff(foo);
}

Unfortunately we can't use the closure sugar because we get this error:

bar.rs:14:18: 14:27 error: mismatched types: expected `extern "Rust" fn(&str)` but found `&fn(<V0>)` (expected extern fn but found fn)
bar.rs:14     do_spawn_stuff(|_s| { });
                            ^~~~~~~~~
bar.rs:14:18: 14:27 error: Unconstrained region variable #0
bar.rs:14     do_spawn_stuff(|_s| { });
                            ^~~~~~~~~
Contributor

@erickt we could make this work, yeah.

@nikomatsakis nikomatsakis added a commit to nikomatsakis/rust that referenced this issue May 7, 2013
@nikomatsakis nikomatsakis Lift restriction on calling extern C functions, and propagate
the ABI as part of the type of an extern fn. cc #3678
387d6c5
@bors bors added a commit that referenced this issue May 8, 2013
@bors bors auto merge of #6309 : nikomatsakis/rust/issue-3678-type-of-extern-fns…
…, r=nikomatsakis

Lift restriction on calling extern C functions, and propagate the ABI as part of the type of an extern fn. cc #3678
1d7a136
Contributor
graydon commented Jul 11, 2013

@nikomatsakis can we get a status update on this? What's left to do here?

Contributor

This is basically implemented, but blocked for months now on a windows issue that I have not had the time to investigate. The patch that I implemented removes all wrappers and causes callers of extern "C" functions to request a large stack frame. One issue is that after a recent rebase I started encountering problems in the threadring test, presumably because we were creating large stack frames too frequently. I have not tracked down the source of this problem but I believe it is either because the spawn function itself invokes a C function or because ~ allocation directly invokes malloc.

Member

Any word on this? It is very important for getting us some great OpenGL bindings.

Contributor
bstrie commented Aug 6, 2013

Echoing @bjz's question. I don't even see this in the bors queue anymore.

Contributor
graydon commented Aug 15, 2013

visiting for triage. apparently any day now. Properly classified, nominating for back-compat or possibly well-defined. Though I imagine it'll land before we've got it accepted/rejected.

Contributor
graydon commented Aug 15, 2013

accepted for backwards-compatible milestone

@bors bors added a commit that closed this issue Aug 19, 2013
@bors bors auto merge of #8535 : nikomatsakis/rust/issue-3678-wrappers-be-gone-2…
…, r=graydon

Long-standing branch to remove foreign function wrappers altogether. Calls to C functions are done "in place" with no stack manipulation; the scheme relies entirely on the correct use of `#[fixed_stack_segment]` to guarantee adequate stack space. A linter is added to detect when `#[fixed_stack_segment]` annotations are missing. An `externfn!` macro is added to make it easier to declare foreign fns and wrappers in one go: this macro may need some refinement, though, for example it might be good to be able to declare a group of foreign fns. I leave that for future work (hopefully somebody else's work :) ).

Fixes #3678.
81a7816
@bors bors closed this in 81a7816 Aug 19, 2013
@toddaaro toddaaro added a commit to toddaaro/rust that referenced this issue Aug 20, 2013
@toddaaro toddaaro Revert "Issue #3678: Remove wrappers and call foreign functions direc…
…tly"

This reverts commit 303f650.
e3ee974
@nikomatsakis nikomatsakis added a commit to nikomatsakis/rust that referenced this issue Aug 21, 2013
@nikomatsakis nikomatsakis Change type of extern fns from `*u8` to `extern "ABI" fn` 82a9abb

It appears that this isn't complete yet: linking to foreign functions with a "Rust" or "RustCall" ABI is not implemented. See here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment