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

modify alias-relate to also normalize ambiguous opaques #120549

Merged
merged 4 commits into from
Feb 13, 2024

Conversation

lcnr
Copy link
Contributor

@lcnr lcnr commented Feb 1, 2024

allows a bunch of further cleanups and generally simplifies the type system. To handle rust-lang/trait-system-refactor-initiative#8 we'll have to add a some additional complexity to the (Alias, Infer) branches in alias-relate, so removing the opaque type special case here is really valuable.

It does worsen deduce_closure_signature and friends even more as they now receive an inference variable which is only constrained via an AliasRelate goal. These probably have to look into alias relate goals somehow. Leaving that for a future PR as this is something we'll have to tackle regardless.

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 labels Feb 1, 2024
@rustbot
Copy link
Collaborator

rustbot commented Feb 1, 2024

Some changes occurred to the core trait solver

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

@lcnr lcnr force-pushed the errs-showcase branch 3 times, most recently from a9cdad2 to 80218a5 Compare February 1, 2024 12:42
@rust-log-analyzer

This comment has been minimized.

@@ -302,6 +302,8 @@ fn typeck_with_fallback<'tcx>(

let typeck_results = fcx.resolve_type_vars_in_body(body);

let _ = fcx.infcx.take_opaque_types();
Copy link
Member

Choose a reason for hiding this comment

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

Why? Needs a comment at least.

@@ -562,7 +562,9 @@ impl<'cx, 'tcx> WritebackCx<'cx, 'tcx> {

#[instrument(skip(self), level = "debug")]
fn visit_opaque_types(&mut self) {
let opaque_types = self.fcx.infcx.take_opaque_types();
// We clone the opaques instead of stealing them here as they are still used for
Copy link
Member

Choose a reason for hiding this comment

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

Oh that's why lol

@compiler-errors
Copy link
Member

@bors r+

@bors
Copy link
Contributor

bors commented Feb 5, 2024

📌 Commit 2adf60c 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 Feb 5, 2024
@compiler-errors
Copy link
Member

oh new solver

@bors rollup

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Feb 6, 2024
modify alias-relate to also normalize ambiguous opaques

allows a bunch of further cleanups and generally simplifies the type system. To handle rust-lang/trait-system-refactor-initiative#8 we'll have to add a some additional complexity to the `(Alias, Infer)` branches in alias-relate, so removing the opaque type special case here is really valuable.

It does worsen `deduce_closure_signature` and friends even more as they now receive an inference variable which is only constrained via an `AliasRelate` goal. These probably have to look into alias relate goals somehow. Leaving that for a future PR as this is something we'll have to tackle regardless.

r? `@compiler-errors`
@matthiaskrgr
Copy link
Member

I suspect that this failed here:
#120710 (comment)

@lcnr
Copy link
Contributor Author

lcnr commented Feb 8, 2024

fixed CI

@bors r=compiler-errors

@bors
Copy link
Contributor

bors commented Feb 8, 2024

📌 Commit 45377df has been approved by compiler-errors

It is now in the queue for this repository.

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Feb 8, 2024
modify alias-relate to also normalize ambiguous opaques

allows a bunch of further cleanups and generally simplifies the type system. To handle rust-lang/trait-system-refactor-initiative#8 we'll have to add a some additional complexity to the `(Alias, Infer)` branches in alias-relate, so removing the opaque type special case here is really valuable.

It does worsen `deduce_closure_signature` and friends even more as they now receive an inference variable which is only constrained via an `AliasRelate` goal. These probably have to look into alias relate goals somehow. Leaving that for a future PR as this is something we'll have to tackle regardless.

r? `@compiler-errors`
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Feb 8, 2024
modify alias-relate to also normalize ambiguous opaques

allows a bunch of further cleanups and generally simplifies the type system. To handle rust-lang/trait-system-refactor-initiative#8 we'll have to add a some additional complexity to the `(Alias, Infer)` branches in alias-relate, so removing the opaque type special case here is really valuable.

It does worsen `deduce_closure_signature` and friends even more as they now receive an inference variable which is only constrained via an `AliasRelate` goal. These probably have to look into alias relate goals somehow. Leaving that for a future PR as this is something we'll have to tackle regardless.

r? ``@compiler-errors``
@matthiaskrgr
Copy link
Member

@bors r-
may need rebase #120795 (comment)

@bors
Copy link
Contributor

bors commented Feb 8, 2024

📌 Commit 89e8cc9 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 Feb 8, 2024
bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 9, 2024
…iaskrgr

Rollup of 7 pull requests

Successful merges:

 - rust-lang#120584 (For a rigid projection, recursively look at the self type's item bounds to fix the `associated_type_bounds` feature)
 - rust-lang#120589 (std::thread::available_parallelism merging linux/android/freebsd version)
 - rust-lang#120596 ([rustdoc] Correctly generate path for non-local items in source code pages)
 - rust-lang#120629 (Move some test files)
 - rust-lang#120846 (Update jobserver-rs to 0.1.28)
 - rust-lang#120850 (ast_lowering: Fix regression in `use ::{}` imports.)
 - rust-lang#120853 (Avoid a collection and iteration on empty passes)

Failed merges:

 - rust-lang#120549 (modify alias-relate to also normalize ambiguous opaques)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors
Copy link
Contributor

bors commented Feb 9, 2024

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

@bors bors 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-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Feb 9, 2024
@lcnr
Copy link
Contributor Author

lcnr commented Feb 13, 2024

@rustbot ready

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Feb 13, 2024
@lcnr
Copy link
Contributor Author

lcnr commented Feb 13, 2024

@bors r=compiler-errors

@bors
Copy link
Contributor

bors commented Feb 13, 2024

📌 Commit 51a1000 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 Feb 13, 2024
bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 13, 2024
…iaskrgr

Rollup of 8 pull requests

Successful merges:

 - rust-lang#114877 (unstable-book: add quick-edit link)
 - rust-lang#120548 (rustdoc: Fix handling of doc_auto_cfg feature for cfg attributes on glob reexport)
 - rust-lang#120549 (modify alias-relate to also normalize ambiguous opaques)
 - rust-lang#120959 (Remove good path delayed bugs)
 - rust-lang#120978 (match lowering: simplify block creation)
 - rust-lang#121019 (coverage: Simplify some parts of the coverage span refiner)
 - rust-lang#121021 (Extend intra-doc link chapter in the rustdoc book)
 - rust-lang#121031 (RustWrapper: adapt for coverage mapping API changes)

Failed merges:

 - rust-lang#121014 (Remove `force_print_diagnostic`)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 65ab663 into rust-lang:master Feb 13, 2024
11 checks passed
@rustbot rustbot added this to the 1.78.0 milestone Feb 13, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Feb 13, 2024
Rollup merge of rust-lang#120549 - lcnr:errs-showcase, r=compiler-errors

modify alias-relate to also normalize ambiguous opaques

allows a bunch of further cleanups and generally simplifies the type system. To handle rust-lang/trait-system-refactor-initiative#8 we'll have to add a some additional complexity to the `(Alias, Infer)` branches in alias-relate, so removing the opaque type special case here is really valuable.

It does worsen `deduce_closure_signature` and friends even more as they now receive an inference variable which is only constrained via an `AliasRelate` goal. These probably have to look into alias relate goals somehow. Leaving that for a future PR as this is something we'll have to tackle regardless.

r? `@compiler-errors`
@lcnr lcnr deleted the errs-showcase branch February 13, 2024 21:34
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Feb 22, 2024
…lcnr

No need to `validate_alias_bound_self_from_param_env` in `assemble_alias_bound_candidates`

We already fully normalize the self type before we reach `assemble_alias_bound_candidates`, so there's no reason to double check that a projection is truly rigid by checking param-env bounds.

I think this is also blocked on us making sure to always normalize opaques: rust-lang#120549.

r? lcnr
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Feb 22, 2024
…lcnr

No need to `validate_alias_bound_self_from_param_env` in `assemble_alias_bound_candidates`

We already fully normalize the self type before we reach `assemble_alias_bound_candidates`, so there's no reason to double check that a projection is truly rigid by checking param-env bounds.

I think this is also blocked on us making sure to always normalize opaques: rust-lang#120549.

r? lcnr
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Feb 22, 2024
Rollup merge of rust-lang#120598 - compiler-errors:no-rigid-check, r=lcnr

No need to `validate_alias_bound_self_from_param_env` in `assemble_alias_bound_candidates`

We already fully normalize the self type before we reach `assemble_alias_bound_candidates`, so there's no reason to double check that a projection is truly rigid by checking param-env bounds.

I think this is also blocked on us making sure to always normalize opaques: rust-lang#120549.

r? lcnr
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

6 participants