Skip to content

remove last vestiges of the old check#244

Merged
nikomatsakis merged 7 commits intorust-lang:mainfrom
nikomatsakis:living-large
Mar 13, 2026
Merged

remove last vestiges of the old check#244
nikomatsakis merged 7 commits intorust-lang:mainfrom
nikomatsakis:living-large

Conversation

@nikomatsakis
Copy link
Copy Markdown
Contributor

No description provided.

@nikomatsakis
Copy link
Copy Markdown
Contributor Author

I have to rebase this and drop the first two commits I think

@rustbot

This comment has been minimized.

@nikomatsakis nikomatsakis requested a review from lqd February 26, 2026 14:49
@nikomatsakis
Copy link
Copy Markdown
Contributor Author

OK, this has been rebased and is ready to go! cc @lqd @tiif

Also cc @BoxyUwU, this modifies slightly the const syntax, but not the new fancy stuff we were talking about.

Copy link
Copy Markdown
Member

@tiif tiif left a comment

Choose a reason for hiding this comment

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

I like the new function argument syntax :) Other than the nit, everything looks good.

Also, the final commit message is different from the diff, did you forget to add the change?

View changes since this review

Comment thread src/test/borrowck.rs
Comment on lines -182 to -206
expect_test::expect![[r#"
the rule "borrow of disjoint places" at (nll.rs) failed because
condition evaluted to false: `place_disjoint_from_place(&loan.place.to_place_expression(), &access.place)`
&loan.place.to_place_expression() = *(local(m))
&access.place = *(local(m))

the rule "loan_not_required_by_universal_regions" at (nll.rs) failed because
condition evaluted to false: `outlived_by_loan.iter().all(|p| match p
{
Parameter::Ty(_) => false, Parameter::Lt(lt) => match lt.data()
{
LtData::Static => false, LtData::Variable(Variable::UniversalVar(_))
=> false, LtData::Variable(Variable::ExistentialVar(_)) => true,
LtData::Variable(Variable::BoundVar(_)) =>
panic!("cannot outlive a bound var"),
}, Parameter::Const(_) => panic!("cannot outlive a constant"),
})`

the rule "write-indirect" at (nll.rs) failed because
pattern `TypedPlaceExpressionKind::Deref(place_loaned_ref)` did not match value `local(m)`

the rule "write-indirect" at (nll.rs) failed because
condition evaluted to false: `place_accessed.is_prefix_of(&place_loaned_ref.to_place_expression())`
place_accessed = *(local(m))
&place_loaned_ref.to_place_expression() = local(m)"#]]
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

It's interesting that this problem case 3 test passed. I suppose it is related to the change where we move verify_universal_outlives to prove_judgment, but I don't really understand why.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

same, I don't understand why this changed either, nor why checking for universal subset errors more or at a different place would change anything here.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The reason that problem case number 3 passes now is because we are no longer collecting all the outlives relations in a pre-pass and then using them everywhere along the graph, but rather accumulating them as we go. This is the heart of polonius, basically, and so it fixes this behavior.

Comment on lines 88 to 105
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We should be able to remove this function, it is no longer used.

Copy link
Copy Markdown
Member

@lqd lqd left a comment

Choose a reason for hiding this comment

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

  • +1 on tiif's comments
  • the new syntax is way better
  • the last 2 commits also likely need to be dropped
  • It'd be cool to discuss the change we don't understand at the next meeting

View changes since this review

@@ -134,19 +135,9 @@ enum AccessKind {
/// Given the `TypeckEnv`, and a (populated) list of `pending_outlives`
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

we can also remove this mention of pending_outlives since it's removed in this commit

Comment thread src/test/mir_typeck.rs Outdated
$*params

#[term(minirust {
// then declare the body
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

"the body" instead?

Comment thread src/test/borrowck.rs
Comment on lines -182 to -206
expect_test::expect![[r#"
the rule "borrow of disjoint places" at (nll.rs) failed because
condition evaluted to false: `place_disjoint_from_place(&loan.place.to_place_expression(), &access.place)`
&loan.place.to_place_expression() = *(local(m))
&access.place = *(local(m))

the rule "loan_not_required_by_universal_regions" at (nll.rs) failed because
condition evaluted to false: `outlived_by_loan.iter().all(|p| match p
{
Parameter::Ty(_) => false, Parameter::Lt(lt) => match lt.data()
{
LtData::Static => false, LtData::Variable(Variable::UniversalVar(_))
=> false, LtData::Variable(Variable::ExistentialVar(_)) => true,
LtData::Variable(Variable::BoundVar(_)) =>
panic!("cannot outlive a bound var"),
}, Parameter::Const(_) => panic!("cannot outlive a constant"),
})`

the rule "write-indirect" at (nll.rs) failed because
pattern `TypedPlaceExpressionKind::Deref(place_loaned_ref)` did not match value `local(m)`

the rule "write-indirect" at (nll.rs) failed because
condition evaluted to false: `place_accessed.is_prefix_of(&place_loaned_ref.to_place_expression())`
place_accessed = *(local(m))
&place_loaned_ref.to_place_expression() = local(m)"#]]
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

same, I don't understand why this changed either, nor why checking for universal subset errors more or at a different place would change anything here.

LivePlaces,
},
mini_rust_check::{ty_is_adt, ty_is_ref, Location, PendingOutlives, TypeckEnv},
mini_rust_check::{ty_is_adt, ty_is_int, ty_is_ref, Location, PendingOutlives, TypeckEnv},
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

did retcon fail in this commit? the message looks more about the previous commit than this one

LivePlaces,
},
mini_rust_check::{ty_is_adt, ty_is_int, ty_is_ref, Location, PendingOutlives, TypeckEnv},
mini_rust_check::{ty_is_adt, ty_is_ref, Location, PendingOutlives, TypeckEnv},
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

this is the inverse of the previous commit? "fix: check switch discriminant is an integer type in NLL"? :/

nikomatsakis and others added 5 commits March 8, 2026 17:20
Move verify_universal_outlives check from the return terminator into
prove_judgment, so it runs every time outlives constraints accumulate.
This catches unsound universal region relationships even in code that
never returns (e.g., infinite loops).

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@rustbot
Copy link
Copy Markdown
Collaborator

rustbot commented Mar 9, 2026

This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

- Remove dead `check_blocks` function
- Update stale `pending_outlives` doc comment
- Fix "then declare the body" → "the body" comment

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@nikomatsakis
Copy link
Copy Markdown
Contributor Author

We discussed this in detail. We decided to merge the code but to open a FIXME about adding back in the outlives constraints needed to accurately model NLL-as-it-is-today.

@lqd
Copy link
Copy Markdown
Member

lqd commented Mar 13, 2026

I was surprised this wasn't merged yet, especially because I mistakenly said it was in our call yesterday, apologies.

So I've made CI green and took care of

but to open a FIXME about adding back in the outlives constraints needed to accurately model NLL-as-it-is-today.

Done in #253

@nikomatsakis nikomatsakis merged commit 1448023 into rust-lang:main Mar 13, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants