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

Coercion sites are not consistent between struct tuples and structs #31260

Closed
nox opened this issue Jan 28, 2016 · 15 comments · Fixed by #42807
Closed

Coercion sites are not consistent between struct tuples and structs #31260

nox opened this issue Jan 28, 2016 · 15 comments · Fixed by #42807
Assignees
Labels
T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@nox
Copy link
Contributor

nox commented Jan 28, 2016

pub struct Struct<K: 'static> {
    pub field: K,
}

static STRUCT: Struct<&'static [u8]> = Struct {
    field: &[1]
};

pub struct Tuple<K: 'static>(K);

static TUPLE: Tuple<&'static [u8]> = Tuple(&[1]);

STRUCT fails to type check, while TUPLE succeeds.

<anon>:7:40: 9:2 error: mismatched types:
 expected `Struct<&'static [u8]>`,
    found `Struct<&[_; 1]>`
(expected slice,
    found array of 1 elements) [E0308]
<anon>:7 static STRUCT: Struct<&'static [u8]> = Struct {
<anon>:8     field: &[1]
<anon>:9 };
<anon>:7:40: 9:2 help: see the detailed explanation for E0308
@nox nox changed the title Coercion sites are not consistent between tuples and structs. Coercion sites are not consistent between tuples and structs Jan 28, 2016
@SimonSapin
Copy link
Contributor

This coercion is important when using b"…" byte string literals.

nox added a commit to nox/rust-phf that referenced this issue Jan 28, 2016
Because of rust-lang/rust#31260, a cast must be inserted
explicitly instead of letting rustc implicitly coercing the type.
@eddyb
Copy link
Member

eddyb commented Jan 29, 2016

The tuple struct case is actually the function call "expected type" propagation, which would have to be adapted to struct literals as well.

@nox nox changed the title Coercion sites are not consistent between tuples and structs Coercion sites are not consistent between struct tuples and structs Jan 29, 2016
@nox
Copy link
Contributor Author

nox commented Jan 29, 2016

Yes.

@SimonSapin
Copy link
Contributor

Coercion already happens in non-generic struct literals:

struct Foo { s: &'static [u8] }
static FOO: Foo = Foo { s: b"foo" };

@eddyb
Copy link
Member

eddyb commented Jan 29, 2016

@SimonSapin Yes, but the field types are taken from the struct and not combined with the overall expected type for the struct literal.

nox added a commit to nox/rust-phf that referenced this issue Jan 31, 2016
Because of rust-lang/rust#31260, a cast must be inserted
explicitly instead of letting rustc implicitly coercing the type.
@abonander
Copy link
Contributor

abonander commented Jun 12, 2016

I just discovered this bug/unexpected behavior today (#34240). I would like to see some movement on it because it's surprising that these two don't work the same way.

Here's a workaround I found; it's ugly as hell but I'm using it in a generated file that no human would want to look through anyway: https://is.gd/boA5ut

I tried type ascription but it seems to not cue coercion, it just moves the type error: https://is.gd/yut0EK

It seems that forcing a concrete type on the struct literal works as well (I didn't expect this syntax to work, honestly): https://is.gd/ZipLo1 Unfortunately this won't work for me because I'm using phf_codegen too and I can't add this extra syntax myself.

@nox
Copy link
Contributor Author

nox commented Jan 18, 2017

Any news on this?

@KalitaAlexey
Copy link
Contributor

Is anyone working on it?

frewsxcv added a commit to frewsxcv/rust that referenced this issue Mar 17, 2017
Propagate expected type hints through struct literals.

Partial fix for rust-lang#31260 to maximize backwards-compatibility, i.e. the hint is provided but not coerced to.

The added test works because `{...; x}` with a hint of `T` coerces `x` to `T`, and the reasoning why that is slightly different has to do with DSTs: `&Struct { tail: [x] }: &Struct<[T]>` has a hint of `[T]` for `[x]`, but the inferred type should be `[T; 1]` to succeed later, so `[x]` shouldn't be *forced* to be `[T]`.

*However*, implementing that complete behavior in a backwards-compatible way may be non-trivial, and has not yet been fully investigated, while this PR fixes rust-lang#40355 and can be backported.

r? @nikomatsakis
frewsxcv added a commit to frewsxcv/rust that referenced this issue Mar 17, 2017
Propagate expected type hints through struct literals.

Partial fix for rust-lang#31260 to maximize backwards-compatibility, i.e. the hint is provided but not coerced to.

The added test works because `{...; x}` with a hint of `T` coerces `x` to `T`, and the reasoning why that is slightly different has to do with DSTs: `&Struct { tail: [x] }: &Struct<[T]>` has a hint of `[T]` for `[x]`, but the inferred type should be `[T; 1]` to succeed later, so `[x]` shouldn't be *forced* to be `[T]`.

*However*, implementing that complete behavior in a backwards-compatible way may be non-trivial, and has not yet been fully investigated, while this PR fixes rust-lang#40355 and can be backported.

r? @nikomatsakis
frewsxcv added a commit to frewsxcv/rust that referenced this issue Mar 18, 2017
Propagate expected type hints through struct literals.

Partial fix for rust-lang#31260 to maximize backwards-compatibility, i.e. the hint is provided but not coerced to.

The added test works because `{...; x}` with a hint of `T` coerces `x` to `T`, and the reasoning why that is slightly different has to do with DSTs: `&Struct { tail: [x] }: &Struct<[T]>` has a hint of `[T]` for `[x]`, but the inferred type should be `[T; 1]` to succeed later, so `[x]` shouldn't be *forced* to be `[T]`.

*However*, implementing that complete behavior in a backwards-compatible way may be non-trivial, and has not yet been fully investigated, while this PR fixes rust-lang#40355 and can be backported.

r? @nikomatsakis
arielb1 pushed a commit to arielb1/rust that referenced this issue Mar 18, 2017
Propagate expected type hints through struct literals.

Partial fix for rust-lang#31260 to maximize backwards-compatibility, i.e. the hint is provided but not coerced to.

The added test works because `{...; x}` with a hint of `T` coerces `x` to `T`, and the reasoning why that is slightly different has to do with DSTs: `&Struct { tail: [x] }: &Struct<[T]>` has a hint of `[T]` for `[x]`, but the inferred type should be `[T; 1]` to succeed later, so `[x]` shouldn't be *forced* to be `[T]`.

*However*, implementing that complete behavior in a backwards-compatible way may be non-trivial, and has not yet been fully investigated, while this PR fixes rust-lang#40355 and can be backported.

r? @nikomatsakis
frewsxcv added a commit to frewsxcv/rust that referenced this issue Mar 20, 2017
Propagate expected type hints through struct literals.

Partial fix for rust-lang#31260 to maximize backwards-compatibility, i.e. the hint is provided but not coerced to.

The added test works because `{...; x}` with a hint of `T` coerces `x` to `T`, and the reasoning why that is slightly different has to do with DSTs: `&Struct { tail: [x] }: &Struct<[T]>` has a hint of `[T]` for `[x]`, but the inferred type should be `[T; 1]` to succeed later, so `[x]` shouldn't be *forced* to be `[T]`.

*However*, implementing that complete behavior in a backwards-compatible way may be non-trivial, and has not yet been fully investigated, while this PR fixes rust-lang#40355 and can be backported.

r? @nikomatsakis
@nikomatsakis
Copy link
Contributor

It seems like @eddyb implemented a partial fix in #40398 but suggests that there are backwards incompatibility issues to be considered that preventing him from going further. I'm not quite sure what he had in mind there, perhaps he can elaborate.

@arielb1 arielb1 added I-nominated T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels May 4, 2017
@arielb1
Copy link
Contributor

arielb1 commented May 4, 2017

Making sure we remember to look at this.

@eddyb
Copy link
Member

eddyb commented May 4, 2017

@nikomatsakis What I did is I computed a "light hint" (which becomes "strong hint", i.e. forcing coercion, if it reaches a block, and it's enough to help integer inference for #40355).

This is a common problem with coercion hints, in that unless there's enough type information to prevent them from triggering, they will trigger early even if that breaks later on.

The thing to try here is to replace these two lines with the simpler:

self.check_expr_coercable_to_type(&field.expr, field_type_hint)

final_field_type doesn't even need to be computed if that works, since field_type_hint is at least as specific - AFAICT, we already have that "specificity" with function calls, but since it hasn't been used with struct literals before, crater/cargobomb might reveal regressions.

@nikomatsakis
Copy link
Contributor

@eddyb ok, I was mostly wondering if you encountered actual regressions in practice, or just theorize that this might lead to problems.

@eddyb
Copy link
Member

eddyb commented May 4, 2017

@nikomatsakis I don't think I got the chance to do a crater run and then I lost track of it.

@nikomatsakis
Copy link
Contributor

@arielb1 will open a PR and we can try a crater run at least.

@arielb1 arielb1 self-assigned this Jun 18, 2017
arielb1 added a commit to arielb1/rust that referenced this issue Jun 21, 2017
Fully fixes rust-lang#31260.

This needs a crater run.
bors added a commit that referenced this issue Jun 30, 2017
Coerce fields to the expected field type

Fully fixes #31260.

This needs a crater run. I was supposed to do this last month but it slipped. Let's get this done.
@burdges
Copy link

burdges commented Jun 27, 2018

There is still a similar problem with the type inference of temporaries when you have an array that should be interpreted as a slice buried in a tuple inside an outer slice.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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.

8 participants