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

Made Self a keyword. #22158

Merged
merged 1 commit into from
Feb 14, 2015
Merged

Conversation

Kimundi
Copy link
Member

@Kimundi Kimundi commented Feb 10, 2015

It is only allowed in paths now, where it will either work inside a trait
or impl item, or not resolve outside of it.

[breaking-change]

Closes #22137

@rust-highfive
Copy link
Collaborator

r? @huonw

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

@alexcrichton
Copy link
Member

Could you tag this as a [breaking-change] as well?

@@ -1797,7 +1797,11 @@ impl<'a> Parser<'a> {
let mut segments = Vec::new();
loop {
// First, parse an identifier.
let identifier = self.parse_ident();
let identifier = if self.is_self_type_ident() {
Copy link
Member

Choose a reason for hiding this comment

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

Maybe this could be factored out into self.parse_ident_or_self_type()?

Copy link
Member Author

Choose a reason for hiding this comment

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

I guess that would make it somewhat more clear, yeah.

},
_ => {
let token_str = self.this_token_to_string();
self.fatal(&format!("expected `Self`, found `{}`",
Copy link
Contributor

Choose a reason for hiding this comment

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

there's no test for this error message

Copy link
Member Author

Choose a reason for hiding this comment

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

Right, this error message can not actually trigger right now, its just there because I copied the two parser functions for the self keyword.

@pnkfelix
Copy link
Member

@bors r+ 0d7ec46

@pnkfelix pnkfelix closed this Feb 12, 2015
@pnkfelix pnkfelix reopened this Feb 12, 2015
@pnkfelix
Copy link
Member

(sorry, hit wrong button by accident)

@pnkfelix
Copy link
Member

@bors r+ 0d7ec46

@bors
Copy link
Contributor

bors commented Feb 12, 2015

⌛ Testing commit 0d7ec46 with merge 0c409ae...

@bors
Copy link
Contributor

bors commented Feb 12, 2015

💔 Test failed - auto-mac-32-opt

@Kimundi
Copy link
Member Author

Kimundi commented Feb 12, 2015

Whoops, forgot to actually test a full bootstrap, and it seems like pattern bindings with the identifier Self are not yet rejected. Will fix asap.

It is only allowed in paths now, where it will either work inside a `trait`
or `impl` item, or not resolve outside of it.

[breaking-change]

Closes rust-lang#22137
@Kimundi
Copy link
Member Author

Kimundi commented Feb 12, 2015

Should pass make check now.

@alexcrichton
Copy link
Member

@bors: r=pnkfelix 07d00de

@bors
Copy link
Contributor

bors commented Feb 12, 2015

⌛ Testing commit 07d00de with merge 7378fd9...

bors added a commit that referenced this pull request Feb 12, 2015
It is only allowed in paths now, where it will either work inside a `trait`
or `impl` item, or not resolve outside of it.

[breaking-change]

Closes #22137
@bors
Copy link
Contributor

bors commented Feb 13, 2015

💔 Test failed - auto-linux-32-opt

@Kimundi
Copy link
Member Author

Kimundi commented Feb 13, 2015

Hm, the failure doesn't seem to be related to the PR, and does not reproduce locally.

@alexcrichton
Copy link
Member

@bors: retry

@bors
Copy link
Contributor

bors commented Feb 14, 2015

⌛ Testing commit 07d00de with merge b63cee4...

bors added a commit that referenced this pull request Feb 14, 2015
It is only allowed in paths now, where it will either work inside a `trait`
or `impl` item, or not resolve outside of it.

[breaking-change]

Closes #22137
@bors bors merged commit 07d00de into rust-lang:master Feb 14, 2015
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.

Make Self a keyword
7 participants