Remove proc keyword #23156
Remove proc keyword #23156
Conversation
|
r? @sfackler (rust_highfive has picked a reviewer for you, use r? to override) |
|
This looks good to me, but someone else (maybe @nikomatsakis ) should r+ it |
| @@ -561,7 +561,6 @@ declare_special_idents_and_keywords! { | |||
| (39, Virtual, "virtual"); | |||
| (40, While, "while"); | |||
| (41, Continue, "continue"); | |||
| (42, Proc, "proc"); | |||
| (43, Box, "box"); | |||
There was a problem hiding this comment.
Should these numbers be collapsed?
There was a problem hiding this comment.
Hum... Actually that's a pretty good question...
|
It's good now ! |
|
Maybe we could leave |
|
Why would you want to keep |
|
Technically, 're-implementing it' might break backwards compatibility, as you could have named something That said, I can see it either way. |
|
Well, as you prefer.
|
|
I would prefer to just leave it as is today. It's a somewhat useful word and we may find use for it one day (especially because it's already reserved). Adding a new keyword forces crates using the keyword to possibly opt-in to having the new keyword semantics, so it's not a free operation. |
|
I re-added it. Waiting for review. |
|
https://github.com/GuillaumeGomez/rust/blob/remove-proc/src/libsyntax/parse/token.rs#L525 |
|
@bombless: Well, I'll wait for others' opinion before doing anything but thanks for your comment ! |
|
Ah yes, could it be moved to the |
|
Yes of course. I change that. |
|
I moved |
| @@ -1052,7 +1052,7 @@ impl<'a> Parser<'a> { | |||
|
|
|||
| // examine next token to decide to do | |||
| if self.eat_keyword_noexpect(keywords::Proc) { | |||
| self.parse_proc_type(lifetime_defs) | |||
| self.fatal("`proc` keyword isn't used for the moment"); | |||
There was a problem hiding this comment.
It's ok to remove this specific handling of the Proc keyword, an error should show up regardless somewhere in the parser eventually.
|
@alexcrichton: I applied your comments. |
| let _ = self.parse_proc_decl(); | ||
| let _ = self.parse_expr(); | ||
| return self.obsolete_expr(span, ObsoleteSyntax::ProcExpr); | ||
| self.fatal("`proc` keyword isn't used for the moment"); |
There was a problem hiding this comment.
(this special case can also be removed)
|
So now we're back to my first commit haha. @alexcrichton r? ? |
| @@ -10,8 +10,8 @@ | |||
|
|
|||
| // Test that we generate obsolete syntax errors around usages of `proc`. | |||
|
|
|||
| fn foo(p: proc()) { } //~ ERROR obsolete syntax: the `proc` type | |||
| fn foo(p: proc()) { } //~ ERROR: the `proc` type isn't used for the moment | |||
There was a problem hiding this comment.
Should this error message be updated? I think the parser modifications you made earlier may have changed the error message (you can check with make check-stage2-pfail)
There was a problem hiding this comment.
Well, I think it's a good thing. Like that it's even more precise.
There was a problem hiding this comment.
Ah sorry, I meant that I'm not sure that this error message is still generated. Have you run make check-stage2-pfail after the recent modifications?
There was a problem hiding this comment.
Ah no, I always stop at stage1. And even if this error message isn't generated, a nice comment is always useful. ^^ Or it wasn't what you meant ?
There was a problem hiding this comment.
What I'm saying is that this error message cannot happen an this test needs to be updated if it wants to get past the bots. The ObsoleteSyntax changes you made above are never actually called (ProcExpr is never referenced and ProcType is only called in a dead function).
Could you:
- Delete
ProcExprandProcType - Delete
parse_proc_type - Update the error messages in this test
There was a problem hiding this comment.
I removed ProcExpr, ProcType and parse_proc_type. I'll run make check-stage2-pfail tomorrow.
|
Ready for review ! |
This is the implementation of the [RFC 584](rust-lang/rfcs#584).
This is the implementation of the RFC 584.