Extern fn declarations can have dangling lifetimes

[Manishearth]

extern {
    fn foo() -> &u8;
}

(playpen)

compiles fine. The &u8 has a dangling lifetime, which doesn't compile for functions that have bodies.

This probably should throw an error.

[Manishearth]

Probably would be a breaking change to fix. But soundness.

[eddyb]

Elision was never added to them. cc @rust-lang/lang

[nagisa]

Seems fine to break to me. References in extern blocks is very likely not what you want anyway for non-Rust ABIs (i.e. is a bug).

[Manishearth]

Well, it also happens with lifetime parameter elision. For the servo-gecko stuff we've managed to configure bindgen to return typedefs to types with lifetimes for things which are conceptually borrows. So we're using lifetime params in ffi today 😄

(thats how I discovered this, someone else added a dangling lifetime and I was bewildered by it. We're going to merge it for now because my safer bindings glue isn't complete yet, so the input lifetime for that function can't exist yet, but it will soon)

[nikomatsakis]

Discussed in @rust-lang/lang meeting. This is definitely a bug; we should "just fix it". We should measure the impact, of course.

[nikomatsakis]

triage: P-medium

[eddyb]

I accidentally unified the two codepaths in my lazy-start branch so consider the impact measured:

• liquidfun (error log)
• xcb (error log)

[Mark-Simulacrum]

@eddyb So I presume that we don't currently have the lazy-start branch in tree, since this hasn't been fixed. Perhaps the two codepaths were un-unified?

[eddyb]

Yeah, I redid some things, and tried to avoid regressing anything at all.

[nikomatsakis]

It would probably be good to deprecate (future compat warning?) code using this pattern.

[nikomatsakis]

Well, for sure we should deprecate. It doesn't necessarily have to be a future-compat warning. i.e., we could live with this.

[Centril]

@eddyb @nagisa The snippet in the OP does not compile today... is there anything left to do here?

[kenta7777]

I'd like to tackle this issue. How shall we begin?

[iluuu1994]

Seems fixed, there's a test for it here.

[Centril]

Indeed it seems so; closing therefore.