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

Can't compile type with lifetime bound in certain order #39085

Closed
frewsxcv opened this issue Jan 15, 2017 · 4 comments
Closed

Can't compile type with lifetime bound in certain order #39085

frewsxcv opened this issue Jan 15, 2017 · 4 comments
Assignees

Comments

@frewsxcv
Copy link
Member

frewsxcv commented Jan 15, 2017

This compiles:

fn lol<'a>() -> Box<Iterator<Item=()> + 'a> {
    unimplemented!()
}

fn main() {
}

This doesn't:

fn lol<'a>() -> Box<Iterator<'a + Item=()>> {
    unimplemented!()
}

fn main() {
}

Error:

rustc 1.16.0-nightly (1a2ed98d3 2017-01-13)
error: expected `,` or `>` after lifetime name, found `+`
 --> <anon>:1:24
  |
1 | fn lol<'a>() -> Box<'a + Iterator<Item=()>> {
  |                        ^
  |
note: did you mean a single argument type &'a Type, or did you mean the comma-separated arguments 'a, Type?
 --> <anon>:1:21
  |
1 | fn lol<'a>() -> Box<'a + Iterator<Item=()>> {
  |                     ^^^^

error: aborting due to previous error
@frewsxcv
Copy link
Member Author

cc @rkruppe

@hanna-kruppe
Copy link
Contributor

The two snippets above are slightly different in where the lifetime applies (Iterator vs. Item). This is shorter and doesn't have such a difference:

type A<'a> = Box<Display + 'a>;
type B<'a> = Box<'a + Display>;

@petrochenkov
Copy link
Contributor

petrochenkov commented Jan 15, 2017

I'm actually working on this right now (I started redoing #37511, but the scope has grown).
Ideally, anything acceptable as a bound list (the part of T: 'a + ?Sized + Trait + for<'a> Trait2<'a> after :) should be accepted as a (trait object) type as well (at parse time).
There are some ambiguities however and the compiler has a bunch of parsing quirks to address them right now. Not accepting lifetimes as first elements in a type sum is one of them.

bors added a commit that referenced this issue Jan 17, 2017
Merge ObjectSum and PolyTraitRef in AST/HIR + some other refactoring

`ObjectSum` and `PolyTraitRef` are the same thing (list of bounds), they exist separately only due to parser quirks. The second commit merges them.

The first commit replaces `Path` with `Ty` in (not yet supported) equality predicates. They are parsed as types anyway and arbitrary types can always be disguised as paths using aliases, so this doesn't add any new functionality.

The third commit uses `Vec` instead of `P<[T]>` in AST. AST is not immutable like HIR and `Vec`s are more convenient for it, unnecessary conversions are also avoided.

The last commit renames `parse_ty_sum` (which is used for parsing types in general) into `parse_ty`, and renames `parse_ty` (which is used restricted contexts where `+` is not permitted due to operator priorities or other reasons) into `parse_ty_no_plus`.

This is the first part of #39085 (comment) and #39080 focused on data changes and mechanical renaming, I'll submit a PR with parser changes a bit later.

r? @eddyb
@petrochenkov petrochenkov self-assigned this Feb 19, 2017
bors added a commit that referenced this issue Mar 22, 2017
Refactor parsing of trait object types

Bugs are fixed and code is cleaned up.

User visible changes:
- `ty` matcher in macros accepts trait object types like `Write + Send` (#39080)
- Buggy priority of `+` in trait object types starting with `for` is fixed (#39317). `&for<'a> Trait<'a> + Send` is now parsed as `(&for<'a> Trait<'a>) + Send` and requires parens `&(for<'a> Trait<'a> + Send)`. For comparison, `&Send + for<'a> Trait<'a>` was parsed like this since [Nov 27, 2014](#19298).
- Trailing `+`s are supported in trait objects, like in other bounds.
- Better error reporting for trait objects starting with `?Sized`.

Fixes #39080
Fixes #39317 [breaking-change]
Closes #39298
cc #39085 (fixed, then reverted #40043 (comment))
cc #39318 (fixed, then reverted #40043 (comment))

r? @nikomatsakis
@petrochenkov
Copy link
Contributor

So, #40043 landed without a fix for this issue (see #40043 (comment) and below).
@nikomatsakis @withoutboats and @solson were mildly-against/mildly-for/undecided with regards to this change, so it is not implemented yet, but may be implemented in the future.

Arguments for:

  • (technical) Bounds in other contexts can start with a lifetime bound - fn f<T: 'a + Trait>() {}, fn f() -> impl 'a + Trait, but bounds in trait objects should start with a type and Box<'a + Trait> is not permitted, this is an inconsistency in the grammar.
  • (technical) 'a + Trait is syntactically unambiguous and can be trivially supported.
  • (philosophical) If A + B is supported, then B + A should be expected to work as well.

Arguments against:

  • (philosophical) 'a + Trait is not an idiomatic style, Trait is usually the most important component and goes first.
  • (philosophical) If Box<'a> is not supported, then Box<'a + Trait> shouldn't be as well.
  • (philosophical) 'a is parsed as a lifetime and not as a type, but 'a + is parsed as a type (but rejected later by a semantic check), this looks strange, trailing +s look strange in general though.

bors added a commit that referenced this issue Apr 28, 2017
syntax: Parse trait object types starting with a lifetime bound

Fixes #39085

This was originally implemented in #40043, then reverted, then there was some [agreement](#39318 (comment)) that it should be supported.
(This is hopefully the last PR related to bound parsing.)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants