-
Notifications
You must be signed in to change notification settings - Fork 13.3k
ICE on stable and nightly: [codegen_fulfill_obligation] checking if std::ops::FnOnce
fulfills its obligations
#80351
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
std::ops::FnOnce
fulfills its obligationsstd::ops::FnOnce
fulfills its obligations
Thanks for minimizing and checking on both stable and nightly! It's very helpful :) |
Looks like this might be a duplicate of #80291. |
Yeah, seems likely. |
Interesting. Yeah, that looks surprisingly similar, although my usecase is maybe a bit different. I want to box and store the closure instead of executing it directly. For that reason, I don't think shifting around the lifetime bounds as mentioned in that discussion would work for me. In case it helps, some additional context why this came up. My actual code is closer to this definition:
As you can see, the Query result is supposed to reference into the Source. At least intuitively, it seems like the higher ranked bounds are necessary to make this sound: Since the boxed Fn is only executed much later, neither Q nor F are allowed to care about the concrete lifetime of the Source parameter. BTW, I didn't write the "Query" trait myself. It's actually this trait from the Legion crate, which is an entity-component system I use. |
Assigning |
At a cursory look it seems to this ICE started after 1.26 |
@apiraino If I use bare
|
Issue: rust-lang/rust#80351
I seem to have run into the same issue. Here is a another minification that should reproduce this error. pub fn main() {
f::<(), _>(|_| {});
}
fn f<S, P>(p: P)
where
P: for<'r> FnOnce(<S as Trait<'r>>::Item),
S: for<'r> Trait<'r>,
{
p(None.unwrap())
}
pub trait Trait<'a> {
type Item;
}
impl<'a, S> Trait<'a> for S {
type Item = ();
} |
I hit this in the wild too. Independently reduced to: trait Viewable<'a> {
type View;
}
impl<'a> Viewable<'a> for u64 {
type View = u64;
}
fn main() {
let _f: for <'a> fn(<u64 as Viewable<'a>>::View) = |_| {};
} |
Pretty error message:
I guess this checking code doesn't understand that |
Apparently, there are two separate times rustc has to look up trait impls:
So, in this case, I guess, we correctly get through all the way to codegen, but then we fail to unify |
See "Trait resolution" in the rustc-dev guide. In |
Some debug output, but it's only what anyone who actually knows this code would already expect:
|
This call to
And we do end up here:
|
The reason typeck succeeds is that it calls
Before that call, Afterwards, it is |
OK, recap time. The Story So Far: trait Viewable<'a> {
type View;
}
impl<'a> Viewable<'a> for u64 {
type View = u64;
}
fn main() {
let _f: for<'a> fn(<u64 as Viewable<'a>>::View) = |_| {};
} Type checking succeeds, but we hit an ICE during codegen, due to failure to select an impl for the obligation This means there is a mismatch between how typeck solves traits and how codegen selects trait impls: typeck was satisfied the impl exists, but codegen can't find it. More specifically: I think typeck realizes |
During typeck, the stack is (most recent call last):
During codegen, the stack is:
So both phases try to normalize. The difference is:
Note the difference in |
@wesleywiser pointed out that the snippet of log under "During codegen" above doesn't seem to correspond to the "During typeck" snippet. Here's the broader context during codegen:
|
#85499 will probably fix this. |
@jorendorff excellent analysis here! You hit the nail on the head on the problem here. Indeed #85499 will fix this. I'm going to close this issue as a duplicate of #62529. There are lots of examples and MCVEs of this. |
Should we add all these minimal examples as tests on the PR then ? The repro code above currently ICEs with #85499:
|
Thanks @lqd. There's over a dozen issues that are basically duplicates of #62529; I added several of the MCVEs in the PR itself, but ultimately they're generally testing the same thing. Seems like this has an extra twist to it that will need further debugging (something I haven't seen for the other MCVEs). I'm going to go ahead and assign this to myself to follow up on after #85499 lands. |
@jorendorff if you still want to help out here, feel more than free to checkout #85499 and run the repro. I imagine we're probably just missing a If you have questions, I can answer questions or chat on zulip or here. (If you aren't interested anymore or don't have time, no worries!) |
Fixed by #85499, closing given that the PR adds a test with the same behavior |
I encountered the following ICE while working on a task queue where the tasks can query for different resources.
Below is a small example. The ICE doesn't seem to be triggered by dead code, which is why I included a main function.
Edit: The following output is from nightly, but I get the same error on stable (rustc 1.48.0 (7eac88a 2020-11-16) running on x86_64-pc-windows-msvc).
Edit 2: I was able to simplify the code quite a bit further. I hope it's close to minimal now.
Code
Meta
rustc --version --verbose
:Error output
cargo build
:Backtrace
Output with
RUST_BACKTRACE=full
:The text was updated successfully, but these errors were encountered: