Skip to content

Inconsistent Self behavior with generic structs #69306

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Closed
arucil opened this issue Feb 20, 2020 · 6 comments · Fixed by #69340
Closed

Inconsistent Self behavior with generic structs #69306

arucil opened this issue Feb 20, 2020 · 6 comments · Fixed by #69340
Assignees
Labels
C-bug Category: This is a bug. P-high High priority T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@arucil
Copy link

arucil commented Feb 20, 2020

Summary from @pnkfelix : Self used as a tuple-struct constructor function should use the fully type-substituted form from the impl block, but does not.

Thus, today this code is accepted, but it should not be.

struct _Bar<T>(T);

impl<T> _Bar<T> {
    fn _map1<U>(x: U) -> _Bar<U> {
        Self(x)
    }
}

(Fixing this would be a breaking-change, so we should take care in how we deploy it.)

Original bug report follows:


I tried this code:

struct _Foo<T> {
    x: T
}

impl<T> _Foo<T> {
    fn _map<U>(x: U) -> _Foo<U> {
        Self { x }
    }
}

struct _Bar<T>(T);

impl<T> _Bar<T> {
    fn _map<U>(x: U) -> _Bar<U> {
        Self(x)
    }
}

In nightly rustc, _Bar compiles while _Foo yields a type mismatch error:

error[E0308]: mismatched types
  --> src/main.rs:10:16
   |
8  | impl<T> _Foo<T> {
   |      - expected type parameter
9  |     fn _map<U>(x: U) -> _Foo<U> {
   |             - found type parameter
10 |         Self { x }
   |                ^ expected type parameter `T`, found type parameter `U`
   |
@arucil arucil added the C-bug Category: This is a bug. label Feb 20, 2020
@petrochenkov
Copy link
Contributor

petrochenkov commented Feb 20, 2020

The _Bar one is incorrect, Self has all its generic parameters substituted, i.e. it's _Foo<T>/_Bar<T> in both cases, type or constructor function.

@jonas-schievink jonas-schievink added I-nominated T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-lang Relevant to the language team, which will review and decide on the PR/issue. labels Feb 20, 2020
@pnkfelix
Copy link
Member

To be 100% clear here: I believe what @petrochenkov is saying is that the compiler has a bug, and that the bug is that the code

impl<T> _Bar<T> {
    fn _map<U>(x: U) -> _Bar<U> {
        Self(x)
    }
}

should be treated as a short-hand for:

impl<T> _Bar<T> {
    fn _map<U>(x: U) -> _Bar<U> {
        _Bar::<T>(x)
    }
}

and thus should be yielding the same error that one sees for the _Foo cases in the code from the description.

However, the _Bar::_map code is not yielding an error at all.

@pnkfelix
Copy link
Member

T-compiler triage: leaving nominated for T-lang. Tagging as P-high priority to resolve whether the semantics described by @petrochenkov is indeed what we want, and if so, how to address deploying a breaking-change to fix it.

@pnkfelix pnkfelix added the P-high High priority label Feb 20, 2020
@Centril
Copy link
Contributor

Centril commented Feb 20, 2020

cc @eddyb

I thought we had fixed this bug in #61896 (and to be clear, it's clearly a bug (as noted by @petrochenkov in #69306 (comment)). It should not compile.

@Centril Centril removed the T-lang Relevant to the language team, which will review and decide on the PR/issue. label Feb 20, 2020
@Centril
Copy link
Contributor

Centril commented Feb 20, 2020

Just to simplify a bit (still passes but shouldn't):

struct _Bar<T>(T);

impl<T> _Bar<T> {
    fn _map1(x: u8) -> _Bar<u8> {
        Self(x)
    }
}

@eddyb
Copy link
Member

eddyb commented Feb 20, 2020

Just checked on godbolt and it seems to compile in every stable release since it was stabilized (1.32.0). I haven't looked into the code again, but I don't really understand how #61896 didn't cover this. Did we forget to add a test with errors, and only tried to prevent ICEs, or something?

@Centril Centril assigned Centril and unassigned varkor Feb 21, 2020
@bors bors closed this as completed in 76fe449 Feb 28, 2020
hannobraun added a commit to hannobraun/vndf-2020 that referenced this issue Apr 28, 2020
That this code was accepted was a compiler bug. See:
rust-lang/rust#69306
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
C-bug Category: This is a bug. P-high High priority T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants