From 5c342a5688fe7a4bb337ce0622968226d524022e Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Sun, 9 Feb 2020 15:32:27 +0000 Subject: Resolve by ref instead of by mut --- dhall/src/syntax/ast/visitor.rs | 166 ---------------------------------------- 1 file changed, 166 deletions(-) (limited to 'dhall/src/syntax/ast/visitor.rs') diff --git a/dhall/src/syntax/ast/visitor.rs b/dhall/src/syntax/ast/visitor.rs index 6a1ce7d..c09b8d4 100644 --- a/dhall/src/syntax/ast/visitor.rs +++ b/dhall/src/syntax/ast/visitor.rs @@ -32,29 +32,6 @@ pub trait ExprKindVisitor<'a, SE1, SE2, E1, E2>: Sized { } } -/// Like `ExprKindVisitor`, but by mutable reference -pub trait ExprKindMutVisitor<'a, SE, E>: Sized { - type Error; - - fn visit_subexpr(&mut self, subexpr: &'a mut SE) - -> Result<(), Self::Error>; - fn visit_embed(self, _embed: &'a mut E) -> Result<(), Self::Error> { - Ok(()) - } - - fn visit_subexpr_under_binder( - mut self, - _label: &'a mut Label, - subexpr: &'a mut SE, - ) -> Result<(), Self::Error> { - self.visit_subexpr(subexpr) - } - - fn visit(self, input: &'a mut ExprKind) -> Result<(), Self::Error> { - visit_mut(self, input) - } -} - fn visit_ref<'a, V, SE1, SE2, E1, E2>( mut v: V, input: &'a ExprKind, @@ -174,128 +151,6 @@ where }) } -fn visit_mut<'a, V, SE, E>( - mut v: V, - input: &'a mut ExprKind, -) -> Result<(), V::Error> -where - V: ExprKindMutVisitor<'a, SE, E>, -{ - fn vec<'a, V, SE, E>(v: &mut V, x: &'a mut Vec) -> Result<(), V::Error> - where - V: ExprKindMutVisitor<'a, SE, E>, - { - for x in x { - v.visit_subexpr(x)?; - } - Ok(()) - } - fn opt<'a, V, SE, E>( - v: &mut V, - x: &'a mut Option, - ) -> Result<(), V::Error> - where - V: ExprKindMutVisitor<'a, SE, E>, - { - if let Some(x) = x { - v.visit_subexpr(x)?; - } - Ok(()) - } - fn dupmap<'a, V, SE, E>( - mut v: V, - x: impl IntoIterator, - ) -> Result<(), V::Error> - where - SE: 'a, - V: ExprKindMutVisitor<'a, SE, E>, - { - for (_, x) in x { - v.visit_subexpr(x)?; - } - Ok(()) - } - fn optdupmap<'a, V, SE, E>( - mut v: V, - x: impl IntoIterator)>, - ) -> Result<(), V::Error> - where - SE: 'a, - V: ExprKindMutVisitor<'a, SE, E>, - { - for (_, x) in x { - opt(&mut v, x)?; - } - Ok(()) - } - - use crate::syntax::ExprKind::*; - match input { - Var(_) | Const(_) | Builtin(_) | BoolLit(_) | NaturalLit(_) - | IntegerLit(_) | DoubleLit(_) => {} - Lam(l, t, e) => { - v.visit_subexpr(t)?; - v.visit_subexpr_under_binder(l, e)?; - } - Pi(l, t, e) => { - v.visit_subexpr(t)?; - v.visit_subexpr_under_binder(l, e)?; - } - Let(l, t, a, e) => { - opt(&mut v, t)?; - v.visit_subexpr(a)?; - v.visit_subexpr_under_binder(l, e)?; - } - App(f, a) => { - v.visit_subexpr(f)?; - v.visit_subexpr(a)?; - } - Annot(x, t) => { - v.visit_subexpr(x)?; - v.visit_subexpr(t)?; - } - TextLit(t) => t.traverse_mut(|e| v.visit_subexpr(e))?, - BinOp(_, x, y) => { - v.visit_subexpr(x)?; - v.visit_subexpr(y)?; - } - BoolIf(b, t, f) => { - v.visit_subexpr(b)?; - v.visit_subexpr(t)?; - v.visit_subexpr(f)?; - } - EmptyListLit(t) => v.visit_subexpr(t)?, - NEListLit(es) => vec(&mut v, es)?, - SomeLit(e) => v.visit_subexpr(e)?, - RecordType(kts) => dupmap(v, kts)?, - RecordLit(kvs) => dupmap(v, kvs)?, - UnionType(kts) => optdupmap(v, kts)?, - Merge(x, y, t) => { - v.visit_subexpr(x)?; - v.visit_subexpr(y)?; - opt(&mut v, t)?; - } - ToMap(x, t) => { - v.visit_subexpr(x)?; - opt(&mut v, t)?; - } - Field(e, _) => v.visit_subexpr(e)?, - Projection(e, _) => v.visit_subexpr(e)?, - ProjectionByExpr(e, x) => { - v.visit_subexpr(e)?; - v.visit_subexpr(x)?; - } - Completion(x, y) => { - v.visit_subexpr(x)?; - v.visit_subexpr(y)?; - } - Assert(e) => v.visit_subexpr(e)?, - Import(i) => i.traverse_mut(|e| v.visit_subexpr(e))?, - Embed(a) => v.visit_embed(a)?, - } - Ok(()) -} - pub struct TraverseRefMaybeBinderVisitor(pub F); impl<'a, SE, E, SE2, Err, F> ExprKindVisitor<'a, SE, SE2, E, E> @@ -321,24 +176,3 @@ where Ok(embed.clone()) } } - -pub struct TraverseMutVisitor { - pub visit_subexpr: F1, -} - -impl<'a, SE, E, Err, F1> ExprKindMutVisitor<'a, SE, E> - for TraverseMutVisitor -where - SE: 'a, - E: 'a, - F1: FnMut(&'a mut SE) -> Result<(), Err>, -{ - type Error = Err; - - fn visit_subexpr( - &mut self, - subexpr: &'a mut SE, - ) -> Result<(), Self::Error> { - (self.visit_subexpr)(subexpr) - } -} -- cgit v1.2.3 From 5a2538d174fd36a8ed7f4fa344b9583fc48bd977 Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Tue, 11 Feb 2020 13:12:13 +0000 Subject: Remove the Embed variant from ExprKind --- dhall/src/syntax/ast/visitor.rs | 30 ++++++++++++------------------ 1 file changed, 12 insertions(+), 18 deletions(-) (limited to 'dhall/src/syntax/ast/visitor.rs') diff --git a/dhall/src/syntax/ast/visitor.rs b/dhall/src/syntax/ast/visitor.rs index c09b8d4..fc90efd 100644 --- a/dhall/src/syntax/ast/visitor.rs +++ b/dhall/src/syntax/ast/visitor.rs @@ -10,11 +10,10 @@ use crate::syntax::*; /// preventing exactly this ! So we have to be more clever. The visitor pattern allows us to have /// only one mutable thing the whole time: the visitor itself. The visitor can then carry around /// multiple closures or just one, and Rust is ok with either. See for example TraverseRefVisitor. -pub trait ExprKindVisitor<'a, SE1, SE2, E1, E2>: Sized { +pub trait ExprKindVisitor<'a, SE1, SE2>: Sized { type Error; fn visit_subexpr(&mut self, subexpr: &'a SE1) -> Result; - fn visit_embed(self, embed: &'a E1) -> Result; fn visit_subexpr_under_binder( mut self, @@ -26,18 +25,18 @@ pub trait ExprKindVisitor<'a, SE1, SE2, E1, E2>: Sized { fn visit( self, - input: &'a ExprKind, - ) -> Result, Self::Error> { + input: &'a ExprKind, + ) -> Result, Self::Error> { visit_ref(self, input) } } -fn visit_ref<'a, V, SE1, SE2, E1, E2>( +fn visit_ref<'a, V, SE1, SE2>( mut v: V, - input: &'a ExprKind, -) -> Result, V::Error> + input: &'a ExprKind, +) -> Result, V::Error> where - V: ExprKindVisitor<'a, SE1, SE2, E1, E2>, + V: ExprKindVisitor<'a, SE1, SE2>, { fn vec<'a, T, U, Err, F: FnMut(&'a T) -> Result>( x: &'a [T], @@ -54,27 +53,27 @@ where None => None, }) } - fn dupmap<'a, V, SE1, SE2, E1, E2, T>( + fn dupmap<'a, V, SE1, SE2, T>( x: impl IntoIterator, mut v: V, ) -> Result where SE1: 'a, T: FromIterator<(Label, SE2)>, - V: ExprKindVisitor<'a, SE1, SE2, E1, E2>, + V: ExprKindVisitor<'a, SE1, SE2>, { x.into_iter() .map(|(k, x)| Ok((k.clone(), v.visit_subexpr(x)?))) .collect() } - fn optdupmap<'a, V, SE1, SE2, E1, E2, T>( + fn optdupmap<'a, V, SE1, SE2, T>( x: impl IntoIterator)>, mut v: V, ) -> Result where SE1: 'a, T: FromIterator<(Label, Option)>, - V: ExprKindVisitor<'a, SE1, SE2, E1, E2>, + V: ExprKindVisitor<'a, SE1, SE2>, { x.into_iter() .map(|(k, x)| { @@ -147,17 +146,15 @@ where } Assert(e) => Assert(v.visit_subexpr(e)?), Import(i) => Import(i.traverse_ref(|e| v.visit_subexpr(e))?), - Embed(a) => Embed(v.visit_embed(a)?), }) } pub struct TraverseRefMaybeBinderVisitor(pub F); -impl<'a, SE, E, SE2, Err, F> ExprKindVisitor<'a, SE, SE2, E, E> +impl<'a, SE, SE2, Err, F> ExprKindVisitor<'a, SE, SE2> for TraverseRefMaybeBinderVisitor where SE: 'a, - E: 'a + Clone, F: FnMut(Option<&'a Label>, &'a SE) -> Result, { type Error = Err; @@ -172,7 +169,4 @@ where ) -> Result { (self.0)(Some(label), subexpr) } - fn visit_embed(self, embed: &'a E) -> Result { - Ok(embed.clone()) - } } -- cgit v1.2.3 From cab75190e86e8fd4ccc209499c0e7f300a669022 Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Tue, 11 Feb 2020 13:33:17 +0000 Subject: Simplify ExprKind visitor --- dhall/src/syntax/ast/visitor.rs | 220 +++++++++++++++------------------------- 1 file changed, 82 insertions(+), 138 deletions(-) (limited to 'dhall/src/syntax/ast/visitor.rs') diff --git a/dhall/src/syntax/ast/visitor.rs b/dhall/src/syntax/ast/visitor.rs index fc90efd..39959ac 100644 --- a/dhall/src/syntax/ast/visitor.rs +++ b/dhall/src/syntax/ast/visitor.rs @@ -2,171 +2,115 @@ use std::iter::FromIterator; use crate::syntax::*; -/// A visitor trait that can be used to traverse `ExprKind`s. We need this pattern so that Rust lets -/// us have as much mutability as we can. -/// For example, `traverse_ref_with_special_handling_of_binders` cannot be made using only -/// `traverse_ref`, because `traverse_ref` takes a `FnMut` so we would need to pass multiple -/// mutable reverences to this argument to `traverse_ref`. But Rust's ownership system is all about -/// preventing exactly this ! So we have to be more clever. The visitor pattern allows us to have -/// only one mutable thing the whole time: the visitor itself. The visitor can then carry around -/// multiple closures or just one, and Rust is ok with either. See for example TraverseRefVisitor. -pub trait ExprKindVisitor<'a, SE1, SE2>: Sized { - type Error; +fn vec<'a, T, U, Err>( + x: &'a [T], + f: impl FnMut(&'a T) -> Result, +) -> Result, Err> { + x.iter().map(f).collect() +} - fn visit_subexpr(&mut self, subexpr: &'a SE1) -> Result; +fn opt<'a, T, U, Err>( + x: &'a Option, + f: impl FnOnce(&'a T) -> Result, +) -> Result, Err> { + Ok(match x { + Some(x) => Some(f(x)?), + None => None, + }) +} - fn visit_subexpr_under_binder( - mut self, - _label: &'a Label, - subexpr: &'a SE1, - ) -> Result { - self.visit_subexpr(subexpr) - } +fn dupmap<'a, SE1, SE2, T, Err>( + x: impl IntoIterator, + mut f: impl FnMut(&'a SE1) -> Result, +) -> Result +where + SE1: 'a, + T: FromIterator<(Label, SE2)>, +{ + x.into_iter().map(|(k, x)| Ok((k.clone(), f(x)?))).collect() +} - fn visit( - self, - input: &'a ExprKind, - ) -> Result, Self::Error> { - visit_ref(self, input) - } +fn optdupmap<'a, SE1, SE2, T, Err>( + x: impl IntoIterator)>, + mut f: impl FnMut(&'a SE1) -> Result, +) -> Result +where + SE1: 'a, + T: FromIterator<(Label, Option)>, +{ + x.into_iter() + .map(|(k, x)| { + Ok(( + k.clone(), + match x { + Some(x) => Some(f(x)?), + None => None, + }, + )) + }) + .collect() } -fn visit_ref<'a, V, SE1, SE2>( - mut v: V, +pub(crate) fn visit_ref<'a, F, SE1, SE2, Err>( input: &'a ExprKind, -) -> Result, V::Error> + mut f: F, +) -> Result, Err> where - V: ExprKindVisitor<'a, SE1, SE2>, + F: FnMut(Option<&'a Label>, &'a SE1) -> Result, { - fn vec<'a, T, U, Err, F: FnMut(&'a T) -> Result>( - x: &'a [T], - f: F, - ) -> Result, Err> { - x.iter().map(f).collect() - } - fn opt<'a, T, U, Err, F: FnOnce(&'a T) -> Result>( - x: &'a Option, - f: F, - ) -> Result, Err> { - Ok(match x { - Some(x) => Some(f(x)?), - None => None, - }) - } - fn dupmap<'a, V, SE1, SE2, T>( - x: impl IntoIterator, - mut v: V, - ) -> Result - where - SE1: 'a, - T: FromIterator<(Label, SE2)>, - V: ExprKindVisitor<'a, SE1, SE2>, - { - x.into_iter() - .map(|(k, x)| Ok((k.clone(), v.visit_subexpr(x)?))) - .collect() - } - fn optdupmap<'a, V, SE1, SE2, T>( - x: impl IntoIterator)>, - mut v: V, - ) -> Result - where - SE1: 'a, - T: FromIterator<(Label, Option)>, - V: ExprKindVisitor<'a, SE1, SE2>, - { - x.into_iter() - .map(|(k, x)| { - Ok(( - k.clone(), - match x { - Some(x) => Some(v.visit_subexpr(x)?), - None => None, - }, - )) - }) - .collect() + // Can't use closures because of borrowing rules + macro_rules! expr { + ($e:expr) => { + f(None, $e) + }; + ($l:expr, $e:expr) => { + f(Some($l), $e) + }; } use crate::syntax::ExprKind::*; Ok(match input { Var(v) => Var(v.clone()), Lam(l, t, e) => { - let t = v.visit_subexpr(t)?; - let e = v.visit_subexpr_under_binder(l, e)?; + let t = expr!(t)?; + let e = expr!(l, e)?; Lam(l.clone(), t, e) } Pi(l, t, e) => { - let t = v.visit_subexpr(t)?; - let e = v.visit_subexpr_under_binder(l, e)?; + let t = expr!(t)?; + let e = expr!(l, e)?; Pi(l.clone(), t, e) } Let(l, t, a, e) => { - let t = opt(t, &mut |e| v.visit_subexpr(e))?; - let a = v.visit_subexpr(a)?; - let e = v.visit_subexpr_under_binder(l, e)?; + let t = opt(t, &mut |e| expr!(e))?; + let a = expr!(a)?; + let e = expr!(l, e)?; Let(l.clone(), t, a, e) } - App(f, a) => App(v.visit_subexpr(f)?, v.visit_subexpr(a)?), - Annot(x, t) => Annot(v.visit_subexpr(x)?, v.visit_subexpr(t)?), + App(f, a) => App(expr!(f)?, expr!(a)?), + Annot(x, t) => Annot(expr!(x)?, expr!(t)?), Const(k) => Const(*k), Builtin(v) => Builtin(*v), BoolLit(b) => BoolLit(*b), NaturalLit(n) => NaturalLit(*n), IntegerLit(n) => IntegerLit(*n), DoubleLit(n) => DoubleLit(*n), - TextLit(t) => TextLit(t.traverse_ref(|e| v.visit_subexpr(e))?), - BinOp(o, x, y) => BinOp(*o, v.visit_subexpr(x)?, v.visit_subexpr(y)?), - BoolIf(b, t, f) => BoolIf( - v.visit_subexpr(b)?, - v.visit_subexpr(t)?, - v.visit_subexpr(f)?, - ), - EmptyListLit(t) => EmptyListLit(v.visit_subexpr(t)?), - NEListLit(es) => NEListLit(vec(es, |e| v.visit_subexpr(e))?), - SomeLit(e) => SomeLit(v.visit_subexpr(e)?), - RecordType(kts) => RecordType(dupmap(kts, v)?), - RecordLit(kvs) => RecordLit(dupmap(kvs, v)?), - UnionType(kts) => UnionType(optdupmap(kts, v)?), - Merge(x, y, t) => Merge( - v.visit_subexpr(x)?, - v.visit_subexpr(y)?, - opt(t, |e| v.visit_subexpr(e))?, - ), - ToMap(x, t) => { - ToMap(v.visit_subexpr(x)?, opt(t, |e| v.visit_subexpr(e))?) - } - Field(e, l) => Field(v.visit_subexpr(e)?, l.clone()), - Projection(e, ls) => Projection(v.visit_subexpr(e)?, ls.clone()), - ProjectionByExpr(e, x) => { - ProjectionByExpr(v.visit_subexpr(e)?, v.visit_subexpr(x)?) - } - Completion(e, x) => { - Completion(v.visit_subexpr(e)?, v.visit_subexpr(x)?) - } - Assert(e) => Assert(v.visit_subexpr(e)?), - Import(i) => Import(i.traverse_ref(|e| v.visit_subexpr(e))?), + TextLit(t) => TextLit(t.traverse_ref(|e| expr!(e))?), + BinOp(o, x, y) => BinOp(*o, expr!(x)?, expr!(y)?), + BoolIf(b, t, f) => BoolIf(expr!(b)?, expr!(t)?, expr!(f)?), + EmptyListLit(t) => EmptyListLit(expr!(t)?), + NEListLit(es) => NEListLit(vec(es, |e| expr!(e))?), + SomeLit(e) => SomeLit(expr!(e)?), + RecordType(kts) => RecordType(dupmap(kts, |e| expr!(e))?), + RecordLit(kvs) => RecordLit(dupmap(kvs, |e| expr!(e))?), + UnionType(kts) => UnionType(optdupmap(kts, |e| expr!(e))?), + Merge(x, y, t) => Merge(expr!(x)?, expr!(y)?, opt(t, |e| expr!(e))?), + ToMap(x, t) => ToMap(expr!(x)?, opt(t, |e| expr!(e))?), + Field(e, l) => Field(expr!(e)?, l.clone()), + Projection(e, ls) => Projection(expr!(e)?, ls.clone()), + ProjectionByExpr(e, x) => ProjectionByExpr(expr!(e)?, expr!(x)?), + Completion(e, x) => Completion(expr!(e)?, expr!(x)?), + Assert(e) => Assert(expr!(e)?), + Import(i) => Import(i.traverse_ref(|e| expr!(e))?), }) } - -pub struct TraverseRefMaybeBinderVisitor(pub F); - -impl<'a, SE, SE2, Err, F> ExprKindVisitor<'a, SE, SE2> - for TraverseRefMaybeBinderVisitor -where - SE: 'a, - F: FnMut(Option<&'a Label>, &'a SE) -> Result, -{ - type Error = Err; - - fn visit_subexpr(&mut self, subexpr: &'a SE) -> Result { - (self.0)(None, subexpr) - } - fn visit_subexpr_under_binder( - mut self, - label: &'a Label, - subexpr: &'a SE, - ) -> Result { - (self.0)(Some(label), subexpr) - } -} -- cgit v1.2.3 From 40bee3cdcb9ac0c76996feeceb6ca160a6bd8b42 Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Tue, 11 Feb 2020 19:04:44 +0000 Subject: Introduce LitKind to factor out common enum nodes --- dhall/src/syntax/ast/visitor.rs | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) (limited to 'dhall/src/syntax/ast/visitor.rs') diff --git a/dhall/src/syntax/ast/visitor.rs b/dhall/src/syntax/ast/visitor.rs index 39959ac..c361bc1 100644 --- a/dhall/src/syntax/ast/visitor.rs +++ b/dhall/src/syntax/ast/visitor.rs @@ -91,10 +91,7 @@ where Annot(x, t) => Annot(expr!(x)?, expr!(t)?), Const(k) => Const(*k), Builtin(v) => Builtin(*v), - BoolLit(b) => BoolLit(*b), - NaturalLit(n) => NaturalLit(*n), - IntegerLit(n) => IntegerLit(*n), - DoubleLit(n) => DoubleLit(*n), + Lit(l) => Lit(l.clone()), TextLit(t) => TextLit(t.traverse_ref(|e| expr!(e))?), BinOp(o, x, y) => BinOp(*o, expr!(x)?, expr!(y)?), BoolIf(b, t, f) => BoolIf(expr!(b)?, expr!(t)?, expr!(f)?), -- cgit v1.2.3