Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

#

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Jump to bottom

Deeply normalize when computing implied outlives bounds #133517

Merged
merged 2 commits into from
Dec 3, 2024
Merged

Deeply normalize when computing implied outlives bounds #133517

merged 2 commits into from
Dec 3, 2024

Conversation

compiler-errors
Copy link
Member

r? lcnr

Unfortunately resolving regions is still slightly scuffed (though in an unrelated way). Specifically, we should be normalizing our param-env outlives when constructing the OutlivesEnv; otherwise, these assumptions (

rust/compiler/rustc_infer/src/infer/outlives/env.rs

Line 78 in dd2837e

for outlives_bound in explicit_outlives_bounds(param_env).chain(extra_bounds) {
) are not constructed correctly.

Let me know if you want us to track that somewhere.

@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. WG-trait-system-refactor The Rustc Trait System Refactor Initiative (-Znext-solver) labels Nov 27, 2024
@compiler-errors compiler-errors mentioned this pull request Nov 27, 2024
Draft
lcnr
lcnr reviewed Nov 27, 2024
View reviewed changes
Copy link
Contributor

@lcnr lcnr left a comment

Choose a reason for hiding this comment

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

nits, then r=me

compiler/rustc_trait_selection/src/traits/query/type_op/implied_outlives_bounds.rs
let normalize_op = |ty| {
let ty = ocx.normalize(&ObligationCause::dummy(), param_env, ty);
let normalize_op = |ty| -> Result<_, NoSolution> {
let ty = ocx
Copy link
Contributor

Choose a reason for hiding this comment

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

pls add a comment for why we deeply_normalize here

Copy link
Contributor

Choose a reason for hiding this comment

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

you didn't do that 😅

@lcnr
Copy link
Contributor

lcnr commented Nov 27, 2024

Unfortunately resolving regions is still slightly scuffed (though in an unrelated way). Specifically, we should be normalizing our param-env outlives when constructing the OutlivesEnv; otherwise, these assumptions (

rust/compiler/rustc_infer/src/infer/outlives/env.rs

Line 78 in dd2837e

for outlives_bound in explicit_outlives_bounds(param_env).chain(extra_bounds) {
) are not constructed correctly.

explicit_outlives_bounds only looks at region outlives bounds:

rust/compiler/rustc_infer/src/infer/outlives/mod.rs

Lines 21 to 30 in 83965ef

pub fn explicit_outlives_bounds<'tcx>(
param_env: ty::ParamEnv<'tcx>,
) -> impl Iterator<Item = OutlivesBound<'tcx>> + 'tcx {
param_env
.caller_bounds()
.into_iter()
.filter_map(ty::Clause::as_region_outlives_clause)
.filter_map(ty::Binder::no_bound_vars)
.map(|ty::OutlivesPredicate(r_a, r_b)| OutlivesBound::RegionSubRegion(r_b, r_a))
}

so this should be fine?

Maybe we should rename that function to explicit_region_outlives_bounds though :3

bors added a commit to rust-lang-ci/rust that referenced this pull request Nov 27, 2024
@bors
Auto merge of rust-lang#133502 - lcnr:rust4, r=<try>
ac5ccad 
[DO NOT MERGE] bootstrap with `-Znext-solver=globally`

A revival of rust-lang#124812.

Current status:

`./x.py b --stage 2` passes 🎉

### commits

- rust-lang#133501
- rust-lang#133493
- 9456bfe and b21b116 reimplement candidate preference based on rust-lang#132325, not yet a separate PR
- c3ef9cd is a rebased version of rust-lang#125334, unsure whether I actually want to land this PR for now
- rust-lang#133517
* rust-lang#133518
* rust-lang#133519
* rust-lang#133520
* rust-lang#133521
* rust-lang#133524

r? `@ghost`
bors added a commit to rust-lang-ci/rust that referenced this pull request Nov 27, 2024
@bors
Auto merge of rust-lang#133502 - lcnr:rust4, r=<try>
550ae0d 
[DO NOT MERGE] bootstrap with `-Znext-solver=globally`

A revival of rust-lang#124812.

Current status:

`./x.py b --stage 2` passes 🎉

### commits

- rust-lang#133501
- rust-lang#133493
- 9456bfe and b21b116 reimplement candidate preference based on rust-lang#132325, not yet a separate PR
- c3ef9cd is a rebased version of rust-lang#125334, unsure whether I actually want to land this PR for now
- rust-lang#133517
* rust-lang#133518
* rust-lang#133519
* rust-lang#133520
* rust-lang#133521
* rust-lang#133524

r? `@ghost`
@rustbot
Copy link
Collaborator

rustbot commented Nov 27, 2024

Some changes occurred to the core trait solver

cc @rust-lang/initiative-trait-system-refactor

@compiler-errors
Copy link
Member Author

Probably could use another review from @lcnr

bors added a commit to rust-lang-ci/rust that referenced this pull request Nov 28, 2024
@bors
Auto merge of rust-lang#133502 - lcnr:rust4, r=<try>
bccf3b7 
[DO NOT MERGE] bootstrap with `-Znext-solver=globally`

A revival of rust-lang#124812.

Current status:

`./x.py b --stage 2` passes 🎉

### commits

- rust-lang#133501
- rust-lang#133493
- 9456bfe and b21b116 reimplement candidate preference based on rust-lang#132325, not yet a separate PR
- c3ef9cd is a rebased version of rust-lang#125334, unsure whether I actually want to land this PR for now
- rust-lang#133517
* rust-lang#133518
* rust-lang#133519
* rust-lang#133520
* rust-lang#133521
* rust-lang#133524

r? `@ghost`
lcnr
lcnr reviewed Nov 28, 2024
View reviewed changes
Copy link
Contributor

@lcnr lcnr left a comment

Choose a reason for hiding this comment

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

with comment r=me

@lcnr lcnr 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 29, 2024
@compiler-errors
Copy link
Member Author

@bors r=lcnr

@bors
Copy link
Contributor

bors commented Dec 2, 2024

📌 Commit 398fd90 has been approved by lcnr

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Dec 2, 2024
@matthiaskrgr matthiaskrgr mentioned this pull request Dec 3, 2024
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Dec 3, 2024
@bors
Auto merge of rust-lang#133788 - matthiaskrgr:rollup-1p100a8, r=matth…
ae3703c 
…iaskrgr

Rollup of 6 pull requests

Successful merges:

 - rust-lang#132723 (Unify `sysroot_target_{bin,lib}dir` handling)
 - rust-lang#133041 (Print name of env var in `--print=deployment-target`)
 - rust-lang#133325 (Reimplement `~const` trait specialization)
 - rust-lang#133395 (Add simd_relaxed_fma intrinsic)
 - rust-lang#133517 (Deeply normalize when computing implied outlives bounds)
 - rust-lang#133785 (Add const evaluation error UI test.)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 6827909 into rust-lang:master Dec 3, 2024
6 checks passed
@rustbot rustbot added this to the 1.85.0 milestone Dec 3, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Dec 3, 2024
@rust-timer
Unrolled build for rust-lang#133517
6f0343f 
Rollup merge of rust-lang#133517 - compiler-errors:deep-norm, r=lcnr

Deeply normalize when computing implied outlives bounds

r? lcnr

Unfortunately resolving regions is still slightly scuffed (though in an unrelated way). Specifically, we should be normalizing our param-env outlives when constructing the `OutlivesEnv`; otherwise, these assumptions (https://github.com/rust-lang/rust/blob/dd2837ec5de4301a692e05a7c4475e980af57a57/compiler/rustc_infer/src/infer/outlives/env.rs#L78) are not constructed correctly.

Let me know if you want us to track that somewhere.
@fmease fmease mentioned this pull request Dec 4, 2024
Closed
@fmease
Copy link
Member

fmease commented Dec 4, 2024

sync @bors r-

@bors bors 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-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Dec 4, 2024
# for free to join this conversation on GitHub. Already have an account? # to comment
Reviewers

@lcnr lcnr lcnr left review comments

Assignees

@lcnr lcnr

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. WG-trait-system-refactor The Rustc Trait System Refactor Initiative (-Znext-solver)
Projects
None yet
Milestone
1.85.0
Development

Successfully merging this pull request may close these issues.
5 participants
@compiler-errors @lcnr @rustbot @bors @fmease