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

Prefer param-env candidates that do no inference in new solver #109579

Closed
wants to merge 2 commits into from
Closed

Prefer param-env candidates that do no inference in new solver #109579

wants to merge 2 commits into from

Conversation

compiler-errors
Copy link
Member

r? @lcnr since we discussed that:

  • in the short term, prefer param-env candidates so we can actually wf-check impls that return non-trivial region constraints (see ui tests) -- this shouldn't affect coherence where we don't see param-env candidates during coherence, since this is pretty badly affecting any testing of hir typeck with the new solver.
  • in the long term, we'd like to take intersections of region responses instead of having to prefer param-env over other candidates.

@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 Mar 25, 2023
@rustbot
Copy link
Collaborator

rustbot commented Mar 25, 2023

Some changes occurred to the core trait solver

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

@rust-cloud-vms rust-cloud-vms bot force-pushed the new-solver-prefer-param-env branch from 201bfac to a22dd55 Compare March 25, 2023 02:27
lcnr
lcnr reviewed Mar 27, 2023
View reviewed changes
Copy link
Contributor

@lcnr lcnr left a comment •
edited
Loading

Choose a reason for hiding this comment

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

would like to talk about this in sync as i have a slightly more involved refactoring to this part of the solver in mind here

@bors
Copy link
Contributor

bors commented Mar 27, 2023

☔ The latest upstream changes (presumably #109662) made this pull request unmergeable. Please resolve the merge conflicts.

@compiler-errors
Copy link
Member Author

@lcnr said they will take this pr over, so i'm gonna mark it as blocked and they can close it when they open up a better pr than this 😸

@rustbot blocked

@rustbot rustbot added S-blocked Status: Blocked on something else such as an RFC or other implementation work. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 27, 2023
@lcnr lcnr mentioned this pull request Mar 29, 2023
Merged
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this pull request Apr 10, 2023
@Dylan-DPC
Rollup merge of rust-lang#109724 - lcnr:prioritize-env, r=compiler-er…
c30d7e9 
…rors

prioritize param env candidates if they don't guide type inference

intended to supersede rust-lang#109579. We disable the prioritization during coherence to maintain completeness.

Long term we can hopefully replace this hack with adding OR to external constraints at which point the only relevant part when merging responses is whether they guide type inference in the same way.

Reuses `try_merge_responses` for assembly and the cleanest way to impl that was to actually split that so that `try_merge_responses` returns `None` if we fail to merge them and then add `flounder` which is used afterwards which is allowed to lower the certainty of our responses.

If, in the future, we add the ability to merge candidates `YES: ?0 = Vec<u32>` and `YES: ?0 = Vec<i32>` to `AMBIG: ?0 = Vec<?1>`, this should happen in `flounder`.

r? `@compiler-errors` `@BoxyUwU`
@compiler-errors compiler-errors deleted the new-solver-prefer-param-env branch August 11, 2023 20:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lcnr lcnr lcnr left review comments

Assignees

@lcnr lcnr

Labels
S-blocked Status: Blocked on something else such as an RFC or other implementation work. 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
No milestone
Development

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