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

Fix GAT lifetime bounds #64756

Closed

Conversation

PlasmaPower
Copy link
Contributor

@PlasmaPower PlasmaPower commented Sep 25, 2019

This makes GAT lifetimes in traits (not impls) late-bound regions, which matches the RFC, stating that bounds placed on the GAT should be interpreted as HRTBs.

This fixes most of #62521, but the last code block in that issue fails because GAT impls are unimplemented (pun not intended).

This makes GAT lifetimes late-bound regions
@rust-highfive
Copy link
Collaborator

r? @eddyb

(rust_highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Sep 25, 2019
let outlives = ty::OutlivesPredicate(param_ty, region_bound);
(ty::Binder::dummy(outlives).to_predicate(), span)
(ty::Binder::bind(outlives).to_predicate(), span)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was flying blind here, and just tried to implement what the previous comment said, with a bit of trial and error. If someone who knows this code could review this that'd be appreciated.

@eddyb
Copy link
Member

eddyb commented Sep 25, 2019

r? @nikomatsakis cc @rust-lang/wg-traits

@rust-highfive rust-highfive assigned nikomatsakis and unassigned eddyb Sep 25, 2019
Copy link
Contributor

@nikomatsakis nikomatsakis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm. I have to think about this. I do not believe these should be late-bound, but rather early-bound -- we basically want to substitute them. In general, it'd be good @PlasmaPower if you wanted to pop into the #wg-traits channel on Zulip. We've been discussing on and off whether to pursue GATs in the context of Rust's current trait solver or postpone that work until we've made more progress on chalk (with an inclination to the latter).

@PlasmaPower
Copy link
Contributor Author

According to the RFC, specifically the Iterable example here, type GAT<'a>: 'a should be roughly interpreted as the following bound on the trait where for<'a> Self::GAT<'a>: 'a which is a late bound region from my understanding. However, this approach doesn't work for type generics, which is a gap in the RFC I think, or at least a gap in how to implement it with the current trait solver. I'll hop onto that channel/stream to discuss this further.

@eddyb
Copy link
Member

eddyb commented Oct 3, 2019

@PlasmaPower You can always synthesize where for<'lt> X<'lt>: 'lt, anonymous or not, but X needs to take an early-bound lifetime for X<'lt> to work at all.

Presumably the only thing missing is converting the (early-bound) lifetime parameter of a GAT to late-bound when generating the bounds.

I think we have a similar problem with constructors and their lifetimes not being turned into late-bound ones when computing the signature (#30904).

@PlasmaPower
Copy link
Contributor Author

To clarify, you're talking about something like fn f<'b>() -> Self::X<'b> later on, not reusing the lifetime with fn f() -> X<'lt> right? (the latter isn't supposed to work)

@eddyb
Copy link
Member

eddyb commented Oct 3, 2019

For a struct constructor, at least, all parameters must be used in the fields, so it would always be valid to use late-bound lifetimes (but this is getting a bit offtopic).

(To be clear, I was talking about the ADT constructors provided by the language, not user-written methods).

@PlasmaPower
Copy link
Contributor Author

Well it sounds like this approach isn't correct, and that it's planned to wait on chalk for GAT implementation, so I'll close this PR.

@PlasmaPower PlasmaPower closed this Oct 3, 2019
@alexreg
Copy link
Contributor

alexreg commented Oct 3, 2019

How close is Chalk to being ready for things like this?

@semtexzv
Copy link

semtexzv commented Oct 4, 2019

GAT and all the other goodness that comes with chalk is one of the things I'm most interested. I'd be glad to help if I can.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants