From f1da1dd51b6868ced3c2ae7f3d946be7cbcf8781 Mon Sep 17 00:00:00 2001
From: Yann Hamdaoui <yann.hamdaoui@tweag.io>
Date: Wed, 20 Nov 2024 10:11:06 +0100
Subject: [PATCH] Fix compilation errors and spurious grammar ambiguity

---
 core/src/bytecode/ast/pattern/mod.rs | 22 ----------------------
 core/src/parser/grammar.lalrpop      | 20 +++++++++++---------
 core/src/parser/utils.rs             | 19 +++++++++++++++++++
 3 files changed, 30 insertions(+), 31 deletions(-)

diff --git a/core/src/bytecode/ast/pattern/mod.rs b/core/src/bytecode/ast/pattern/mod.rs
index 032f390d4..e682c58cf 100644
--- a/core/src/bytecode/ast/pattern/mod.rs
+++ b/core/src/bytecode/ast/pattern/mod.rs
@@ -66,28 +66,6 @@ pub struct FieldPattern<'ast> {
     pub pos: TermPos,
 }
 
-/// The last match in a data structure pattern. This can either be a normal match, or an ellipsis
-/// which can capture the rest of the data structure. The type parameter `P` is the type of the
-/// pattern of the data structure: currently, ellipsis matches are only supported for record, but
-/// we'll probably support them for arrays as well.
-///
-/// This enum is mostly used during parsing.
-///
-/// # Example
-///
-/// - In `{foo={}, bar}`, the last match is an normal match.
-/// - In `{foo={}, bar, ..}`, the last match is a non-capturing ellipsis.
-/// - In `{foo={}, bar, ..rest}`, the last match is a capturing ellipsis.
-#[derive(Debug, PartialEq, Clone)]
-pub enum PatternTail<'ast, P> {
-    /// The last field is a normal match. In this case the pattern is "closed" so every record
-    /// fields should be matched.
-    Normal(&'ast P),
-    /// The pattern is "open" `, ..}`. Optionally you can bind a record containing the remaining
-    /// fields to an `Identifier` using the syntax `, ..y}`.
-    Ellipsis(Option<LocIdent>),
-}
-
 /// A record pattern.
 #[derive(Debug, PartialEq, Clone)]
 pub struct RecordPattern<'ast> {
diff --git a/core/src/parser/grammar.lalrpop b/core/src/parser/grammar.lalrpop
index c13d9607d..73f9c49be 100644
--- a/core/src/parser/grammar.lalrpop
+++ b/core/src/parser/grammar.lalrpop
@@ -116,9 +116,9 @@ RepeatSep<Rule, Sep>: Vec<Rule> = <mut elems: (<Rule> Sep)*> <last: Rule?> => {
     elems
 };
 
-// Same as `RepeatSep`, but repeat the rule at least once (one or more), instead of zero or
-// more. 
-RepeatSep1<Rule, Sep>: Vec<Rule> = <mut elems: (Sep <Rule>)*> <last: Rule> Sep? => {
+// Same as `RepeatSep`, but repeat the rule at least once (one or more), instead
+// of zero or more.
+RepeatSep1<Rule, Sep>: Vec<Rule> = <mut elems: (<Rule> Sep)*> <last: Rule> Sep? => {
     elems.push(last);
     elems
 };
@@ -934,12 +934,14 @@ FieldPattern: FieldPattern<'ast> = {
         },
 };
 
-// Last field pattern of a record pattern. We need this rule (together with
-// `LastElemPat`) combining both a field and a potential ellipsis because
-// putting the ellipsis in a separate rule AND handling the case of zero fields
-// (`{..}`) isn't possible: the fact that the ellipsis will need a "," separator
-// before it will depend on the presence of zero or more fields. A stand-alone
-// ellipsis rule would have no way to know that.
+// Last field pattern of a record pattern.
+//
+// We need this rule (together with `LastElemPat`) combining both a last field
+// or a potential ellipsis because putting the ellipsis in a separate rule AND
+// handling the case of zero fields (`{..}`) isn't possible: the fact that the
+// ellipsis will need a "," separator before depends on the presence of zero or
+// more fields, but a stand-alone ellipsis rule has no way to get this
+// information about previous match.
 LastFieldPat: LastPattern<FieldPattern<'ast>> = {
     FieldPattern => LastPattern::Normal(<>),
     ".." <Ident?> => LastPattern::Ellipsis(<>),
diff --git a/core/src/parser/utils.rs b/core/src/parser/utils.rs
index 9aa7e57ad..5e22878d8 100644
--- a/core/src/parser/utils.rs
+++ b/core/src/parser/utils.rs
@@ -215,6 +215,25 @@ pub enum RecordLastField<'ast> {
     Ellipsis,
 }
 
+/// The last match in a data structure pattern. This can either be a normal match, or an ellipsis
+/// which can capture the rest of the data structure. The type parameter `P` is the type of the
+/// pattern of the data structure (ellipsis are supported for both array and record patterns).
+///
+/// # Example
+///
+/// - In `{foo={}, bar}`, the last match is an normal match.
+/// - In `{foo={}, bar, ..}`, the last match is a non-capturing ellipsis.
+/// - In `{foo={}, bar, ..rest}`, the last match is a capturing ellipsis.
+#[derive(Debug, PartialEq, Clone)]
+pub enum LastPattern<P> {
+    /// The last field is a normal match. In this case the pattern is "closed" so every record
+    /// fields should be matched.
+    Normal(P),
+    /// The pattern is "open" `, ..}`. Optionally you can bind a record containing the remaining
+    /// fields to an `Identifier` using the syntax `, ..y}`.
+    Ellipsis(Option<LocIdent>),
+}
+
 /// Trait for operators that can be eta-expanded to a function.
 pub(super) trait EtaExpand {
     /// Eta-expand an operator. This wraps an operator, for example `==`, as a function `fun x1 x2