Repository: tweag/nickel
Pull request #567 (2022-01-11 16:16:42)

Feature/funargs destructuring
Implement destructuring for functions arguments as described in the last point of #81:
```text
let f = fun {x; y; z;} => x + y + z in foo
/* bind x, y and z to the corresponding fields of the argument in the body of f */
```


Patch:
diff --git a/src/eval/mod.rs b/src/eval/mod.rs
index f71a9919..cdc4cde5 100644
--- a/src/eval/mod.rs
+++ b/src/eval/mod.rs
@@ -746,6 +746,7 @@ pub fn subst(rt: RichTerm, global_env: &Environment, env: &Environment) -> RichT
                 RichTerm::new(Term::Let(id, t1, t2, btype), pos)
             }
             p @ Term::LetPattern(..) => panic!("Pattern {:?} has not been transformed before evaluation", p),
+            p @ Term::FunPattern(..) => panic!("Pattern {:?} has not been transformed before evaluation", p),
             Term::App(t1, t2) => {
                 let t1 = subst_(t1, global_env, env, Cow::Borrowed(bound.as_ref()));
                 let t2 = subst_(t2, global_env, env, bound);
diff --git a/src/grammar.lalrpop b/src/grammar.lalrpop
index b342de00..5c52f28b 100644
--- a/src/grammar.lalrpop
+++ b/src/grammar.lalrpop
@@ -93,10 +93,10 @@ RootTerm: RichTerm = {
 
         mk_term::let_pat(pat.0, pat.1, t1, t2)
     },
-    <l: @L> "fun" <ids:Ident+> "=>" <t: Term> <r: @R> => {
+    <l: @L> "fun" <pats: Pattern+> "=>" <t: Term> <r: @R> => {
         let pos = mk_pos(src_id, l, r);
-        ids.into_iter().rev().fold(t, |t, p| RichTerm {
-            term: SharedTerm::new(Term::Fun(p, t)),
+        pats.into_iter().rev().fold(t, |t, (id, destruct)| RichTerm {
+            term: SharedTerm::new(Term::FunPattern(id, destruct, t)),
             pos,
         })
     },
diff --git a/src/term.rs b/src/term.rs
index ad8e728d..03a8c340 100644
--- a/src/term.rs
+++ b/src/term.rs
@@ -57,9 +57,12 @@ pub enum Term {
     /// efficiently from the back of it.
     #[serde(skip)]
     StrChunks(Vec<StrChunk<RichTerm>>),
-    /// A function.
+    /// A standard function.
     #[serde(skip)]
     Fun(Ident, RichTerm),
+    /// A function able to destruct its arguments.
+    #[serde(skip)]
+    FunPattern(Option<Ident>, Destruct, RichTerm),
     /// A blame label.
     #[serde(skip)]
     Lbl(Label),
@@ -342,7 +345,10 @@ impl Term {
             }
             Bool(_) | Num(_) | Str(_) | Lbl(_) | Var(_) | Sym(_) | Enum(_) | Import(_)
             | ResolvedImport(_) => {}
-            Fun(_, ref mut t) | Op1(_, ref mut t) | Wrapped(_, ref mut t) => {
+            Fun(_, ref mut t)
+            | FunPattern(_, _, ref mut t)
+            | Op1(_, ref mut t)
+            | Wrapped(_, ref mut t) => {
                 func(t);
             }
             MetaValue(ref mut meta) => {
@@ -383,7 +389,7 @@ impl Term {
             Term::Bool(_) => Some("Bool"),
             Term::Num(_) => Some("Num"),
             Term::Str(_) => Some("Str"),
-            Term::Fun(_, _) => Some("Fun"),
+            Term::Fun(_, _) | Term::FunPattern(_, _, _) => Some("Fun"),
             Term::Lbl(_) => Some("Label"),
             Term::Enum(_) => Some("Enum"),
             Term::Record(..) | Term::RecRecord(..) => Some("Record"),
@@ -427,7 +433,7 @@ impl Term {
 
                 format!("\"{}\"", chunks_str.join(""))
             }
-            Term::Fun(_, _) => String::from("<func>"),
+            Term::Fun(_, _) | Term::FunPattern(_, _, _) => String::from("<func>"),
             Term::Lbl(_) => String::from("<label>"),
             Term::Enum(id) => {
                 let re = regex::Regex::new("_?[a-zA-Z][_a-zA-Z0-9]*").unwrap();
@@ -521,6 +527,7 @@ impl Term {
             | Term::Sym(_) => true,
             Term::Let(..)
             | Term::LetPattern(..)
+            | Term::FunPattern(..)
             | Term::App(_, _)
             | Term::Var(_)
             | Term::Switch(..)
@@ -560,6 +567,7 @@ impl Term {
             | Term::Record(..)
             | Term::List(_)
             | Term::Fun(_, _)
+            | Term::FunPattern(_, _, _)
             | Term::App(_, _)
             | Term::Switch(..)
             | Term::Var(_)
@@ -970,6 +978,13 @@ impl RichTerm {
                     pos,
                 )
             },
+            Term::FunPattern(id, d, t) => {
+                let t = t.traverse(f, state, method)?;
+                RichTerm::new(
+                    Term::FunPattern(id, d, t),
+                    pos,
+                )
+            },
             Term::Let(id, t1, t2, btype) => {
                 let t1 = t1.traverse(f, state, method)?;
                 let t2 = t2.traverse(f, state, method)?;
diff --git a/src/transform/desugar_destructuring.rs b/src/transform/desugar_destructuring.rs
index a1931424..8b3d3858 100644
--- a/src/transform/desugar_destructuring.rs
+++ b/src/transform/desugar_destructuring.rs
@@ -6,7 +6,7 @@
 //!
 //! # Example
 //!
-//! The let pattern:
+//! ## The let pattern:
 //! ```text
 //! let x @ {a, b=d, ..} = {a=1,b=2,c="ignored"} in ...
 //! ```
@@ -17,6 +17,19 @@
 //! let d = x.b in
 //! ...
 //! ```
+//!
+//! ## The function pattern
+//! ```text
+//! let f = fun x@{a, b=c} {d ? 2, ..w} => <do_something> in ...
+//! ```
+//! will be transformed to:
+//! ```text
+//! let f = fun x %unnamed% => (
+//!     let {a, b=c} = x in
+//!     let {d ? 2, ..w} = %unnamed% in
+//!     <do_something>
+//! ) in ...
+//! ```
 use crate::destruct::{Destruct, Match};
 use crate::identifier::Ident;
 use crate::match_sharedterm;
@@ -25,10 +38,49 @@ use crate::term::{
     BinaryOp::DynRemove, BindingType, MetaValue, RichTerm, Term, UnaryOp::StaticAccess,
 };
 
-/// Wrap the desugar term in a meta value containing the "Record contract" needed to check the
+/// Entry point of the patterns desugaring.
+/// It desugar a `RichTerm` if possible (the term is a let pattern or a function with patterns in
+/// its arguments).
+/// ## Warning:
+/// The transformation is generaly not recursive. The result can contain patterns itself.
+pub fn transform_one(rt: RichTerm) -> RichTerm {
+    match *rt.term {
+        Term::LetPattern(..) => desugar_with_contract(rt),
+        Term::FunPattern(..) => desugar_fun(rt),
+        _ => rt,
+    }
+}
+
+/// Desugar a function with patterns as arguments.
+/// This function does not perform nested transformation because internaly it's only used in a top
+/// down traversal. This means that the return value is a normal `Term::Fun` but it can contain
+/// `Term::FunPattern` and `Term::LetPattern` inside.
+pub fn desugar_fun(rt: RichTerm) -> RichTerm {
+    match_sharedterm! {rt.term, with {
+        Term::FunPattern(x, pat, t_) if !pat.is_empty() => {
+            let x = x.unwrap_or_else(super::fresh_var);
+            let t_pos = t_.pos;
+            RichTerm::new(
+                Term::Fun(
+                    x.clone(),
+                    RichTerm::new(Term::LetPattern(None, pat, Term::Var(x).into(), t_), t_pos /* TODO: should we use rt.pos? */),
+                ),
+                rt.pos,
+            )
+        },
+        Term::FunPattern(Some(x), Destruct::Empty, t_) => RichTerm::new(Term::Fun(x, t_), rt.pos),
+        t@Term::FunPattern(..) => panic!(
+            "A function can not have empty pattern without name in {:?}",
+            t
+        ),
+    } else rt
+    }
+}
+
+/// Wrap the desugar `LetPattern` in a meta value containing the "Record contract" needed to check the
 /// pattern exhaustively and also fill the default values (`?` operator) if not presents in the
-/// record. This function should be, in the general case, considered as the entry point of this
-/// transformation.
+/// record. This function should be, in the general case, considered as the entry point of the let
+/// patterns transformation.
 pub fn desugar_with_contract(rt: RichTerm) -> RichTerm {
     match_sharedterm!(rt.term,
         with {
diff --git a/src/transform/mod.rs b/src/transform/mod.rs
index cb4fd827..c27467b2 100644
--- a/src/transform/mod.rs
+++ b/src/transform/mod.rs
@@ -1,3 +1,4 @@
+//! Various post transformations of nickel code.
 use crate::cache::ImportResolver;
 use crate::eval::{lazy::Thunk, Closure, Environment, IdentKind};
 use crate::identifier::Ident;
@@ -23,7 +24,7 @@ pub fn transform(rt: RichTerm) -> RichTerm {
         .traverse(
             &mut |rt: RichTerm, _| -> Result<RichTerm, ()> {
                 // before anything, we have to desugar the syntax
-                let rt = desugar_destructuring::desugar_with_contract(rt);
+                let rt = desugar_destructuring::transform_one(rt);
                 // We need to do contract generation before wrapping stuff in variables
                 let rt = apply_contracts::transform_one(rt);
                 Ok(rt)
diff --git a/src/typecheck/mod.rs b/src/typecheck/mod.rs
index bb10647f..607063a7 100644
--- a/src/typecheck/mod.rs
+++ b/src/typecheck/mod.rs
@@ -281,6 +281,29 @@ fn type_check_<L: Linearizer>(
     rt: &RichTerm,
     ty: TypeWrapper,
 ) -> Result<(), TypecheckError> {
+    use crate::destruct::*;
+    // TODO: The insertion of values in the type environment is done but everything is
+    // typed as `Dyn`.
+    fn inject_pat_vars(pat: &Destruct, envs: &mut Envs) {
+        if let Destruct::Record(matches, _, rst) = pat {
+            if let Some(id) = rst {
+                envs.insert(id.clone(), TypeWrapper::Concrete(AbsType::Dyn()));
+            }
+            matches.iter().for_each(|m| match m {
+                Match::Simple(id, ..) => {
+                    envs.insert(id.clone(), TypeWrapper::Concrete(AbsType::Dyn()))
+                }
+                Match::Assign(id, _, (bind_id, pat)) => {
+                    let id = bind_id.as_ref().unwrap_or(id);
+                    envs.insert(id.clone(), TypeWrapper::Concrete(AbsType::Dyn()));
+                    if !pat.is_empty() {
+                        inject_pat_vars(&pat, envs);
+                    }
+                }
+            });
+        }
+    }
+
     let RichTerm { term: t, pos } = rt;
     linearizer.add_term(lin, t, *pos, ty.clone());
 
@@ -330,6 +353,20 @@ fn type_check_<L: Linearizer>(
             envs.insert(x.clone(), src);
             type_check_(state, envs, lin, linearizer, strict, t, trg)
         }
+        Term::FunPattern(x, pat, t) => {
+            let src = TypeWrapper::Ptr(new_var(state.table));
+            // TODO what to do here, this makes more sense to me, but it means let x = foo in bar
+            // behaves quite different to (\x.bar) foo, worth considering if it's ok to type these two differently
+            let trg = TypeWrapper::Ptr(new_var(state.table));
+            let arr = mk_tyw_arrow!(src.clone(), trg.clone());
+            if let Some(x) = x {
+                linearizer.retype_ident(lin, x, src.clone());
+                envs.insert(x.clone(), src);
+            }
+            inject_pat_vars(pat, &mut envs);
+            unify(state, strict, ty, arr).map_err(|err| err.into_typecheck_err(state, rt.pos))?;
+            type_check_(state, envs, lin, linearizer, strict, t, trg)
+        }
         Term::List(terms) => {
             let ty_elts = TypeWrapper::Ptr(new_var(state.table));
 
@@ -374,28 +411,6 @@ fn type_check_<L: Linearizer>(
             type_check_(state, envs, lin, linearizer, strict, rt, ty)
         }
         Term::LetPattern(x, pat, re, rt) => {
-            use crate::destruct::*;
-            // TODO: The insertion of values in the type environment is done but everything is
-            // typed as `Dyn`.
-            fn inject_pat_vars(pat: &Destruct, envs: &mut Envs) {
-                if let Destruct::Record(matches, _, rst) = pat {
-                    if let Some(id) = rst {
-                        envs.insert(id.clone(), TypeWrapper::Concrete(AbsType::Dyn()));
-                    }
-                    matches.iter().for_each(|m| match m {
-                        Match::Simple(id, ..) => {
-                            envs.insert(id.clone(), TypeWrapper::Concrete(AbsType::Dyn()))
-                        }
-                        Match::Assign(id, _, (bind_id, pat)) => {
-                            let id = bind_id.as_ref().unwrap_or(id);
-                            envs.insert(id.clone(), TypeWrapper::Concrete(AbsType::Dyn()));
-                            if !pat.is_empty() {
-                                inject_pat_vars(&pat, envs);
-                            }
-                        }
-                    });
-                }
-            }
             let ty_let = binding_type(re.as_ref(), &envs, state.table, strict, state.resolver);
             type_check_(
                 state,


Test patch:
diff --git a/src/parser/tests.rs b/src/parser/tests.rs
index 5c10d20f..792d979c 100644
--- a/src/parser/tests.rs
+++ b/src/parser/tests.rs
@@ -125,7 +125,10 @@ fn variables() {
 
 #[test]
 fn functions() {
-    assert_eq!(parse_without_pos("fun x => x"), mk_term::id());
+    assert_eq!(
+        crate::transform::desugar_destructuring::desugar_fun(parse_without_pos("fun x => x")),
+        mk_term::id()
+    );
 }
 
 #[test]
diff --git a/tests/destructuring.rs b/tests/destructuring.rs
index 740eb0ac..672d4d29 100644
--- a/tests/destructuring.rs
+++ b/tests/destructuring.rs
@@ -83,3 +83,8 @@ fn typecontract_fail() {
         Err(Error::EvalError(EvalError::BlameError(..)))
     );
 }
+
+#[test]
+fn fun() {
+    assert_eq!(eval_file("destructuring/fun.ncl"), Ok(Term::Bool(true)));
+}
diff --git a/tests/destructuring/fun.ncl b/tests/destructuring/fun.ncl
new file mode 100644
index 00000000..3509e88f
--- /dev/null
+++ b/tests/destructuring/fun.ncl
@@ -0,0 +1,2 @@
+let f = fun x@{a, b} {c=d, ..} => a - x.a + b - x.b + d in
+f {a=1, b=2} {c=3, d=4} == 3