-
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
Fix late-bound lifetime closure ICEs in HIR typeck and MIR borrowck #103780
Fix late-bound lifetime closure ICEs in HIR typeck and MIR borrowck #103780
Conversation
r? @wesleywiser (rustbot has picked a reviewer for you, use r? to override) |
@compiler-errors are you missing part of the first commit? The diff is unrelated to the message. |
This comment has been minimized.
This comment has been minimized.
@cjgillot I didn't forget anything, though I could've named it better. Let me rename it. |
6282e0c
to
9c9282d
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
f6b3cca
to
c012e56
Compare
if let Some(late_bounds) = tcx.is_late_bound_map(fn_def_id.expect_local()) { | ||
for ®ion_def_id in late_bounds.iter() { | ||
let name = tcx.item_name(region_def_id.to_def_id()); | ||
let typeck_root_def_id = tcx.typeck_root_def_id(mir_def_id.to_def_id()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To have a single loop, we can use while tcx.is_typeck_child(mir_def_id.to_def_id())
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That doesn't loop over the root def id, but stops one def id before it -- I'm not sure if I understand?
c012e56
to
2768c2f
Compare
) -> IndexVec<RegionVid, ty::Region<'tcx>> { | ||
let mut region_mapping = IndexVec::with_capacity(expected_num_vars); | ||
region_mapping.push(tcx.lifetimes.re_static); | ||
tcx.for_each_free_region(&closure_substs, |fr| { | ||
region_mapping.push(fr); | ||
}); | ||
|
||
for_each_late_bound_region_defined_on(tcx, typeck_root_def_id, |r| { | ||
for_each_late_bound_region_in_recursive_scope(tcx, tcx.local_parent(closure_def_id), |r| { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: This functions now assumes being called from the strict parent of the closure, not any higher parent. Can we make it a method on UniversalRegions and add assert_eq!(self.len(), expected_num_vars)
and assert_eq!(self.defining_ty.def_id(), tcx.parent(closure_def_id))
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we make it a method on UniversalRegions
Make what a method? for_each_late_bound_region_in_recursive_scope
?
and add assert_eq!(self.len(), expected_num_vars)
What's the difference between this assertion and the one that happens below?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Make what a method?
I meant closure_mapping
.
What's the difference between this assertion and the one that happens below?
Oops, I missed that. I guess it is sufficient then.
r=me with or without @aliemjay's comment |
@bors r=jackh726 |
…iaskrgr Rollup of 7 pull requests Successful merges: - rust-lang#103680 (CStr: add some doc links) - rust-lang#103780 (Fix late-bound lifetime closure ICEs in HIR typeck and MIR borrowck) - rust-lang#103845 (Add track_caller to some Lock methods) - rust-lang#103935 (Remove rustdoc clean::Visibility type) - rust-lang#103941 (Fixed typos) - rust-lang#103950 (Fix ICE when negative impl is collected during eager mono) - rust-lang#103953 (Remove unused argument from `throw_unresolved_import_error`) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup
During HIR typeck, we need to teach astconv to treat late-bound regions within a closure body as free, fixing escaping bound vars ICEs in both of the issues below.
However, this then gets us to MIR borrowck, which itself needs to be taught how to instantiate free region vids for late-bound regions that come from items that aren't the typeck root (for now, just closures).
Fixes #103771
Fixes #103736