Skip to content

Conversation

GuillaumeGomez
Copy link
Member

Fixes #43286.

@rust-highfive
Copy link
Contributor

r? @pnkfelix

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

@carols10cents carols10cents added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jul 24, 2017
@arielb1
Copy link
Contributor

arielb1 commented Jul 24, 2017

r? @petrochenkov

self.span_err(self.span, &format!("expected identifier, found {}",
self.this_token_descr()));
let desc = self.this_token_descr();
if desc == "keyword `ref`" {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think the correct way of doing this is self.token.is_keyword(keyword::Ref)

Copy link
Member Author

Choose a reason for hiding this comment

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

Good point!

self.this_token_descr()));
let desc = self.this_token_descr();
if desc == "keyword `ref`" {
self.span_err_help(self.span,
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this is a prime candidate for span_suggestion

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 wondered for a while between the both of them. But if you prefer suggestion, I'll totally update to it.

@GuillaumeGomez
Copy link
Member Author

Updated.

@petrochenkov
Copy link
Contributor

fn parse_ident is not a good place for this diagnostics, it will fire in many totally unrelated situations.
Also, no recovery is happening.

I believe the correct fix should look like this:

--- a/src/libsyntax/parse/parser.rs
+++ b/src/libsyntax/parse/parser.rs
@@ -3523,8 +3523,18 @@ impl<'a> Parser<'a> {
             }
             // At this point, token != _, &, &&, (, [
             _ => if self.eat_keyword(keywords::Mut) {
-                // Parse mut ident @ pat
-                pat = self.parse_pat_ident(BindingMode::ByValue(Mutability::Mutable))?;
+                // Parse mut ident @ pat / mut ref ident @ pat
+                let mutref_span = self.prev_span.to(self.span);
+                let binding_mode = if self.eat_keyword(keywords::Ref) {
+                    self.diagnostic()
+                        .struct_span_err(mutref_span, "the order of `mut` and `ref` is incorrect")
+                        .span_suggestion(mutref_span, "try switching the order", "ref mut".into())
+                        .emit();
+                    BindingMode::ByRef(Mutability::Mutable)
+                } else {
+                    BindingMode::ByValue(Mutability::Mutable)
+                };
+                pat = self.parse_pat_ident(binding_mode)?;
             } else if self.eat_keyword(keywords::Ref) {
                 // Parse ref ident @ pat / ref mut ident @ pat
                 let mutbl = self.parse_mutability();

@GuillaumeGomez
Copy link
Member Author

@petrochenkov: Well, you did it. I can take your patch and apply but you sure you don't want to open a PR yourself? I'd feel bad to take it from you. :-/

@petrochenkov
Copy link
Contributor

@GuillaumeGomez
Okay, submitted #43489

@GuillaumeGomez GuillaumeGomez deleted the ref-mut-help branch July 26, 2017 18:09
bors added a commit that referenced this pull request Jul 27, 2017
Better diagnostics and recovery for `mut ref` in patterns

Fixes #43286
Supersedes #43451

r? @GuillaumeGomez
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.

7 participants