From 77af0bbc171618f48531cc6b1d77e18089928885 Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Tue, 13 Aug 2019 20:54:15 +0200 Subject: Stop tracking the absence of Embed values at the type level --- dhall_syntax/src/core/context.rs | 2 +- dhall_syntax/src/core/expr.rs | 16 ++-------------- dhall_syntax/src/core/visitor.rs | 15 +-------------- 3 files changed, 4 insertions(+), 29 deletions(-) (limited to 'dhall_syntax/src/core') diff --git a/dhall_syntax/src/core/context.rs b/dhall_syntax/src/core/context.rs index eeec121..6844baa 100644 --- a/dhall_syntax/src/core/context.rs +++ b/dhall_syntax/src/core/context.rs @@ -4,7 +4,7 @@ use std::hash::Hash; /// A `(Context a)` associates `Text` labels with values of type `a` /// -/// The `Context` is used for type-checking when `(a = Expr X)` +/// The `Context` is used for type-checking when `(a = Expr)` /// /// * You create a `Context` using `empty` and `insert` /// * You transform a `Context` using `fmap` diff --git a/dhall_syntax/src/core/expr.rs b/dhall_syntax/src/core/expr.rs index 0cbece3..e08d816 100644 --- a/dhall_syntax/src/core/expr.rs +++ b/dhall_syntax/src/core/expr.rs @@ -10,9 +10,9 @@ pub type Double = NaiveDouble; /// An empty type #[derive(Debug, Copy, Clone, PartialEq, Eq)] -pub enum X {} +pub enum Void {} -pub fn trivial_result(x: Result) -> T { +pub fn trivial_result(x: Result) -> T { match x { Ok(x) => x, Err(e) => match e {}, @@ -351,12 +351,6 @@ impl Expr { } } -impl Expr { - pub fn absurd(&self) -> Expr { - self.visit(&mut visitor::AbsurdVisitor) - } -} - impl SubExpr { pub fn as_ref(&self) -> &Expr { &self.0.as_ref().0 @@ -416,12 +410,6 @@ impl SubExpr { } } -impl SubExpr { - pub fn absurd(&self) -> SubExpr { - SubExpr::from_expr_no_span(self.as_ref().absurd()) - } -} - impl Clone for SubExpr { fn clone(&self) -> Self { SubExpr(Rc::clone(&self.0)) diff --git a/dhall_syntax/src/core/visitor.rs b/dhall_syntax/src/core/visitor.rs index 50ec68a..e39e95b 100644 --- a/dhall_syntax/src/core/visitor.rs +++ b/dhall_syntax/src/core/visitor.rs @@ -243,7 +243,7 @@ impl<'a, T, SE1, SE2, E1, E2> ExprFFallibleVisitor<'a, SE1, SE2, E1, E2> where T: ExprFInFallibleVisitor<'a, SE1, SE2, E1, E2>, { - type Error = X; + type Error = Void; fn visit_subexpr(&mut self, subexpr: &'a SE1) -> Result { Ok(self.0.visit_subexpr(subexpr)) @@ -379,16 +379,3 @@ where (self.0)(embed) } } - -pub struct AbsurdVisitor; - -impl<'a, 'b, E> ExprFInFallibleVisitor<'a, SubExpr, SubExpr, X, E> - for &'b mut AbsurdVisitor -{ - fn visit_subexpr(&mut self, subexpr: &'a SubExpr) -> SubExpr { - SubExpr::from_expr_no_span(subexpr.as_ref().visit(&mut **self)) - } - fn visit_embed(self, embed: &'a X) -> E { - match *embed {} - } -} -- cgit v1.3.1 From 51dbaa0b66089bca63aa9cf69a1e0ec59df053b9 Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Tue, 13 Aug 2019 21:10:59 +0200 Subject: Considerably simplify Embed handling --- dhall/src/core/thunk.rs | 2 - dhall/src/core/value.rs | 8 ++- dhall/src/phase/normalize.rs | 1 - dhall/src/phase/typecheck.rs | 3 +- dhall_syntax/src/core/expr.rs | 110 +++++++++------------------------------ dhall_syntax/src/core/visitor.rs | 60 ++++++--------------- dhall_syntax/src/printer.rs | 2 +- 7 files changed, 48 insertions(+), 138 deletions(-) (limited to 'dhall_syntax/src/core') diff --git a/dhall/src/core/thunk.rs b/dhall/src/core/thunk.rs index 4d193f9..7c5c537 100644 --- a/dhall/src/core/thunk.rs +++ b/dhall/src/core/thunk.rs @@ -309,7 +309,6 @@ impl Shift for ThunkInternal { e.traverse_ref_with_special_handling_of_binders( |v| Ok(v.shift(delta, var)?), |x, v| Ok(v.shift(delta, &var.under_binder(x))?), - |x| Ok(Normalized::clone(x)), )?, ), ThunkInternal::Value(m, v) => { @@ -356,7 +355,6 @@ impl Subst for ThunkInternal { &val.under_binder(x), ) }, - Normalized::clone, ), ), ThunkInternal::Value(_, v) => { diff --git a/dhall/src/core/value.rs b/dhall/src/core/value.rs index 88d7a20..20a6021 100644 --- a/dhall/src/core/value.rs +++ b/dhall/src/core/value.rs @@ -243,7 +243,7 @@ impl Value { y.normalize_to_expr_maybe_alpha(alpha), )), Value::PartialExpr(e) => { - rc(e.map_ref_simple(|v| v.normalize_to_expr_maybe_alpha(alpha))) + rc(e.map_ref(|v| v.normalize_to_expr_maybe_alpha(alpha))) } } } @@ -333,8 +333,8 @@ impl Value { y.normalize_mut(); } Value::PartialExpr(e) => { - // TODO: need map_mut_simple - e.map_ref_simple(|v| { + // TODO: need map_mut + e.map_ref(|v| { v.normalize_nf(); }); } @@ -475,7 +475,6 @@ impl Shift for Value { e.traverse_ref_with_special_handling_of_binders( |v| Ok(v.shift(delta, var)?), |x, v| Ok(v.shift(delta, &var.under_binder(x))?), - |x| Ok(Normalized::clone(x)), )?, ), }) @@ -500,7 +499,6 @@ impl Subst for Value { &val.under_binder(x), ) }, - Normalized::clone, )) } // Retry normalizing since substituting may allow progress diff --git a/dhall/src/phase/normalize.rs b/dhall/src/phase/normalize.rs index f1045a5..2970f5f 100644 --- a/dhall/src/phase/normalize.rs +++ b/dhall/src/phase/normalize.rs @@ -381,7 +381,6 @@ pub fn normalize_whnf(ctx: NormalizationContext, expr: InputSubExpr) -> Value { expr.as_ref().map_ref_with_special_handling_of_binders( |e| Thunk::new(ctx.clone(), e.clone()), |x, e| Thunk::new(ctx.skip(x), e.clone()), - |_| unreachable!(), ); normalize_one_layer(expr) diff --git a/dhall/src/phase/typecheck.rs b/dhall/src/phase/typecheck.rs index 96ff246..e8b2544 100644 --- a/dhall/src/phase/typecheck.rs +++ b/dhall/src/phase/typecheck.rs @@ -376,12 +376,11 @@ fn type_with( e.as_ref().traverse_ref_with_special_handling_of_binders( |e| type_with(ctx, e.clone()), |_, _| unreachable!(), - |_| unreachable!(), )?; let ret = type_last_layer(ctx, &expr)?; match ret { RetTypeOnly(typ) => { - let expr = expr.map_ref_simple(|typed| typed.to_thunk()); + let expr = expr.map_ref(|typed| typed.to_thunk()); Typed::from_thunk_and_type( Thunk::from_partial_expr(expr), typ, diff --git a/dhall_syntax/src/core/expr.rs b/dhall_syntax/src/core/expr.rs index e08d816..74eb993 100644 --- a/dhall_syntax/src/core/expr.rs +++ b/dhall_syntax/src/core/expr.rs @@ -245,92 +245,62 @@ impl ExprF { v.visit(self) } - pub fn traverse_ref_with_special_handling_of_binders<'a, SE2, E2, Err>( + pub fn traverse_ref_with_special_handling_of_binders<'a, SE2, Err>( &'a self, visit_subexpr: impl FnMut(&'a SE) -> Result, visit_under_binder: impl FnOnce(&'a Label, &'a SE) -> Result, - visit_embed: impl FnOnce(&'a E) -> Result, - ) -> Result, Err> { + ) -> Result, Err> + where + E: Clone, + { self.visit(visitor::TraverseRefWithBindersVisitor { visit_subexpr, visit_under_binder, - visit_embed, }) } - fn traverse_ref<'a, SE2, E2, Err>( + fn traverse_ref<'a, SE2, Err>( &'a self, visit_subexpr: impl FnMut(&'a SE) -> Result, - visit_embed: impl FnOnce(&'a E) -> Result, - ) -> Result, Err> { - self.visit(visitor::TraverseRefVisitor { - visit_subexpr, - visit_embed, - }) + ) -> Result, Err> + where + E: Clone, + { + self.visit(visitor::TraverseRefVisitor { visit_subexpr }) } - pub fn map_ref_with_special_handling_of_binders<'a, SE2, E2>( + pub fn map_ref_with_special_handling_of_binders<'a, SE2>( &'a self, mut map_subexpr: impl FnMut(&'a SE) -> SE2, mut map_under_binder: impl FnMut(&'a Label, &'a SE) -> SE2, - map_embed: impl FnOnce(&'a E) -> E2, - ) -> ExprF { + ) -> ExprF + where + E: Clone, + { trivial_result(self.traverse_ref_with_special_handling_of_binders( |x| Ok(map_subexpr(x)), |l, x| Ok(map_under_binder(l, x)), - |x| Ok(map_embed(x)), )) } - pub fn map_ref<'a, SE2, E2>( + pub fn map_ref<'a, SE2>( &'a self, mut map_subexpr: impl FnMut(&'a SE) -> SE2, - map_embed: impl FnOnce(&'a E) -> E2, - ) -> ExprF { - trivial_result( - self.traverse_ref(|x| Ok(map_subexpr(x)), |x| Ok(map_embed(x))), - ) - } - - pub fn traverse_ref_simple<'a, SE2, Err>( - &'a self, - visit_subexpr: impl FnMut(&'a SE) -> Result, - ) -> Result, Err> - where - E: Clone, - { - self.traverse_ref(visit_subexpr, |x| Ok(E::clone(x))) - } - - pub fn map_ref_simple<'a, SE2>( - &'a self, - map_subexpr: impl Fn(&'a SE) -> SE2, ) -> ExprF where E: Clone, { - self.map_ref(map_subexpr, E::clone) + trivial_result(self.traverse_ref(|x| Ok(map_subexpr(x)))) } } -impl Expr { - fn traverse_embed( - &self, - visit_embed: impl FnMut(&E) -> Result, - ) -> Result, Err> { - self.visit(&mut visitor::TraverseEmbedVisitor(visit_embed)) - } - - fn map_embed(&self, mut map_embed: impl FnMut(&E) -> E2) -> Expr { - trivial_result(self.traverse_embed(|x| Ok(map_embed(x)))) - } - +impl Expr { pub fn traverse_resolve( &self, - visit_embed: impl FnMut(&E) -> Result, + visit_import: impl FnMut(&Import) -> Result, ) -> Result, Err> { self.traverse_resolve_with_visitor(&mut visitor::ResolveVisitor( - visit_embed, + visit_import, )) } @@ -339,7 +309,7 @@ impl Expr { visitor: &mut visitor::ResolveVisitor, ) -> Result, Err> where - F1: FnMut(&E) -> Result, + F1: FnMut(&Import) -> Result, { match self { ExprF::BinOp(BinOp::ImportAlt, l, r) => l @@ -371,42 +341,14 @@ impl SubExpr { pub fn rewrap(&self, x: Expr) -> SubExpr { SubExpr(Rc::new((x, (self.0).1.clone()))) } +} - pub fn traverse_embed( - &self, - visit_embed: impl FnMut(&E) -> Result, - ) -> Result, Err> { - Ok(self.rewrap(self.as_ref().traverse_embed(visit_embed)?)) - } - - pub fn map_embed( - &self, - map_embed: impl FnMut(&E) -> E2, - ) -> SubExpr { - self.rewrap(self.as_ref().map_embed(map_embed)) - } - - pub fn map_subexprs_with_special_handling_of_binders<'a>( - &'a self, - map_expr: impl FnMut(&'a Self) -> Self, - map_under_binder: impl FnMut(&'a Label, &'a Self) -> Self, - ) -> Self { - match self.as_ref() { - ExprF::Embed(_) => SubExpr::clone(self), - // This calls ExprF::map_ref - e => self.rewrap(e.map_ref_with_special_handling_of_binders( - map_expr, - map_under_binder, - |_| unreachable!(), - )), - } - } - +impl SubExpr { pub fn traverse_resolve( &self, - visit_embed: impl FnMut(&E) -> Result, + visit_import: impl FnMut(&Import) -> Result, ) -> Result, Err> { - Ok(self.rewrap(self.as_ref().traverse_resolve(visit_embed)?)) + Ok(self.rewrap(self.as_ref().traverse_resolve(visit_import)?)) } } diff --git a/dhall_syntax/src/core/visitor.rs b/dhall_syntax/src/core/visitor.rs index e39e95b..647f76f 100644 --- a/dhall_syntax/src/core/visitor.rs +++ b/dhall_syntax/src/core/visitor.rs @@ -278,21 +278,18 @@ where } } -pub struct TraverseRefWithBindersVisitor { +pub struct TraverseRefWithBindersVisitor { pub visit_subexpr: F1, pub visit_under_binder: F2, - pub visit_embed: F4, } -impl<'a, SE, E, SE2, E2, Err, F1, F2, F4> - ExprFFallibleVisitor<'a, SE, SE2, E, E2> - for TraverseRefWithBindersVisitor +impl<'a, SE, E, SE2, Err, F1, F2> ExprFFallibleVisitor<'a, SE, SE2, E, E> + for TraverseRefWithBindersVisitor where SE: 'a, - E: 'a, + E: 'a + Clone, F1: FnMut(&'a SE) -> Result, F2: FnOnce(&'a Label, &'a SE) -> Result, - F4: FnOnce(&'a E) -> Result, { type Error = Err; @@ -306,68 +303,45 @@ where ) -> Result { (self.visit_under_binder)(label, subexpr) } - fn visit_embed(self, embed: &'a E) -> Result { - (self.visit_embed)(embed) + fn visit_embed(self, embed: &'a E) -> Result { + Ok(embed.clone()) } } -pub struct TraverseRefVisitor { +pub struct TraverseRefVisitor { pub visit_subexpr: F1, - pub visit_embed: F3, } -impl<'a, SE, E, SE2, E2, Err, F1, F3> ExprFFallibleVisitor<'a, SE, SE2, E, E2> - for TraverseRefVisitor +impl<'a, SE, E, SE2, Err, F1> ExprFFallibleVisitor<'a, SE, SE2, E, E> + for TraverseRefVisitor where SE: 'a, - E: 'a, + E: 'a + Clone, F1: FnMut(&'a SE) -> Result, - F3: FnOnce(&'a E) -> Result, { type Error = Err; fn visit_subexpr(&mut self, subexpr: &'a SE) -> Result { (self.visit_subexpr)(subexpr) } - fn visit_embed(self, embed: &'a E) -> Result { - (self.visit_embed)(embed) - } -} - -pub struct TraverseEmbedVisitor(pub F1); - -impl<'a, 'b, E, E2, Err, F1> - ExprFFallibleVisitor<'a, SubExpr, SubExpr, E, E2> - for &'b mut TraverseEmbedVisitor -where - F1: FnMut(&E) -> Result, -{ - type Error = Err; - - fn visit_subexpr( - &mut self, - subexpr: &'a SubExpr, - ) -> Result, Self::Error> { - Ok(subexpr.rewrap(subexpr.as_ref().visit(&mut **self)?)) - } - fn visit_embed(self, embed: &'a E) -> Result { - (self.0)(embed) + fn visit_embed(self, embed: &'a E) -> Result { + Ok(embed.clone()) } } pub struct ResolveVisitor(pub F1); -impl<'a, 'b, E, E2, Err, F1> - ExprFFallibleVisitor<'a, SubExpr, SubExpr, E, E2> +impl<'a, 'b, E2, Err, F1> + ExprFFallibleVisitor<'a, SubExpr, SubExpr, Import, E2> for &'b mut ResolveVisitor where - F1: FnMut(&E) -> Result, + F1: FnMut(&Import) -> Result, { type Error = Err; fn visit_subexpr( &mut self, - subexpr: &'a SubExpr, + subexpr: &'a SubExpr, ) -> Result, Self::Error> { Ok(subexpr.rewrap( subexpr @@ -375,7 +349,7 @@ where .traverse_resolve_with_visitor(&mut **self)?, )) } - fn visit_embed(self, embed: &'a E) -> Result { + fn visit_embed(self, embed: &'a Import) -> Result { (self.0)(embed) } } diff --git a/dhall_syntax/src/printer.rs b/dhall_syntax/src/printer.rs index 25d4ca8..48e56be 100644 --- a/dhall_syntax/src/printer.rs +++ b/dhall_syntax/src/printer.rs @@ -156,7 +156,7 @@ impl Expr { }; // Annotate subexpressions with the appropriate phase, defaulting to Base - let phased_self = match self.map_ref_simple(|e| PhasedExpr(e, Base)) { + let phased_self = match self.map_ref(|e| PhasedExpr(e, Base)) { Pi(a, b, c) => { if &String::from(&a) == "_" { Pi(a, b.phase(Operator), c) -- cgit v1.3.1 From 07956ccb1daf4a6819f64776f70b6f5f26869184 Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Tue, 13 Aug 2019 21:38:17 +0200 Subject: Cleanup visitor code --- dhall_syntax/src/core/visitor.rs | 193 +++++++++++---------------------------- 1 file changed, 54 insertions(+), 139 deletions(-) (limited to 'dhall_syntax/src/core') diff --git a/dhall_syntax/src/core/visitor.rs b/dhall_syntax/src/core/visitor.rs index 647f76f..f2f3b32 100644 --- a/dhall_syntax/src/core/visitor.rs +++ b/dhall_syntax/src/core/visitor.rs @@ -2,52 +2,56 @@ use crate::*; use std::iter::FromIterator; /// A way too generic Visitor trait. -pub trait GenericVisitor: Sized { - fn visit(self, input: Input) -> Return; +pub trait GenericVisitor: Sized { + fn visit(self, input: Input) -> Output; } -/// A visitor trait that can be used to traverse `ExprF`s. We need this pattern -/// so that Rust lets us have as much mutability as we can. -/// For example, `traverse_embed` 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 -/// and TraverseEmbedVisitor. -/// This is very generic. For a more legible trait, see ExprFInFallibleVisitor -pub trait ExprFVeryGenericVisitor<'a, Ret, SE1, E1>: Sized { +/// A visitor trait that can be used to traverse `ExprF`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 ExprFFallibleVisitor<'a, SE1, SE2, E1, E2>: Sized { type Error; - type SE2; - type E2; - fn visit_subexpr( - &mut self, - subexpr: &'a SE1, - ) -> Result; + fn visit_subexpr(&mut self, subexpr: &'a SE1) -> Result; + fn visit_embed(self, embed: &'a E1) -> Result; fn visit_subexpr_under_binder( - self, - label: &'a Label, + mut self, + _label: &'a Label, subexpr: &'a SE1, - ) -> Result; + ) -> Result { + self.visit_subexpr(subexpr) + } +} - fn visit_embed_squash(self, embed: &'a E1) -> Result; +/// Like ExprFFallibleVisitor, but without the error handling. +pub trait ExprFInFallibleVisitor<'a, SE1, SE2, E1, E2>: Sized { + fn visit_subexpr(&mut self, subexpr: &'a SE1) -> SE2; + fn visit_embed(self, embed: &'a E1) -> E2; - // Called with the result of the map, in the non-embed case. - // Useful to change the result type, and/or avoid some loss of info - fn visit_resulting_exprf( - result: ExprF, - ) -> Result; + fn visit_subexpr_under_binder( + mut self, + _label: &'a Label, + subexpr: &'a SE1, + ) -> SE2 { + self.visit_subexpr(subexpr) + } } -impl<'a, T, Ret, SE1, E1> - GenericVisitor<&'a ExprF, Result> for T +impl<'a, T, SE1, SE2, E1, E2> + GenericVisitor<&'a ExprF, Result, T::Error>> for T where - T: ExprFVeryGenericVisitor<'a, Ret, SE1, E1>, + T: ExprFFallibleVisitor<'a, SE1, SE2, E1, E2>, { - fn visit(self, input: &'a ExprF) -> Result { + fn visit( + self, + input: &'a ExprF, + ) -> Result, T::Error> { fn vec<'a, T, U, Err, F: FnMut(&'a T) -> Result>( x: &'a [T], f: F, @@ -63,27 +67,27 @@ where None => None, }) } - fn dupmap<'a, V, Ret, SE, E, T>( - x: impl IntoIterator, + fn dupmap<'a, V, SE1, SE2, E1, E2, T>( + x: impl IntoIterator, mut v: V, ) -> Result where - SE: 'a, - T: FromIterator<(Label, V::SE2)>, - V: ExprFVeryGenericVisitor<'a, Ret, SE, E>, + SE1: 'a, + T: FromIterator<(Label, SE2)>, + V: ExprFFallibleVisitor<'a, SE1, SE2, E1, E2>, { x.into_iter() .map(|(k, x)| Ok((k.clone(), v.visit_subexpr(x)?))) .collect() } - fn optdupmap<'a, V, Ret, SE, E, T>( - x: impl IntoIterator)>, + fn optdupmap<'a, V, SE1, SE2, E1, E2, T>( + x: impl IntoIterator)>, mut v: V, ) -> Result where - SE: 'a, - T: FromIterator<(Label, Option)>, - V: ExprFVeryGenericVisitor<'a, Ret, SE, E>, + SE1: 'a, + T: FromIterator<(Label, Option)>, + V: ExprFFallibleVisitor<'a, SE1, SE2, E1, E2>, { x.into_iter() .map(|(k, x)| { @@ -100,7 +104,7 @@ where let mut v = self; use crate::ExprF::*; - T::visit_resulting_exprf(match input { + Ok(match input { Var(v) => Var(v.clone()), Lam(l, t, e) => { let t = v.visit_subexpr(t)?; @@ -149,90 +153,18 @@ where Field(e, l) => Field(v.visit_subexpr(e)?, l.clone()), Projection(e, ls) => Projection(v.visit_subexpr(e)?, ls.clone()), Assert(e) => Assert(v.visit_subexpr(e)?), - Embed(a) => return v.visit_embed_squash(a), + Embed(a) => Embed(v.visit_embed(a)?), }) } } -/// Like ExprFVeryGenericVisitor, but sets the return -/// type to ExprF<_> -pub trait ExprFFallibleVisitor<'a, SE1, SE2, E1, E2>: 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, - _label: &'a Label, - subexpr: &'a SE1, - ) -> Result { - self.visit_subexpr(subexpr) - } - - fn visit_embed_squash( - self, - embed: &'a E1, - ) -> Result, Self::Error> { - Ok(ExprF::Embed(self.visit_embed(embed)?)) - } -} - -impl<'a, T, SE1, SE2, E1, E2> - ExprFVeryGenericVisitor<'a, ExprF, SE1, E1> for T +impl<'a, T, SE1, SE2, E1, E2> GenericVisitor<&'a ExprF, ExprF> + for T where - T: ExprFFallibleVisitor<'a, SE1, SE2, E1, E2>, + T: ExprFInFallibleVisitor<'a, SE1, SE2, E1, E2>, { - type Error = T::Error; - type SE2 = SE2; - type E2 = E2; - - fn visit_subexpr( - &mut self, - subexpr: &'a SE1, - ) -> Result { - self.visit_subexpr(subexpr) - } - - fn visit_subexpr_under_binder( - self, - label: &'a Label, - subexpr: &'a SE1, - ) -> Result { - self.visit_subexpr_under_binder(label, subexpr) - } - - fn visit_embed_squash( - self, - embed: &'a E1, - ) -> Result, Self::Error> { - self.visit_embed_squash(embed) - } - - // Called with the result of the map, in the non-embed case. - // Useful to change the result type, and/or avoid some loss of info - fn visit_resulting_exprf( - result: ExprF, - ) -> Result, Self::Error> { - Ok(result) - } -} - -/// Like ExprFFallibleVisitor, but without the error handling. -pub trait ExprFInFallibleVisitor<'a, SE1, SE2, E1, E2>: Sized { - fn visit_subexpr(&mut self, subexpr: &'a SE1) -> SE2; - fn visit_embed(self, embed: &'a E1) -> E2; - - fn visit_subexpr_under_binder( - mut self, - _label: &'a Label, - subexpr: &'a SE1, - ) -> SE2 { - self.visit_subexpr(subexpr) - } - - fn visit_embed_squash(self, embed: &'a E1) -> ExprF { - ExprF::Embed(self.visit_embed(embed)) + fn visit(self, input: &'a ExprF) -> ExprF { + trivial_result(InfallibleWrapper(self).visit(input)) } } @@ -259,23 +191,6 @@ where ) -> Result { Ok(self.0.visit_subexpr_under_binder(label, subexpr)) } - - fn visit_embed_squash( - self, - embed: &'a E1, - ) -> Result, Self::Error> { - Ok(self.0.visit_embed_squash(embed)) - } -} - -impl<'a, T, SE1, SE2, E1, E2> GenericVisitor<&'a ExprF, ExprF> - for T -where - T: ExprFInFallibleVisitor<'a, SE1, SE2, E1, E2>, -{ - fn visit(self, input: &'a ExprF) -> ExprF { - trivial_result(InfallibleWrapper(self).visit(input)) - } } pub struct TraverseRefWithBindersVisitor { -- cgit v1.3.1 From 8d45d633dfa60e8d64c9e6e742de4e33496bf0fa Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Tue, 13 Aug 2019 22:11:45 +0200 Subject: Store Imports in their own node instead of in Embed --- dhall/src/phase/binary.rs | 24 ++++++++++++++---------- dhall/src/phase/mod.rs | 8 +++++--- dhall/src/phase/normalize.rs | 3 +++ dhall/src/phase/typecheck.rs | 3 +++ dhall_syntax/src/core/expr.rs | 15 ++++++++++++--- dhall_syntax/src/core/visitor.rs | 11 ++++++----- dhall_syntax/src/parser.rs | 14 +++++++------- dhall_syntax/src/printer.rs | 16 +++++++++++----- 8 files changed, 61 insertions(+), 33 deletions(-) (limited to 'dhall_syntax/src/core') diff --git a/dhall/src/phase/binary.rs b/dhall/src/phase/binary.rs index 3292617..b3a80aa 100644 --- a/dhall/src/phase/binary.rs +++ b/dhall/src/phase/binary.rs @@ -19,7 +19,6 @@ pub fn decode(data: &[u8]) -> Result { } } -//TODO: encode normalized expression too pub fn encode(expr: &ParsedSubExpr) -> Result, EncodeError> { serde_cbor::ser::to_vec(&Serialize::Expr(expr)) .map_err(|e| EncodeError::CBORError(e)) @@ -264,7 +263,7 @@ fn cbor_value_to_dhall( // TODO // Some(x) => { // match cbor_value_to_dhall(&x)?.as_ref() { - // Embed(import) => Some(Box::new( + // Import(import) => Some(Box::new( // import.location_hashed.clone(), // )), // _ => Err(DecodeError::WrongFormatError( @@ -340,7 +339,7 @@ fn cbor_value_to_dhall( "import/type".to_owned(), ))?, }; - Embed(Import { + Import(dhall_syntax::Import { mode, location_hashed: ImportHashed { hash, location }, }) @@ -573,7 +572,10 @@ where .chain(once(expr(x))) .chain(ls.iter().map(label)), ), - Embed(import) => serialize_import(ser, import), + Import(import) => serialize_import(ser, import), + Embed(_) => unimplemented!( + "An expression with resolved imports cannot be binary-encoded" + ), } } @@ -631,12 +633,14 @@ where ImportLocation::Remote(url) => { match &url.headers { None => ser_seq.serialize_element(&Null)?, - Some(location_hashed) => ser_seq.serialize_element( - &self::Serialize::Expr(&rc(ExprF::Embed(Import { - mode: ImportMode::Code, - location_hashed: location_hashed.as_ref().clone(), - }))), - )?, + Some(location_hashed) => { + ser_seq.serialize_element(&self::Serialize::Expr(&rc( + ExprF::Import(dhall_syntax::Import { + mode: ImportMode::Code, + location_hashed: location_hashed.as_ref().clone(), + }), + )))? + } }; ser_seq.serialize_element(&url.authority)?; for p in &url.path { diff --git a/dhall/src/phase/mod.rs b/dhall/src/phase/mod.rs index 7364949..a1e3f29 100644 --- a/dhall/src/phase/mod.rs +++ b/dhall/src/phase/mod.rs @@ -2,7 +2,7 @@ use std::borrow::Cow; use std::fmt::Display; use std::path::Path; -use dhall_syntax::{Const, Import, SubExpr}; +use dhall_syntax::{Const, SubExpr, Void}; use crate::core::thunk::{Thunk, TypedThunk}; use crate::core::value::Value; @@ -17,8 +17,8 @@ pub(crate) mod parse; pub(crate) mod resolve; pub(crate) mod typecheck; -pub type ParsedSubExpr = SubExpr; -pub type DecodedSubExpr = SubExpr; +pub type ParsedSubExpr = SubExpr; +pub type DecodedSubExpr = SubExpr; pub type ResolvedSubExpr = SubExpr; pub type NormalizedSubExpr = SubExpr; @@ -26,6 +26,8 @@ pub type NormalizedSubExpr = SubExpr; pub struct Parsed(ParsedSubExpr, ImportRoot); /// An expression where all imports have been resolved +/// +/// Invariant: there must be no `Import` nodes or `ImportAlt` operations left. #[derive(Debug, Clone)] pub struct Resolved(ResolvedSubExpr); diff --git a/dhall/src/phase/normalize.rs b/dhall/src/phase/normalize.rs index 2970f5f..adbcb02 100644 --- a/dhall/src/phase/normalize.rs +++ b/dhall/src/phase/normalize.rs @@ -644,6 +644,9 @@ pub fn normalize_one_layer(expr: ExprF) -> Value { }; let ret = match expr { + ExprF::Import(_) => unreachable!( + "There should remain no imports in a resolved expression" + ), ExprF::Embed(_) => unreachable!(), ExprF::Var(_) => unreachable!(), ExprF::Annot(x, _) => Ret::Thunk(x), diff --git a/dhall/src/phase/typecheck.rs b/dhall/src/phase/typecheck.rs index e8b2544..89e2da8 100644 --- a/dhall/src/phase/typecheck.rs +++ b/dhall/src/phase/typecheck.rs @@ -407,6 +407,9 @@ fn type_last_layer( let mkerr = |msg: TypeMessage| TypeError::new(ctx, msg); match e { + Import(_) => unreachable!( + "There should remain no imports in a resolved expression" + ), Lam(_, _, _) | Pi(_, _, _) | Let(_, _, _, _) | Embed(_) | Var(_) => { unreachable!() } diff --git a/dhall_syntax/src/core/expr.rs b/dhall_syntax/src/core/expr.rs index 74eb993..4c809f8 100644 --- a/dhall_syntax/src/core/expr.rs +++ b/dhall_syntax/src/core/expr.rs @@ -12,6 +12,12 @@ pub type Double = NaiveDouble; #[derive(Debug, Copy, Clone, PartialEq, Eq)] pub enum Void {} +impl std::fmt::Display for Void { + fn fmt(&self, _f: &mut std::fmt::Formatter) -> Result<(), std::fmt::Error> { + match *self {} + } +} + pub fn trivial_result(x: Result) -> T { match x { Ok(x) => x, @@ -233,7 +239,9 @@ pub enum ExprF { Field(SubExpr, Label), /// `e.{ x, y, z }` Projection(SubExpr, DupTreeSet { // Precedence is magically handled by the ordering of BinOps. ExprF::BinOp(op, _, _) if phase > PrintPhase::BinOp(*op) => true, ExprF::App(_, _) if phase > PrintPhase::App => true, - Field(_, _) | Projection(_, _) if phase > Import => true, + Field(_, _) | Projection(_, _) if phase > PrintPhase::Import => { + true + } _ => false, }; @@ -165,8 +168,8 @@ impl Expr { } } Merge(a, b, c) => Merge( - a.phase(Import), - b.phase(Import), + a.phase(PrintPhase::Import), + b.phase(PrintPhase::Import), c.map(|x| x.phase(PrintPhase::App)), ), Annot(a, b) => Annot(a.phase(Operator), b), @@ -175,8 +178,11 @@ impl Expr { a.phase(PrintPhase::BinOp(op)), b.phase(PrintPhase::BinOp(op)), ), - SomeLit(e) => SomeLit(e.phase(Import)), - ExprF::App(f, a) => ExprF::App(f.phase(Import), a.phase(Import)), + SomeLit(e) => SomeLit(e.phase(PrintPhase::Import)), + ExprF::App(f, a) => ExprF::App( + f.phase(PrintPhase::Import), + a.phase(PrintPhase::Import), + ), Field(a, b) => Field(a.phase(Primitive), b), Projection(e, ls) => Projection(e.phase(Primitive), ls), e => e, -- cgit v1.3.1 From 43094c8c52319aa82b03e73700ca1b5f65da13c6 Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Tue, 13 Aug 2019 22:24:04 +0200 Subject: The syntax has very little sharing; no need for Rc anymore --- dhall_syntax/src/core/expr.rs | 16 +++++----------- 1 file changed, 5 insertions(+), 11 deletions(-) (limited to 'dhall_syntax/src/core') diff --git a/dhall_syntax/src/core/expr.rs b/dhall_syntax/src/core/expr.rs index 4c809f8..e3f8cec 100644 --- a/dhall_syntax/src/core/expr.rs +++ b/dhall_syntax/src/core/expr.rs @@ -168,8 +168,8 @@ pub enum Builtin { } // Each node carries an annotation. In practice it's either X (no annotation) or a Span. -#[derive(Debug)] -pub struct SubExpr(Rc<(Expr, Option)>); +#[derive(Debug, Clone)] +pub struct SubExpr(Box<(Expr, Option)>); impl std::cmp::PartialEq for SubExpr { fn eq(&self, other: &Self) -> bool { @@ -336,11 +336,11 @@ impl SubExpr { } pub fn new(x: Expr, n: Span) -> Self { - SubExpr(Rc::new((x, Some(n)))) + SubExpr(Box::new((x, Some(n)))) } pub fn from_expr_no_span(x: Expr) -> Self { - SubExpr(Rc::new((x, None))) + SubExpr(Box::new((x, None))) } pub fn from_builtin(b: Builtin) -> Self { @@ -348,7 +348,7 @@ impl SubExpr { } pub fn rewrap(&self, x: Expr) -> SubExpr { - SubExpr(Rc::new((x, (self.0).1.clone()))) + SubExpr(Box::new((x, (self.0).1.clone()))) } } @@ -361,12 +361,6 @@ impl SubExpr { } } -impl Clone for SubExpr { - fn clone(&self) -> Self { - SubExpr(Rc::clone(&self.0)) - } -} - // Should probably rename this pub fn rc(x: Expr) -> SubExpr { SubExpr::from_expr_no_span(x) -- cgit v1.3.1 From c600bae72198350b78fe19cf993b7f4c6f35225a Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Tue, 13 Aug 2019 23:08:48 +0200 Subject: Implement Hash for ParsedSubExpr --- dhall/src/phase/binary.rs | 41 ++++++++++++++++++----------------------- dhall/src/phase/mod.rs | 16 ++++++++++++++++ dhall_syntax/src/core/expr.rs | 28 +++++++++++++++++++++++----- dhall_syntax/src/core/text.rs | 2 +- 4 files changed, 58 insertions(+), 29 deletions(-) (limited to 'dhall_syntax/src/core') diff --git a/dhall/src/phase/binary.rs b/dhall/src/phase/binary.rs index b3a80aa..9ed823c 100644 --- a/dhall/src/phase/binary.rs +++ b/dhall/src/phase/binary.rs @@ -10,7 +10,7 @@ use dhall_syntax::{ }; use crate::error::{DecodeError, EncodeError}; -use crate::phase::{DecodedSubExpr, ParsedSubExpr}; +use crate::phase::DecodedSubExpr; pub fn decode(data: &[u8]) -> Result { match serde_cbor::de::from_slice(data) { @@ -19,7 +19,7 @@ pub fn decode(data: &[u8]) -> Result { } } -pub fn encode(expr: &ParsedSubExpr) -> Result, EncodeError> { +pub fn encode(expr: &SubExpr) -> Result, EncodeError> { serde_cbor::ser::to_vec(&Serialize::Expr(expr)) .map_err(|e| EncodeError::CBORError(e)) } @@ -423,11 +423,11 @@ where .collect::>() } -enum Serialize<'a> { - Expr(&'a ParsedSubExpr), +enum Serialize<'a, E> { + Expr(&'a SubExpr), CBOR(cbor::Value), - RecordMap(&'a DupTreeMap), - UnionMap(&'a DupTreeMap>), + RecordMap(&'a DupTreeMap>), + UnionMap(&'a DupTreeMap>>), } macro_rules! count { @@ -447,7 +447,7 @@ macro_rules! ser_seq { }}; } -fn serialize_subexpr(ser: S, e: &ParsedSubExpr) -> Result +fn serialize_subexpr(ser: S, e: &SubExpr) -> Result where S: serde::ser::Serializer, { @@ -457,21 +457,14 @@ where use std::iter::once; use self::Serialize::{RecordMap, UnionMap}; - fn expr(x: &ParsedSubExpr) -> self::Serialize<'_> { + fn expr(x: &SubExpr) -> self::Serialize<'_, E> { self::Serialize::Expr(x) } - fn cbor<'a>(v: cbor::Value) -> self::Serialize<'a> { - self::Serialize::CBOR(v) - } - fn tag<'a>(x: u64) -> self::Serialize<'a> { - cbor(U64(x)) - } - fn null<'a>() -> self::Serialize<'a> { - cbor(cbor::Value::Null) - } - fn label<'a>(l: &Label) -> self::Serialize<'a> { - cbor(cbor::Value::String(l.into())) - } + let cbor = + |v: cbor::Value| -> self::Serialize<'_, E> { self::Serialize::CBOR(v) }; + let tag = |x: u64| cbor(U64(x)); + let null = || cbor(cbor::Value::Null); + let label = |l: &Label| cbor(cbor::Value::String(l.into())); match e.as_ref() { Const(c) => ser.serialize_str(&c.to_string()), @@ -634,12 +627,14 @@ where match &url.headers { None => ser_seq.serialize_element(&Null)?, Some(location_hashed) => { - ser_seq.serialize_element(&self::Serialize::Expr(&rc( + let e = rc( ExprF::Import(dhall_syntax::Import { mode: ImportMode::Code, location_hashed: location_hashed.as_ref().clone(), }), - )))? + ); + let s: Serialize<'_, ()> = self::Serialize::Expr(&e); + ser_seq.serialize_element(&s)? } }; ser_seq.serialize_element(&url.authority)?; @@ -665,7 +660,7 @@ where ser_seq.end() } -impl<'a> serde::ser::Serialize for Serialize<'a> { +impl<'a, E> serde::ser::Serialize for Serialize<'a, E> { fn serialize(&self, ser: S) -> Result where S: serde::ser::Serializer, diff --git a/dhall/src/phase/mod.rs b/dhall/src/phase/mod.rs index a1e3f29..b73597c 100644 --- a/dhall/src/phase/mod.rs +++ b/dhall/src/phase/mod.rs @@ -157,6 +157,10 @@ impl Typed { } impl Normalized { + pub fn encode(&self) -> Result, EncodeError> { + crate::phase::binary::encode(&self.to_expr()) + } + #[allow(dead_code)] pub fn to_expr(&self) -> NormalizedSubExpr { self.0.to_expr() @@ -220,6 +224,18 @@ derive_traits_for_wrapper_struct!(Parsed); derive_traits_for_wrapper_struct!(Resolved); derive_traits_for_wrapper_struct!(Normalized); +impl std::hash::Hash for Normalized { + fn hash(&self, state: &mut H) + where + H: std::hash::Hasher, + { + match self.encode() { + Ok(vec) => vec.hash(state), + Err(_) => {} + } + } +} + impl Eq for Typed {} impl PartialEq for Typed { fn eq(&self, other: &Self) -> bool { diff --git a/dhall_syntax/src/core/expr.rs b/dhall_syntax/src/core/expr.rs index e3f8cec..9729efb 100644 --- a/dhall_syntax/src/core/expr.rs +++ b/dhall_syntax/src/core/expr.rs @@ -61,6 +61,15 @@ impl PartialEq for NaiveDouble { impl Eq for NaiveDouble {} +impl std::hash::Hash for NaiveDouble { + fn hash(&self, state: &mut H) + where + H: std::hash::Hasher, + { + self.0.to_bits().hash(state) + } +} + impl From for NaiveDouble { fn from(x: f64) -> Self { NaiveDouble(x) @@ -74,7 +83,7 @@ impl From for f64 { } /// Constants for a pure type system -#[derive(Debug, Copy, Clone, PartialEq, Eq, PartialOrd, Ord)] +#[derive(Debug, Copy, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)] pub enum Const { Type, Kind, @@ -86,7 +95,7 @@ pub enum Const { /// The `Label` field is the variable's name (i.e. \"`x`\"). /// The `Int` field is a DeBruijn index. /// See dhall-lang/standard/semantics.md for details -#[derive(Debug, Clone, PartialEq, Eq)] +#[derive(Debug, Clone, PartialEq, Eq, Hash)] pub struct V