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

Allow constraining opaque types during subtyping in the trait system #125447

Merged
merged 1 commit into from
Jun 19, 2024

Conversation

oli-obk
Copy link
Contributor

@oli-obk oli-obk commented May 23, 2024

Previous attempt: #123979

Sometimes we don't immediately perform subtyping, but instead register a subtyping obligation and solve that obligation when its inference variables become resolved. Unlike immediate subtyping, we currently do not allow registering hidden types for opaque types. This PR also allows that.

@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. labels May 23, 2024
@rust-log-analyzer

This comment has been minimized.

@rustbot rustbot added A-testsuite Area: The testsuite used to check the correctness of rustc T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) labels May 23, 2024
@rustbot

This comment was marked as resolved.

@rust-log-analyzer

This comment has been minimized.

@compiler-errors

This comment was marked as resolved.

@oli-obk

This comment was marked as resolved.

@compiler-errors

This comment was marked as resolved.

@oli-obk

This comment was marked as resolved.

@lcnr
Copy link
Contributor

lcnr commented May 24, 2024

A minimized test:

fn foo() -> impl Default {
    let x = Default::default();
    // add `Subtype(?x, ?y)` obligation
    let y = x;

    // Make a tuple `(?x, ?y)` and equate it with `(impl Default, u32)`.
    // For us to try and prove a `Subtype(impl Default, u32)` obligation,
    // we have to instantiate both `?x` and `?y` without any
    // `select_where_possible` calls inbetween.
    let tup = &mut (x, y);
    let assign_tup = &mut (foo(), 1u32);
    tup = assign_tup;
    1u32
}

fn main() {}

I think this change is clearly desirable, even if it did no go through an FCP. From what I can tell it also doesn't block any other work, as I want us to convert all places to use DefineOpaqueTypes::Yes before stabilizing ATPIT. I would like us to FCP this.

While I think it would have been fine to FCP merge this without first reverting this PR, we would only have 1 week to go through the FCP to make it in time for its stabilization. Given that we're missing some existing uncertainty wrt the impact of this change, please revert #123979 and remerge it after a types FCP.

@oli-obk

This comment was marked as resolved.

@bors

This comment was marked as resolved.

@oli-obk oli-obk changed the title Remove assertion now that we have a test covering the code path Allow constraining opaque types during subtyping in the trait system May 27, 2024
@rustbot

This comment was marked as resolved.

@rustbot rustbot added the has-merge-commits PR has merge commits, merge with caution. label May 27, 2024
@rustbot

This comment was marked as resolved.

@rustbot rustbot added the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label May 27, 2024
@oli-obk oli-obk added T-types Relevant to the types team, which will review and decide on the PR/issue. and removed T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) labels May 27, 2024
@rustbot rustbot removed the has-merge-commits PR has merge commits, merge with caution. label May 27, 2024
@oli-obk oli-obk added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-fcp Status: PR is in FCP and is awaiting for FCP to complete. labels Jun 10, 2024
@apiraino apiraino removed the to-announce Announce this issue on triage meeting label Jun 14, 2024
@compiler-errors
Copy link
Member

@bors r+

@bors
Copy link
Contributor

bors commented Jun 18, 2024

📌 Commit 1e9cd07 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 Jun 18, 2024
jieyouxu added a commit to jieyouxu/rust that referenced this pull request Jun 19, 2024
…-errors

Allow constraining opaque types during subtyping in the trait system

Previous attempt: rust-lang#123979

Sometimes we don't immediately perform subtyping, but instead register a subtyping obligation and solve that obligation when its inference variables become resolved. Unlike immediate subtyping, we currently do not allow registering hidden types for opaque types. This PR also allows that.
@jieyouxu
Copy link
Contributor

UI tests might need rebless #126653 (comment)
@bors r-

@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 Jun 19, 2024
@oli-obk
Copy link
Contributor Author

oli-obk commented Jun 19, 2024

@bors r=compiler-errors

@bors
Copy link
Contributor

bors commented Jun 19, 2024

📌 Commit ba4510e 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 Jun 19, 2024
bors added a commit to rust-lang-ci/rust that referenced this pull request Jun 19, 2024
Rollup of 6 pull requests

Successful merges:

 - rust-lang#125447 (Allow constraining opaque types during subtyping in the trait system)
 - rust-lang#125766 (MCDC Coverage: instrument last boolean RHS operands from condition coverage)
 - rust-lang#125880 (Remove `src/tools/rust-demangler`)
 - rust-lang#126154 (StorageLive: refresh storage (instead of UB) when local is already live)
 - rust-lang#126572 (override user defined channel when using precompiled rustc)
 - rust-lang#126662 (Unconditionally warn on usage of `wasm32-wasi`)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 25d47fe into rust-lang:master Jun 19, 2024
6 checks passed
@rustbot rustbot added this to the 1.81.0 milestone Jun 19, 2024
@bors
Copy link
Contributor

bors commented Jun 19, 2024

⌛ Testing commit ba4510e with merge 3186d17...

rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Jun 19, 2024
Rollup merge of rust-lang#125447 - oli-obk:eq_opaque_pred, r=compiler-errors

Allow constraining opaque types during subtyping in the trait system

Previous attempt: rust-lang#123979

Sometimes we don't immediately perform subtyping, but instead register a subtyping obligation and solve that obligation when its inference variables become resolved. Unlike immediate subtyping, we currently do not allow registering hidden types for opaque types. This PR also allows that.
@fmease
Copy link
Member

fmease commented Jun 19, 2024

It's already merged, bors. What the hell?
@bors r-

Edit: We currently have three full-build pending PRs, omg.

@fmease
Copy link
Member

fmease commented Jun 19, 2024

@bors retry r-

@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 Jun 19, 2024
@oli-obk oli-obk deleted the eq_opaque_pred branch June 19, 2024 13:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-testsuite Area: The testsuite used to check the correctness of rustc disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. finished-final-comment-period The final comment period is finished for this PR / Issue. S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-types Relevant to the types team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet