From 29016b78736dca857e4e7f7c4dc68ed5e30c28bb Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Mon, 19 Aug 2019 12:25:09 +0200 Subject: s/to_valuef/to_whnf/ and avoid cloning ValueFs when possible --- dhall/src/core/value.rs | 38 ++++----- dhall/src/core/valuef.rs | 7 +- dhall/src/error/mod.rs | 20 ++--- dhall/src/phase/mod.rs | 15 ++-- dhall/src/phase/normalize.rs | 94 ++++++++++---------- dhall/src/phase/typecheck.rs | 198 ++++++++++++++++++++----------------------- dhall/src/tests.rs | 2 +- 7 files changed, 182 insertions(+), 192 deletions(-) (limited to 'dhall/src') diff --git a/dhall/src/core/value.rs b/dhall/src/core/value.rs index ef43e3e..213a2bd 100644 --- a/dhall/src/core/value.rs +++ b/dhall/src/core/value.rs @@ -193,23 +193,17 @@ impl Value { Ref::map(self.as_internal(), ValueInternal::as_nf) } - /// WARNING: avoid normalizing any thunk while holding on to this ref - /// or you could run into BorrowMut panics - /// TODO: deprecated because of misleading name - pub(crate) fn as_valuef(&self) -> Ref { - self.as_whnf() - } - - /// WARNING: avoid normalizing any thunk while holding on to this ref - /// or you could run into BorrowMut panics + /// This is what you want if you want to pattern-match on the value. + /// WARNING: drop this ref before normalizing the same value or you will run into BorrowMut + /// panics. pub(crate) fn as_whnf(&self) -> Ref { self.do_normalize_whnf(); Ref::map(self.as_internal(), ValueInternal::as_whnf) } - /// TODO: deprecated because of misleading name - pub(crate) fn to_valuef(&self) -> ValueF { - self.as_valuef().clone() + /// TODO: cloning a valuef can often be avoided + pub(crate) fn to_whnf(&self) -> ValueF { + self.as_whnf().clone() } pub(crate) fn normalize_to_expr_maybe_alpha( @@ -275,14 +269,19 @@ impl TypedValue { TypedValue(Value::from_valuef_and_type(v, t)) } - pub(crate) fn to_valuef(&self) -> ValueF { - self.0.to_valuef() + /// WARNING: drop this ref before normalizing the same value or you will run into BorrowMut + /// panics. + pub(crate) fn as_whnf(&self) -> Ref { + self.0.as_whnf() + } + pub(crate) fn to_whnf(&self) -> ValueF { + self.0.to_whnf() } pub(crate) fn to_expr(&self) -> NormalizedSubExpr { - self.to_valuef().normalize_to_expr() + self.as_whnf().normalize_to_expr() } pub(crate) fn to_expr_alpha(&self) -> NormalizedSubExpr { - self.to_valuef().normalize_to_expr_maybe_alpha(true) + self.as_whnf().normalize_to_expr_maybe_alpha(true) } pub(crate) fn to_value(&self) -> Value { self.0.clone() @@ -294,8 +293,7 @@ impl TypedValue { Typed::from_typedvalue(self) } pub(crate) fn as_const(&self) -> Option { - // TODO: avoid clone - match &self.to_valuef() { + match &*self.as_whnf() { ValueF::Const(c) => Some(*c), _ => None, } @@ -357,14 +355,14 @@ impl Subst for TypedValue { impl std::cmp::PartialEq for Value { fn eq(&self, other: &Self) -> bool { - *self.as_valuef() == *other.as_valuef() + *self.as_whnf() == *other.as_whnf() } } impl std::cmp::Eq for Value {} impl std::cmp::PartialEq for TypedValue { fn eq(&self, other: &Self) -> bool { - self.to_valuef() == other.to_valuef() + &*self.as_whnf() == &*other.as_whnf() } } impl std::cmp::Eq for TypedValue {} diff --git a/dhall/src/core/valuef.rs b/dhall/src/core/valuef.rs index 9e6e8c8..0ba1d59 100644 --- a/dhall/src/core/valuef.rs +++ b/dhall/src/core/valuef.rs @@ -12,8 +12,9 @@ use crate::phase::{Normalized, Typed}; /// A semantic value. Subexpressions are Values, which are partially evaluated expressions that are /// normalized on-demand. -/// Equality is up to alpha-equivalence (renaming of bound variables) and beta-equivalence -/// (normalization). Equality will normalize as needed. +/// If you compare for equality two `ValueF`s in WHNF, then equality will be up to +/// alpha-equivalence (renaming of bound variables) and beta-equivalence (normalization). It will +/// recursively normalize as needed. #[derive(Debug, Clone, PartialEq, Eq)] pub enum ValueF { /// Closures @@ -348,7 +349,7 @@ impl Subst for ValueF { t.subst_shift(var, val), e.subst_shift(&var.under_binder(x), &val.under_binder(x)), ), - ValueF::Var(v) if v == var => val.to_valuef(), + ValueF::Var(v) if v == var => val.to_whnf(), ValueF::Var(v) => ValueF::Var(v.shift(-1, var).unwrap()), ValueF::Const(c) => ValueF::Const(*c), ValueF::BoolLit(b) => ValueF::BoolLit(*b), diff --git a/dhall/src/error/mod.rs b/dhall/src/error/mod.rs index 8f6ff51..34a89c0 100644 --- a/dhall/src/error/mod.rs +++ b/dhall/src/error/mod.rs @@ -4,7 +4,7 @@ use dhall_syntax::{BinOp, Import, Label, ParseError, V}; use crate::core::context::TypecheckContext; use crate::phase::resolve::ImportStack; -use crate::phase::{Normalized, NormalizedSubExpr, Type, Typed}; +use crate::phase::{NormalizedSubExpr, Type, Typed}; pub type Result = std::result::Result; @@ -48,24 +48,24 @@ pub struct TypeError { #[derive(Debug)] pub(crate) enum TypeMessage { UnboundVariable(V