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

rustc has a stack overflow when using impl Trait of a trait with a generic parameter with a default type of Self #49376

Closed
quininer opened this issue Mar 26, 2018 · 4 comments
Labels
A-impl-trait Area: `impl Trait`. Universally / existentially quantified anonymous types with static dispatch. C-bug Category: This is a bug. I-crash Issue: The compiler crashes (SIGSEGV, SIGABRT, etc). Use I-ICE instead when the compiler panics.

Comments

@quininer
Copy link
Contributor

https://play.rust-lang.org/?gist=a827cb4bc9dd29324227e3ac676dd47a&version=nightly

#![crate_type = "rlib"] 
#![feature(conservative_impl_trait)]

struct Bar {}
trait Foo<T = Self> {}
impl Foo for Bar {}

fn foo() -> impl Foo {
    Bar {}
}
@matthewjasper matthewjasper added I-crash Issue: The compiler crashes (SIGSEGV, SIGABRT, etc). Use I-ICE instead when the compiler panics. A-impl-trait Area: `impl Trait`. Universally / existentially quantified anonymous types with static dispatch. labels Mar 26, 2018
@matthewjasper
Copy link
Contributor

I guess this should be erroring with E0393 or E0666 (nested impl trait is not allowed).

@shepmaster shepmaster added the C-bug Category: This is a bug. label May 10, 2018
@shepmaster
Copy link
Member

Continues crashing in 1.26 stable.

@shepmaster shepmaster changed the title rustc stack overflow when impl Trait rustc has a stack overflow when using impl Trait of a trait with a generic parameter with a default type of Self May 10, 2018
@shepmaster
Copy link
Member

This does not crash:

struct Bar {}
trait Foo<T = i32> {}
impl Foo for Bar {}

fn foo() -> impl Foo {
    Bar {}
}

fn main() {}

@leoyvens
Copy link
Contributor

leoyvens commented May 10, 2018

Particularly unfortunate because of PartialEq and PartialOrd.

What is going on here: There is a syntactical check to forbid nested impl Trait, but the default sneaks past that check, so we probably have to check for that substitution after lowering from HIR in type collection. Though it can also be done in the syntax check with some contortion.

I was thinking that this shouldn't be allowed because it's like nested impl Trait, but impl PartialEq is not at all like impl PartialEq<impl PartialEq> because the latter allows the inner type to be different from the outer one, while the former doesn't. So it might just be a case of fixing the overflow itself here.

I can take this myself but someone looking for mentorship or who is just faster is welcome.

leoyvens added a commit to leoyvens/rust that referenced this issue May 12, 2018
A high impact bug because a lot of common traits use a `Self`
substitution by default. Should be backported to beta.

There was a check for this which wasn't catching all cases, it was made
more robust.

Fixes rust-lang#49376
Fixes rust-lang#50626

r? @petrochenkov
bors added a commit that referenced this issue May 14, 2018
…r=petrochenkov

Fix self referential impl Trait substitutions

A high impact bug because a lot of common traits use a `Self` substitution by default. Should be backported to beta.

There was a check for this which wasn't catching all cases, it was made more robust.

Fixes #49376
Fixes #50626

r? @petrochenkov
leoyvens added a commit to leoyvens/rust that referenced this issue May 14, 2018
A high impact bug because a lot of common traits use a `Self`
substitution by default. Should be backported to beta.

There was a check for this which wasn't catching all cases, it was made
more robust.

Fixes rust-lang#49376
Fixes rust-lang#50626

r? @petrochenkov
pietroalbini pushed a commit to pietroalbini/rust that referenced this issue May 15, 2018
A high impact bug because a lot of common traits use a `Self`
substitution by default. Should be backported to beta.

There was a check for this which wasn't catching all cases, it was made
more robust.

Fixes rust-lang#49376
Fixes rust-lang#50626

r? @petrochenkov
Mark-Simulacrum pushed a commit to Mark-Simulacrum/rust that referenced this issue May 24, 2018
A high impact bug because a lot of common traits use a `Self`
substitution by default. Should be backported to beta.

There was a check for this which wasn't catching all cases, it was made
more robust.

Fixes rust-lang#49376
Fixes rust-lang#50626

r? @petrochenkov
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. I-crash Issue: The compiler crashes (SIGSEGV, SIGABRT, etc). Use I-ICE instead when the compiler panics.
Projects
None yet
Development

No branches or pull requests

4 participants