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

TAIT: "unconstrained opaque type" error even if it's constrained #96572

Closed
Dirbaio opened this issue Apr 30, 2022 · 7 comments · Fixed by #98582 or #99806
Closed

TAIT: "unconstrained opaque type" error even if it's constrained #96572

Dirbaio opened this issue Apr 30, 2022 · 7 comments · Fixed by #98582 or #99806
Assignees
Labels
A-impl-trait Area: impl Trait. Universally / existentially quantified anonymous types with static dispatch. C-bug Category: This is a bug. F-type_alias_impl_trait `#[feature(type_alias_impl_trait)]` glacier ICE tracked in rust-lang/glacier. I-ICE Issue: The compiler panicked, giving an Internal Compilation Error (ICE) ❄️ T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@Dirbaio
Copy link
Contributor

Dirbaio commented Apr 30, 2022

This is allowed:

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

But this isn't:

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

It seems strange to me that adding code causes a type that was previously properly constrained to not longer be so. I'm not sure if it's a diagnostics issue, or if it should indeed be allowed...

See playground

rustc 1.62.0-nightly (a707f40 2022-04-29)

@rustbot label A-impl-trait F-type_alias_impl_trait

@rustbot rustbot added A-impl-trait Area: impl Trait. Universally / existentially quantified anonymous types with static dispatch. F-type_alias_impl_trait `#[feature(type_alias_impl_trait)]` labels Apr 30, 2022
@oli-obk oli-obk self-assigned this May 3, 2022
@oli-obk
Copy link
Contributor

oli-obk commented May 3, 2022

UPDATE: the below ICE is fixed now!

I don't think we can really allow this. No pattern but a straight up binding is a valid pattern for an opaque type. If you actually give that pattern a type

#![feature(type_alias_impl_trait)]

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

then we get a lot of ICEs from borrowck. The interesting ones are

error: internal compiler error: broken MIR in DefId(0:3 ~ inference[4f8b]::main) ((_1.0: u32)): can't project out of PlaceTy { ty: main::T, variant_index: None }
  --> /home/ubuntu/rust5/src/test/ui/type-alias-impl-trait/inference.rs:6:10
   |
LL |     let (a, b): (u32, u32) = foo;
   |          ^
   |
   = note: delayed at compiler/rustc_borrowck/src/type_check/mod.rs:855:31

error: internal compiler error: broken MIR in DefId(0:3 ~ inference[4f8b]::main) ((_1.1: u32)): can't project out of PlaceTy { ty: main::T, variant_index: None }
  --> /home/ubuntu/rust5/src/test/ui/type-alias-impl-trait/inference.rs:6:13
   |
LL |     let (a, b): (u32, u32) = foo;
   |             ^
   |
   = note: delayed at compiler/rustc_borrowck/src/type_check/mod.rs:855:31

These happen because we assume that if a pattern type checks, that we can actually destructure the thing that was moved into the pattern. Unfortunately for opaque types that isn't true.

I guess the simple solution would be to just forbid it. We could try to allow it, but I'm not sure how to do that in general without just throwing more logic into match checking and mir building. Just patching the type of foo as seen in the pattern logic could work though... basically only touch mir typeck and verification/sanitation to teach them about the types inferred from patterns

@oli-obk oli-obk added I-ICE Issue: The compiler panicked, giving an Internal Compilation Error (ICE) ❄️ I-needs-decision Issue: In need of a decision. labels May 3, 2022
@rust-lang-glacier-bot rust-lang-glacier-bot added the glacier ICE tracked in rust-lang/glacier. label May 3, 2022
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue May 7, 2022
…Simulacrum

Add regression and bug tests

this tracks the behaviour from rust-lang#96572 in our test suite
@nikomatsakis
Copy link
Contributor

nikomatsakis commented May 12, 2022

UPDATE: the below ICE is fixed now!

Other examples:

#![feature(type_alias_impl_trait)]

fn main() {
    type T = impl Copy;
    let foo: T = (1u32, 2u32);
    match foo {
        (a, b) => (),
    }
}

also gives an error (playground)

@oli-obk oli-obk removed the I-needs-decision Issue: In need of a decision. label May 12, 2022
@nikomatsakis
Copy link
Contributor

nikomatsakis commented May 12, 2022

Also ICEs (playground):

#![feature(type_alias_impl_trait)]

fn main() {
    type T = impl Copy;
    let foo: T = Some((1u32, 2u32));
    match foo {
        None => (),
        Some((a, b)) => (),
    }
}

@oli-obk
Copy link
Contributor

oli-obk commented May 12, 2022

triage:

Implement a "opaque type downcast projection" in mir and insert appropriately

@JohnTitor
Copy link
Member

Triage: #96572 (comment) and #96572 (comment) are fixed by #96515. The last one (#96572 (comment)) is still ICE.

@JohnTitor JohnTitor added C-bug Category: This is a bug. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels May 28, 2022
@oli-obk
Copy link
Contributor

oli-obk commented Jun 28, 2022

Fun: these can all be triggered on stable, too:

#[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)
    }
}

and

fn foo(b: bool) -> Option<impl Copy> {
    if b {
        return None;
    }
    match foo(!b) {
        Some((mut x, mut y)) => {
            x = 42;
            y = "foo";
        }
        None => {}
    }
    None
}

@JohnTitor
Copy link
Member

Re-opening as #98582 has been reverted: #99495

@bors bors closed this as completed in 4136b59 Sep 20, 2022
flip1995 pushed a commit to flip1995/rust that referenced this issue Oct 6, 2022
…estebank

Allow patterns to constrain the hidden type of opaque types

fixes rust-lang#96572

reverts a revert as original PR was a perf regression that was fixed by reverting it: rust-lang#99368 (comment))

TODO:

* check if rust-lang#99685 is avoided
bjorn3 pushed a commit to bjorn3/rust that referenced this issue Oct 23, 2022
…estebank

Allow patterns to constrain the hidden type of opaque types

fixes rust-lang#96572

reverts a revert as original PR was a perf regression that was fixed by reverting it: rust-lang#99368 (comment))

TODO:

* check if rust-lang#99685 is avoided
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-impl-trait Area: impl Trait. Universally / existentially quantified anonymous types with static dispatch. C-bug Category: This is a bug. F-type_alias_impl_trait `#[feature(type_alias_impl_trait)]` glacier ICE tracked in rust-lang/glacier. I-ICE Issue: The compiler panicked, giving an Internal Compilation Error (ICE) ❄️ T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
6 participants