-
Notifications
You must be signed in to change notification settings - Fork 13.3k
ICE "anonymous bound region BrAnon(0) in return but not args" #43567
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Comments
@petrochenkov Do you mean a patch that would make |
@SimonSapin |
Ok, thanks for looking into it. |
Performance collection for the style crate is also broken because of this. |
cc @nikomatsakis Should we add elision logic to foreign functions? |
#43594 has a simpler testcase that also involves elision. Also, yes, I think elision should work on FFI. Firstly, it used to, and it's a breaking change otherwise. Secondly, this is extremely useful when paired with bindgen to have a modicum of safety wrt returning non-owners pointers from FFI. Dereferenced raw pointers yield values of arbitrary lifetime, and guarding against this in FFI-heavy code is extremely tedious. In an FFI-heavy project the risk of introducing unsafety in the FFI layer can outweigh the risk of just writing it all in C++, largely due to things like this. |
@Manishearth Elision never worked, what you were getting was an unbound lifetime in the return. |
@Manishearth It was even you who reported #35851. I found two crates that would break so we didn't go through with adding elision to them. |
Wait, what? I could swear we've had lifetime errors on this before. Or perhaps they were in nearby FFI wrappers. Hmm. |
Oh, right, I didn't remember that was all elision. Opt in would be okay by me too, really. Then again, we also need this on stable. Can we add elision for the cases where there isn't a function returning a dangling lifetime (and deal with the backcompat hazard separately?). That seems like it can be done a without breaking anyone. As it stands this will break stylo, and it is super nontrivial to fix given the amount of FFI we do. |
I can also try to fix those crates preemptively. |
If anyone wants to test with the no-elision-in-FFI hack turned off, change this |
I think this is not a case of valid elision. If I add a non-FFI function with the same signature it fails to build with a proper error message because there is no implicit lifetime in the function’s parameters. pub fn xcb_screen_allowed_depths_iterator__ (R: *const xcb_screen_t)
-> xcb_depth_iterator_t { unimplemented!() } error[E0106]: missing lifetime specifier
--> src/ffi/xproto.rs:8:16
|
8 | -> xcb_depth_iterator_t { unimplemented!() }
| ^^^^^^^^^^^^^^^^^^^^ expected lifetime parameter
|
= help: this function's return type contains a borrowed value with an elided lifetime, but the lifetime cannot be derived from the arguments
= help: consider giving it an explicit bounded or 'static lifetime |
FWIW the testcase in #43594 is a case of valid elision, even though it's invalid here. |
Actually, it occurs to me that this bound region bug is going to happen anyway and break all of the crates rely on this anyway, so we should just enable elision for extern fns .. ? |
PR to fix rust-xcb: https://github.com/rtbo/rust-xcb/pull/41 |
Fixed liquidfun-rust rjanicek/liquidfun-rust#4 Might be worth a fresh crater run. |
cc @Mark-Simulacrum Cargobomb would be helpful. |
I don't have access to cargobomb myself, but @tomprince and @brson do, I believe. A patch would be useful either way. |
I've ran into the same error. Here's a minimal case: extern "C" {
fn ice(ice: &mut u32) -> &u32;
}
rustc 1.21.0-nightly (1d2a6df 2017-08-03) running on x86_64-apple-darwin |
Fix ICE with elided lifetimes in return type of foreign functions cc #43567 This is for a preliminary crater/cargobomb run. Lifetime elision still produces a late-bound lifetime, but it is caught by the same check as catches `type F = for<'a> fn() -> &'a u8` and reported as an error. If the breakage is significant, I'll have to partially revert #43543 (all the stuff that was required for dealing with late bound lifetimes in this position). r? @eddyb
This also happens on Stylo FWIW. |
(we know, see #43594) |
I've fixed up stylo so that the fix for this issue will play nice with it (and the fix for this issue should be in nightly now) |
It panics on current master with |
Compiles for me on latest nightly :) |
Still failing here: https://travis-ci.org/main--/windows-gaming/jobs/262206955#L1731 |
I’m also still seeing this failure with rustc 1.21.0-nightly (cbbe17a 2017-08-07) (same as @main--) on Servo. @Manishearth, which version did you use, and what did you test? |
Oh, right, I used stable by accident. Oops. Looks like it never landed on master, only try. |
@Manishearth I think @tomprince started the cargobomb run only today. |
Discussed in the compiler team meeting. P-high. |
In the meantime, could someone fix these issues in xcb/stylo/etc by adding explicit lifetimes? |
It should be fixed in stylo. I am unable to fix XCB, they use a pretty weird custom bindings generation setup that uses lifetimes but doesn't carry enough info around for those lifetimes to be used consistently. A maintainer of that crate will have to fix it; I'm just not familiar enough with this custom setup to deal with it. |
( |
This kind of bug is why |
Fix ICE with elided lifetimes in return type of foreign functions cc #43567 This is for a preliminary crater/cargobomb run. Lifetime elision in foreign functions now works exactly like in other functions or function-like entities. If the breakage is significant, I'll have to partially revert #43543 (all the stuff that was required for dealing with late bound lifetimes in this position). r? @eddyb
Fixed in #43651 |
Confirmed that Firefox compiles on Rust Nightly, with #43651. Thanks! |
rustc 1.21.0-nightly (aac223f 2017-07-30) ICEs on https://github.com/rtbo/rust-xcb (full output below), although 1.20.0-beta.1 and some not-much-older nightlies succeed.
Some more context around the line in the error message:
So the
xcb_depth_iterator_t
does have lifetime parameter that’s elided in this function declaration’s return type without any elided lifetime in the parameters. Maybe this code should indeed be rejected, but in that case we need a proper error message rather than a compiler panic. Although it used to be accepted, so this is a breaking change. Adding an explicit lifetime parameter to the function fixes this, but that’s not easy to do properly since this is generated code.Full output:
The text was updated successfully, but these errors were encountered: