Skip to content

Better closure requirement propagation V2 #154271

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.

Sign up for GitHub

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

Open
LorrensP-2158466 wants to merge 1 commit into
base: main
Choose a base branch
from
+52 −1
Open

Better closure requirement propagation V2 #154271

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 27 additions & 1 deletion compiler/rustc_borrowck/src/region_infer/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -713,9 +713,35 @@ impl<'tcx> RegionInferenceContext<'tcx> {
for ur in self.scc_values.universal_regions_outlived_by(r_scc) {
found_outlived_universal_region = true;
debug!("universal_region_outlived_by ur={:?}", ur);
let non_local_ub = self.universal_region_relations.non_local_upper_bounds(ur);
let mut non_local_ub = self.universal_region_relations.non_local_upper_bounds(ur);
debug!(?non_local_ub);

// We don't need to propagate every `T: 'ub` for soundness:
// Say we have the following outlives constraints given (`'b: 'a` == `a -> b`):
//
// a
// \
// -> c
// /
// b
Copy link
Contributor

@lcnr lcnr Mar 24, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

that's still odd and the test feels confusing/easier to fix than the one I wrote 🤔

In your test we have

'a: 'c
'b: 'c
T: 'a

We should clearly not propagate T: 'c and 'c: 'a. It's weird that we look at at universal_regions_outlived_by at all. Why are we not directly looking at the non_local_upper_bounds of lower_bound?

At least only look at the smallest such universal region

Copy link
Contributor Author

@LorrensP-2158466 LorrensP-2158466 Mar 24, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

We should clearly not propagate T: 'c and 'c: 'a

And that doesn't happen, I don't know what you mean with this.

It's weird that we look at at universal_regions_outlived_by at all. Why are we not directly looking at the non_local_upper_bounds of lower_bound

The function comment:

    /// Once we've promoted T, we have to "promote" `'X` to some region
    /// that is "external" to the closure. Generally speaking, a region
    /// may be the union of some points in the closure body as well as
    /// various free lifetimes. We can ignore the points in the closure
    /// body: if the type T can be expressed in terms of external regions,
    /// we know it outlives the points in the closure body. That
    /// just leaves the free regions.

Otherwise I have no clue, dev-guide didn't give a reason. If I try to do non_local_upper_bounds of lower_bound the compiler panics:

thread 'rustc' (6493769) panicked at compiler/rustc_borrowck/src/type_check/free_region_relations.rs:113:9:
assertion failed: self.universal_regions.is_universal_region(fr0)

At least only look at the smallest such universal region

How is this possible?

//
// And we are doing the type test `T: 'a`.
//
// The `lower_bound_universal_regions` of `'a` are `['a, 'c]`. The `upper_bounds` of `'a`
// is `['a]`, so we propagate `T: 'a`.
// The `upper_bounds` of `'c` are `['a, 'b]`, propagating `T: 'a` is correct;
// `T: 'b` is redundant because it provides no value to proving `T: 'a`.
//
// So we filter the set of upper_bounds to regions already outliving `lower_bound`,
// but if this subset is empty, we fall back to the original one.
let subset: Vec<_> = non_local_ub
.iter()
.copied()
.filter(|ub| self.eval_outlives(*ub, lower_bound))
.collect();
non_local_ub = if subset.is_empty() { non_local_ub } else { subset };
debug!(?non_local_ub, "upper_bounds after filtering");

// This is slightly too conservative. To show T: '1, given `'2: '1`
// and `'3: '1` we only need to prove that T: '2 *or* T: '3, but to
// avoid potential non-determinism we approximate this by requiring
Expand Down
25 changes: 25 additions & 0 deletions tests/ui/nll/closure-requirements/type-test-issue-154267.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
//@ check-pass
// This test checks that the compiler doesn't propagate `T: 'b` during the `T: 'a` type test.
// If it did, it would fail to compile, even though the program is sound.

struct Arg<'a: 'c, 'b: 'c, 'c, T> {
field: *mut (&'a (), &'b (), &'c (), T),
}

impl<'a, 'b, 'c, T> Arg<'a, 'b, 'c, T> {
fn constrain(self)
where
T: 'a,
{
}
}

fn takes_closure<'a, 'b, T>(_: impl for<'c> FnOnce(Arg<'a, 'b, 'c, T>)) {}

// We have `'a: 'c` and `'b: 'c`, requiring `T: 'a` in `constrain` should not need
// `T: 'b` here.
fn error<'a, 'b, T: 'a>() {
takes_closure::<'a, 'b, T>(|arg| arg.constrain());
}

fn main() {}
Loading