syntax: remove non-capturing groups from HIR

It turns out they are completely superfluous in the HIR, so we can drop
them completely. We only need to explicitly represent capturing groups.

Author: BurntSushi

regex-syntax/src/hir/mod.rs

@@ -1118,39 +1118,25 @@ impl Look {
     }
 }
 
-/// The high-level intermediate representation for a group.
+/// The high-level intermediate representation for a capturing group.
 ///
-/// This represents one of three possible group types:
+/// A capturing group always has an index and a child expression. It may
+/// also have a name associated with it (e.g., `(?P<foo>\w)`), but it's not
+/// necessary.
 ///
-/// 1. A non-capturing group (e.g., `(?:expr)`).
-/// 2. A capturing group (e.g., `(expr)`).
-/// 3. A named capturing group (e.g., `(?P<name>expr)`).
+/// Note that there is no explicit representation of a non-capturing group
+/// in a `Hir`. Instead, non-capturing grouping is handled automatically by
+/// the recursive structure of the `Hir` itself.
 #[derive(Clone, Debug, Eq, PartialEq)]
 pub struct Group {
-    /// The kind of this group. If it is a capturing group, then the kind
-    /// contains the capture group index (and the name, if it is a named
-    /// group).
-    pub kind: GroupKind,
+    /// The capture index of the group.
+    pub index: u32,
+    /// The name of the group, if it exists.
+    pub name: Option<Box<str>>,
     /// The expression inside the capturing group, which may be empty.
     pub hir: Box<Hir>,
 }
 
-/// The kind of group.
-#[derive(Clone, Debug, Eq, PartialEq)]
-pub enum GroupKind {
-    /// A non-capturing group.
-    NonCapturing,
-    /// A capturing group with an optional name.
-    ///
-    /// The value is the capture index of the group.
-    Capture {
-        /// The capture index of the group.
-        index: u32,
-        /// The name of the group, if it exists.
-        name: Option<String>,
-    },
-}
-
 /// The high-level intermediate representation of a repetition operator.
 ///
 /// A repetition operator permits the repetition of an arbitrary
@@ -2452,7 +2438,8 @@ mod tests {
             let mut expr = Hir::empty();
             for _ in 0..100 {
                 expr = Hir::group(Group {
-                    kind: GroupKind::NonCapturing,
+                    index: 1,
+                    name: None,
                     hir: Box::new(expr),
                 });
                 expr = Hir::repetition(Repetition {

regex-syntax/src/hir/print.rs

@@ -176,17 +176,12 @@ impl<W: fmt::Write> Visitor for Writer<W> {
                     self.wtr.write_str(r"\B")?;
                 }
             },
-            HirKind::Group(ref x) => match x.kind {
-                hir::GroupKind::Capture { ref name, .. } => {
-                    self.wtr.write_str("(")?;
-                    if let Some(ref name) = *name {
-                        write!(self.wtr, "?P<{}>", name)?;
-                    }
-                }
-                hir::GroupKind::NonCapturing => {
-                    self.wtr.write_str("(?:")?;
+            HirKind::Group(hir::Group { ref name, .. }) => {
+                self.wtr.write_str("(")?;
+                if let Some(ref name) = *name {
+                    write!(self.wtr, "?P<{}>", name)?;
                 }
-            },
+            }
             // Why do this? Wrapping concats and alts in non-capturing groups
             // is not *always* necessary, but is sometimes necessary. For
             // example, 'concat(a, alt(b, c))' should be written as 'a(?:b|c)'
@@ -415,11 +410,11 @@ mod tests {
     fn print_group() {
         roundtrip("()", "()");
         roundtrip("(?P<foo>)", "(?P<foo>)");
-        roundtrip("(?:)", "(?:)");
+        roundtrip("(?:)", "");
 
         roundtrip("(a)", "(a)");
         roundtrip("(?P<foo>a)", "(?P<foo>a)");
-        roundtrip("(?:a)", "(?:a)");
+        roundtrip("(?:a)", "a");
 
         roundtrip("((((a))))", "((((a))))");
     }

regex-syntax/src/hir/translate.rs

@@ -915,17 +915,16 @@ impl<'t, 'p> TranslatorI<'t, 'p> {
     }
 
     fn hir_group(&self, group: &ast::Group, expr: Hir) -> Hir {
-        let kind = match group.kind {
-            ast::GroupKind::CaptureIndex(index) => {
-                hir::GroupKind::Capture { index, name: None }
-            }
-            ast::GroupKind::CaptureName(ref cap) => hir::GroupKind::Capture {
-                index: cap.index,
-                name: Some(cap.name.clone()),
-            },
-            ast::GroupKind::NonCapturing(_) => hir::GroupKind::NonCapturing,
+        let (index, name) = match group.kind {
+            ast::GroupKind::CaptureIndex(index) => (index, None),
+            ast::GroupKind::CaptureName(ref cap) => {
+                (cap.index, Some(cap.name.clone().into_boxed_str()))
+            }
+            // The HIR doesn't need to use non-capturing groups, since the way
+            // in which the data type is defined handles this automatically.
+            ast::GroupKind::NonCapturing(_) => return expr,
         };
-        Hir::group(hir::Group { kind, hir: Box::new(expr) })
+        Hir::group(hir::Group { index, name, hir: Box::new(expr) })
     }
 
     fn hir_repetition(&self, rep: &ast::Repetition, expr: Hir) -> Hir {
@@ -1349,26 +1348,14 @@ mod tests {
         Hir::literal(s)
     }
 
-    fn hir_group(i: u32, expr: Hir) -> Hir {
-        Hir::group(hir::Group {
-            kind: hir::GroupKind::Capture { index: i, name: None },
-            hir: Box::new(expr),
-        })
+    fn hir_group(index: u32, expr: Hir) -> Hir {
+        Hir::group(hir::Group { index, name: None, hir: Box::new(expr) })
     }
 
-    fn hir_group_name(i: u32, name: &str, expr: Hir) -> Hir {
+    fn hir_group_name(index: u32, name: &str, expr: Hir) -> Hir {
         Hir::group(hir::Group {
-            kind: hir::GroupKind::Capture {
-                index: i,
-                name: Some(name.to_string()),
-            },
-            hir: Box::new(expr),
-        })
-    }
-
-    fn hir_group_nocap(expr: Hir) -> Hir {
-        Hir::group(hir::Group {
-            kind: hir::GroupKind::NonCapturing,
+            index,
+            name: Some(name.into()),
             hir: Box::new(expr),
         })
     }
@@ -1519,7 +1506,7 @@ mod tests {
         assert_eq!(t(""), Hir::empty());
         assert_eq!(t("(?i)"), Hir::empty());
         assert_eq!(t("()"), hir_group(1, Hir::empty()));
-        assert_eq!(t("(?:)"), hir_group_nocap(Hir::empty()));
+        assert_eq!(t("(?:)"), Hir::empty());
         assert_eq!(t("(?P<wat>)"), hir_group_name(1, "wat", Hir::empty()));
         assert_eq!(t("|"), hir_alt(vec![Hir::empty(), Hir::empty()]));
         assert_eq!(
@@ -1592,10 +1579,7 @@ mod tests {
         #[cfg(feature = "unicode-case")]
         assert_eq!(t("(?i)a"), hir_uclass(&[('A', 'A'), ('a', 'a'),]));
         #[cfg(feature = "unicode-case")]
-        assert_eq!(
-            t("(?i:a)"),
-            hir_group_nocap(hir_uclass(&[('A', 'A'), ('a', 'a')],))
-        );
+        assert_eq!(t("(?i:a)"), hir_uclass(&[('A', 'A'), ('a', 'a')]));
         #[cfg(feature = "unicode-case")]
         assert_eq!(
             t("a(?i)a(?-i)a"),
@@ -1757,20 +1741,17 @@ mod tests {
                 hir_group_name(2, "bar", hir_lit("b")),
             ])
         );
-        assert_eq!(t("(?:)"), hir_group_nocap(Hir::empty()));
-        assert_eq!(t("(?:a)"), hir_group_nocap(hir_lit("a")));
+        assert_eq!(t("(?:)"), Hir::empty());
+        assert_eq!(t("(?:a)"), hir_lit("a"));
         assert_eq!(
             t("(?:a)(b)"),
-            hir_cat(vec![
-                hir_group_nocap(hir_lit("a")),
-                hir_group(1, hir_lit("b")),
-            ])
+            hir_cat(vec![hir_lit("a"), hir_group(1, hir_lit("b")),])
         );
         assert_eq!(
             t("(a)(?:b)(c)"),
             hir_cat(vec![
                 hir_group(1, hir_lit("a")),
-                hir_group_nocap(hir_lit("b")),
+                hir_lit("b"),
                 hir_group(2, hir_lit("c")),
             ])
         );
@@ -1793,22 +1774,21 @@ mod tests {
         #[cfg(feature = "unicode-case")]
         assert_eq!(
             t("(?i:a)a"),
-            hir_cat(vec![
-                hir_group_nocap(hir_uclass(&[('A', 'A'), ('a', 'a')])),
-                hir_lit("a"),
-            ])
+            hir_cat(
+                vec![hir_uclass(&[('A', 'A'), ('a', 'a')]), hir_lit("a"),]
+            )
         );
         assert_eq!(
             t("(?i-u:a)β"),
             hir_cat(vec![
-                hir_group_nocap(hir_bclass(&[(b'A', b'A'), (b'a', b'a')])),
+                hir_bclass(&[(b'A', b'A'), (b'a', b'a')]),
                 hir_lit("β"),
             ])
         );
         assert_eq!(
             t("(?:(?i-u)a)b"),
             hir_cat(vec![
-                hir_group_nocap(hir_bclass(&[(b'A', b'A'), (b'a', b'a')])),
+                hir_bclass(&[(b'A', b'A'), (b'a', b'a')]),
                 hir_lit("b"),
             ])
         );
@@ -1822,10 +1802,9 @@ mod tests {
         #[cfg(feature = "unicode-case")]
         assert_eq!(
             t("(?i)(?-i:a)a"),
-            hir_cat(vec![
-                hir_group_nocap(hir_lit("a")),
-                hir_uclass(&[('A', 'A'), ('a', 'a')]),
-            ])
+            hir_cat(
+                vec![hir_lit("a"), hir_uclass(&[('A', 'A'), ('a', 'a')]),]
+            )
         );
         #[cfg(feature = "unicode-case")]
         assert_eq!(
@@ -1858,21 +1837,21 @@ mod tests {
         assert_eq!(
             t("(?:a(?i)a)a"),
             hir_cat(vec![
-                hir_group_nocap(hir_cat(vec![
+                hir_cat(vec![
                     hir_lit("a"),
                     hir_uclass(&[('A', 'A'), ('a', 'a')]),
-                ])),
+                ]),
                 hir_lit("a"),
             ])
         );
         #[cfg(feature = "unicode-case")]
         assert_eq!(
             t("(?i)(?:a(?-i)a)a"),
             hir_cat(vec![
-                hir_group_nocap(hir_cat(vec![
+                hir_cat(vec![
                     hir_uclass(&[('A', 'A'), ('a', 'a')]),
                     hir_lit("a"),
-                ])),
+                ]),
                 hir_uclass(&[('A', 'A'), ('a', 'a')]),
             ])
         );

src/compile.rs

@@ -368,19 +368,19 @@ impl Compiler {
                     self.c_empty_look(prog::EmptyLook::NotWordBoundary)
                 }
             },
-            Group(ref g) => match g.kind {
-                hir::GroupKind::NonCapturing => self.c(&g.hir),
-                hir::GroupKind::Capture { index, ref name } => {
-                    if index as usize >= self.compiled.captures.len() {
-                        self.compiled.captures.push(name.clone());
-                        if let Some(ref name) = *name {
-                            self.capture_name_idx
-                                .insert(name.clone(), index as usize);
-                        }
+            Group(hir::Group { index, ref name, ref hir }) => {
+                if index as usize >= self.compiled.captures.len() {
+                    let name = match *name {
+                        None => None,
+                        Some(ref boxed_str) => Some(boxed_str.to_string()),
+                    };
+                    self.compiled.captures.push(name.clone());
+                    if let Some(name) = name {
+                        self.capture_name_idx.insert(name, index as usize);
                     }
-                    self.c_capture(2 * index as usize, &g.hir)
                 }
-            },
+                self.c_capture(2 * index as usize, hir)
+            }
             Concat(ref es) => {
                 if self.compiled.is_reverse {
                     self.c_concat(es.iter().rev())