From 055e70f52bb0d8740ce6ac00b98ae856c29642b2 Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Mon, 2 Nov 2020 01:03:59 +0000 Subject: Try to keep ownership of `Nir`s when convenient The goal is that we might avoid cloning internals in the common case where a value is not shared --- dhall/src/semantics/nze/normalize.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'dhall/src/semantics') diff --git a/dhall/src/semantics/nze/normalize.rs b/dhall/src/semantics/nze/normalize.rs index d042f3f..3b40fac 100644 --- a/dhall/src/semantics/nze/normalize.rs +++ b/dhall/src/semantics/nze/normalize.rs @@ -144,7 +144,7 @@ pub fn normalize_one_layer(expr: ExprKind, env: &NzEnv) -> NirKind { ExprKind::UnionType(kvs) => { ret_kind(UnionType(kvs.into_iter().collect())) } - ExprKind::Op(ref op) => normalize_operation(op), + ExprKind::Op(op) => normalize_operation(op), ExprKind::Annot(x, _) => ret_nir(x), ExprKind::Assert(x) => ret_kind(Assert(x)), ExprKind::Import(..) => { -- cgit v1.2.3 From 9e8ae42b2742e27a70a7fb8ea79ae21060d43fc1 Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Mon, 2 Nov 2020 03:47:15 +0000 Subject: Normalize `with` by mutation. This is Cow-style mutation: we clone only what's shared and then mutate it. This it more legible and more efficient than the immutable version. --- dhall/src/semantics/nze/lazy.rs | 21 +++++++++++++++++---- dhall/src/semantics/nze/nir.rs | 24 ++++++++++++++++++++++++ dhall/src/semantics/nze/normalize.rs | 2 +- 3 files changed, 42 insertions(+), 5 deletions(-) (limited to 'dhall/src/semantics') diff --git a/dhall/src/semantics/nze/lazy.rs b/dhall/src/semantics/nze/lazy.rs index d361313..550f69a 100644 --- a/dhall/src/semantics/nze/lazy.rs +++ b/dhall/src/semantics/nze/lazy.rs @@ -35,6 +35,22 @@ where let _ = lazy.tgt.set(tgt); lazy } + + pub fn force(&self) -> &Tgt { + self.tgt.get_or_init(|| { + let src = self.src.take().unwrap(); + src.eval() + }) + } + + pub fn get_mut(&mut self) -> &mut Tgt { + self.force(); + self.tgt.get_mut().unwrap() + } + pub fn into_inner(self) -> Tgt { + self.force(); + self.tgt.into_inner().unwrap() + } } impl Deref for Lazy @@ -43,10 +59,7 @@ where { type Target = Tgt; fn deref(&self) -> &Self::Target { - self.tgt.get_or_init(|| { - let src = self.src.take().unwrap(); - src.eval() - }) + self.force() } } diff --git a/dhall/src/semantics/nze/nir.rs b/dhall/src/semantics/nze/nir.rs index 12f1b14..95eeba1 100644 --- a/dhall/src/semantics/nze/nir.rs +++ b/dhall/src/semantics/nze/nir.rs @@ -138,6 +138,24 @@ impl Nir { self.0.kind() } + /// The contents of a `Nir` are immutable and shared. If however we happen to be the sole + /// owners, we can mutate it directly. Otherwise, this clones the internal value first. + pub fn kind_mut(&mut self) -> &mut NirKind { + if Rc::get_mut(&mut self.0).is_none() { + // Clone self + let kind = self.kind().clone(); + *self = Nir::from_kind(kind); + } + Rc::get_mut(&mut self.0).unwrap().kind_mut() + } + /// If we are the sole owner of this Nir, we can avoid a clone. + pub fn into_kind(self) -> NirKind { + match Rc::try_unwrap(self.0) { + Ok(int) => int.into_kind(), + Err(rc) => rc.kind().clone(), + } + } + pub fn to_type(&self, u: impl Into) -> Type { Type::new(self.clone(), u.into()) } @@ -291,6 +309,12 @@ impl NirInternal { fn kind(&self) -> &NirKind { &self.kind } + fn kind_mut(&mut self) -> &mut NirKind { + self.kind.get_mut() + } + fn into_kind(self) -> NirKind { + self.kind.into_inner() + } } impl NirKind { diff --git a/dhall/src/semantics/nze/normalize.rs b/dhall/src/semantics/nze/normalize.rs index 3b40fac..62efc5f 100644 --- a/dhall/src/semantics/nze/normalize.rs +++ b/dhall/src/semantics/nze/normalize.rs @@ -84,7 +84,7 @@ where pub type Ret = NirKind; pub fn ret_nir(x: Nir) -> Ret { - ret_ref(&x) + x.into_kind() } pub fn ret_kind(x: NirKind) -> Ret { x -- cgit v1.2.3 From 85d946016d67515aa70fa01338512e3fd7df408e Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Mon, 2 Nov 2020 04:06:05 +0000 Subject: Typecheck `with` using mutation --- dhall/src/semantics/tck/tir.rs | 3 +++ dhall/src/semantics/tck/typecheck.rs | 4 ++-- 2 files changed, 5 insertions(+), 2 deletions(-) (limited to 'dhall/src/semantics') diff --git a/dhall/src/semantics/tck/tir.rs b/dhall/src/semantics/tck/tir.rs index ec15a1f..f34802c 100644 --- a/dhall/src/semantics/tck/tir.rs +++ b/dhall/src/semantics/tck/tir.rs @@ -117,6 +117,9 @@ impl<'hir> Tir<'hir> { pub fn ty(&self) -> &Type { &self.ty } + pub fn into_ty(self) -> Type { + self.ty + } pub fn to_hir(&self) -> Hir { self.as_hir().clone() diff --git a/dhall/src/semantics/tck/typecheck.rs b/dhall/src/semantics/tck/typecheck.rs index d21c7ce..498bd76 100644 --- a/dhall/src/semantics/tck/typecheck.rs +++ b/dhall/src/semantics/tck/typecheck.rs @@ -35,7 +35,7 @@ fn type_one_layer( ) -> Result { let span_err = |msg: &str| mk_span_err(span.clone(), msg); - Ok(match &ekind { + Ok(match ekind { ExprKind::Import(..) => { unreachable!("This case should have been handled in resolution") } @@ -57,7 +57,7 @@ fn type_one_layer( NumKind::Double(_) => Builtin::Double, }), ExprKind::Builtin(b) => { - let t_hir = type_of_builtin(*b); + let t_hir = type_of_builtin(b); typecheck(&t_hir)?.eval_to_type(env)? } ExprKind::TextLit(interpolated) => { -- cgit v1.2.3 From ecc5242463308c16f38dbd5015b9f264f990b76a Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Tue, 3 Nov 2020 20:35:22 +0000 Subject: Remove NirInternal --- dhall/src/semantics/nze/lazy.rs | 11 ++++++++ dhall/src/semantics/nze/nir.rs | 62 ++++++++--------------------------------- 2 files changed, 23 insertions(+), 50 deletions(-) (limited to 'dhall/src/semantics') diff --git a/dhall/src/semantics/nze/lazy.rs b/dhall/src/semantics/nze/lazy.rs index 550f69a..d3b5c8d 100644 --- a/dhall/src/semantics/nze/lazy.rs +++ b/dhall/src/semantics/nze/lazy.rs @@ -63,6 +63,17 @@ where } } +/// This implementation evaluates before cloning, because we can't clone the contents of a `Cell`. +impl Clone for Lazy +where + Src: Eval, + Tgt: Clone, +{ + fn clone(&self) -> Self { + Self::new_completed(self.force().clone()) + } +} + impl Debug for Lazy where Tgt: Debug, diff --git a/dhall/src/semantics/nze/nir.rs b/dhall/src/semantics/nze/nir.rs index 95eeba1..7bda836 100644 --- a/dhall/src/semantics/nze/nir.rs +++ b/dhall/src/semantics/nze/nir.rs @@ -14,18 +14,13 @@ use crate::syntax::{ use crate::ToExprOptions; /// Stores a possibly unevaluated value. Gets (partially) normalized on-demand, sharing computation -/// automatically. Uses a Rc to share computation. +/// automatically. Uses a Rc to share computation. /// If you compare for equality two `Nir`s, then equality will be up to alpha-equivalence /// (renaming of bound variables) and beta-equivalence (normalization). It will recursively /// normalize as needed. -/// Stands for "Normalized intermediate representation" +/// Stands for "Normalized Intermediate Representation" #[derive(Clone)] -pub struct Nir(Rc); - -#[derive(Debug)] -struct NirInternal { - kind: lazy::Lazy, -} +pub struct Nir(Rc>); /// An unevaluated subexpression #[derive(Debug, Clone)] @@ -101,17 +96,17 @@ pub enum NirKind { impl Nir { /// Construct a Nir from a completely unnormalized expression. pub fn new_thunk(env: NzEnv, hir: Hir) -> Nir { - NirInternal::from_thunk(Thunk::new(env, hir)).into_nir() + Nir(Rc::new(lazy::Lazy::new(Thunk::new(env, hir)))) } /// Construct a Nir from a partially normalized expression that's not in WHNF. pub fn from_partial_expr(e: ExprKind) -> Nir { // TODO: env let env = NzEnv::new(); - NirInternal::from_thunk(Thunk::from_partial_expr(env, e)).into_nir() + Nir(Rc::new(lazy::Lazy::new(Thunk::from_partial_expr(env, e)))) } /// Make a Nir from a NirKind pub fn from_kind(v: NirKind) -> Nir { - NirInternal::from_whnf(v).into_nir() + Nir(Rc::new(lazy::Lazy::new_completed(v))) } pub fn from_const(c: Const) -> Self { Self::from_kind(NirKind::Const(c)) @@ -135,24 +130,19 @@ impl Nir { /// This is what you want if you want to pattern-match on the value. pub fn kind(&self) -> &NirKind { - self.0.kind() + &*self.0 } /// The contents of a `Nir` are immutable and shared. If however we happen to be the sole /// owners, we can mutate it directly. Otherwise, this clones the internal value first. pub fn kind_mut(&mut self) -> &mut NirKind { - if Rc::get_mut(&mut self.0).is_none() { - // Clone self - let kind = self.kind().clone(); - *self = Nir::from_kind(kind); - } - Rc::get_mut(&mut self.0).unwrap().kind_mut() + Rc::make_mut(&mut self.0).get_mut() } /// If we are the sole owner of this Nir, we can avoid a clone. pub fn into_kind(self) -> NirKind { match Rc::try_unwrap(self.0) { - Ok(int) => int.into_kind(), - Err(rc) => rc.kind().clone(), + Ok(lazy) => lazy.into_inner(), + Err(rc) => (**rc).clone(), } } @@ -291,32 +281,6 @@ impl Nir { } } -impl NirInternal { - fn from_whnf(k: NirKind) -> Self { - NirInternal { - kind: lazy::Lazy::new_completed(k), - } - } - fn from_thunk(th: Thunk) -> Self { - NirInternal { - kind: lazy::Lazy::new(th), - } - } - fn into_nir(self) -> Nir { - Nir(Rc::new(self)) - } - - fn kind(&self) -> &NirKind { - &self.kind - } - fn kind_mut(&mut self) -> &mut NirKind { - self.kind.get_mut() - } - fn into_kind(self) -> NirKind { - self.kind.into_inner() - } -} - impl NirKind { pub fn into_nir(self) -> Nir { Nir::from_kind(self) @@ -472,13 +436,11 @@ impl std::cmp::Eq for Closure {} impl std::fmt::Debug for Nir { fn fmt(&self, fmt: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - let vint: &NirInternal = &self.0; - let kind = vint.kind(); - if let NirKind::Const(c) = kind { + if let NirKind::Const(c) = self.kind() { return write!(fmt, "{:?}", c); } let mut x = fmt.debug_struct("Nir"); - x.field("kind", kind); + x.field("kind", self.kind()); x.finish() } } -- cgit v1.2.3