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 destructuring opaque types in their defining scopes #98582

Merged
merged 11 commits into from
Jul 16, 2022

Conversation

oli-obk
Copy link
Contributor

@oli-obk oli-obk commented Jun 27, 2022

fixes #96572

Before this PR, the following code snippet failed with an incomprehensible error, and similar code just ICEd in mir borrowck.

    type T = impl Copy;
    let foo: T = (1u32, 2u32);
    let (a, b) = foo; 

The problem was that the last line created MIR projections of the form foo.0 and foo.1, but foo's type is T, which doesn't have fields (only its hidden type does). But the pattern supplies enough type information (a tuple of two different inference types) to bind a hidden type.

@rustbot rustbot added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Jun 27, 2022
@rust-highfive
Copy link
Collaborator

Some changes occured to the CTFE / Miri engine

cc @rust-lang/miri

@rust-highfive
Copy link
Collaborator

r? @nagisa

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jun 27, 2022
@bors
Copy link
Contributor

bors commented Jun 27, 2022

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

@rustbot
Copy link
Collaborator

rustbot commented Jun 28, 2022

Some changes occurred to the CTFE / Miri engine

cc @rust-lang/miri

Some changes occurred to MIR optimizations

cc @rust-lang/mir-opt

Some changes occurred in src/tools/clippy

cc @rust-lang/clippy

Some changes occurred in compiler/rustc_codegen_cranelift

cc @bjorn3

@oli-obk oli-obk marked this pull request as ready for review June 28, 2022 13:50
@bors
Copy link
Contributor

bors commented Jun 30, 2022

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

@oli-obk
Copy link
Contributor Author

oli-obk commented Jun 30, 2022

cc @rust-lang/wg-mir-opt this is solely a MIR building/mir-borrowck concern, any takers?

@bors
Copy link
Contributor

bors commented Jul 13, 2022

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

@oli-obk
Copy link
Contributor Author

oli-obk commented Jul 15, 2022

r? rust-lang/compiler-team

@oli-obk
Copy link
Contributor Author

oli-obk commented Jul 15, 2022

r? compiler-team

@rust-highfive rust-highfive assigned estebank and unassigned nagisa Jul 15, 2022
Copy link
Contributor

@estebank estebank left a comment

Choose a reason for hiding this comment

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

@bors r+

Comment on lines +3 to +13
#[allow(unconditional_recursion)]
fn foo(b: bool) -> impl Copy {
let (mut x, mut y) = foo(false);
x = 42;
y = "foo";
if b {
panic!()
} else {
foo(true)
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

This looks like a bug on the lint?

{
place = test_place_builder.into_place(self.tcx, self.typeck_results);
} else {
return;
Copy link
Contributor

Choose a reason for hiding this comment

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

So the early return is no longer needed because of the changes in the files above?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

into_place ICEs if try_upvars_resolved would have returned Err. So this early return was always dead code as I could not figure out when it would ever happen and nothing in our test suite actually hit it.

Comment on lines +832 to +838
// Opaque types can't get destructured/split, but the patterns can
// actually hint at hidden types, so we use the patterns' types instead.
if let ty::Opaque(..) = v.head().ty().kind() {
if let Some(row) = rows.first() {
ty = row.head().ty();
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm guessing this is the meat of this change to get things working.

@estebank
Copy link
Contributor

@bors r+

@bors
Copy link
Contributor

bors commented Jul 16, 2022

📌 Commit 57e9f7a has been approved by estebank

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 Jul 16, 2022
bors added a commit to rust-lang-ci/rust that referenced this pull request Jul 16, 2022
…askrgr

Rollup of 10 pull requests

Successful merges:

 - rust-lang#98582 (Allow destructuring opaque types in their defining scopes)
 - rust-lang#99213 (migrate some of `rustc_passes::check_attr`'s diagnostics and derive improvements)
 - rust-lang#99258 (Provide structured suggestion for dropped temp value)
 - rust-lang#99259 (interpret/visitor: support visiting with a PlaceTy)
 - rust-lang#99287 ([rustdoc-json] JSON no longer inlines)
 - rust-lang#99290 (Revert "Highlight conflicting param-env candidates")
 - rust-lang#99316 (docs: add missing word)
 - rust-lang#99317 (Borrow Vec<T, A> as [T])
 - rust-lang#99323 (Fix flakyness of GUI tests)
 - rust-lang#99342 (Avoid some `Symbol` to `String` conversions)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 6f8fb91 into rust-lang:master Jul 16, 2022
@rustbot rustbot added this to the 1.64.0 milestone Jul 16, 2022
@oli-obk oli-obk deleted the unconstrained_opaque_type branch July 17, 2022 08:12
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Jul 17, 2022
…e_type, r=estebank"

This reverts commit 6f8fb91, reversing
changes made to 7210e46.
oli-obk added a commit to oli-obk/rust that referenced this pull request Jul 20, 2022
…e_type, r=estebank"

This reverts commit 6f8fb91, reversing
changes made to 7210e46.
bors added a commit to rust-lang-ci/rust that referenced this pull request Jul 20, 2022
Revert "Rollup merge of rust-lang#98582 - oli-obk:unconstrained_opaque_type, r…

…=estebank"

This reverts commit 6f8fb91, reversing
changes made to 7210e46.

r? `@ghost`

rebase of rust-lang#99368
JohnTitor pushed a commit to JohnTitor/rust that referenced this pull request Jul 26, 2022
…e_type, r=estebank"

This reverts commit 6f8fb91, reversing
changes made to 7210e46.
flip1995 pushed a commit to flip1995/rust that referenced this pull request Jul 28, 2022
…e_type, r=estebank"

This reverts commit 6f8fb91, reversing
changes made to 7210e46.
bors added a commit to rust-lang-ci/rust that referenced this pull request Aug 4, 2022
…rors

Some `is_useful` cleanups

rust-lang#98582 was reverted because it was a perf regression.

rust-lang#99806 reintroduces the changes, but this PR picks individual ones that have no regressions.
oli-obk added a commit to oli-obk/rust that referenced this pull request Sep 16, 2022
…ed_opaque_type, r=estebank""

This reverts commit 4a742a6.
flip1995 pushed a commit to flip1995/rust that referenced this pull request Oct 6, 2022
…ed_opaque_type, r=estebank""

This reverts commit 4a742a6.
bjorn3 pushed a commit to bjorn3/rust that referenced this pull request Oct 23, 2022
…ed_opaque_type, r=estebank""

This reverts commit 4a742a6.
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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

TAIT: "unconstrained opaque type" error even if it's constrained
6 participants