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

librustc: Implement the fully-expanded, UFCS form of explicit self. #14022

Merged
merged 3 commits into from
Jul 17, 2014

Conversation

pcwalton
Copy link
Contributor

@pcwalton pcwalton commented May 7, 2014

@nikomatsakis
Copy link
Contributor

I will (and want to) review this, but probably not till Monday (other things in the meantime).

That said, I have some concerns about the syntax we agreed upon, and I want to revisit it. Specifically, I think we should make it:

<type as Trait::<X,Y,Z>>::member

The reason being that this will be fully parsable and avoids all ambiguities.

For example, one can write <&mut [int] as Foo>::Bar etc.

@nikomatsakis
Copy link
Contributor

OK, so, I think this patch is pretty OK, modulo the concern about running region inference. Probably doesn't make sense to try and do anything more invasive (i.e., supporting arbitrary types), since I'm in the process of implementing #5527 which will permit that.

@pcwalton
Copy link
Contributor Author

@nikomatsakis Does that mean r+? :)

@nikomatsakis
Copy link
Contributor

@pcwalton seems like we should run region inference, if we're going to be checking the expected types. All that means is calling resolve_regions_and_report_errors(&self) after you create a new infer context. (Did you verify if my broken test indeed passed?)

@alexcrichton
Copy link
Member

ping @pcwalton, any progress on this?

@pcwalton
Copy link
Contributor Author

Will get to it after these couple of other P-backcompat-lang issues I'm working on.

@pcwalton
Copy link
Contributor Author

@nikomatsakis We can't unify types with regions in them because we don't know the variance of the regions yet during astconv. Boo. I'm not sure how to solve that problem. :(

@pcwalton
Copy link
Contributor Author

pcwalton commented Jul 8, 2014

Updated with a solution for the issue that @nikomatsakis brought up.

r? @pnkfelix

@pcwalton
Copy link
Contributor Author

pcwalton commented Jul 8, 2014

This is a prerequisite to fixing #15517.

}

impl<'a,'b> Foo<'a,'b> {
// The number of errors is related to the way invariance works.
Copy link
Member

Choose a reason for hiding this comment

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

hmm. I wonder if we should try to address that in some way. I'll file a ticket.

pcwalton added 2 commits July 16, 2014 20:01
This makes two changes to region inference: (1) it allows region
inference to relate early-bound regions; and (2) it allows regions to be
related before variance runs. The former is needed because there is no
relation between the two regions before region substitution happens,
while the latter is needed because type collection has to run before
variance. We assume that, before variance is inferred, that lifetimes
are invariant. This is a conservative overapproximation.

This relates to rust-lang#13885. This does not remove `~self` from the language
yet, however.

[breaking-change]
It'll be complex to port to the new explicit-self regime and it seems to
be unused.
@pcwalton pcwalton mentioned this pull request Jul 17, 2014
@bors bors closed this Jul 17, 2014
@bors bors merged commit 00c70d1 into rust-lang:master Jul 17, 2014
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

Successfully merging this pull request may close these issues.

5 participants