Skip to content

do not constrain infer vars in find_best_leaf_obligation - #133493

Merged
bors merged 2 commits into
rust-lang:masterfrom
lcnr:fulfill-fudge
Nov 27, 2024
Merged

do not constrain infer vars in find_best_leaf_obligation#133493
bors merged 2 commits into
rust-lang:masterfrom
lcnr:fulfill-fudge

Conversation

@lcnr

@lcnr lcnr commented Nov 26, 2024 •
edited
Loading

Copy link
Copy Markdown
Contributor

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

rustbot commented Nov 26, 2024

Copy link
Copy Markdown
Collaborator

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

compiler-errors commented Nov 27, 2024

Copy link
Copy Markdown
Contributor

@bors r+ rollup

@bors

bors commented Nov 27, 2024

Copy link
Copy Markdown
Collaborator

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

1 more reviewer
@compiler-errors compiler-errors compiler-errors approved these changes
Reviewers whose approvals may not affect merge requirements

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