Skip to content

Conversation

@JohnTitor
Copy link
Member

@JohnTitor JohnTitor commented Oct 13, 2025

PR #135479 changed how late-bound regions are registered for MIR borrowck, switching from for_each_late_bound_region_in_item to iterating over bound_inputs_and_output.bound_vars(). However, for regular functions, the latter only includes late-bound lifetimes that appear in the function's inputs or output, missing unused lifetime parameters.

This caused an ICE when replace_bound_regions_with_nll_infer_vars tried to look up these unregistered regions.

This ensures all late-bound regions are properly registered in the universal regions map before they're needed during region substitution.

Fixes #135845

PR rust-lang#135479 changed how late-bound regions are registered for MIR
borrowck, switching from `for_each_late_bound_region_in_item` to
iterating over `bound_inputs_and_output.bound_vars()`. However, for
regular functions, the latter only includes late-bound lifetimes that
appear in the function's inputs or output, missing unused lifetime
parameters.

This caused an ICE when `replace_bound_regions_with_nll_infer_vars`
tried to look up these unregistered regions.

This ensures all late-bound regions are properly registered in the
universal regions map before they're needed during region substitution.
@rustbot
Copy link
Collaborator

rustbot commented Oct 13, 2025

This PR changes a file inside tests/crashes. If a crash was fixed, please move into the corresponding ui subdir and add 'Fixes #' to the PR description to autoclose the issue upon merge.

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Oct 13, 2025
@rustbot
Copy link
Collaborator

rustbot commented Oct 13, 2025

r? @davidtwco

rustbot has assigned @davidtwco.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot
Copy link
Collaborator

rustbot commented Oct 13, 2025

⚠️ Warning ⚠️

  • There are issue links (such as #123) in the commit messages of the following commits.
    Please move them to the PR description, to avoid spamming the issues with references to the commit, and so this bot can automatically canonicalize them to avoid issues with subtree.

@davidtwco
Copy link
Member

r? types

@rustbot rustbot added the T-types Relevant to the types team, which will review and decide on the PR/issue. label Oct 13, 2025
@rustbot rustbot assigned jackh726 and unassigned davidtwco Oct 13, 2025
| DefiningTy::Coroutine(..)
| DefiningTy::CoroutineClosure(..) => {
// For closures/coroutines, iterate over bound_vars to include implicit regions.
for (idx, bound_var) in bound_inputs_and_output.bound_vars().iter().enumerate() {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, so. This seems incompatible with the closure_lifetime_binder feature?

I think we should get @lcnr's thoughts on this, on why this switch was made (and if it's appropriate to go back to calling for_each_late_bound_region_in_item on everything).

@lcnr
Copy link
Contributor

lcnr commented Oct 29, 2025

However, for regular functions, the latter only includes late-bound lifetimes that appear in the function's inputs or output, missing unused lifetime parameters.

That doesn't seem to be quite true 🤔 from what I can tell all the ICE happen in cases where the fn-sig is malformed. The following compiles:

struct S<'a, T: ?Sized>(&'a T);

fn b<'a>() -> S<'static, u32> {
    S::<'a>(&0)
}

Hmm, so. This seems incompatible with the closure_lifetime_binder feature?

I think we should get @lcnr's thoughts on this, on why this switch was made (and if it's appropriate to go back to calling for_each_late_bound_region_in_item on everything).

I would also expect the same to be true for closures. The Binder should reference all late-bound regions, even if they're unused.

#![feature(closure_lifetime_binder)]
struct S<'a, T: ?Sized>(&'a T);

fn b() {
    for<'a> || -> S<'static, u32> {
        S::<'a>(&0)
    };
}

I personally believe the bug here to be that we don't include late bound regions in the binder if there's an error. We could even add an assert in query fn_sig that all late-bound regions are present in the binder to detect such issues more eagerly

@lcnr
Copy link
Contributor

lcnr commented Oct 29, 2025

on why this switch was made

Iirc the main reason behind #135479 was that previously we lazily converted bound regions which feels questionable to me. I switched us to first add nll vars for every late-bound region, and then convert as normal, without having to optionally create new bound regions

@jackh726
Copy link
Member

I would also expect the same to be true for closures. The Binder should reference all late-bound regions, even if they're unused.

Okay, yes, that's actually what I would expect too! But wasn't quite sure.

@jackh726
Copy link
Member

jackh726 commented Nov 3, 2025

Okay, so, I think both @lcnr and I are on the same page that this isn't quite the right solution. Going to mark this as waiting on author until an alternate solution is proposed.

@rustbot author

@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Nov 3, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-types Relevant to the types team, which will review and decide on the PR/issue.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ICE: cannot convert ReLateParam to a region vid

5 participants