Skip to content

Conversation

lcnr
Copy link
Contributor

@lcnr lcnr commented Mar 15, 2023

based on the idea of #108389. Moved define_opaque_types into the actual operations, e.g. eq, instead of infcx.at because normalization doesn't use define_opaque_types and even creates it's own At with a different define_opaque_types internally.

Somewhat surprisingly, coherence actually relies on DefineOpaqueTypes::Yes for soundness which was revealed because I've incorrectly used DefineOpaqueTypes::No in equate_impl_headers. It feels concerning that even though this is the case, we still sometimes use DefineOpaqueTypes::No in coherence. I did not look into this as part of this PR as it is purely changing the structure of the code without changing behavior in any way.

r? @oli-obk

@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. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. WG-trait-system-refactor The Rustc Trait System Refactor Initiative (-Znext-solver) labels Mar 15, 2023
@rustbot
Copy link
Collaborator

rustbot commented Mar 15, 2023

Some changes occurred to the core trait solver

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

Some changes occurred in engine.rs, potentially modifying the public API of ObligationCtxt.

cc @lcnr, @compiler-errors

@lcnr lcnr changed the title always make define_opaque_types explicit make define_opaque_types fully explicit Mar 15, 2023
@oli-obk
Copy link
Contributor

oli-obk commented Mar 15, 2023

@bors r+

@bors
Copy link
Collaborator

bors commented Mar 15, 2023

📌 Commit d2b7604 has been approved by oli-obk

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 Mar 15, 2023
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Mar 15, 2023
… r=oli-obk

make `define_opaque_types` fully explicit

based on the idea of rust-lang#108389. Moved `define_opaque_types` into the actual operations, e.g. `eq`, instead of `infcx.at` because normalization doesn't use `define_opaque_types` and even creates it's own `At` with a different `define_opaque_types` internally.

Somewhat surprisingly, coherence actually relies on `DefineOpaqueTypes::Yes` for soundness which was revealed because I've incorrectly used `DefineOpaqueTypes::No` in `equate_impl_headers`. It feels concerning that even though this is the case, we still sometimes use `DefineOpaqueTypes::No` in coherence. I did not look into this as part of this PR as it is purely changing the structure of the code without changing behavior in any way.

r? `@oli-obk`
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Mar 16, 2023
… r=oli-obk

make `define_opaque_types` fully explicit

based on the idea of rust-lang#108389. Moved `define_opaque_types` into the actual operations, e.g. `eq`, instead of `infcx.at` because normalization doesn't use `define_opaque_types` and even creates it's own `At` with a different `define_opaque_types` internally.

Somewhat surprisingly, coherence actually relies on `DefineOpaqueTypes::Yes` for soundness which was revealed because I've incorrectly used `DefineOpaqueTypes::No` in `equate_impl_headers`. It feels concerning that even though this is the case, we still sometimes use `DefineOpaqueTypes::No` in coherence. I did not look into this as part of this PR as it is purely changing the structure of the code without changing behavior in any way.

r? ``@oli-obk``
bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 16, 2023
…iaskrgr

Rollup of 10 pull requests

Successful merges:

 - rust-lang#108875 (rustdoc: fix type search for `Option` combinators)
 - rust-lang#108971 (error-msg: impl better suggestion for `E0532`)
 - rust-lang#109139 (rustdoc: DocFS: Replace rayon with threadpool and enable it for all targets)
 - rust-lang#109151 (Assert def-kind is correct for alias types)
 - rust-lang#109158 (error-msg: expand suggestion for `unused_def` lint)
 - rust-lang#109166 (make `define_opaque_types` fully explicit)
 - rust-lang#109171 (Some cleanups in our normalization logic)
 - rust-lang#109180 (Unequal → Not equal)
 - rust-lang#109185 (rustdoc: remove `std::` from primitive intra-doc link tooltips)
 - rust-lang#109192 (Mention UEFI target promotion in release notes for 1.67.0)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit f0205d5 into rust-lang:master Mar 16, 2023
@rustbot rustbot added this to the 1.70.0 milestone Mar 16, 2023
@lcnr lcnr deleted the define_opaque_types-explicit branch March 16, 2023 13:47
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. T-rustdoc Relevant to the rustdoc 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.

4 participants