Allow generic foreign functions.

[rpjohnst]

This allows for things like this:

extern "C" fn callback<T>(t: T) { /* ... */ }
extern "C" {
    fn take_callback(c: extern fn(i32));
}

and later:

take_callback(callback::<i32>);

Closes #12502.

[alexcrichton]

cc #10353, in particular this comment:

Current thinking is to put in a proper error message now (as in, don't add support in the short term for 1.0).

Then post 1.0 we can backward-compatibly extend rustc to add support for this.

That being said, I believe times have changed such that an extern fn is now a first-class type instead of having the type *u8 (in the very olden days), and I've certainly found this to be useful from time to time.

One common use of extern fn is to attach the #[no_mangle] attribute to it, however, and I believe that this implementation may lead to surprising results. Could you add a test to make sure nothing too surprising happens?

[alexcrichton]

This doesn't seem to be used from test.rs, could you be sure to use it and the struct?

[huonw]

Maybe #[no_mangle] should just be illegal on a generic function? (What do we do now for non extern generic functions?)

[rpjohnst]

Currently, #[no_mangle] is ignored on generic functions. With this patch, that behavior is preserved so #[no_mangle] works on non-generic externs and is ignored on generic externs. It would be nice to have an error or warning in the cases it's currently ignored, but that's probably a separate issue.

I've pushed a new commit that (hopefully) addresses @alexcrichton's comments above, adding a test that non-generic functions (extern and non) can be successfully #[no_mangle]d, and using testcrate::foo in test.rs (it looks like it already was, but without the testcrate:: ...?)

[sfackler]

If it's ignored "properly", it'll at least trigger the unused attribute warning.

[rpjohnst]

It's ignored improperly in both cases right now. I've filed bug #15844.

[alexcrichton]

Could you add a test to ensure that this works as expected?

#[no_mangle]
extern fn foo<T: Int>(a: T, b: T) -> T { a + b }

fn main() {
    assert_eq!(99u8, foo(255u8, 100u8));
    assert_eq!(99u16, foo(65535u16, 100u16));
}

In an ideal world it would yield a warning that the #[no_mangle] attribute is unused, but I want to make sure that we don't ICE or cause linker errors in today's world. It's ok if it doesn't yield a warning just yet.

[sfackler]

I'll look into the lack of a warning.

[lilyball]

Travis failure looks legit. 14 run-pass tests either crashed the compiler or, interestingly, failed to invoke the linker due to memory exhaustion.

[alexcrichton]

That actually happens pretty frequently, it's safe to ignore. The travis builders don't necessarily have a lot of umph behind them.

[lilyball]

@alexcrichton If it was just memory, sure. But a bunch of tests actually crashed with SIGILL, which AFAIK is not a known expected failure case of Travis.

[sfackler]

An OOM will result in a SIGILL since it calls the abort intrinsic.

[lilyball]

Without printing anything to stdout? Oh. That's kind of surprising. Also, I was under the impression that allocation failure resulted in task failure, but I don't know where I got that idea from.

[thestinger]

Task failure requires memory allocation.

[brson]

I recall us not wanting generic types in extern fns. I'd like to have more light on this issue before merging.

[emberian]

@brson The only situation it's useful in is passing a callback to a C function. It needs to have the right ABI, but you also want to be able to pass in a (fully instantiated, of course) pointer to a generic function.

One workaround that works today is extern "C" fn foo_for_int(...) { foo::<int>() }. Not nice, of course...

[rpjohnst]

I just realized I've got multiple commits here where the earlier ones don't fully implement the feature. What's the policy on that? Do I need to squash them?

[nikomatsakis]

I think it's fine for extern "C" fn's declared in Rust to be generic, but:

• It should be an error to combine a generic fn with #[no_mangle], since we always mangle the name anyway.
• I'd like to be sure that there are still tests ensuring that you can't reference an "external" fn (one not defined in the crate)
• What happens if you reference an external Rust fn that has generics, is that supported? (Tested?)

[alexcrichton]

@nikomatsakis, just to clarify, this is what you're thinking, right?

extern foo<T>(t: T) {} // ok

#[no_mangle]
extern foo2<T>(t: T) {} // error

extern {
    fn bar<T>(); // error
}

fn main() {
    // ok
    let _f: extern fn(int) = foo;
    let _f: extern fn(i16) = foo;

    // ok
    fn inner<T>(f: extern fn(T), t: T) { f(t) }
}

@rpjohnst it sounds like we'd like to move forward with this, but we're a little hesitant about the amount of testing here. We want to try to think of all the fun ways these can pop up and test them ahead of time to make sure nothing goes awry.

[rpjohnst]

I reorganized the commits and added a few more tests that should cover all the mentioned cases now. The failed test is run-pass/spawn-stack-too-big, which is entirely unrelated and passes on my machine- is that a Travis failure rather than a test failure?

[lilyball]

It's a Travis issue that all recent PRs have been hitting.

[alexcrichton]

Thanks @rpjohnst, this looks great!

[rpjohnst]

Do make check and Travis not do the run-make tests? The cross-crate generic extern is causing a fail in run-make/extern-fn-generic that didn't show up until bors (actually bors errored with missing pubs but fixing that exposed this error).

It looks like I need to skip generics in metadata::encode::encode_reachable_extern_fns and make sure the ABI information gets stored correctly in wherever it encodes generics.

[alexcrichton]

Travis does not execute the run-make tests, but make check should.

[rpjohnst]

It seems make check was not running the run-make tests on my machine when I used -j. The tests should all be fixed now.

[rpjohnst]

The logs end with this:

test task::task_abort_no_kill_runtime ... ok

command timed out: 3600 seconds without output, attempting to kill

task_abort_no_kill_runtime has a call to timer::sleep(1000) but that shouldn't go 3600 seconds and it looks like it completed okay anyway. The next output on my machine (also linux 64-bit) is a bunch of ignored tests before it moves on to test libgreen.

Broken bot?