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: it's possible to impl a trait for a tait by using projections #99840

Closed
lcnr opened this issue Jul 28, 2022 · 12 comments · Fixed by #103488
Closed

TAIT: it's possible to impl a trait for a tait by using projections #99840

lcnr opened this issue Jul 28, 2022 · 12 comments · Fixed by #103488
Labels
F-type_alias_impl_trait `#[feature(type_alias_impl_trait)]` I-ICE Issue: The compiler panicked, giving an Internal Compilation Error (ICE) ❄️ I-unsound Issue: A soundness hole (worst kind of bug), see: https://en.wikipedia.org/wiki/Soundness requires-nightly This issue requires a nightly compiler in some way. T-types Relevant to the types team, which will review and decide on the PR/issue.

Comments

@lcnr
Copy link
Contributor

lcnr commented Jul 28, 2022

#![feature(type_alias_impl_trait)]
type Alias = impl Sized;

fn constrain() -> Alias {
    1i32
}

trait HideIt {
    type Assoc;
}

impl HideIt for () {
    type Assoc = Alias;
}

pub trait Yay {}

impl Yay for <() as HideIt>::Assoc {}
// impl Yay for i32 {} // this errors
// impl Yay for u32 {} // this also errors

As coherence checking keeps opaque types opaque this should not be unsound. Considering that a direct impl for opaque types is forbidden, this is an inconsistency we should resolve though.

cc @oli-obk

@lcnr lcnr added F-type_alias_impl_trait `#[feature(type_alias_impl_trait)]` T-types Relevant to the types team, which will review and decide on the PR/issue. labels Jul 28, 2022
@oli-obk
Copy link
Contributor

oli-obk commented Jul 28, 2022

@oli-obk oli-obk added the I-ICE Issue: The compiler panicked, giving an Internal Compilation Error (ICE) ❄️ label Jul 28, 2022
@lcnr
Copy link
Contributor Author

lcnr commented Jul 28, 2022

what, is that a recent regression? :o these do error with rustc 1.63.0-nightly (5435ed691 2022-06-07)

@lcnr lcnr added I-unsound Issue: A soundness hole (worst kind of bug), see: https://en.wikipedia.org/wiki/Soundness requires-nightly This issue requires a nightly compiler in some way. labels Jul 28, 2022
@oli-obk oli-obk added the E-needs-bisection Call for participation: This issue needs bisection: https://github.com/rust-lang/cargo-bisect-rustc label Jul 28, 2022
@lqd
Copy link
Member

lqd commented Jul 28, 2022

It looks to be from last week's #99506, and out of that rollup #99345 looks likely.

@oli-obk
Copy link
Contributor

oli-obk commented Jul 28, 2022

Hmm... that one got reverted in #99079

cc @compiler-errors

@lqd
Copy link
Member

lqd commented Jul 28, 2022

Was that a revert, or a targeted fix for closures ?

@oli-obk
Copy link
Contributor

oli-obk commented Jul 28, 2022

revert + actual fix of the original issue

@lqd
Copy link
Member

lqd commented Jul 28, 2022

The behavior is the same on master, no errors + ICEs. I'll try to manually bisect within the rollup to confirm cargo-bisect-rustc's findings.

@compiler-errors
Copy link
Member

Isn't this just another form of #99554?

@lqd
Copy link
Member

lqd commented Jul 28, 2022

(I wish we had try builds for each rolled up PR 😅) It was indeed introduced in the same rollup: by eecfdfb / #99383.

Reverting that does indeed bring the conflicting implementation errors back, and fixes the ICE.

@lcnr
Copy link
Contributor Author

lcnr commented Jul 28, 2022

Isn't this just another form of #99554?

no, #99554 is using projections in ways which should fail the orphan check, here we have projections which should pass the orphan check, but can be normalized to opaque types.

These issues need two distinct fixes

the fix for this being: add tests for auto trait stuff with opaque types but allow them in impls. As coherence doesn't use Reveal::All, having one impl trait ref with impl A<X, opaque, Y> for Z should conflict with all impl A<X, whatever, Y> for Z. Have to think more about this though

@oli-obk
Copy link
Contributor

oli-obk commented Jul 28, 2022

I have a proper fix locally, but we should investigate how #99383 affected this

@lqd lqd removed the E-needs-bisection Call for participation: This issue needs bisection: https://github.com/rust-lang/cargo-bisect-rustc label Jul 29, 2022
@ouz-a
Copy link
Contributor

ouz-a commented Aug 1, 2022

(I wish we had try builds for each rolled up PR 😅) It was indeed introduced in the same rollup: by eecfdfb / #99383.

Reverting that does indeed bring the conflicting implementation errors back, and fixes the ICE.

Although it looks suspicious this is not caused by #99383 I reverted all changes made by it and tested it, this still passes without any errors. Here https://github.com/ouz-a/rust/tree/undo_formalize

Manishearth added a commit to Manishearth/rust that referenced this issue Nov 22, 2022
Allow opaque types in trait impl headers and rely on coherence to reject unsound cases

r? `@lcnr`

fixes rust-lang#99840
Manishearth added a commit to Manishearth/rust that referenced this issue Nov 22, 2022
Allow opaque types in trait impl headers and rely on coherence to reject unsound cases

r? ``@lcnr``

fixes rust-lang#99840
Manishearth added a commit to Manishearth/rust that referenced this issue Nov 23, 2022
Allow opaque types in trait impl headers and rely on coherence to reject unsound cases

r? ```@lcnr```

fixes rust-lang#99840
@bors bors closed this as completed in 53eab24 Nov 23, 2022
flip1995 pushed a commit to flip1995/rust that referenced this issue Dec 1, 2022
Allow opaque types in trait impl headers and rely on coherence to reject unsound cases

r? ````@lcnr````

fixes rust-lang#99840
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
F-type_alias_impl_trait `#[feature(type_alias_impl_trait)]` I-ICE Issue: The compiler panicked, giving an Internal Compilation Error (ICE) ❄️ I-unsound Issue: A soundness hole (worst kind of bug), see: https://en.wikipedia.org/wiki/Soundness requires-nightly This issue requires a nightly compiler in some way. T-types Relevant to the types team, which will review and decide on the PR/issue.
5 participants