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

hide impls if trait bound is proven from env #120836

Merged
merged 2 commits into from
Feb 9, 2024

Conversation

lcnr
Copy link
Contributor

@lcnr lcnr commented Feb 9, 2024

AVERT YOUR EYES @compiler-errors

fixes rust-lang/trait-system-refactor-initiative#76 and rust-lang/trait-system-refactor-initiative#12 (comment)

this is kinda ugly and I hate it, but I wasn't able to think of a cleaner approach for now. I am also unsure whether we have to refine this filtering later on, so by making the change pretty minimal it should be easier to improve going forward.

r? @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 (-Znext-solver) labels Feb 9, 2024
@rustbot
Copy link
Collaborator

rustbot commented Feb 9, 2024

Some changes occurred to the core trait solver

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

@lcnr lcnr force-pushed the param-env-hide-impl branch 3 times, most recently from 36a5590 to 50ea079 Compare February 9, 2024 09:49
@rust-log-analyzer

This comment has been minimized.

@BoxyUwU
Copy link
Member

BoxyUwU commented Feb 9, 2024

r=me once you fix typo and the link

@lcnr
Copy link
Contributor Author

lcnr commented Feb 9, 2024

@bors r=BoxyUwU rollup=always (new solver)

@bors
Copy link
Contributor

bors commented Feb 9, 2024

📌 Commit 5051637 has been approved by BoxyUwU

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 Feb 9, 2024
@jackh726
Copy link
Member

jackh726 commented Feb 9, 2024

This is super weird

@lcnr
Copy link
Contributor Author

lcnr commented Feb 9, 2024

This is super weird

yes, this matches the old solver where it may feel a bit more natural:

  • project only uses impls if candidate selection for the trait goal results in that impl candidate
  • if trait goal selection is ambig, then project is also ambig
  • candidate selection for trait goals prefers param env candidates if there are any

The effect is the same as in this PR (or well, even worse given the complex behavior of fn candidate_should_be_dropped_in_favor_of in the old solver), but this ugliness is really prominent in the new solver

bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 9, 2024
…iaskrgr

Rollup of 11 pull requests

Successful merges:

 - rust-lang#120351 (Implement SystemTime for UEFI)
 - rust-lang#120354 (improve normalization of `Pointee::Metadata`)
 - rust-lang#120776 (Move path implementations into `sys`)
 - rust-lang#120790 (better error message on download CI LLVM failure)
 - rust-lang#120806 (Clippy subtree update)
 - rust-lang#120815 (Improve `Option::inspect` docs)
 - rust-lang#120822 (Emit more specific diagnostics when enums fail to cast with `as`)
 - rust-lang#120827 (Print image input file and checksum in CI only)
 - rust-lang#120836 (hide impls if trait bound is proven from env)
 - rust-lang#120844 (Build DebugInfo for async closures)
 - rust-lang#120851 (Remove duplicate release note)

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 9, 2024
…iaskrgr

Rollup of 11 pull requests

Successful merges:

 - rust-lang#120351 (Implement SystemTime for UEFI)
 - rust-lang#120354 (improve normalization of `Pointee::Metadata`)
 - rust-lang#120776 (Move path implementations into `sys`)
 - rust-lang#120790 (better error message on download CI LLVM failure)
 - rust-lang#120806 (Clippy subtree update)
 - rust-lang#120815 (Improve `Option::inspect` docs)
 - rust-lang#120822 (Emit more specific diagnostics when enums fail to cast with `as`)
 - rust-lang#120827 (Print image input file and checksum in CI only)
 - rust-lang#120836 (hide impls if trait bound is proven from env)
 - rust-lang#120844 (Build DebugInfo for async closures)
 - rust-lang#120851 (Remove duplicate release note)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 2515845 into rust-lang:master Feb 9, 2024
11 checks passed
@rustbot rustbot added this to the 1.78.0 milestone Feb 9, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Feb 9, 2024
Rollup merge of rust-lang#120836 - lcnr:param-env-hide-impl, r=BoxyUwU

hide impls if trait bound is proven from env

AVERT YOUR EYES `@compiler-errors`

fixes rust-lang/trait-system-refactor-initiative#76 and rust-lang/trait-system-refactor-initiative#12 (comment)

this is kinda ugly and I hate it, but I wasn't able to think of a cleaner approach for now. I am also unsure whether we have to refine this filtering later on, so by making the change pretty minimal it should be easier to improve going forward.

r? `@BoxyUwU`
@lcnr lcnr deleted the param-env-hide-impl branch February 12, 2024 10:39
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 (-Znext-solver)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

using impl candidates shadowed by where-bound overflows by using impl with only super trait bounds
6 participants