Support parentheses in patterns under feature gate

Improve recovery for trailing comma after `..`
rust-lang · Feb 28, 2018 · c9aff92 · c9aff92
1 parent 0ff9872
commit c9aff92
Show file tree

Hide file tree

Showing 12 changed files with 166 additions and 107 deletions.
diff --git a/src/librustc/hir/lowering.rs b/src/librustc/hir/lowering.rs
@@ -2465,86 +2465,88 @@ impl<'a> LoweringContext<'a> {
     }
 
     fn lower_pat(&mut self, p: &Pat) -> P<hir::Pat> {
-        let LoweredNodeId { node_id, hir_id } = self.lower_node_id(p.id);
+        let node = match p.node {
+            PatKind::Wild => hir::PatKind::Wild,
+            PatKind::Ident(ref binding_mode, pth1, ref sub) => {
+                match self.resolver.get_resolution(p.id).map(|d| d.base_def()) {
+                    // `None` can occur in body-less function signatures
+                    def @ None | def @ Some(Def::Local(_)) => {
+                        let canonical_id = match def {
+                            Some(Def::Local(id)) => id,
+                            _ => p.id
+                        };
+                        hir::PatKind::Binding(self.lower_binding_mode(binding_mode),
+                                                canonical_id,
+                                                respan(pth1.span, pth1.node.name),
+                                                sub.as_ref().map(|x| self.lower_pat(x)))
+                    }
+                    Some(def) => {
+                        hir::PatKind::Path(hir::QPath::Resolved(None, P(hir::Path {
+                            span: pth1.span,
+                            def,
+                            segments: hir_vec![
+                                hir::PathSegment::from_name(pth1.node.name)
+                            ],
+                        })))
+                    }
+                }
+            }
+            PatKind::Lit(ref e) => hir::PatKind::Lit(P(self.lower_expr(e))),
+            PatKind::TupleStruct(ref path, ref pats, ddpos) => {
+                let qpath = self.lower_qpath(p.id, &None, path, ParamMode::Optional,
+                                                ImplTraitContext::Disallowed);
+                hir::PatKind::TupleStruct(qpath,
+                                            pats.iter().map(|x| self.lower_pat(x)).collect(),
+                                            ddpos)
+            }
+            PatKind::Path(ref qself, ref path) => {
+                hir::PatKind::Path(self.lower_qpath(p.id, qself, path, ParamMode::Optional,
+                                                    ImplTraitContext::Disallowed))
+            }
+            PatKind::Struct(ref path, ref fields, etc) => {
+                let qpath = self.lower_qpath(p.id, &None, path, ParamMode::Optional,
+                                                ImplTraitContext::Disallowed);
+
+                let fs = fields.iter()
+                                .map(|f| {
+                                    Spanned {
+                                        span: f.span,
+                                        node: hir::FieldPat {
+                                            name: self.lower_ident(f.node.ident),
+                                            pat: self.lower_pat(&f.node.pat),
+                                            is_shorthand: f.node.is_shorthand,
+                                        },
+                                    }
+                                })
+                                .collect();
+                hir::PatKind::Struct(qpath, fs, etc)
+            }
+            PatKind::Tuple(ref elts, ddpos) => {
+                hir::PatKind::Tuple(elts.iter().map(|x| self.lower_pat(x)).collect(), ddpos)
+            }
+            PatKind::Box(ref inner) => hir::PatKind::Box(self.lower_pat(inner)),
+            PatKind::Ref(ref inner, mutbl) => {
+                hir::PatKind::Ref(self.lower_pat(inner), self.lower_mutability(mutbl))
+            }
+            PatKind::Range(ref e1, ref e2, ref end) => {
+                hir::PatKind::Range(P(self.lower_expr(e1)),
+                                    P(self.lower_expr(e2)),
+                                    self.lower_range_end(end))
+            }
+            PatKind::Slice(ref before, ref slice, ref after) => {
+                hir::PatKind::Slice(before.iter().map(|x| self.lower_pat(x)).collect(),
+                            slice.as_ref().map(|x| self.lower_pat(x)),
+                            after.iter().map(|x| self.lower_pat(x)).collect())
+            }
+            PatKind::Paren(ref inner) => return self.lower_pat(inner),
+            PatKind::Mac(_) => panic!("Shouldn't exist here"),
+        };
 
+        let LoweredNodeId { node_id, hir_id } = self.lower_node_id(p.id);
         P(hir::Pat {
             id: node_id,
             hir_id,
-            node: match p.node {
-                PatKind::Wild => hir::PatKind::Wild,
-                PatKind::Ident(ref binding_mode, pth1, ref sub) => {
-                    match self.resolver.get_resolution(p.id).map(|d| d.base_def()) {
-                        // `None` can occur in body-less function signatures
-                        def @ None | def @ Some(Def::Local(_)) => {
-                            let canonical_id = match def {
-                                Some(Def::Local(id)) => id,
-                                _ => p.id
-                            };
-                            hir::PatKind::Binding(self.lower_binding_mode(binding_mode),
-                                                  canonical_id,
-                                                  respan(pth1.span, pth1.node.name),
-                                                  sub.as_ref().map(|x| self.lower_pat(x)))
-                        }
-                        Some(def) => {
-                            hir::PatKind::Path(hir::QPath::Resolved(None, P(hir::Path {
-                                span: pth1.span,
-                                def,
-                                segments: hir_vec![
-                                    hir::PathSegment::from_name(pth1.node.name)
-                                ],
-                            })))
-                        }
-                    }
-                }
-                PatKind::Lit(ref e) => hir::PatKind::Lit(P(self.lower_expr(e))),
-                PatKind::TupleStruct(ref path, ref pats, ddpos) => {
-                    let qpath = self.lower_qpath(p.id, &None, path, ParamMode::Optional,
-                                                 ImplTraitContext::Disallowed);
-                    hir::PatKind::TupleStruct(qpath,
-                                              pats.iter().map(|x| self.lower_pat(x)).collect(),
-                                              ddpos)
-                }
-                PatKind::Path(ref qself, ref path) => {
-                    hir::PatKind::Path(self.lower_qpath(p.id, qself, path, ParamMode::Optional,
-                                                        ImplTraitContext::Disallowed))
-                }
-                PatKind::Struct(ref path, ref fields, etc) => {
-                    let qpath = self.lower_qpath(p.id, &None, path, ParamMode::Optional,
-                                                 ImplTraitContext::Disallowed);
-
-                    let fs = fields.iter()
-                                   .map(|f| {
-                                       Spanned {
-                                           span: f.span,
-                                           node: hir::FieldPat {
-                                               name: self.lower_ident(f.node.ident),
-                                               pat: self.lower_pat(&f.node.pat),
-                                               is_shorthand: f.node.is_shorthand,
-                                           },
-                                       }
-                                   })
-                                   .collect();
-                    hir::PatKind::Struct(qpath, fs, etc)
-                }
-                PatKind::Tuple(ref elts, ddpos) => {
-                    hir::PatKind::Tuple(elts.iter().map(|x| self.lower_pat(x)).collect(), ddpos)
-                }
-                PatKind::Box(ref inner) => hir::PatKind::Box(self.lower_pat(inner)),
-                PatKind::Ref(ref inner, mutbl) => {
-                    hir::PatKind::Ref(self.lower_pat(inner), self.lower_mutability(mutbl))
-                }
-                PatKind::Range(ref e1, ref e2, ref end) => {
-                    hir::PatKind::Range(P(self.lower_expr(e1)),
-                                        P(self.lower_expr(e2)),
-                                        self.lower_range_end(end))
-                }
-                PatKind::Slice(ref before, ref slice, ref after) => {
-                    hir::PatKind::Slice(before.iter().map(|x| self.lower_pat(x)).collect(),
-                                slice.as_ref().map(|x| self.lower_pat(x)),
-                                after.iter().map(|x| self.lower_pat(x)).collect())
-                }
-                PatKind::Mac(_) => panic!("Shouldn't exist here"),
-            },
+            node,
             span: p.span,
         })
     }

diff --git a/src/librustc_lint/builtin.rs b/src/librustc_lint/builtin.rs
@@ -737,6 +737,7 @@ impl EarlyLintPass for IllegalFloatLiteralPattern {
                 PatKind::TupleStruct(..) |
                 PatKind::Ref(..) |
                 PatKind::Box(..) |
+                PatKind::Paren(..) |
                 PatKind::Slice(..) => (),
 
                 // Extract the expressions and check them

diff --git a/src/libsyntax/ast.rs b/src/libsyntax/ast.rs
@@ -562,7 +562,7 @@ impl Pat {
             PatKind::TupleStruct(_, ref s, _) | PatKind::Tuple(ref s, _) => {
                 s.iter().all(|p| p.walk(it))
             }
-            PatKind::Box(ref s) | PatKind::Ref(ref s, _) => {
+            PatKind::Box(ref s) | PatKind::Ref(ref s, _) | PatKind::Paren(ref s) => {
                 s.walk(it)
             }
             PatKind::Slice(ref before, ref slice, ref after) => {
@@ -656,6 +656,8 @@ pub enum PatKind {
     /// `[a, b, ..i, y, z]` is represented as:
     ///     `PatKind::Slice(box [a, b], Some(i), box [y, z])`
     Slice(Vec<P<Pat>>, Option<P<Pat>>, Vec<P<Pat>>),
+    /// Parentheses in patters used for grouping, i.e. `(PAT)`.
+    Paren(P<Pat>),
     /// A macro pattern; pre-expansion
     Mac(Mac),
 }

diff --git a/src/libsyntax/feature_gate.rs b/src/libsyntax/feature_gate.rs
@@ -449,6 +449,9 @@ declare_features! (
 
     // Multiple patterns with `|` in `if let` and `while let`
     (active, if_while_or_patterns, "1.26.0", Some(48215)),
+
+    // Parentheses in patterns
+    (active, pattern_parentheses, "1.26.0", None),
 );
 
 declare_features! (
@@ -1663,6 +1666,10 @@ impl<'a> Visitor<'a> for PostExpansionVisitor<'a> {
                 gate_feature_post!(&self, dotdoteq_in_patterns, pattern.span,
                                    "`..=` syntax in patterns is experimental");
             }
+            PatKind::Paren(..) => {
+                gate_feature_post!(&self, pattern_parentheses, pattern.span,
+                                   "parentheses in patterns are unstable");
+            }
             _ => {}
         }
         visit::walk_pat(self, pattern)

diff --git a/src/libsyntax/fold.rs b/src/libsyntax/fold.rs
@@ -1148,6 +1148,7 @@ pub fn noop_fold_pat<T: Folder>(p: P<Pat>, folder: &mut T) -> P<Pat> {
                        slice.map(|x| folder.fold_pat(x)),
                        after.move_map(|x| folder.fold_pat(x)))
             }
+            PatKind::Paren(inner) => PatKind::Paren(folder.fold_pat(inner)),
             PatKind::Mac(mac) => PatKind::Mac(folder.fold_mac(mac))
         },
         span: folder.new_span(span)

diff --git a/src/libsyntax/parse/parser.rs b/src/libsyntax/parse/parser.rs
@@ -3484,33 +3484,47 @@ impl<'a> Parser<'a> {
         };
     }
 
-    fn parse_pat_tuple_elements(&mut self, unary_needs_comma: bool)
-                                -> PResult<'a, (Vec<P<Pat>>, Option<usize>)> {
-        let mut fields = vec![];
-        let mut ddpos = None;
+    // Parses a parenthesized list of patterns like
+    // `()`, `(p)`, `(p,)`, `(p, q)`, or `(p, .., q)`. Returns:
+    // - a vector of the patterns that were parsed
+    // - an option indicating the index of the `..` element
+    // - a boolean indicating whether a trailing comma was present.
+    // Trailing commas are significant because (p) and (p,) are different patterns.
+    fn parse_parenthesized_pat_list(&mut self) -> PResult<'a, (Vec<P<Pat>>, Option<usize>, bool)> {
+        self.expect(&token::OpenDelim(token::Paren))?;
 
-        while !self.check(&token::CloseDelim(token::Paren)) {
-            if ddpos.is_none() && self.eat(&token::DotDot) {
-                ddpos = Some(fields.len());
-                if self.eat(&token::Comma) {
-                    // `..` needs to be followed by `)` or `, pat`, `..,)` is disallowed.
-                    fields.push(self.parse_pat()?);
+        let mut fields = Vec::new();
+        let mut ddpos = None;
+        let mut trailing_comma = false;
+        loop {
+            if self.eat(&token::DotDot) {
+                if ddpos.is_none() {
+                    ddpos = Some(fields.len());
+                } else {
+                    // Emit a friendly error, ignore `..` and continue parsing
+                    self.span_err(self.prev_span,
+                                  "`..` can only be used once per tuple or tuple struct pattern");
                 }
-            } else if ddpos.is_some() && self.eat(&token::DotDot) {
-                // Emit a friendly error, ignore `..` and continue parsing
-                self.span_err(self.prev_span, "`..` can only be used once per \
-                                               tuple or tuple struct pattern");
-            } else {
+            } else if !self.check(&token::CloseDelim(token::Paren)) {
                 fields.push(self.parse_pat()?);
+            } else {
+                break
             }
 
-            if !self.check(&token::CloseDelim(token::Paren)) ||
-                    (unary_needs_comma && fields.len() == 1 && ddpos.is_none()) {
-                self.expect(&token::Comma)?;
+            trailing_comma = self.eat(&token::Comma);
+            if !trailing_comma {
+                break
             }
         }
 
-        Ok((fields, ddpos))
+        if ddpos == Some(fields.len()) && trailing_comma {
+            // `..` needs to be followed by `)` or `, pat`, `..,)` is disallowed.
+            self.span_err(self.prev_span, "trailing comma is not permitted after `..`");
+        }
+
+        self.expect(&token::CloseDelim(token::Paren))?;
+
+        Ok((fields, ddpos, trailing_comma))
     }
 
     fn parse_pat_vec_elements(
@@ -3714,10 +3728,12 @@ impl<'a> Parser<'a> {
             }
             token::OpenDelim(token::Paren) => {
                 // Parse (pat,pat,pat,...) as tuple pattern
-                self.bump();
-                let (fields, ddpos) = self.parse_pat_tuple_elements(true)?;
-                self.expect(&token::CloseDelim(token::Paren))?;
-                pat = PatKind::Tuple(fields, ddpos);
+                let (fields, ddpos, trailing_comma) = self.parse_parenthesized_pat_list()?;
+                pat = if fields.len() == 1 && ddpos.is_none() && !trailing_comma {
+                    PatKind::Paren(fields.into_iter().nth(0).unwrap())
+                } else {
+                    PatKind::Tuple(fields, ddpos)
+                };
             }
             token::OpenDelim(token::Bracket) => {
                 // Parse [pat,pat,...] as slice pattern
@@ -3807,9 +3823,7 @@ impl<'a> Parser<'a> {
                             return Err(self.fatal("unexpected `(` after qualified path"));
                         }
                         // Parse tuple struct or enum pattern
-                        self.bump();
-                        let (fields, ddpos) = self.parse_pat_tuple_elements(false)?;
-                        self.expect(&token::CloseDelim(token::Paren))?;
+                        let (fields, ddpos, _) = self.parse_parenthesized_pat_list()?;
                         pat = PatKind::TupleStruct(path, fields, ddpos)
                     }
                     _ => pat = PatKind::Path(qself, path),

diff --git a/src/libsyntax/print/pprust.rs b/src/libsyntax/print/pprust.rs
@@ -2659,6 +2659,11 @@ impl<'a> State<'a> {
                                    |s, p| s.print_pat(p))?;
                 self.s.word("]")?;
             }
+            PatKind::Paren(ref inner) => {
+                self.popen()?;
+                self.print_pat(inner)?;
+                self.pclose()?;
+            }
             PatKind::Mac(ref m) => self.print_mac(m, token::Paren)?,
         }
         self.ann.post(self, NodePat(pat))

diff --git a/src/libsyntax/visit.rs b/src/libsyntax/visit.rs
@@ -425,7 +425,8 @@ pub fn walk_pat<'a, V: Visitor<'a>>(visitor: &mut V, pattern: &'a Pat) {
             walk_list!(visitor, visit_pat, tuple_elements);
         }
         PatKind::Box(ref subpattern) |
-        PatKind::Ref(ref subpattern, _) => {
+        PatKind::Ref(ref subpattern, _) |
+        PatKind::Paren(ref subpattern) => {
             visitor.visit_pat(subpattern)
         }
         PatKind::Ident(_, ref pth1, ref optional_subpattern) => {

diff --git a/src/test/parse-fail/pat-tuple-2.rs b/src/test/parse-fail/pat-tuple-2.rs
@@ -12,6 +12,6 @@
 
 fn main() {
     match 0 {
-        (pat, ..,) => {} //~ ERROR expected pattern, found `)`
+        (pat, ..,) => {} //~ ERROR trailing comma is not permitted after `..`
     }
 }
diff --git a/src/test/parse-fail/pat-tuple-6.rs → src/test/run-pass/pat-tuple-7.rs b/src/test/parse-fail/pat-tuple-6.rs → src/test/run-pass/pat-tuple-7.rs
@@ -8,10 +8,10 @@
 // option. This file may not be copied, modified, or distributed
 // except according to those terms.
 
-// compile-flags: -Z parse-only
+#![feature(pattern_parentheses)]
 
 fn main() {
     match 0 {
-        (pat) => {} //~ ERROR expected one of `,` or `@`, found `)`
+        (pat) => assert_eq!(pat, 0)
     }
 }
diff --git a/src/test/ui/feature-gate-pattern_parentheses.rs b/src/test/ui/feature-gate-pattern_parentheses.rs
@@ -0,0 +1,15 @@
+// Copyright 2017 The Rust Project Developers. See the COPYRIGHT
+// file at the top-level directory of this distribution and at
+// http://rust-lang.org/COPYRIGHT.
+//
+// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+fn main() {
+    match 0 {
+        (pat) => {} //~ ERROR parentheses in patterns are unstable
+    }
+}
diff --git a/src/test/ui/feature-gate-pattern_parentheses.stderr b/src/test/ui/feature-gate-pattern_parentheses.stderr
@@ -0,0 +1,11 @@
+error[E0658]: parentheses in patterns are unstable
+  --> $DIR/feature-gate-pattern_parentheses.rs:13:9
+   |
+LL |         (pat) => {} //~ ERROR parentheses in patterns are unstable
+   |         ^^^^^
+   |
+   = help: add #![feature(pattern_parentheses)] to the crate attributes to enable
+
+error: aborting due to previous error
+
+If you want more information on this error, try using "rustc --explain E0658"