Let rec patterns (#2031)

* Add attrs to let patterns

* Implement let-rec patterns

* Fix markdown

* More markdown

* Put a test in the right place

* Revert the comment change

* Review comments

cli/tests/snapshot/snapshots/snapshot__eval_stderr_destructuring_closed_fail.ncl.snap

@@ -2,11 +2,10 @@
 source: cli/tests/snapshot/main.rs
 expression: err
 ---
-error: unmatched pattern
+error: destructuring failed
   ┌─ [INPUTS_PATH]/errors/destructuring_closed_fail.ncl:3:5
   │
 3 │ let {a} = {a=1, b=2}
-  │     ^^^^^^^^^^^^^^^^
-  │     │     │
-  │     │     this value doesn't match any branch
-  │     in this match expression
+  │     ^^^   ---------- this value failed to match
+  │     │      
+  │     this pattern

core/src/error/mod.rs

@@ -27,7 +27,7 @@ use crate::{
     position::{RawSpan, TermPos},
     repl,
     serialize::{ExportFormat, NickelPointer},
-    term::{record::FieldMetadata, Number, RichTerm, Term},
+    term::{pattern::Pattern, record::FieldMetadata, Number, RichTerm, Term},
     typ::{EnumRow, RecordRow, Type, TypeF, VarKindDiscriminant},
 };
 
@@ -168,6 +168,12 @@ pub enum EvalError {
         /// The position of the `match` expression
         pos: TermPos,
     },
+    FailedDestructuring {
+        /// The original term matched.
+        value: RichTerm,
+        /// The pattern that failed to match.
+        pattern: Pattern,
+    },
     /// Tried to query a field of something that wasn't a record.
     QueryNonRecord {
         /// Position of the original unevaluated expression.
@@ -1468,6 +1474,20 @@ impl IntoDiagnostics<FileId> for EvalError {
                     .with_message("unmatched pattern")
                     .with_labels(labels)]
             }
+            EvalError::FailedDestructuring { value, pattern } => {
+                let mut labels = Vec::new();
+
+                if let Some(span) = pattern.pos.into_opt() {
+                    labels.push(primary(&span).with_message("this pattern"));
+                }
+
+                labels
+                    .push(secondary_term(&value, files).with_message("this value failed to match"));
+
+                vec![Diagnostic::error()
+                    .with_message("destructuring failed")
+                    .with_labels(labels)]
+            }
             EvalError::IllegalPolymorphicTailAccess {
                 action,
                 label: contract_label,

core/src/parser/grammar.lalrpop

@@ -232,11 +232,11 @@ UniTerm: UniTerm = {
     InfixExpr,
     AnnotatedInfixExpr,
     AsUniTerm<Forall>,
-    "let" <l: @L> <recursive:"rec"?> <r: @R>
+    "let" <recursive:"rec"?>
         <mut bindings:(<LetBinding> ",")*> <last:LetBinding> ","?
         "in" <body: Term> =>? {
         bindings.push(last);
-        Ok(UniTerm::from(mk_let(recursive.is_some(), bindings, body, mk_span(src_id, l, r))?))
+        Ok(UniTerm::from(mk_let(recursive.is_some(), bindings, body)?))
     },
     <l: @L> "fun" <pats: PatternFun+> "=>" <t: Term> <r: @R> => {
         let pos = mk_pos(src_id, l, r);

core/src/parser/utils.rs

@@ -630,14 +630,11 @@ pub fn mk_merge_label(src_id: FileId, l: usize, r: usize) -> MergeLabel {
 }
 
 /// Generate a `Let` or a `LetPattern` (depending on whether there's a binding
-/// with a record pattern) from the parsing of a let definition. This function
-/// fails if the definition has both a pattern and is recursive because
-/// recursive let-patterns are currently not supported.
+/// with a record pattern) from the parsing of a let definition.
 pub fn mk_let(
     rec: bool,
     bindings: Vec<LetBinding>,
     body: RichTerm,
-    span: RawSpan,
 ) -> Result<RichTerm, ParseError> {
     let all_simple = bindings
         .iter()
@@ -678,10 +675,9 @@ pub fn mk_let(
             }),
             body,
         ))
-    } else if rec {
-        Err(ParseError::RecursiveLetPattern(span))
     } else {
         Ok(mk_term::let_pat_in(
+            rec,
             bindings.into_iter().map(|mut b| {
                 if let Some(ann) = b.annot {
                     b.value = ann.annotation.attach_term(b.value);

core/src/pretty.rs

@@ -864,9 +864,10 @@ where
             Lbl(_lbl) => allocator.text("%<label>").append(allocator.line()),
             Let(bindings, body, attrs) => docs![
                 allocator,
-                "let ",
+                "let",
+                allocator.space(),
                 if attrs.rec {
-                    allocator.text("rec ")
+                    docs![allocator, "rec", allocator.space()]
                 } else {
                     allocator.nil()
                 },
@@ -883,9 +884,15 @@ where
             .append(allocator.line())
             .append(body.pretty(allocator).nest(2))
             .group(),
-            LetPattern(bindings, body) => docs![
+            LetPattern(bindings, body, attrs) => docs![
                 allocator,
-                "let ",
+                "let",
+                allocator.space(),
+                if attrs.rec {
+                    docs![allocator, "rec", allocator.space()]
+                } else {
+                    allocator.nil()
+                },
                 allocator.intersperse(
                     bindings
                         .iter()

core/src/term/mod.rs

@@ -114,7 +114,7 @@ pub enum Term {
 
     /// A destructuring let-binding.
     #[serde(skip)]
-    LetPattern(SmallVec<[(Pattern, RichTerm); 1]>, RichTerm),
+    LetPattern(SmallVec<[(Pattern, RichTerm); 1]>, RichTerm, LetAttrs),
 
     /// An application.
     #[serde(skip)]
@@ -345,7 +345,9 @@ impl PartialEq for Term {
             (Self::FunPattern(l0, l1), Self::FunPattern(r0, r1)) => l0 == r0 && l1 == r1,
             (Self::Lbl(l0), Self::Lbl(r0)) => l0 == r0,
             (Self::Let(l0, l1, l2), Self::Let(r0, r1, r2)) => l0 == r0 && l1 == r1 && l2 == r2,
-            (Self::LetPattern(l0, l1), Self::LetPattern(r0, r1)) => l0 == r0 && l1 == r1,
+            (Self::LetPattern(l0, l1, l2), Self::LetPattern(r0, r1, r2)) => {
+                l0 == r0 && l1 == r1 && l2 == r2
+            }
             (Self::App(l0, l1), Self::App(r0, r1)) => l0 == r0 && l1 == r1,
             (Self::Var(l0), Self::Var(r0)) => l0 == r0,
             (Self::Enum(l0), Self::Enum(r0)) => l0 == r0,
@@ -2205,13 +2207,13 @@ impl Traverse<RichTerm> for RichTerm {
                 let body = body.traverse(f, order)?;
                 RichTerm::new(Term::Let(bindings, body, attrs), pos)
             }
-            Term::LetPattern(bindings, body) => {
+            Term::LetPattern(bindings, body, attrs) => {
                 let bindings = bindings
                     .into_iter()
                     .map(|(key, val)| Ok((key, val.traverse(f, order)?)))
                     .collect::<Result<_, E>>()?;
                 let body = body.traverse(f, order)?;
-                RichTerm::new(Term::LetPattern(bindings, body), pos)
+                RichTerm::new(Term::LetPattern(bindings, body, attrs), pos)
             }
             Term::App(t1, t2) => {
                 let t1 = t1.traverse(f, order)?;
@@ -2404,7 +2406,7 @@ impl Traverse<RichTerm> for RichTerm {
                 .iter()
                 .find_map(|(_id, t)| t.traverse_ref(f, state))
                 .or_else(|| body.traverse_ref(f, state)),
-            Term::LetPattern(bindings, body) => bindings
+            Term::LetPattern(bindings, body, _) => bindings
                 .iter()
                 .find_map(|(_pat, t)| t.traverse_ref(f, state))
                 .or_else(|| body.traverse_ref(f, state)),
@@ -2763,23 +2765,29 @@ pub mod make {
         Term::LetPattern(
             std::iter::once((pat.into(), t1.into())).collect(),
             t2.into(),
+            LetAttrs::default(),
         )
         .into()
     }
 
-    pub fn let_pat_in<D, T1, T2, Iter>(bindings: Iter, body: T2) -> RichTerm
+    pub fn let_pat_in<D, T1, T2, Iter>(rec: bool, bindings: Iter, body: T2) -> RichTerm
     where
         T1: Into<RichTerm>,
         T2: Into<RichTerm>,
         D: Into<Pattern>,
         Iter: IntoIterator<Item = (D, T1)>,
     {
+        let attrs = LetAttrs {
+            binding_type: BindingType::Normal,
+            rec,
+        };
         Term::LetPattern(
             bindings
                 .into_iter()
                 .map(|(pat, t)| (pat.into(), t.into()))
                 .collect(),
             body.into(),
+            attrs,
         )
         .into()
     }