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.

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

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

Merged
merged 3 commits into from
Apr 10, 2023

Conversation

lcnr
Copy link
Contributor

@lcnr lcnr commented Mar 29, 2023

intended to supersede #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

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

rustbot commented Mar 29, 2023

Some changes occurred to the core trait solver

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

Comment on lines +326 to +332
response.value.certainty == Certainty::Yes
&& response.has_no_inference_or_external_constraints()
Copy link
Member

Choose a reason for hiding this comment

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

Noting that while sound, returning yes if we find a candidate with no external constraint eventually may need to be weakened for lcnr/solver-woes#12 (comment) to compile.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i hope to move the prioritization to compute_alias_eq_goal instead and keep try_merge_candidates complete. might not work out perfectly but that's my vibe here for now.

@compiler-errors
Copy link
Member

r=me, nits or not (i guess there's only like one nit, the rest are just observations)

@bors
Copy link
Contributor

bors commented Mar 30, 2023

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

@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@bors
Copy link
Contributor

bors commented Mar 31, 2023

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

Copy link
Member

@compiler-errors compiler-errors left a comment

Choose a reason for hiding this comment

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

r=me

@compiler-errors compiler-errors 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 Apr 9, 2023
@lcnr
Copy link
Contributor Author

lcnr commented Apr 10, 2023

@bors r=compiler-errors rollup

@bors
Copy link
Contributor

bors commented Apr 10, 2023

📌 Commit 3fab7f7 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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Apr 10, 2023
bors added a commit to rust-lang-ci/rust that referenced this pull request Apr 10, 2023
Rollup of 6 pull requests

Successful merges:

 - rust-lang#109724 (prioritize param env candidates if they don't guide type inference)
 - rust-lang#110021 (Fix a couple ICEs in the new `CastKind::Transmute` code)
 - rust-lang#110044 (Avoid some manual slice length calculation)
 - rust-lang#110115 (compiletest: Use remap-path-prefix only in CI)
 - rust-lang#110121 (Fix `x check --stage 1` when download-rustc is enabled)
 - rust-lang#110124 (Some clippy fixes in the compiler)

Failed merges:

 - rust-lang#109752 (Stall auto trait assembly in new solver for int/float vars)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit c30d7e9 into rust-lang:master Apr 10, 2023
@rustbot rustbot added this to the 1.70.0 milestone Apr 10, 2023
@lcnr lcnr deleted the prioritize-env branch April 10, 2023 14:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants