-
Notifications
You must be signed in to change notification settings - Fork 12.9k
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
ICE when existential type is exported from one crate and used in another #53443
Comments
Oops. Yea. I assumed this would work since |
Is the fix just to have rust/src/librustc/middle/resolve_lifetime.rs Line 630 in b559042
where the If you think the fix for this ICE is just that one |
I think they are closely related. We should probably run {
intravisit::walk_ty(self, ty);
return;
} in case |
My real code's existential types do not have lifetimes, though they do have pub existential type GetResponse: Future<Output = crate::Result<crate::Mod>> + 'static;
pub existential type DownloadResponse: Stream<Item = crate::Result<crate::reqwest_async::Chunk>> + 'static; Adding The concrete type for the first one is compiler-created because it's the return type of an async block. pub fn get(&self, mod_name: &factorio_mods_common::ModName) -> GetResponse {
...
async {
....
}
} I believe this is sugar for The second one's concrete type is my own Stream impl that also does not have lifetimes. It'll take me some time to get a repro because something is broken with the rustc I built locally (it started hitting #53469 even though it was fine before, not sure why). If you want you can find the real code at https://github.com/Arnavion/fac-rs/tree/3a6afed4. The existential types are in the |
Rust + factorio? <3
the issue you are hitting is that a |
@oli-obk The #53469 I started hitting was because of two different versions of libsyntax_pos in stage1 builds, as I described here. So with stage2 builds I don't have that problem, So I've got a repro for the issue where
extern crate a;
struct Bar(a::Foo);
fn main() {
} ICE
|
Your suggestion did fix that error by the way. --- a/src/librustc/middle/resolve_lifetime.rs
+++ b/src/librustc/middle/resolve_lifetime.rs
@@ -623,7 +623,13 @@ impl<'a, 'tcx> Visitor<'tcx> for LifetimeContext<'a, 'tcx> {
}
hir::TyKind::Path(hir::QPath::Resolved(None, ref path)) => {
if let Def::Existential(exist_ty_did) = path.def {
- let id = self.tcx.hir.as_local_node_id(exist_ty_did).unwrap();
+ let id = match self.tcx.hir.as_local_node_id(exist_ty_did) {
+ Some(id) => id,
+ None => {
+ intravisit::walk_ty(self, ty);
+ return;
+ },
+ };
// Resolve the lifetimes in the bounds to the lifetime defs in the generics.
// `fn foo<'a>() -> impl MyTrait<'a> { ... }` desugars to |
With the above patch, there is a new ICE when using the field inside an inherent method. extern crate a;
struct Bar(a::Foo);
impl Bar {
fn zero(&self) -> &a::Foo {
&self.0
}
}
fn main() {
} ICE
|
For me another variant of this issues is blocker:
#![feature(existential_type)]
pub trait View {
type Tmp: Iterator<Item = u32>;
fn test(&self) -> Self::Tmp;
}
pub struct X;
impl View for X {
existential type Tmp: Iterator<Item = u32>;
fn test(&self) -> Self::Tmp {
vec![1,2,3].into_iter()
}
} and extern crate a;
use a::View;
fn main() {
let v = a::X;
v.test();
} ICE
|
Is there any progress on this? |
@Arnavion how do you feel about making a PR with your patch that has a partial fix? It looks like that would be enough to unblock me from using existential types in my project. |
A bunch of stuff changed with oli-obk's recent change so the second patch won't apply directly. I'll retest and put up a fork. |
For the first ICE (function that returns existential type in crate A called from crate B), the patch (on top of 46880f4) is diff --git a/src/librustc_resolve/build_reduced_graph.rs b/src/librustc_resolve/build_reduced_graph.rs
index 25a7ff9cd3..02a28a1e2f 100644
--- a/src/librustc_resolve/build_reduced_graph.rs
+++ b/src/librustc_resolve/build_reduced_graph.rs
@@ -736,6 +736,9 @@ impl<'a, 'cl> Resolver<'a, 'cl> {
Def::Macro(..) => {
self.define(parent, ident, MacroNS, (def, vis, DUMMY_SP, expansion));
}
+ Def::Existential(..) => {
+ self.define(parent, ident, TypeNS, (def, vis, DUMMY_SP, expansion));
+ }
_ => bug!("unexpected definition: {:?}", def)
}
} The second ICE (existential type exported from crate A used as struct member in crate B) is already fixed. The third ICE (existential type exported from crate A used in return type of a function in crate B) still exists. And, while I'm at it, @andreytkachenko's ICE also still exists. |
@Arnavion thanks so much for following up on this! Do you want to open a PR, since you've done all the work? 😁 |
The patch is just guesswork on my part, and the whole thing is still broken anyway, so there's no point to me making a PR. If you want to try it you can apply that patch locally. |
Just encountered an ICE and searching led me here. Is this just another instance of the same bug or should I report it separately? Run
|
I'm running into this with ICE
|
@jonhoo Yes. If you see the "Easy playground repro" link in the OP, that happens because the doctest is a separate crate. |
Sadly these do not work yet due to the ICE from rust-lang/rust#53443
Sorry about taking so long, it was just a few small fixes in the end. I think #57836 fixes all the ICEs mentioned in this issue. |
Thanks. I confirmed this fixed all the issues with my original project too. |
Sadly these do not work yet due to the ICE from rust-lang/rust#53443
a/Cargo.toml
a/src/lib.rs
b/Cargo.toml
b/src/main.rs
ICE
rust/src/librustc_resolve/build_reduced_graph.rs
Line 741 in b559042
Resolver::build_reduced_graph_for_external_crate_def
needs an arm forDef::Existential
? @oli-obkEdit: Easy playground repro
The text was updated successfully, but these errors were encountered: