Rust: With a generic struct, `Self(x)` works where `x` is of the wrong type argument

Created on 16 Jun 2019  路  5Comments  路  Source: rust-lang/rust

In particular, we can write:

struct A<T>(T);

impl A<bool> {
    const B: A<u8> = Self(0); // OK but shouldn't be?

    const C: A<u8> = Self { 0: 0 }; // Correctly rejected.
}

This can lead to the following interesting situation:

struct A<T>(T);

impl A<bool> {
    const B: Self = Self(0); // Rejected, so `Self(..)` is not of type `Self`?
}

cc @petrochenkov, @varkor, @eddyb, and @alexreg

This does seem like a bug to me... In particular, type_alias_enum_variants does not allow Self::Some(transform(x)) where Self == Option<T> but where Option<U> is expected.

cc @rust-lang/lang

C-bug T-compiler

Most helpful comment

Self constructors were stabilized not too long ago, so this should preferably be fixed soon and backported to beta.

All 5 comments

Yes, that's a bug, Self in the example should be A<bool> and A::<bool>(0) should report an error.

We missed a test for this in https://github.com/rust-lang/rust/pull/53751 somehow (for Self types this property is tested in struct-path-self-type-mismatch.rs).

Self constructors were stabilized not too long ago, so this should preferably be fixed soon and backported to beta.

Ouch, this explains why we were able to remove the Ty result from rewrite_self_ctor as redundant in #61085 / #61189 - there was a subst call missing on that type.
But that's not the end of it, this is incorrectly allowed right now, as well:

struct A<T>(T);

impl A<bool> {
    const B: A<u8> = Self::<u8>(0);
}

So the order here is wrong: https://github.com/rust-lang/rust/blob/374c63e0fc356eb61b1966cb6026a2a49fe9226d/src/librustc_typeck/check/mod.rs#L5368-L5377
In that Res::SelfCtor really needs to be treated like Res::Local.

In general, I think instead of rewrite_self_ctor we need a method that only returns the right information (such as the modified Res and substituted Ty) given the impl DefId inside Res::SelfCtor.
And every call should be audited.
EDIT: there's only one call now, huh, so it can be inlined.
I guess I could prepare a master and beta PR?

We shouldn't forget tests like A<&'static T> being created with Self(non_static_ref).

Looks like the test added in #59025 for #57924 has an explicit generic arg: Self::<E>(e), but somehow @varkor and I didn't spot that during the review, whoops.

Was this page helpful?
0 / 5 - 0 ratings