Skip to content
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

Jump to bottom

do not constrain infer vars in find_best_leaf_obligation #133493

Merged
merged 2 commits into from
Nov 27, 2024
Merged

do not constrain infer vars in find_best_leaf_obligation #133493

merged 2 commits into from
Nov 27, 2024

Conversation

lcnr
Copy link
Contributor

@lcnr lcnr commented Nov 26, 2024 •
edited
Loading

This ended up causing an ICE by making the following code path reachable by incorrectly constraining an inference variable while computing the best obligation for a preceding ambiguity. Closes #129444.

rust/compiler/rustc_trait_selection/src/solve/fulfill.rs

Lines 312 to 314 in f2abf82

Ok((_, Certainty::Yes)) => {
bug!("did not expect successful goal when collecting ambiguity errors")
}

I have to be honest, I don't fully understand how that change removes all the additional diagnostics :3

r? @compiler-errors

@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 26, 2024
@rustbot
Copy link
Collaborator

rustbot commented Nov 26, 2024

Some changes occurred to the core trait solver

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

@lcnr lcnr mentioned this pull request Nov 26, 2024
Draft
@compiler-errors
Copy link
Member

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Nov 27, 2024

📌 Commit fa66288 has been approved by compiler-errors

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-review Status: Awaiting review from the assignee but also interested parties. labels Nov 27, 2024
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>
ca04381 
[DO NOT MERGE] bootstrap with `-Znext-solver=globally`

A revival of rust-lang#124812.

Current status, we're failing in:

- failing in `rustc_next_trait_solver` with 126 instances of the following error
```
error[E0311]: the parameter type `I` may not live long enough
    |
help: consider adding an explicit lifetime bound
   --> compiler/rustc_next_trait_solver/src/solve/trait_goals.rs:624:53
    |
624 ~     fn consider_structural_builtin_unsize_candidates<'a>(
625 ~         ecx: &mut EvalCtxt<'a, D>,
626 |         goal: Goal<I, Self>,
627 ~     ) -> Vec<Candidate<I>> where I: 'a {
```
- `itertools` hangs, we're already encountered this hang in our previous attempt due to large type sizes. I believe that it's simply caused by a missing cache somewhere, potentially in `wf.rs`, but other visitors may also be responsible. See rust-itertools/itertools#945 for more details

### 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

r? `@ghost`
@matthiaskrgr matthiaskrgr mentioned this pull request Nov 27, 2024
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Nov 27, 2024
@bors
Auto merge of rust-lang#133527 - matthiaskrgr:rollup-kyre1df, r=matth…
5f8a240 
…iaskrgr

Rollup of 6 pull requests

Successful merges:

 - rust-lang#132979 (use `--exact` on `--skip` to avoid unintended substring matches)
 - rust-lang#133248 (CI: split x86_64-msvc-ext job)
 - rust-lang#133449 (std: expose `const_io_error!` as `const_error!`)
 - rust-lang#133453 (Commit license-metadata.json to git and check it's correct in CI)
 - rust-lang#133457 (miri: implement `TlsFree`)
 - rust-lang#133493 (do not constrain infer vars in `find_best_leaf_obligation`)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 762a661 into rust-lang:master Nov 27, 2024
6 checks passed
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Nov 27, 2024
@rust-timer
Unrolled build for rust-lang#133493
0a0e1a2 
Rollup merge of rust-lang#133493 - lcnr:fulfill-fudge, r=compiler-errors

do not constrain infer vars in `find_best_leaf_obligation`

This ended up causing an ICE by making the following code path reachable by incorrectly constraining an inference variable while computing the best obligation for a preceding ambiguity. Closes rust-lang#129444.

https://github.com/rust-lang/rust/blob/f2abf827c128120ed7a874d02973947968c158b8/compiler/rustc_trait_selection/src/solve/fulfill.rs#L312-L314

I have to be honest, I don't fully understand how that change removes all the additional diagnostics :3

r? `@compiler-errors`
@rustbot rustbot added this to the 1.85.0 milestone Nov 27, 2024
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`
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`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@compiler-errors compiler-errors compiler-errors approved these changes

Assignees

@compiler-errors compiler-errors

Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. 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.

ICE: coherence: did not expect successful goal when collecting ambiguity errors
4 participants
@lcnr @rustbot @compiler-errors @bors