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

T: ?Sized does not work in where clauses #20503

Closed
ftxqxd opened this issue Jan 4, 2015 · 15 comments
Closed

T: ?Sized does not work in where clauses #20503

ftxqxd opened this issue Jan 4, 2015 · 15 comments
Labels
A-DSTs Area: Dynamically-sized types (DSTs)

Comments

@ftxqxd
Copy link
Contributor

ftxqxd commented Jan 4, 2015

fn foo<T>(x: &T) where T: ?Sized {} // error: unexpected `?`

The RFC states that the syntax should work in where clauses.

@nrc nrc assigned nrc and unassigned nrc Jan 4, 2015
@nrc
Copy link
Member

nrc commented Jan 4, 2015

Note that we don't currently accept where Sized? T, so there is no backwards compatibility hazard here. Although we should certainly fix this, it doesn't need to be done for 1.0 (it probably should be though).

@nrc nrc added the A-DSTs Area: Dynamically-sized types (DSTs) label Jan 4, 2015
tomprogrammer referenced this issue in pfalabella/rust-ascii Jan 4, 2015
@jroesch
Copy link
Member

jroesch commented Jan 6, 2015

@nick29581 I will poke into this.

@ftxqxd
Copy link
Contributor Author

ftxqxd commented May 16, 2015

There are a few edge cases here. In particular, specifying ?Sized in a where clause should probably only be allowed at the type parameter’s definition site; e.g., the following probably shouldn’t be allowed:

impl<T> Foo<T> {
    fn foo(&self) where T: ?Sized { ... }
}

Although allowing that could potentially be useful, I suspect it would complicate the implementation and be slightly unclear anyway.

Additionally, specifying ?Sized on an associated type is unnecessary. In general, specifying ?Sized on anything that isn’t a type parameter is somewhat nonsensical: where Option<T>: ?Sized doesn’t really make much sense either.

@aochagavia
Copy link
Contributor

What needs to be done in order to get this working? Is it just a parsing issue?

@Gankra
Copy link
Contributor

Gankra commented Aug 5, 2015

Triage: still a really annoying problem.

@jroesch
Copy link
Member

jroesch commented Aug 5, 2015

@gankro I can fix this, probably my fault anyways.

@kamalmarhubi
Copy link
Contributor

This is still a bit of a wart, and tripped me up the first time I wanted to use a ?Sized bound.

@ihrwein
Copy link
Contributor

ihrwein commented Apr 12, 2016

I hit this issue today.

@kamalmarhubi
Copy link
Contributor

@jroesch although it was a few months ago, do you remember enough context to write a very quick summary of where changes would need to be made to fix this?

@tinyplasticgreyknight
Copy link

tinyplasticgreyknight commented Jun 12, 2016

Ran into this one today. Is it just a parsing problem?

@jonas-schievink
Copy link
Contributor

IIRC this also needs to be handled in astconv

@jonas-schievink
Copy link
Contributor

add_unsized_bound would have to take the where-clause of the item into account, but I think the where-clause isn't available there.

@johanneshoehn
Copy link

There at least should be some better error message telling one to put ?Sized into the definition instead of the where clause.

@matthieu-m
Copy link
Contributor

matthieu-m commented Oct 29, 2016

This is really a papercut issue.

While when I hit I generally remember "Oh yeah, this bug again" and promptly fixes it, it still would be a much smoother experience if I did not have to.

Note that in the same vein, it is not supported in a trait bounds either (eg: trait X: ?Sized does not work).

@petrochenkov
Copy link
Contributor

I tried to fix this but have immediately drowned in philosophical questions.

?Sized at the moment is not a true bound, but a command - "do not implicitly add Sized bound".
I.e.

fn f<FreshParameter: ?Sized + OtherBounds1>() where ExistingType: OtherBounds2 { ... }

is desugared into

// `Sized` bound is not implicitly added
fn f<FreshParameter: OtherBounds1>() where ExistingType: OtherBounds2 { ... }

and

fn f<FreshParameter: OtherBounds1>() where ExistingType: OtherBounds2 { ... }

is desugared into

`Sized` bound is implicitly added
fn f<FreshParameter: Sized + OtherBounds1>() where ExistingType: OtherBounds2 { ... }

A few conclusions from this:

  • where clauses don't implicitly add Sized bounds so the command ?Sized don't do anything on them
  • ?Sized makes sense only when we define some new entity (like type parameter introduction or associated type definition do) and don't make sense when we require some bounds from an already existing entity.

So, how I think the fix should look:

  • TYPE: ?Trait bounds from where clauses are internally moved on parameter definitions if and only if TYPE is a bare type parameter defined in the current item.
  • if TYPE is a type parameter from one of parent items or any other arbitrary type, then error is reported.
  • where (FreshParameter): ?Sized is a questionable case. I'd classify it as "any other arbitrary type" and report an error, but the information about parens is lost in HIR.

Other stuff:

  1. It's not just parsing issue.

  2. @matthieu-m

    it is not supported in a trait bounds either (eg: trait X: ?Sized does not work).

    traits are ?Sized by default, so trait X: ?Sized doesn't make much sense. It's matter of a better error message (and the error should certainly not be reported in the parser, like now).

bors added a commit that referenced this issue Nov 28, 2016
Support `?Sized` in where clauses

Implemented as described in #20503 (comment) - `?Trait` bounds are moved on type parameter definitions when possible, reported as errors otherwise.
(It'd be nice to unify bounds and where clauses in HIR, but this is mostly blocked by rustdoc now - it needs to render bounds in pleasant way and the best way to do it so far is to mirror what was written in source code.)

Fixes #20503
r? @nikomatsakis
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-DSTs Area: Dynamically-sized types (DSTs)
Projects
None yet
Development

No branches or pull requests