-
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
[NLL] issue-34780.rs test ICEs #47590
Comments
#47577 is probably the same issue. |
Not a dup of #47153 |
or rather, with the fix applied, I get another ICE =) |
|
That new ICE seems similar to #47587 :) |
Hmm, yes, but it's definitely not an issue with |
This probably has something to do with the fact that consts use the identity substitutions when computing universal regions substitutions: rust/src/librustc_mir/borrow_check/nll/universal_regions.rs Lines 581 to 607 in 70f7d58
since those are always going to be generating ReEarlyBound regions:Lines 182 to 187 in 70f7d58
I have yet to track down the code that should be resolving those early bounds and why it isn't doing so. |
Hmm, @bobtwinkles you are in the right area of the code. But I think the problem runs just a touch deeper. Actually the comments here are not that great. I should do a pass to improve them. But let me try to first explain what's happening a bit and then how to fix it. What is the "defining type"The role of the
This is perhaps easiest to explain with a function item. Assume that we are working with the MIR for this function fn foo<'a, T, U>(x: &'a T, y: &'a U) where T: Trait<'a> { ... } In that case, the defining type will be the rust/src/librustc_mir/borrow_check/nll/universal_regions.rs Lines 121 to 123 in def3269
You can see that it is actually sort of minimal: just a def-id and a substs array. In this case, the substs array will consist of three things, first a lifetime variable (representing
(This Given the Note that this substs array does not, for example, include the types of the arguments etc. That's because those types can be reconstructed using queries. For example, the rust/src/librustc_mir/borrow_check/nll/universal_regions.rs Lines 657 to 660 in def3269
Part of the reason it is done this way is that then the types of both arguments ( How does this relate to constants?For constants, our defining-type variant is currently the type of the constant: rust/src/librustc_mir/borrow_check/nll/universal_regions.rs Lines 125 to 128 in def3269
This is actually wrong, and is roughly the equivalent of storing the types of the arguments directly in the We probably want to change that variant to something more like Fn: Const(DefId, Substs<'tcx>), then we want to do is to refactor match tcx.hir.body_owner_kind(self.mir_node_id) {
BodyOwnerKind::Fn => {
let closure_base_def_id = tcx.closure_base_def_id(self.mir_def_id);
let defining_ty = if self.mir_def_id == closure_base_def_id {
tcx.type_of(closure_base_def_id)
} else {
let tables = tcx.typeck_tables_of(self.mir_def_id);
tables.node_id_to_type(self.mir_hir_id)
};
let defining_ty = self.infcx.replace_free_regions_with_nll_infer_vars(FR, &defining_ty);
match defining_ty.sty {
ty::TyClosure(def_id, substs) => DefiningTy::Closure(def_id, substs),
ty::TyGenerator(def_id, substs, interior) => {
DefiningTy::Generator(def_id, substs, interior)
}
ty::TyFnDef(def_id, substs) => DefiningTy::FnDef(def_id, substs),
_ => span_bug!(
tcx.def_span(self.mir_def_id),
"expected defining type for `{:?}`: `{:?}`",
self.mir_def_id,
defining_ty
),
}
}
BodyOwnerKind::Const | BodyOwnerKind::Static(..) => {
// new code will go here, see below
}
} meanwhile, for assert_eq!(tcx.closure_base_def_id(self.mir_def_id), self.mir_def_id);
let identity_substs = Substs::identity_for_item(gcx, closure_base_def_id);
let substs = self.infcx.replace_free_regions_with_nll_infer_vars(FR, &identity_substs);
DefiningTy::Const(self.mir_def_id, substs) Then we can change the other bits of code to match. For example: rust/src/librustc_mir/borrow_check/nll/universal_regions.rs Lines 599 to 601 in def3269
becomes
When we wish to compute the "return type" of the closure, then we can do the rust/src/librustc_mir/borrow_check/nll/universal_regions.rs Lines 663 to 665 in def3269
becomes:
I think that ought to work. cc @bobtwinkles |
Fixes rust-lang#47590 by fixing the way DefiningTy represents constants. Previously, constants were represented using just the type of the variable. However, this will fail to capture early-bound regions as NLL inference vars, resulting in an ICE when we try to compute region VIDs a little bit later in the universal region resolution process.
[NLL] Improve DefiningTy::Const Fixes #47590 by fixing the way DefiningTy represents constants. Previously, constants were represented using just the type of the variable. However, this will fail to capture early-bound regions as NLL inference vars, resulting in an ICE when we try to compute region VIDs a little bit later in the universal region resolution process. (ref #47590)
The text was updated successfully, but these errors were encountered: