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 'lifetime become a valid "trait object" type? #39298

Closed
petrochenkov opened this issue Jan 25, 2017 · 1 comment · Fixed by #40043
Closed

Can 'lifetime become a valid "trait object" type? #39298

petrochenkov opened this issue Jan 25, 2017 · 1 comment · Fixed by #40043
Assignees
Labels
A-parser Area: The parsing of Rust source code to an AST.

Comments

@petrochenkov
Copy link
Contributor

petrochenkov commented Jan 25, 2017

I'm going to fix #39085 by accepting arbitrary bound lists as trait object types and trying to figure out future compatibility concerns.

Consider these examples:

#![feature(conservative_impl_trait)]

// Static anonymization via associated type
trait Tr {
    type A: Send; // We know only that A is Sized + Send
    type B: 'static; // We know only that A is Sized + 'static
    type C; // We know only that C is Sized
}

// Static anonymization via impl Trait
fn f() -> impl Send {} // We know only that returned type is Sized + Send
fn g() -> impl 'static {} // We know only that returned type is Sized + 'static
fn h() -> impl {} // We know only that returned type is Sized
// Fun fact: `h` is currently parsed successfully and rejected by a semantic check

// Dynamic anonymization via trait objects
fn i<'a>(arg: &'a Send) {} // We know that the argument is Sized + Send
fn j<'a>(arg: &'a 'static /*What?*/) {} // We know that the argument is Sized + 'static
fn k<'a>(arg: &'a /*WTF, I don't even. Also ambiguity.*/) {} // We know that the argument is Sized

fn main() {}

As you can see there are two interesting cases - the list is empty and the list contains a single lifetime bound.

I'm not seriously considering *nothing* becoming accepted as a valid type at parse time, but 'lifetime case is more ambiguous.
I don't see syntactic or semantic reasons to prohibit something like plain 'static from being used in impl Trait type or an object type, maybe it can even be useful sometimes? So far I'm going to structure the parsing code with this possibility in mind.

cc @nikomatsakis @eddyb

@nikomatsakis
Copy link
Contributor

My preference would be to disallow it. I agree there is no particular reason to do so, but there is also little reason to accept it, and I would rather start out more conservatively and expand later. =)

@petrochenkov petrochenkov self-assigned this Feb 19, 2017
@petrochenkov petrochenkov added the A-parser Area: The parsing of Rust source code to an AST. label 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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-parser Area: The parsing of Rust source code to an AST.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants