From 697e93e0f56e3c063ce253983f703be88d468b47 Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Mon, 7 Dec 2020 11:56:00 +0000 Subject: Don't store internal structures of `dhall` in `serde_dhall` --- serde_dhall/src/options/de.rs | 11 +++-- serde_dhall/src/value.rs | 99 ++++++++++++++++++++++++++------------- serde_dhall/tests/simple_value.rs | 14 +++++- 3 files changed, 86 insertions(+), 38 deletions(-) diff --git a/serde_dhall/src/options/de.rs b/serde_dhall/src/options/de.rs index 22fa7ba..39ab5ca 100644 --- a/serde_dhall/src/options/de.rs +++ b/serde_dhall/src/options/de.rs @@ -229,7 +229,7 @@ impl<'a, A> Deserializer<'a, A> { // self // } - fn _parse(&self) -> dhall::error::Result + fn _parse(&self) -> dhall::error::Result> where A: TypeAnnot, T: HasAnnot, @@ -246,9 +246,12 @@ impl<'a, A> Deserializer<'a, A> { }; let typed = match &T::get_annot(self.annot) { None => resolved.typecheck()?, - Some(ty) => resolved.typecheck_with(ty.to_value().as_hir())?, + Some(ty) => resolved.typecheck_with(&ty.to_hir())?, }; - Ok(Value::from_nir(typed.normalize().as_nir())) + Ok(Value::from_nir_and_ty( + typed.normalize().as_nir(), + typed.ty().as_nir(), + )) } /// Parses the chosen dhall value with the options provided. @@ -274,7 +277,7 @@ impl<'a, A> Deserializer<'a, A> { let val = self ._parse::() .map_err(ErrorKind::Dhall) - .map_err(Error)?; + .map_err(Error)??; T::from_dhall(&val) } } diff --git a/serde_dhall/src/value.rs b/serde_dhall/src/value.rs index b8ad199..4b22158 100644 --- a/serde_dhall/src/value.rs +++ b/serde_dhall/src/value.rs @@ -8,16 +8,18 @@ use dhall::syntax::{Expr, ExprKind, Span}; use crate::{Error, ErrorKind, FromDhall, Result, ToDhall}; +#[derive(Debug, Clone)] +enum ValueKind { + /// Invariant: the value must be printable with the given type. + Val(SimpleValue, Option), + Ty(SimpleType), +} + #[doc(hidden)] /// An arbitrary Dhall value. -#[derive(Debug, Clone)] +#[derive(Debug, Clone, PartialEq, Eq)] pub struct Value { - /// Invariant: in normal form - hir: Hir, - /// Cached conversions because they are annoying to construct from Hir. - /// At most one of them will be `Some`. - as_simple_val: Option, - as_simple_ty: Option, + kind: ValueKind, } /// A value of the kind that can be decoded by `serde_dhall`, e.g. `{ x = True, y = [1, 2, 3] }`. @@ -204,31 +206,48 @@ pub enum SimpleType { } impl Value { - pub(crate) fn from_nir(x: &Nir) -> Self { - Value { - hir: x.to_hir_noenv(), - as_simple_val: SimpleValue::from_nir(x), - as_simple_ty: SimpleType::from_nir(x), - } - } - - pub(crate) fn as_hir(&self) -> &Hir { - &self.hir + pub(crate) fn from_nir_and_ty(x: &Nir, ty: &Nir) -> Result { + Ok(if let Some(val) = SimpleValue::from_nir(x) { + // The type must be simple if the value is simple. + let ty = SimpleType::from_nir(ty).unwrap(); + Value { + kind: ValueKind::Val(val, Some(ty)), + } + } else if let Some(ty) = SimpleType::from_nir(x) { + Value { + kind: ValueKind::Ty(ty), + } + } else { + let expr = x.to_hir_noenv().to_expr(Default::default()); + return Err(Error(ErrorKind::Deserialize(format!( + "this is neither a simple type nor a simple value: {}", + expr + )))); + }) } /// Converts a Value into a SimpleValue. pub(crate) fn to_simple_value(&self) -> Option { - self.as_simple_val.clone() + match &self.kind { + ValueKind::Val(val, _) => Some(val.clone()), + _ => None, + } } /// Converts a Value into a SimpleType. pub(crate) fn to_simple_type(&self) -> Option { - self.as_simple_ty.clone() + match &self.kind { + ValueKind::Ty(ty) => Some(ty.clone()), + _ => None, + } } /// Converts a value back to the corresponding AST expression. pub(crate) fn to_expr(&self) -> Expr { - self.hir.to_expr(Default::default()) + match &self.kind { + ValueKind::Val(val, ty) => val.to_expr(ty.as_ref()).unwrap(), + ValueKind::Ty(ty) => ty.to_expr(), + } } } @@ -411,12 +430,17 @@ impl SimpleValue { Ok(hir(kind)) } pub(crate) fn into_value(self, ty: Option<&SimpleType>) -> Result { + // Check that the value is printable with the given type. + self.to_hir(ty)?; Ok(Value { - hir: self.to_hir(ty)?, - as_simple_val: Some(self), - as_simple_ty: None, + kind: ValueKind::Val(self, ty.cloned()), }) } + + /// Converts back to the corresponding AST expression. + pub(crate) fn to_expr(&self, ty: Option<&SimpleType>) -> Result { + Ok(self.to_hir(ty)?.to_expr(Default::default())) + } } impl SimpleType { @@ -457,13 +481,6 @@ impl SimpleType { }) } - pub(crate) fn to_value(&self) -> Value { - Value { - hir: self.to_hir(), - as_simple_val: None, - as_simple_ty: Some(self.clone()), - } - } pub(crate) fn to_hir(&self) -> Hir { let hir = |k| Hir::new(HirKind::Expr(k), Span::Artificial); hir(match self { @@ -526,10 +543,15 @@ impl ToDhall for Value { } } -impl Eq for Value {} -impl PartialEq for Value { +impl Eq for ValueKind {} +impl PartialEq for ValueKind { fn eq(&self, other: &Self) -> bool { - self.hir == other.hir + use ValueKind::*; + match (self, other) { + (Val(a, _), Val(b, _)) => a == b, + (Ty(a), Ty(b)) => a == b, + _ => false, + } } } impl std::fmt::Display for Value { @@ -556,3 +578,14 @@ fn test_display_simpletype() { let ty = List(Box::new(Optional(Box::new(Natural)))); assert_eq!(ty.to_string(), "List (Optional Natural)".to_string()) } + +#[test] +fn test_display_value() { + use SimpleType::*; + let ty = List(Box::new(Optional(Box::new(Natural)))); + let val = SimpleValue::List(vec![]); + let val = Value { + kind: ValueKind::Val(val, Some(ty)), + }; + assert_eq!(val.to_string(), "[] : List (Optional Natural)".to_string()) +} diff --git a/serde_dhall/tests/simple_value.rs b/serde_dhall/tests/simple_value.rs index 3bd9d64..3d40da1 100644 --- a/serde_dhall/tests/simple_value.rs +++ b/serde_dhall/tests/simple_value.rs @@ -1,7 +1,7 @@ mod simple_value { use serde::{Deserialize, Serialize}; use serde_dhall::{ - from_str, serialize, FromDhall, NumKind, SimpleValue, ToDhall, + from_str, serialize, FromDhall, NumKind, SimpleValue, ToDhall, Value, }; fn assert_de(s: &str, x: T) @@ -54,5 +54,17 @@ mod simple_value { foo: bool_true.clone(), }, ); + + // Neither a simple value or a simple type. + let not_simple = "Type → Type"; + assert_eq!( + from_str(not_simple) + .parse::() + .map_err(|e| e.to_string()), + Err(format!( + "this is neither a simple type nor a simple value: {}", + not_simple + )) + ); } } -- cgit v1.2.3 From 3a623acaf70c934ee9dbd74dfadcaa2c612160c5 Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Sun, 6 Dec 2020 20:10:51 +0000 Subject: Make global store of imports and import results --- Cargo.lock | 16 +++++ dhall/Cargo.toml | 1 + dhall/src/ctxt.rs | 109 +++++++++++++++++++++++++++++++ dhall/src/lib.rs | 3 + dhall/src/semantics/resolve/env.rs | 37 +++++------ dhall/src/semantics/resolve/resolve.rs | 113 ++++++++++++++++++++------------- 6 files changed, 217 insertions(+), 62 deletions(-) create mode 100644 dhall/src/ctxt.rs diff --git a/Cargo.lock b/Cargo.lock index fdbb3ed..b89c7cd 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -300,6 +300,7 @@ dependencies = [ "annotate-snippets", "anyhow", "colored-diff", + "elsa", "fs_extra", "hex", "itertools 0.9.0", @@ -373,6 +374,15 @@ version = "1.6.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "e78d4f1cc4ae33bbfc157ed5d5a5ef3bc29227303d595861deb238fcec4e9457" +[[package]] +name = "elsa" +version = "1.3.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "0daf12e2857c9485cb2386b18a9ecd5a17c7cee2fd10afb87d436b10ced678e7" +dependencies = [ + "stable_deref_trait", +] + [[package]] name = "encoding_rs" version = "0.8.24" @@ -1456,6 +1466,12 @@ dependencies = [ "winapi 0.3.9", ] +[[package]] +name = "stable_deref_trait" +version = "1.2.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a8f112729512f8e442d81f95a8a7ddf2b7c6b8a1a6f509a95864142b30cab2d3" + [[package]] name = "static_assertions" version = "1.1.0" diff --git a/dhall/Cargo.toml b/dhall/Cargo.toml index 1b4f9a3..1e9cfbc 100644 --- a/dhall/Cargo.toml +++ b/dhall/Cargo.toml @@ -21,6 +21,7 @@ path = "tests/spec.rs" [dependencies] annotate-snippets = "0.9.0" +elsa = "1.3.2" hex = "0.4.2" itertools = "0.9.0" lazy_static = "1.4.0" diff --git a/dhall/src/ctxt.rs b/dhall/src/ctxt.rs new file mode 100644 index 0000000..1d97232 --- /dev/null +++ b/dhall/src/ctxt.rs @@ -0,0 +1,109 @@ +use elsa::vec::FrozenVec; +use once_cell::sync::OnceCell; + +use crate::semantics::{Import, ImportLocation}; +use crate::syntax::Span; +use crate::Typed; + +#[derive(Debug, Copy, Clone, PartialEq, Eq, Hash)] +pub struct ImportId(usize); +#[derive(Debug, Copy, Clone, PartialEq, Eq, Hash)] +pub struct ImportResultId(usize); + +struct StoredImport { + base_location: ImportLocation, + import: Import, + span: Span, + result: OnceCell, +} + +#[derive(Default)] +struct CtxtS { + imports: FrozenVec>, + import_results: FrozenVec>, +} + +/// Context for the dhall compiler. Stores various global maps. +#[derive(Copy, Clone)] +pub struct Ctxt<'cx>(&'cx CtxtS); + +impl Ctxt<'_> { + pub fn with_new(f: impl for<'cx> FnOnce(Ctxt<'cx>) -> T) -> T { + let cx = CtxtS::default(); + let cx = Ctxt(&cx); + f(cx) + } +} +impl<'cx> Ctxt<'cx> { + fn get_stored_import(self, import: ImportId) -> &'cx StoredImport { + self.0.imports.get(import.0).unwrap() + } + pub fn get_import_result(self, id: ImportResultId) -> &'cx Typed { + &self.0.import_results.get(id.0).unwrap() + } + + /// Store an import and the location relative to which it must be resolved. + pub fn push_import( + self, + base_location: ImportLocation, + import: Import, + span: Span, + ) -> ImportId { + let stored = StoredImport { + base_location, + import, + span, + result: OnceCell::new(), + }; + let id = self.0.imports.len(); + self.0.imports.push(Box::new(stored)); + ImportId(id) + } + /// Store the result of fetching an import. + pub fn push_import_result(self, res: Typed) -> ImportResultId { + let id = self.0.import_results.len(); + self.0.import_results.push(Box::new(res)); + ImportResultId(id) + } + + pub fn get_import(self, import: ImportId) -> &'cx Import { + &self.get_stored_import(import).import + } + pub fn get_import_base_location( + self, + import: ImportId, + ) -> &'cx ImportLocation { + &self.get_stored_import(import).base_location + } + pub fn get_import_span(self, import: ImportId) -> Span { + self.get_stored_import(import).span.clone() + } + /// Get the result of fetching this import. Returns `None` if the result has not yet been + /// fetched. + pub fn get_result_of_import(self, import: ImportId) -> Option<&'cx Typed> { + let res = self.get_resultid_of_import(import)?; + Some(self.get_import_result(res)) + } + /// Get the id of the result of fetching this import. Returns `None` if the result has not yet + /// been fetched. + pub fn get_resultid_of_import( + self, + import: ImportId, + ) -> Option { + self.get_stored_import(import).result.get().copied() + } + /// Store the result of fetching this import. + pub fn set_result_of_import( + self, + import: ImportId, + res: Typed, + ) -> ImportResultId { + let res = self.push_import_result(res); + self.set_resultid_of_import(import, res); + res + } + /// Store the result of fetching this import. + pub fn set_resultid_of_import(self, import: ImportId, res: ImportResultId) { + let _ = self.get_stored_import(import).result.set(res); + } +} diff --git a/dhall/src/lib.rs b/dhall/src/lib.rs index 6747eff..d079e92 100644 --- a/dhall/src/lib.rs +++ b/dhall/src/lib.rs @@ -9,6 +9,7 @@ )] pub mod builtins; +pub mod ctxt; pub mod error; pub mod operations; pub mod semantics; @@ -26,6 +27,8 @@ use crate::semantics::resolve::ImportLocation; use crate::semantics::{typecheck, typecheck_with, Hir, Nir, Tir, Type}; use crate::syntax::Expr; +pub use ctxt::{Ctxt, ImportId, ImportResultId}; + #[derive(Debug, Clone)] pub struct Parsed(Expr, ImportLocation); diff --git a/dhall/src/semantics/resolve/env.rs b/dhall/src/semantics/resolve/env.rs index 29dd16b..82f21e8 100644 --- a/dhall/src/semantics/resolve/env.rs +++ b/dhall/src/semantics/resolve/env.rs @@ -3,7 +3,7 @@ use std::collections::HashMap; use crate::error::{Error, ImportError}; use crate::semantics::{AlphaVar, Cache, ImportLocation, VarEnv}; use crate::syntax::{Hash, Label, V}; -use crate::Typed; +use crate::{Ctxt, ImportResultId, Typed}; /// Environment for resolving names. #[derive(Debug, Clone, Default)] @@ -11,14 +11,13 @@ pub struct NameEnv { names: Vec, { - let parsed = match &self.source { - Source::Str(s) => Parsed::parse_str(s)?, - Source::File(p) => Parsed::parse_file(p.as_ref())?, - Source::BinaryFile(p) => Parsed::parse_binary_file(p.as_ref())?, - }; - let resolved = if self.allow_imports { - parsed.resolve()? - } else { - parsed.skip_resolve()? - }; - let typed = match &T::get_annot(self.annot) { - None => resolved.typecheck()?, - Some(ty) => resolved.typecheck_with(&ty.to_hir())?, - }; - Ok(Value::from_nir_and_ty( - typed.normalize().as_nir(), - typed.ty().as_nir(), - )) + Ctxt::with_new(|cx| { + let parsed = match &self.source { + Source::Str(s) => Parsed::parse_str(s)?, + Source::File(p) => Parsed::parse_file(p.as_ref())?, + Source::BinaryFile(p) => Parsed::parse_binary_file(p.as_ref())?, + }; + let resolved = if self.allow_imports { + parsed.resolve(cx)? + } else { + parsed.skip_resolve()? + }; + let typed = match &T::get_annot(self.annot) { + None => resolved.typecheck(cx)?, + Some(ty) => resolved.typecheck_with(cx, &ty.to_hir())?, + }; + Ok(Value::from_nir_and_ty( + typed.normalize(cx).as_nir(), + typed.ty().as_nir(), + )) + }) } /// Parses the chosen dhall value with the options provided. diff --git a/serde_dhall/src/value.rs b/serde_dhall/src/value.rs index 4b22158..e5f5acd 100644 --- a/serde_dhall/src/value.rs +++ b/serde_dhall/src/value.rs @@ -335,7 +335,7 @@ impl SimpleValue { // Converts this to `Hir`, using the optional type annotation. Without the type, things like // empty lists and unions will fail to convert. - fn to_hir(&self, ty: Option<&SimpleType>) -> Result { + fn to_hir<'cx>(&self, ty: Option<&SimpleType>) -> Result> { use SimpleType as T; use SimpleValue as V; let hir = |k| Hir::new(HirKind::Expr(k), Span::Artificial); @@ -481,7 +481,7 @@ impl SimpleType { }) } - pub(crate) fn to_hir(&self) -> Hir { + pub(crate) fn to_hir<'cx>(&self) -> Hir<'cx> { let hir = |k| Hir::new(HirKind::Expr(k), Span::Artificial); hir(match self { SimpleType::Bool => ExprKind::Builtin(Builtin::Bool), -- cgit v1.2.3 From 3c522217b7445c6df9e170d830f485086ad7e062 Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Mon, 7 Dec 2020 13:47:21 +0000 Subject: Tag cx ids with the cx lifetime To make sure we don't let ids escape and we don't mix scopes. --- dhall/src/ctxt.rs | 42 ++++++++++++++++++++++------------ dhall/src/semantics/resolve/env.rs | 6 ++--- dhall/src/semantics/resolve/resolve.rs | 2 +- 3 files changed, 31 insertions(+), 19 deletions(-) diff --git a/dhall/src/ctxt.rs b/dhall/src/ctxt.rs index a171ebe..8bdf99d 100644 --- a/dhall/src/ctxt.rs +++ b/dhall/src/ctxt.rs @@ -1,14 +1,16 @@ use elsa::vec::FrozenVec; use once_cell::sync::OnceCell; +use std::marker::PhantomData; +use std::ops::{Deref, Index}; use crate::semantics::{Import, ImportLocation}; use crate::syntax::Span; use crate::Typed; #[derive(Debug, Copy, Clone, PartialEq, Eq, Hash)] -pub struct ImportId(usize); +pub struct ImportId<'cx>(usize, PhantomData<&'cx ()>); #[derive(Debug, Copy, Clone, PartialEq, Eq, Hash)] -pub struct ImportResultId(usize); +pub struct ImportResultId<'cx>(usize, PhantomData<&'cx ()>); /// What's stored for each `ImportId`. Allows getting and setting a result for this import. pub struct StoredImport<'cx> { @@ -16,7 +18,7 @@ pub struct StoredImport<'cx> { pub base_location: ImportLocation, pub import: Import, pub span: Span, - result: OnceCell, + result: OnceCell>, } /// Implementation detail. Made public for the `Index` instances. @@ -45,7 +47,7 @@ impl<'cx> Ctxt<'cx> { base_location: ImportLocation, import: Import, span: Span, - ) -> ImportId { + ) -> ImportId<'cx> { let stored = StoredImport { cx: self, base_location, @@ -55,24 +57,24 @@ impl<'cx> Ctxt<'cx> { }; let id = self.0.imports.len(); self.0.imports.push(Box::new(stored)); - ImportId(id) + ImportId(id, PhantomData) } /// Store the result of fetching an import. - pub fn push_import_result(self, res: Typed<'cx>) -> ImportResultId { + pub fn push_import_result(self, res: Typed<'cx>) -> ImportResultId<'cx> { let id = self.0.import_results.len(); self.0.import_results.push(Box::new(res)); - ImportResultId(id) + ImportResultId(id, PhantomData) } } impl<'cx> StoredImport<'cx> { /// Get the id of the result of fetching this import. Returns `None` if the result has not yet /// been fetched. - pub fn get_resultid(&self) -> Option { + pub fn get_resultid(&self) -> Option> { self.result.get().copied() } /// Store the result of fetching this import. - pub fn set_resultid(&self, res: ImportResultId) { + pub fn set_resultid(&self, res: ImportResultId<'cx>) { let _ = self.result.set(res); } /// Get the result of fetching this import. Returns `None` if the result has not yet been @@ -82,32 +84,42 @@ impl<'cx> StoredImport<'cx> { Some(&self.cx[res]) } /// Store the result of fetching this import. - pub fn set_result(&self, res: Typed<'cx>) -> ImportResultId { + pub fn set_result(&self, res: Typed<'cx>) -> ImportResultId<'cx> { let res = self.cx.push_import_result(res); self.set_resultid(res); res } } -impl<'cx> std::ops::Deref for Ctxt<'cx> { +impl<'cx> Deref for Ctxt<'cx> { type Target = &'cx CtxtS<'cx>; fn deref(&self) -> &&'cx CtxtS<'cx> { &self.0 } } -impl<'cx> std::ops::Index for CtxtS<'cx> { +impl<'cx> Index> for CtxtS<'cx> { type Output = StoredImport<'cx>; - fn index(&self, id: ImportId) -> &StoredImport<'cx> { + fn index(&self, id: ImportId<'cx>) -> &StoredImport<'cx> { &self.imports[id.0] } } -impl<'cx> std::ops::Index for CtxtS<'cx> { +impl<'cx> Index> for CtxtS<'cx> { type Output = Typed<'cx>; - fn index(&self, id: ImportResultId) -> &Typed<'cx> { + fn index(&self, id: ImportResultId<'cx>) -> &Typed<'cx> { &self.import_results[id.0] } } +impl<'a, 'cx, T> Index<&'a T> for CtxtS<'cx> +where + Self: Index, + T: Copy, +{ + type Output = >::Output; + fn index(&self, id: &'a T) -> &Self::Output { + &self[*id] + } +} /// Empty impl, because `FrozenVec` does not implement `Debug` and I can't be bothered to do it /// myself. diff --git a/dhall/src/semantics/resolve/env.rs b/dhall/src/semantics/resolve/env.rs index 2b998f8..ba6205f 100644 --- a/dhall/src/semantics/resolve/env.rs +++ b/dhall/src/semantics/resolve/env.rs @@ -17,7 +17,7 @@ pub type CyclesStack = Vec; pub struct ImportEnv<'cx> { cx: Ctxt<'cx>, disk_cache: Option, // `None` if it failed to initialize - mem_cache: HashMap, + mem_cache: HashMap>, stack: CyclesStack, } @@ -82,7 +82,7 @@ impl<'cx> ImportEnv<'cx> { pub fn get_from_mem_cache( &self, location: &ImportLocation, - ) -> Option { + ) -> Option> { Some(*self.mem_cache.get(location)?) } @@ -98,7 +98,7 @@ impl<'cx> ImportEnv<'cx> { pub fn write_to_mem_cache( &mut self, location: ImportLocation, - result: ImportResultId, + result: ImportResultId<'cx>, ) { self.mem_cache.insert(location, result); } diff --git a/dhall/src/semantics/resolve/resolve.rs b/dhall/src/semantics/resolve/resolve.rs index 488c516..9021155 100644 --- a/dhall/src/semantics/resolve/resolve.rs +++ b/dhall/src/semantics/resolve/resolve.rs @@ -398,7 +398,7 @@ fn traverse_resolve_expr<'cx>( /// Fetch the import and store the result in the global context. fn fetch_import<'cx>( env: &mut ImportEnv<'cx>, - import_id: ImportId, + import_id: ImportId<'cx>, ) -> Result<(), Error> { let cx = env.cx(); let import = &cx[import_id].import; -- cgit v1.2.3 From 8c5b3ff15f2125e9d731fc199e194e1993c36b37 Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Mon, 7 Dec 2020 14:15:43 +0000 Subject: Thread cx everywhere else imports are read --- dhall/src/ctxt.rs | 6 +++++ dhall/src/lib.rs | 42 ++++++++-------------------------- dhall/src/semantics/nze/nir.rs | 4 ++-- dhall/src/semantics/resolve/cache.rs | 11 ++++++--- dhall/src/semantics/resolve/env.rs | 18 +++++++++++---- dhall/src/semantics/resolve/hir.rs | 20 ++++++++-------- dhall/src/semantics/resolve/resolve.rs | 36 +++++++++++++++-------------- dhall/tests/spec.rs | 27 ++++++++++------------ serde_dhall/src/options/de.rs | 1 + serde_dhall/src/value.rs | 17 ++++++++++---- 10 files changed, 93 insertions(+), 89 deletions(-) diff --git a/dhall/src/ctxt.rs b/dhall/src/ctxt.rs index 8bdf99d..3809bc9 100644 --- a/dhall/src/ctxt.rs +++ b/dhall/src/ctxt.rs @@ -83,6 +83,12 @@ impl<'cx> StoredImport<'cx> { let res = self.get_resultid()?; Some(&self.cx[res]) } + /// Get the result of fetching this import. Panicx if the result has not yet been + /// fetched. + pub fn unwrap_result(&self) -> &'cx Typed<'cx> { + self.get_result() + .expect("imports should all have been resolved at this stage") + } /// Store the result of fetching this import. pub fn set_result(&self, res: Typed<'cx>) -> ImportResultId<'cx> { let res = self.cx.push_import_result(res); diff --git a/dhall/src/lib.rs b/dhall/src/lib.rs index ec2b813..7f77334 100644 --- a/dhall/src/lib.rs +++ b/dhall/src/lib.rs @@ -5,8 +5,7 @@ clippy::needless_lifetimes, clippy::new_ret_no_self, clippy::new_without_default, - clippy::useless_format, - unreachable_code + clippy::useless_format )] pub mod builtins; @@ -17,7 +16,6 @@ pub mod semantics; pub mod syntax; pub mod utils; -use std::fmt::Display; use std::path::Path; use url::Url; @@ -102,8 +100,8 @@ impl<'cx> Resolved<'cx> { Ok(Typed::from_tir(typecheck_with(cx, &self.0, ty)?)) } /// Converts a value back to the corresponding AST expression. - pub fn to_expr(&self) -> Expr { - self.0.to_expr_noopts() + pub fn to_expr(&self, cx: Ctxt<'cx>) -> Expr { + self.0.to_expr_noopts(cx) } } @@ -120,8 +118,8 @@ impl<'cx> Typed<'cx> { } /// Converts a value back to the corresponding AST expression. - fn to_expr(&self) -> Expr { - self.hir.to_expr(ToExprOptions { alpha: false }) + fn to_expr(&self, cx: Ctxt<'cx>) -> Expr { + self.hir.to_expr(cx, ToExprOptions { alpha: false }) } pub fn ty(&self) -> &Type<'cx> { @@ -134,8 +132,8 @@ impl<'cx> Typed<'cx> { impl<'cx> Normalized<'cx> { /// Converts a value back to the corresponding AST expression. - pub fn to_expr(&self) -> Expr { - self.0.to_expr(ToExprOptions::default()) + pub fn to_expr(&self, cx: Ctxt<'cx>) -> Expr { + self.0.to_expr(cx, ToExprOptions::default()) } /// Converts a value back to the corresponding Hir expression. pub fn to_hir(&self) -> Hir<'cx> { @@ -145,8 +143,8 @@ impl<'cx> Normalized<'cx> { &self.0 } /// Converts a value back to the corresponding AST expression, alpha-normalizing in the process. - pub fn to_expr_alpha(&self) -> Expr { - self.0.to_expr(ToExprOptions { alpha: true }) + pub fn to_expr_alpha(&self, cx: Ctxt<'cx>) -> Expr { + self.0.to_expr(cx, ToExprOptions { alpha: true }) } } @@ -178,23 +176,6 @@ impl From for Expr { other.to_expr() } } -impl<'cx> From> for Expr { - fn from(other: Normalized<'cx>) -> Self { - other.to_expr() - } -} - -impl<'cx> Display for Resolved<'cx> { - fn fmt(&self, f: &mut std::fmt::Formatter) -> Result<(), std::fmt::Error> { - self.to_expr().fmt(f) - } -} - -impl<'cx> Display for Typed<'cx> { - fn fmt(&self, f: &mut std::fmt::Formatter) -> Result<(), std::fmt::Error> { - self.to_expr().fmt(f) - } -} impl<'cx> Eq for Normalized<'cx> {} impl<'cx> PartialEq for Normalized<'cx> { @@ -202,8 +183,3 @@ impl<'cx> PartialEq for Normalized<'cx> { self.0 == other.0 } } -impl<'cx> Display for Normalized<'cx> { - fn fmt(&self, f: &mut std::fmt::Formatter) -> Result<(), std::fmt::Error> { - self.to_expr().fmt(f) - } -} diff --git a/dhall/src/semantics/nze/nir.rs b/dhall/src/semantics/nze/nir.rs index 538b8ea..8cf06c5 100644 --- a/dhall/src/semantics/nze/nir.rs +++ b/dhall/src/semantics/nze/nir.rs @@ -148,8 +148,8 @@ impl<'cx> Nir<'cx> { Type::new(self.clone(), u.into()) } /// Converts a value back to the corresponding AST expression. - pub fn to_expr(&self, opts: ToExprOptions) -> Expr { - self.to_hir_noenv().to_expr(opts) + pub fn to_expr(&self, cx: Ctxt<'cx>, opts: ToExprOptions) -> Expr { + self.to_hir_noenv().to_expr(cx, opts) } pub fn to_expr_tyenv(&self, tyenv: &TyEnv<'cx>) -> Expr { self.to_hir(tyenv.as_varenv()).to_expr_tyenv(tyenv) diff --git a/dhall/src/semantics/resolve/cache.rs b/dhall/src/semantics/resolve/cache.rs index 59463dd..9a5e835 100644 --- a/dhall/src/semantics/resolve/cache.rs +++ b/dhall/src/semantics/resolve/cache.rs @@ -68,11 +68,12 @@ impl Cache { pub fn insert<'cx>( &self, + cx: Ctxt<'cx>, hash: &Hash, expr: &Typed<'cx>, ) -> Result<(), Error> { let path = self.entry_path(hash); - write_cache_file(&path, expr) + write_cache_file(cx, &path, expr) } } @@ -97,8 +98,12 @@ fn read_cache_file<'cx>( } /// Write a file to the cache. -fn write_cache_file(path: &Path, expr: &Typed) -> Result<(), Error> { - let data = binary::encode(&expr.to_expr())?; +fn write_cache_file<'cx>( + cx: Ctxt<'cx>, + path: &Path, + expr: &Typed<'cx>, +) -> Result<(), Error> { + let data = binary::encode(&expr.to_expr(cx))?; File::create(path)?.write_all(data.as_slice())?; Ok(()) } diff --git a/dhall/src/semantics/resolve/env.rs b/dhall/src/semantics/resolve/env.rs index ba6205f..0d1952b 100644 --- a/dhall/src/semantics/resolve/env.rs +++ b/dhall/src/semantics/resolve/env.rs @@ -1,9 +1,9 @@ use std::collections::HashMap; use crate::error::{Error, ImportError}; -use crate::semantics::{AlphaVar, Cache, ImportLocation, VarEnv}; +use crate::semantics::{check_hash, AlphaVar, Cache, ImportLocation, VarEnv}; use crate::syntax::{Hash, Label, V}; -use crate::{Ctxt, ImportResultId, Typed}; +use crate::{Ctxt, ImportId, ImportResultId, Typed}; /// Environment for resolving names. #[derive(Debug, Clone, Default)] @@ -95,6 +95,11 @@ impl<'cx> ImportEnv<'cx> { Some(expr) } + /// Invariant: the import must have been fetched. + pub fn check_hash(&self, import: ImportId<'cx>) -> Result<(), Error> { + check_hash(self.cx(), import) + } + pub fn write_to_mem_cache( &mut self, location: ImportLocation, @@ -103,10 +108,13 @@ impl<'cx> ImportEnv<'cx> { self.mem_cache.insert(location, result); } - pub fn write_to_disk_cache(&self, hash: &Option, expr: &Typed<'cx>) { + /// Invariant: the import must have been fetched. + pub fn write_to_disk_cache(&self, import: ImportId<'cx>) { + let import = &self.cx()[import]; if let Some(disk_cache) = self.disk_cache.as_ref() { - if let Some(hash) = hash { - let _ = disk_cache.insert(hash, &expr); + if let Some(hash) = &import.import.hash { + let expr = import.unwrap_result(); + let _ = disk_cache.insert(self.cx(), hash, expr); } } } diff --git a/dhall/src/semantics/resolve/hir.rs b/dhall/src/semantics/resolve/hir.rs index c67ab75..5575888 100644 --- a/dhall/src/semantics/resolve/hir.rs +++ b/dhall/src/semantics/resolve/hir.rs @@ -51,22 +51,23 @@ impl<'cx> Hir<'cx> { } /// Converts a closed Hir expr back to the corresponding AST expression. - pub fn to_expr(&self, opts: ToExprOptions) -> Expr { - hir_to_expr(self, opts, &mut NameEnv::new()) + pub fn to_expr(&self, cx: Ctxt<'cx>, opts: ToExprOptions) -> Expr { + hir_to_expr(cx, self, opts, &mut NameEnv::new()) } /// Converts a closed Hir expr back to the corresponding AST expression. - pub fn to_expr_noopts(&self) -> Expr { + pub fn to_expr_noopts(&self, cx: Ctxt<'cx>) -> Expr { let opts = ToExprOptions { alpha: false }; - self.to_expr(opts) + self.to_expr(cx, opts) } - pub fn to_expr_alpha(&self) -> Expr { + pub fn to_expr_alpha(&self, cx: Ctxt<'cx>) -> Expr { let opts = ToExprOptions { alpha: true }; - self.to_expr(opts) + self.to_expr(cx, opts) } pub fn to_expr_tyenv(&self, env: &TyEnv<'cx>) -> Expr { let opts = ToExprOptions { alpha: false }; + let cx = env.cx(); let mut env = env.as_nameenv().clone(); - hir_to_expr(self, opts, &mut env) + hir_to_expr(cx, self, opts, &mut env) } /// Typecheck the Hir. @@ -95,6 +96,7 @@ impl<'cx> Hir<'cx> { } fn hir_to_expr<'cx>( + cx: Ctxt<'cx>, hir: &Hir<'cx>, opts: ToExprOptions, env: &mut NameEnv, @@ -103,14 +105,14 @@ fn hir_to_expr<'cx>( HirKind::Var(v) if opts.alpha => ExprKind::Var(V("_".into(), v.idx())), HirKind::Var(v) => ExprKind::Var(env.label_var(*v)), HirKind::Import(hir, _) => { - return hir_to_expr(hir, opts, &mut NameEnv::new()) + return hir_to_expr(cx, hir, opts, &mut NameEnv::new()); } HirKind::Expr(e) => { let e = e.map_ref_maybe_binder(|l, hir| { if let Some(l) = l { env.insert_mut(l); } - let e = hir_to_expr(hir, opts, env); + let e = hir_to_expr(cx, hir, opts, env); if l.is_some() { env.remove_mut(); } diff --git a/dhall/src/semantics/resolve/resolve.rs b/dhall/src/semantics/resolve/resolve.rs index 9021155..db162ef 100644 --- a/dhall/src/semantics/resolve/resolve.rs +++ b/dhall/src/semantics/resolve/resolve.rs @@ -287,19 +287,21 @@ fn make_aslocation_uniontype() -> Expr { mkexpr(ExprKind::UnionType(union)) } -fn check_hash<'cx>( - import: &Import, - typed: &Typed<'cx>, - span: Span, +/// Invariant: the import must have been fetched. +pub fn check_hash<'cx>( + cx: Ctxt<'cx>, + import: ImportId<'cx>, ) -> Result<(), Error> { + let import = &cx[import]; if let (ImportMode::Code, Some(Hash::SHA256(hash))) = - (import.mode, &import.hash) + (import.import.mode, &import.import.hash) { - let actual_hash = typed.hir.to_expr_alpha().sha256_hash()?; + let expr = import.unwrap_result().hir.to_expr_alpha(cx); + let actual_hash = expr.sha256_hash()?; if hash[..] != actual_hash[..] { mkerr( ErrorBuilder::new("hash mismatch") - .span_err(span, "hash mismatch") + .span_err(import.span.clone(), "hash mismatch") .note(format!("Expected sha256:{}", hex::encode(hash))) .note(format!( "Found sha256:{}", @@ -411,15 +413,14 @@ fn fetch_import<'cx>( cx[import_id].set_resultid(res_id); // The same location may be used with different or no hashes. Thus we need to check // the hashes every time. - let typed = &cx[res_id]; - check_hash(import, typed, span)?; - env.write_to_disk_cache(&import.hash, typed); + env.check_hash(import_id)?; + env.write_to_disk_cache(import_id); return Ok(()); } if let Some(typed) = env.get_from_disk_cache(&import.hash) { - // No need to check the hash, it was checked before reading the file. We also don't - // write to the in-memory cache, because the location might be completely unrelated - // to the cached file (e.g. `missing sha256:...` is valid). + // No need to check the hash, it was checked before reading the file. + // We also don't write to the in-memory cache, because the location might be completely + // unrelated to the cached file (e.g. `missing sha256:...` is valid). // This actually means that importing many times a same hashed import will take // longer than importing many times a same non-hashed import. cx[import_id].set_result(typed); @@ -441,9 +442,10 @@ fn fetch_import<'cx>( }; // Add the resolved import to the caches - check_hash(import, &typed, span)?; - env.write_to_disk_cache(&import.hash, &typed); - let res_id = cx[import_id].set_result(typed); + let res_id = cx[import_id].set_result(typed.clone()); + env.check_hash(import_id)?; + env.write_to_disk_cache(import_id); + // Cache the mapping from this location to the result. env.write_to_mem_cache(location, res_id); Ok(()) @@ -462,7 +464,7 @@ fn resolve_with_env<'cx>( env.cx().push_import(base_location.clone(), import, span); fetch_import(env, import_id)?; // TODO: store import id in Hir - Ok(env.cx()[import_id].get_result().unwrap().clone()) + Ok(env.cx()[import_id].unwrap_result().clone()) }, )?; Ok(Resolved(resolved)) diff --git a/dhall/tests/spec.rs b/dhall/tests/spec.rs index ab1dc2f..a0fe583 100644 --- a/dhall/tests/spec.rs +++ b/dhall/tests/spec.rs @@ -160,8 +160,7 @@ impl TestFile { } /// Check that the provided expression matches the file contents. - pub fn compare(&self, expr: impl Into) -> Result<()> { - let expr = expr.into(); + pub fn compare(&self, expr: Expr) -> Result<()> { if !self.path().is_file() { return self.write_expr(expr); } @@ -177,8 +176,7 @@ impl TestFile { Ok(()) } /// Check that the provided expression matches the file contents. - pub fn compare_debug(&self, expr: impl Into) -> Result<()> { - let expr = expr.into(); + pub fn compare_debug(&self, expr: Expr) -> Result<()> { if !self.path().is_file() { return self.write_expr(expr); } @@ -194,8 +192,7 @@ impl TestFile { Ok(()) } /// Check that the provided expression matches the file contents. - pub fn compare_binary(&self, expr: impl Into) -> Result<()> { - let expr = expr.into(); + pub fn compare_binary(&self, expr: Expr) -> Result<()> { match self { TestFile::Binary(_) => {} _ => Err(TestError(format!("This is not a binary file")))?, @@ -591,7 +588,7 @@ fn run_test(test: &SpecTest) -> Result<()> { ParserSuccess => { let expr = expr.parse()?; // This exercices both parsing and binary decoding - expected.compare_debug(expr)?; + expected.compare_debug(expr.to_expr())?; } ParserFailure => { use std::io; @@ -612,11 +609,11 @@ fn run_test(test: &SpecTest) -> Result<()> { } BinaryEncoding => { let expr = expr.parse()?; - expected.compare_binary(expr)?; + expected.compare_binary(expr.to_expr())?; } BinaryDecodingSuccess => { let expr = expr.parse()?; - expected.compare_debug(expr)?; + expected.compare_debug(expr.to_expr())?; } BinaryDecodingFailure => { let err = unwrap_err(expr.parse())?; @@ -627,24 +624,24 @@ fn run_test(test: &SpecTest) -> Result<()> { // Round-trip pretty-printer let reparsed = Parsed::parse_str(&parsed.to_string())?; assert_eq!(reparsed, parsed); - expected.compare_ui(parsed)?; + expected.compare_ui(parsed.to_expr())?; } ImportSuccess => { let expr = expr.normalize(cx)?; - expected.compare(expr)?; + expected.compare(expr.to_expr(cx))?; } ImportFailure => { let err = unwrap_err(expr.resolve(cx))?; expected.compare_ui(err)?; } SemanticHash => { - let expr = expr.normalize(cx)?.to_expr_alpha(); + let expr = expr.normalize(cx)?.to_expr_alpha(cx); let hash = hex::encode(expr.sha256_hash()?); expected.compare_ui(format!("sha256:{}", hash))?; } TypeInferenceSuccess => { let ty = expr.typecheck(cx)?.get_type()?; - expected.compare(ty)?; + expected.compare(ty.to_expr(cx))?; } TypeInferenceFailure => { let err = unwrap_err(expr.typecheck(cx))?; @@ -652,10 +649,10 @@ fn run_test(test: &SpecTest) -> Result<()> { } Normalization => { let expr = expr.normalize(cx)?; - expected.compare(expr)?; + expected.compare(expr.to_expr(cx))?; } AlphaNormalization => { - let expr = expr.normalize(cx)?.to_expr_alpha(); + let expr = expr.normalize(cx)?.to_expr_alpha(cx); expected.compare(expr)?; } } diff --git a/serde_dhall/src/options/de.rs b/serde_dhall/src/options/de.rs index d57759a..fc8c6dc 100644 --- a/serde_dhall/src/options/de.rs +++ b/serde_dhall/src/options/de.rs @@ -250,6 +250,7 @@ impl<'a, A> Deserializer<'a, A> { Some(ty) => resolved.typecheck_with(cx, &ty.to_hir())?, }; Ok(Value::from_nir_and_ty( + cx, typed.normalize(cx).as_nir(), typed.ty().as_nir(), )) diff --git a/serde_dhall/src/value.rs b/serde_dhall/src/value.rs index e5f5acd..b259c4b 100644 --- a/serde_dhall/src/value.rs +++ b/serde_dhall/src/value.rs @@ -5,6 +5,7 @@ use dhall::operations::OpKind; use dhall::semantics::{Hir, HirKind, Nir, NirKind}; pub use dhall::syntax::NumKind; use dhall::syntax::{Expr, ExprKind, Span}; +use dhall::Ctxt; use crate::{Error, ErrorKind, FromDhall, Result, ToDhall}; @@ -206,7 +207,11 @@ pub enum SimpleType { } impl Value { - pub(crate) fn from_nir_and_ty(x: &Nir, ty: &Nir) -> Result { + pub(crate) fn from_nir_and_ty<'cx>( + cx: Ctxt<'cx>, + x: &Nir<'cx>, + ty: &Nir<'cx>, + ) -> Result { Ok(if let Some(val) = SimpleValue::from_nir(x) { // The type must be simple if the value is simple. let ty = SimpleType::from_nir(ty).unwrap(); @@ -218,7 +223,7 @@ impl Value { kind: ValueKind::Ty(ty), } } else { - let expr = x.to_hir_noenv().to_expr(Default::default()); + let expr = x.to_hir_noenv().to_expr(cx, Default::default()); return Err(Error(ErrorKind::Deserialize(format!( "this is neither a simple type nor a simple value: {}", expr @@ -342,7 +347,7 @@ impl SimpleValue { let type_error = || { Error(ErrorKind::Serialize(format!( "expected a value of type {}, found {:?}", - ty.unwrap().to_hir().to_expr(Default::default()), + ty.unwrap().to_expr(), self ))) }; @@ -439,7 +444,9 @@ impl SimpleValue { /// Converts back to the corresponding AST expression. pub(crate) fn to_expr(&self, ty: Option<&SimpleType>) -> Result { - Ok(self.to_hir(ty)?.to_expr(Default::default())) + Ctxt::with_new(|cx| { + Ok(self.to_hir(ty)?.to_expr(cx, Default::default())) + }) } } @@ -514,7 +521,7 @@ impl SimpleType { /// Converts back to the corresponding AST expression. pub(crate) fn to_expr(&self) -> Expr { - self.to_hir().to_expr(Default::default()) + Ctxt::with_new(|cx| self.to_hir().to_expr(cx, Default::default())) } } -- cgit v1.2.3 From c1fe26d45c831eec015ad5c015236fce1928613a Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Mon, 7 Dec 2020 14:32:06 +0000 Subject: Pass import results via the global context --- dhall/src/semantics/nze/normalize.rs | 5 ++++- dhall/src/semantics/resolve/hir.rs | 13 +++++++------ dhall/src/semantics/resolve/resolve.rs | 9 ++++----- dhall/src/semantics/tck/typecheck.rs | 5 ++++- 4 files changed, 19 insertions(+), 13 deletions(-) diff --git a/dhall/src/semantics/nze/normalize.rs b/dhall/src/semantics/nze/normalize.rs index cce421d..d545750 100644 --- a/dhall/src/semantics/nze/normalize.rs +++ b/dhall/src/semantics/nze/normalize.rs @@ -155,7 +155,10 @@ pub fn normalize_one_layer<'cx>(expr: ExprKind>) -> NirKind<'cx> { pub fn normalize_hir<'cx>(env: &NzEnv<'cx>, hir: &Hir<'cx>) -> NirKind<'cx> { match hir.kind() { HirKind::Var(var) => env.lookup_val(*var), - HirKind::Import(hir, _) => normalize_hir(env, hir), + HirKind::Import(import) => { + let typed = env.cx()[import].unwrap_result(); + normalize_hir(env, &typed.hir) + } HirKind::Expr(ExprKind::Lam(binder, annot, body)) => { let annot = annot.eval(env); NirKind::LamClosure { diff --git a/dhall/src/semantics/resolve/hir.rs b/dhall/src/semantics/resolve/hir.rs index 5575888..cfde47e 100644 --- a/dhall/src/semantics/resolve/hir.rs +++ b/dhall/src/semantics/resolve/hir.rs @@ -1,7 +1,7 @@ use crate::error::TypeError; -use crate::semantics::{type_with, NameEnv, Nir, NzEnv, Tir, TyEnv, Type}; +use crate::semantics::{type_with, NameEnv, Nir, NzEnv, Tir, TyEnv}; use crate::syntax::{Expr, ExprKind, Span, V}; -use crate::{Ctxt, ToExprOptions}; +use crate::{Ctxt, ImportId, ToExprOptions}; /// Stores an alpha-normalized variable. #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] @@ -13,8 +13,8 @@ pub struct AlphaVar { pub enum HirKind<'cx> { /// A resolved variable (i.e. a DeBruijn index) Var(AlphaVar), - /// Result of resolving an import. - Import(Hir<'cx>, Type<'cx>), + /// An import. It must have been resolved by the time we get to typechecking/normalization. + Import(ImportId<'cx>), // Forbidden ExprKind variants: Var, Import, Completion Expr(ExprKind>), } @@ -104,8 +104,9 @@ fn hir_to_expr<'cx>( let kind = match hir.kind() { HirKind::Var(v) if opts.alpha => ExprKind::Var(V("_".into(), v.idx())), HirKind::Var(v) => ExprKind::Var(env.label_var(*v)), - HirKind::Import(hir, _) => { - return hir_to_expr(cx, hir, opts, &mut NameEnv::new()); + HirKind::Import(import) => { + let typed = cx[import].unwrap_result(); + return hir_to_expr(cx, &typed.hir, opts, &mut NameEnv::new()); } HirKind::Expr(e) => { let e = e.map_ref_maybe_binder(|l, hir| { diff --git a/dhall/src/semantics/resolve/resolve.rs b/dhall/src/semantics/resolve/resolve.rs index db162ef..011ed45 100644 --- a/dhall/src/semantics/resolve/resolve.rs +++ b/dhall/src/semantics/resolve/resolve.rs @@ -348,7 +348,7 @@ fn desugar(expr: &Expr) -> Cow<'_, Expr> { fn traverse_resolve_expr<'cx>( name_env: &mut NameEnv, expr: &Expr, - f: &mut impl FnMut(Import, Span) -> Result, Error>, + f: &mut impl FnMut(Import, Span) -> Result, Error>, ) -> Result, Error> { let expr = desugar(expr); Ok(match expr.kind() { @@ -387,8 +387,8 @@ fn traverse_resolve_expr<'cx>( ExprKind::Import(import) => { // TODO: evaluate import headers let import = import.traverse_ref(|_| Ok::<_, Error>(()))?; - let imported = f(import, expr.span())?; - HirKind::Import(imported.hir, imported.ty) + let import_id = f(import, expr.span())?; + HirKind::Import(import_id) } kind => HirKind::Expr(kind), }; @@ -463,8 +463,7 @@ fn resolve_with_env<'cx>( let import_id = env.cx().push_import(base_location.clone(), import, span); fetch_import(env, import_id)?; - // TODO: store import id in Hir - Ok(env.cx()[import_id].unwrap_result().clone()) + Ok(import_id) }, )?; Ok(Resolved(resolved)) diff --git a/dhall/src/semantics/tck/typecheck.rs b/dhall/src/semantics/tck/typecheck.rs index 8218368..f47563e 100644 --- a/dhall/src/semantics/tck/typecheck.rs +++ b/dhall/src/semantics/tck/typecheck.rs @@ -184,7 +184,10 @@ pub fn type_with<'cx, 'hir>( ) -> Result, TypeError> { let tir = match hir.kind() { HirKind::Var(var) => Tir::from_hir(hir, env.lookup(*var)), - HirKind::Import(_, ty) => Tir::from_hir(hir, ty.clone()), + HirKind::Import(import) => { + let typed = env.cx()[import].unwrap_result(); + Tir::from_hir(hir, typed.ty.clone()) + } HirKind::Expr(ExprKind::Var(_)) => { unreachable!("Hir should contain no unresolved variables") } -- cgit v1.2.3 From 922199ab322efa7b62bf4698cf5ed9e2d7a378c0 Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Mon, 7 Dec 2020 15:24:36 +0000 Subject: Unify `skip_resolve_expr` with normal resolution --- dhall/src/builtins.rs | 19 ++++++---- dhall/src/lib.rs | 15 +++++++- dhall/src/semantics/resolve/cache.rs | 2 +- dhall/src/semantics/resolve/hir.rs | 4 +- dhall/src/semantics/resolve/resolve.rs | 69 ++++++++++++++++++++++------------ dhall/src/semantics/tck/typecheck.rs | 4 +- serde_dhall/src/options/de.rs | 2 +- serde_dhall/tests/serde.rs | 19 ++++++++++ 8 files changed, 94 insertions(+), 40 deletions(-) diff --git a/dhall/src/builtins.rs b/dhall/src/builtins.rs index 39cc4ef..123e03d 100644 --- a/dhall/src/builtins.rs +++ b/dhall/src/builtins.rs @@ -2,15 +2,13 @@ use std::collections::{BTreeMap, HashMap}; use std::convert::TryInto; use crate::operations::{BinOp, OpKind}; -use crate::semantics::{ - nze, skip_resolve_expr, typecheck, Hir, HirKind, Nir, NirKind, NzEnv, - VarEnv, -}; +use crate::semantics::{nze, Hir, HirKind, Nir, NirKind, NzEnv, VarEnv}; use crate::syntax::Const::Type; use crate::syntax::{ Const, Expr, ExprKind, InterpolatedText, InterpolatedTextContents, Label, NaiveDouble, NumKind, Span, UnspannedExpr, V, }; +use crate::{Ctxt, Parsed}; /// Built-ins #[derive(Debug, Copy, Clone, PartialEq, Eq, Hash)] @@ -178,7 +176,7 @@ macro_rules! make_type { }; } -pub fn type_of_builtin<'cx>(b: Builtin) -> Hir<'cx> { +pub fn type_of_builtin<'cx>(cx: Ctxt<'cx>, b: Builtin) -> Hir<'cx> { use Builtin::*; let expr = match b { Bool | Natural | Integer | Double | Text => make_type!(Type), @@ -253,7 +251,10 @@ pub fn type_of_builtin<'cx>(b: Builtin) -> Hir<'cx> { forall (A: Type) -> Optional A ), }; - skip_resolve_expr(&expr).unwrap() + Parsed::from_expr_without_imports(expr) + .resolve(cx) + .unwrap() + .0 } // Ad-hoc macro to help construct closures @@ -323,8 +324,12 @@ fn apply_builtin<'cx>( DoneAsIs, } let make_closure = |e| { - typecheck(cx, &skip_resolve_expr(&e).unwrap()) + Parsed::from_expr_without_imports(e) + .resolve(cx) + .unwrap() + .typecheck(cx) .unwrap() + .as_hir() .eval(env.clone()) }; diff --git a/dhall/src/lib.rs b/dhall/src/lib.rs index 7f77334..c27b633 100644 --- a/dhall/src/lib.rs +++ b/dhall/src/lib.rs @@ -58,6 +58,11 @@ pub struct ToExprOptions { } impl Parsed { + /// Construct from an `Expr`. This `Expr` will have imports disabled. + pub fn from_expr_without_imports(e: Expr) -> Self { + Parsed(e, ImportLocation::dhall_code_without_imports()) + } + pub fn parse_file(f: &Path) -> Result { parse::parse_file(f) } @@ -78,8 +83,11 @@ impl Parsed { pub fn resolve<'cx>(self, cx: Ctxt<'cx>) -> Result, Error> { resolve::resolve(cx, self) } - pub fn skip_resolve<'cx>(self) -> Result, Error> { - resolve::skip_resolve(self) + pub fn skip_resolve<'cx>( + self, + cx: Ctxt<'cx>, + ) -> Result, Error> { + resolve::skip_resolve(cx, self) } /// Converts a value back to the corresponding AST expression. @@ -122,6 +130,9 @@ impl<'cx> Typed<'cx> { self.hir.to_expr(cx, ToExprOptions { alpha: false }) } + pub fn as_hir(&self) -> &Hir<'cx> { + &self.hir + } pub fn ty(&self) -> &Type<'cx> { &self.ty } diff --git a/dhall/src/semantics/resolve/cache.rs b/dhall/src/semantics/resolve/cache.rs index 9a5e835..1fc8dd3 100644 --- a/dhall/src/semantics/resolve/cache.rs +++ b/dhall/src/semantics/resolve/cache.rs @@ -94,7 +94,7 @@ fn read_cache_file<'cx>( } } - Ok(parse_binary(&data)?.skip_resolve()?.typecheck(cx)?) + Ok(parse_binary(&data)?.resolve(cx)?.typecheck(cx)?) } /// Write a file to the cache. diff --git a/dhall/src/semantics/resolve/hir.rs b/dhall/src/semantics/resolve/hir.rs index cfde47e..3e282b4 100644 --- a/dhall/src/semantics/resolve/hir.rs +++ b/dhall/src/semantics/resolve/hir.rs @@ -1,5 +1,5 @@ use crate::error::TypeError; -use crate::semantics::{type_with, NameEnv, Nir, NzEnv, Tir, TyEnv}; +use crate::semantics::{type_with, typecheck, NameEnv, Nir, NzEnv, Tir, TyEnv}; use crate::syntax::{Expr, ExprKind, Span, V}; use crate::{Ctxt, ImportId, ToExprOptions}; @@ -81,7 +81,7 @@ impl<'cx> Hir<'cx> { &'hir self, cx: Ctxt<'cx>, ) -> Result, TypeError> { - self.typecheck(&TyEnv::new(cx)) + typecheck(cx, self) } /// Eval the Hir. It will actually get evaluated only as needed on demand. diff --git a/dhall/src/semantics/resolve/resolve.rs b/dhall/src/semantics/resolve/resolve.rs index 011ed45..e040cf4 100644 --- a/dhall/src/semantics/resolve/resolve.rs +++ b/dhall/src/semantics/resolve/resolve.rs @@ -29,8 +29,10 @@ enum ImportLocationKind { Remote(Url), /// Environment variable Env(String), - /// Data without a location + /// Data without a location; chaining will start from current directory. Missing, + /// Token to signal that thi sfile should contain no imports. + NoImport, } /// The location of some data. @@ -101,6 +103,7 @@ impl ImportLocationKind { url = url.join(&path.file_path.join("/"))?; ImportLocationKind::Remote(url) } + ImportLocationKind::NoImport => unreachable!(), }) } @@ -120,6 +123,7 @@ impl ImportLocationKind { ImportLocationKind::Missing => { return Err(ImportError::Missing.into()) } + ImportLocationKind::NoImport => unreachable!(), }) } @@ -134,6 +138,7 @@ impl ImportLocationKind { ImportLocationKind::Missing => { return Err(ImportError::Missing.into()) } + ImportLocationKind::NoImport => unreachable!(), }) } @@ -149,6 +154,7 @@ impl ImportLocationKind { ("Environment", Some(name.clone())) } ImportLocationKind::Missing => ("Missing", None), + ImportLocationKind::NoImport => unreachable!(), }; let asloc_ty = make_aslocation_uniontype(); @@ -171,6 +177,12 @@ impl ImportLocation { mode: ImportMode::Code, } } + pub fn dhall_code_without_imports() -> Self { + ImportLocation { + kind: ImportLocationKind::NoImport, + mode: ImportMode::Code, + } + } pub fn local_dhall_code(path: PathBuf) -> Self { ImportLocation { kind: ImportLocationKind::Local(path), @@ -191,6 +203,10 @@ impl ImportLocation { fn chain(&self, import: &Import) -> Result { // Makes no sense to chain an import if the current file is not a dhall file. assert!(matches!(self.mode, ImportMode::Code)); + if matches!(self.kind, ImportLocationKind::NoImport) { + Err(ImportError::UnexpectedImport(import.clone()))?; + } + let kind = match &import.location { ImportTarget::Local(prefix, path) => { self.kind.chain_local(*prefix, path)? @@ -232,30 +248,37 @@ impl ImportLocation { env: &mut ImportEnv<'cx>, span: Span, ) -> Result, Error> { - let (hir, ty) = match self.mode { + let cx = env.cx(); + let typed = match self.mode { ImportMode::Code => { let parsed = self.kind.fetch_dhall()?; - let typed = - resolve_with_env(env, parsed)?.typecheck(env.cx())?; - let hir = typed.normalize(env.cx()).to_hir(); - (hir, typed.ty) + let typed = resolve_with_env(env, parsed)?.typecheck(cx)?; + Typed { + // TODO: manage to keep the Nir around. Will need fixing variables. + hir: typed.normalize(cx).to_hir(), + ty: typed.ty, + } } ImportMode::RawText => { let text = self.kind.fetch_text()?; - let hir = Hir::new( - HirKind::Expr(ExprKind::TextLit(text.into())), - span, - ); - (hir, Type::from_builtin(env.cx(), Builtin::Text)) + Typed { + hir: Hir::new( + HirKind::Expr(ExprKind::TextLit(text.into())), + span, + ), + ty: Type::from_builtin(cx, Builtin::Text), + } } ImportMode::Location => { let expr = self.kind.to_location(); - let hir = skip_resolve_expr(&expr)?; - let ty = hir.typecheck_noenv(env.cx())?.ty().clone(); - (hir, ty) + Parsed::from_expr_without_imports(expr) + .resolve(cx) + .unwrap() + .typecheck(cx) + .unwrap() } }; - Ok(Typed { hir, ty }) + Ok(typed) } } @@ -476,16 +499,12 @@ pub fn resolve<'cx>( resolve_with_env(&mut ImportEnv::new(cx), parsed) } -pub fn skip_resolve_expr<'cx>(expr: &Expr) -> Result, Error> { - traverse_resolve_expr(&mut NameEnv::new(), expr, &mut |import, _span| { - Err(ImportError::UnexpectedImport(import).into()) - }) -} - -pub fn skip_resolve<'cx>(parsed: Parsed) -> Result, Error> { - let Parsed(expr, _) = parsed; - let resolved = skip_resolve_expr(&expr)?; - Ok(Resolved(resolved)) +pub fn skip_resolve<'cx>( + cx: Ctxt<'cx>, + parsed: Parsed, +) -> Result, Error> { + let parsed = Parsed::from_expr_without_imports(parsed.0); + Ok(resolve(cx, parsed)?) } pub trait Canonicalize { diff --git a/dhall/src/semantics/tck/typecheck.rs b/dhall/src/semantics/tck/typecheck.rs index f47563e..5a44c9c 100644 --- a/dhall/src/semantics/tck/typecheck.rs +++ b/dhall/src/semantics/tck/typecheck.rs @@ -62,8 +62,8 @@ fn type_one_layer<'cx>( }, ), ExprKind::Builtin(b) => { - let t_hir = type_of_builtin(b); - typecheck(env.cx(), &t_hir)?.eval_to_type(env)? + let t_hir = type_of_builtin(cx, b); + typecheck(cx, &t_hir)?.eval_to_type(env)? } ExprKind::TextLit(interpolated) => { let text_type = Type::from_builtin(cx, Builtin::Text); diff --git a/serde_dhall/src/options/de.rs b/serde_dhall/src/options/de.rs index fc8c6dc..30846a2 100644 --- a/serde_dhall/src/options/de.rs +++ b/serde_dhall/src/options/de.rs @@ -243,7 +243,7 @@ impl<'a, A> Deserializer<'a, A> { let resolved = if self.allow_imports { parsed.resolve(cx)? } else { - parsed.skip_resolve()? + parsed.skip_resolve(cx)? }; let typed = match &T::get_annot(self.annot) { None => resolved.typecheck(cx)?, diff --git a/serde_dhall/tests/serde.rs b/serde_dhall/tests/serde.rs index 1181f72..fe0bffb 100644 --- a/serde_dhall/tests/serde.rs +++ b/serde_dhall/tests/serde.rs @@ -226,6 +226,25 @@ mod serde { Ok(true) ); } + + #[test] + fn test_import() { + assert_de( + "../dhall-lang/tests/parser/success/unit/BoolLitTrueA.dhall", + true, + ); + assert_eq!( + serde_dhall::from_str( + "../dhall-lang/tests/parser/success/unit/BoolLitTrueA.dhall" + ) + .imports(false) + .static_type_annotation() + .parse::() + .map_err(|e| e.to_string()), + Err("UnexpectedImport(Import { mode: Code, location: Local(Parent, FilePath { file_path: [\"dhall-lang\", \"tests\", \"parser\", \"success\", \"unit\", \"BoolLitTrueA.dhall\"] }), hash: None })".to_string()) + ); + } + // TODO: test various builder configurations // In particular test cloning and reusing builder } -- cgit v1.2.3 From ff070d5e5815b6cef27ff40383153226ddfd4a61 Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Mon, 7 Dec 2020 15:37:29 +0000 Subject: Add tests --- dhall/tests/import/success/unit/AlternativeWithWrongVariable1A.dhall | 1 + dhall/tests/import/success/unit/AlternativeWithWrongVariable1B.dhall | 1 + dhall/tests/import/success/unit/AlternativeWithWrongVariable2A.dhall | 1 + dhall/tests/import/success/unit/AlternativeWithWrongVariable2B.dhall | 1 + 4 files changed, 4 insertions(+) create mode 100644 dhall/tests/import/success/unit/AlternativeWithWrongVariable1A.dhall create mode 100644 dhall/tests/import/success/unit/AlternativeWithWrongVariable1B.dhall create mode 100644 dhall/tests/import/success/unit/AlternativeWithWrongVariable2A.dhall create mode 100644 dhall/tests/import/success/unit/AlternativeWithWrongVariable2B.dhall diff --git a/dhall/tests/import/success/unit/AlternativeWithWrongVariable1A.dhall b/dhall/tests/import/success/unit/AlternativeWithWrongVariable1A.dhall new file mode 100644 index 0000000..e6fa8d5 --- /dev/null +++ b/dhall/tests/import/success/unit/AlternativeWithWrongVariable1A.dhall @@ -0,0 +1 @@ +\(x: Natural) -> x ? y diff --git a/dhall/tests/import/success/unit/AlternativeWithWrongVariable1B.dhall b/dhall/tests/import/success/unit/AlternativeWithWrongVariable1B.dhall new file mode 100644 index 0000000..04a1712 --- /dev/null +++ b/dhall/tests/import/success/unit/AlternativeWithWrongVariable1B.dhall @@ -0,0 +1 @@ +λ(x : Natural) → x diff --git a/dhall/tests/import/success/unit/AlternativeWithWrongVariable2A.dhall b/dhall/tests/import/success/unit/AlternativeWithWrongVariable2A.dhall new file mode 100644 index 0000000..8089500 --- /dev/null +++ b/dhall/tests/import/success/unit/AlternativeWithWrongVariable2A.dhall @@ -0,0 +1 @@ +\(x: Natural) -> (y + missing) ? x diff --git a/dhall/tests/import/success/unit/AlternativeWithWrongVariable2B.dhall b/dhall/tests/import/success/unit/AlternativeWithWrongVariable2B.dhall new file mode 100644 index 0000000..04a1712 --- /dev/null +++ b/dhall/tests/import/success/unit/AlternativeWithWrongVariable2B.dhall @@ -0,0 +1 @@ +λ(x : Natural) → x -- cgit v1.2.3 From f478bc16a3b8414770d84dd87f3e46b869d31750 Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Mon, 7 Dec 2020 17:16:06 +0000 Subject: Avoid storing an import before we checked its hash --- dhall/src/semantics/resolve/env.rs | 21 +++++++++++++-------- dhall/src/semantics/resolve/resolve.rs | 30 +++++++++++++++--------------- 2 files changed, 28 insertions(+), 23 deletions(-) diff --git a/dhall/src/semantics/resolve/env.rs b/dhall/src/semantics/resolve/env.rs index 0d1952b..bde99d1 100644 --- a/dhall/src/semantics/resolve/env.rs +++ b/dhall/src/semantics/resolve/env.rs @@ -95,9 +95,12 @@ impl<'cx> ImportEnv<'cx> { Some(expr) } - /// Invariant: the import must have been fetched. - pub fn check_hash(&self, import: ImportId<'cx>) -> Result<(), Error> { - check_hash(self.cx(), import) + pub fn check_hash( + &self, + import: ImportId<'cx>, + result: ImportResultId<'cx>, + ) -> Result<(), Error> { + check_hash(self.cx(), import, result) } pub fn write_to_mem_cache( @@ -108,12 +111,14 @@ impl<'cx> ImportEnv<'cx> { self.mem_cache.insert(location, result); } - /// Invariant: the import must have been fetched. - pub fn write_to_disk_cache(&self, import: ImportId<'cx>) { - let import = &self.cx()[import]; + pub fn write_to_disk_cache( + &self, + hash: &Option, + result: ImportResultId<'cx>, + ) { if let Some(disk_cache) = self.disk_cache.as_ref() { - if let Some(hash) = &import.import.hash { - let expr = import.unwrap_result(); + if let Some(hash) = hash { + let expr = &self.cx()[result]; let _ = disk_cache.insert(self.cx(), hash, expr); } } diff --git a/dhall/src/semantics/resolve/resolve.rs b/dhall/src/semantics/resolve/resolve.rs index e040cf4..7450825 100644 --- a/dhall/src/semantics/resolve/resolve.rs +++ b/dhall/src/semantics/resolve/resolve.rs @@ -15,7 +15,7 @@ use crate::syntax::{ Expr, ExprKind, FilePath, FilePrefix, Hash, ImportMode, ImportTarget, Span, UnspannedExpr, URL, }; -use crate::{Ctxt, ImportId, Parsed, Resolved, Typed}; +use crate::{Ctxt, ImportId, ImportResultId, Parsed, Resolved, Typed}; // TODO: evaluate import headers pub type Import = syntax::Import<()>; @@ -310,16 +310,16 @@ fn make_aslocation_uniontype() -> Expr { mkexpr(ExprKind::UnionType(union)) } -/// Invariant: the import must have been fetched. pub fn check_hash<'cx>( cx: Ctxt<'cx>, import: ImportId<'cx>, + result: ImportResultId<'cx>, ) -> Result<(), Error> { let import = &cx[import]; if let (ImportMode::Code, Some(Hash::SHA256(hash))) = (import.import.mode, &import.import.hash) { - let expr = import.unwrap_result().hir.to_expr_alpha(cx); + let expr = cx[result].hir.to_expr_alpha(cx); let actual_hash = expr.sha256_hash()?; if hash[..] != actual_hash[..] { mkerr( @@ -424,7 +424,7 @@ fn traverse_resolve_expr<'cx>( fn fetch_import<'cx>( env: &mut ImportEnv<'cx>, import_id: ImportId<'cx>, -) -> Result<(), Error> { +) -> Result, Error> { let cx = env.cx(); let import = &cx[import_id].import; let span = cx[import_id].span.clone(); @@ -433,12 +433,11 @@ fn fetch_import<'cx>( // If the import is in the in-memory cache, or the hash is in the on-disk cache, return // the cached contents. if let Some(res_id) = env.get_from_mem_cache(&location) { - cx[import_id].set_resultid(res_id); // The same location may be used with different or no hashes. Thus we need to check // the hashes every time. - env.check_hash(import_id)?; - env.write_to_disk_cache(import_id); - return Ok(()); + env.check_hash(import_id, res_id)?; + env.write_to_disk_cache(&import.hash, res_id); + return Ok(res_id); } if let Some(typed) = env.get_from_disk_cache(&import.hash) { // No need to check the hash, it was checked before reading the file. @@ -446,8 +445,8 @@ fn fetch_import<'cx>( // unrelated to the cached file (e.g. `missing sha256:...` is valid). // This actually means that importing many times a same hashed import will take // longer than importing many times a same non-hashed import. - cx[import_id].set_result(typed); - return Ok(()); + let res_id = cx.push_import_result(typed); + return Ok(res_id); } // Resolve this import, making sure that recursive imports don't cycle back to the @@ -465,13 +464,13 @@ fn fetch_import<'cx>( }; // Add the resolved import to the caches - let res_id = cx[import_id].set_result(typed.clone()); - env.check_hash(import_id)?; - env.write_to_disk_cache(import_id); + let res_id = cx.push_import_result(typed); + env.check_hash(import_id, res_id)?; + env.write_to_disk_cache(&import.hash, res_id); // Cache the mapping from this location to the result. env.write_to_mem_cache(location, res_id); - Ok(()) + Ok(res_id) } fn resolve_with_env<'cx>( @@ -485,7 +484,8 @@ fn resolve_with_env<'cx>( &mut |import, span| { let import_id = env.cx().push_import(base_location.clone(), import, span); - fetch_import(env, import_id)?; + let res_id = fetch_import(env, import_id)?; + env.cx()[import_id].set_resultid(res_id); Ok(import_id) }, )?; -- cgit v1.2.3 From 7a392b07166c089979e69d4c8a68da3298964c28 Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Mon, 7 Dec 2020 17:48:20 +0000 Subject: Defer name errors to typechecking We aren't supposed to inspect anything before alternatives are chosen --- dhall/src/semantics/nze/normalize.rs | 1 + dhall/src/semantics/resolve/hir.rs | 3 +++ dhall/src/semantics/resolve/resolve.rs | 9 ++++----- dhall/src/semantics/tck/typecheck.rs | 5 +++++ 4 files changed, 13 insertions(+), 5 deletions(-) diff --git a/dhall/src/semantics/nze/normalize.rs b/dhall/src/semantics/nze/normalize.rs index d545750..0a09a80 100644 --- a/dhall/src/semantics/nze/normalize.rs +++ b/dhall/src/semantics/nze/normalize.rs @@ -154,6 +154,7 @@ pub fn normalize_one_layer<'cx>(expr: ExprKind>) -> NirKind<'cx> { /// Normalize Hir into WHNF pub fn normalize_hir<'cx>(env: &NzEnv<'cx>, hir: &Hir<'cx>) -> NirKind<'cx> { match hir.kind() { + HirKind::MissingVar(..) => unreachable!("ruled out by typechecking"), HirKind::Var(var) => env.lookup_val(*var), HirKind::Import(import) => { let typed = env.cx()[import].unwrap_result(); diff --git a/dhall/src/semantics/resolve/hir.rs b/dhall/src/semantics/resolve/hir.rs index 3e282b4..05a8550 100644 --- a/dhall/src/semantics/resolve/hir.rs +++ b/dhall/src/semantics/resolve/hir.rs @@ -13,6 +13,8 @@ pub struct AlphaVar { pub enum HirKind<'cx> { /// A resolved variable (i.e. a DeBruijn index) Var(AlphaVar), + /// A variable that couldn't be resolved. Detected during resolution, but causes an error during typeck. + MissingVar(V), /// An import. It must have been resolved by the time we get to typechecking/normalization. Import(ImportId<'cx>), // Forbidden ExprKind variants: Var, Import, Completion @@ -104,6 +106,7 @@ fn hir_to_expr<'cx>( let kind = match hir.kind() { HirKind::Var(v) if opts.alpha => ExprKind::Var(V("_".into(), v.idx())), HirKind::Var(v) => ExprKind::Var(env.label_var(*v)), + HirKind::MissingVar(v) => ExprKind::Var(v.clone()), HirKind::Import(import) => { let typed = cx[import].unwrap_result(); return hir_to_expr(cx, &typed.hir, opts, &mut NameEnv::new()); diff --git a/dhall/src/semantics/resolve/resolve.rs b/dhall/src/semantics/resolve/resolve.rs index 7450825..c4cd518 100644 --- a/dhall/src/semantics/resolve/resolve.rs +++ b/dhall/src/semantics/resolve/resolve.rs @@ -377,11 +377,7 @@ fn traverse_resolve_expr<'cx>( Ok(match expr.kind() { ExprKind::Var(var) => match name_env.unlabel_var(&var) { Some(v) => Hir::new(HirKind::Var(v), expr.span()), - None => mkerr( - ErrorBuilder::new(format!("unbound variable `{}`", var)) - .span_err(expr.span(), "not found in this scope") - .format(), - )?, + None => Hir::new(HirKind::MissingVar(var.clone()), expr.span()), }, ExprKind::Op(OpKind::BinOp(BinOp::ImportAlt, l, r)) => { match traverse_resolve_expr(name_env, l, f) { @@ -492,6 +488,8 @@ fn resolve_with_env<'cx>( Ok(Resolved(resolved)) } +/// Resolves all imports and names. Returns errors if importing failed. Name errors are deferred to +/// typechecking. pub fn resolve<'cx>( cx: Ctxt<'cx>, parsed: Parsed, @@ -499,6 +497,7 @@ pub fn resolve<'cx>( resolve_with_env(&mut ImportEnv::new(cx), parsed) } +/// Resolves names and errors if we find any imports. pub fn skip_resolve<'cx>( cx: Ctxt<'cx>, parsed: Parsed, diff --git a/dhall/src/semantics/tck/typecheck.rs b/dhall/src/semantics/tck/typecheck.rs index 5a44c9c..e14bcc6 100644 --- a/dhall/src/semantics/tck/typecheck.rs +++ b/dhall/src/semantics/tck/typecheck.rs @@ -184,6 +184,11 @@ pub fn type_with<'cx, 'hir>( ) -> Result, TypeError> { let tir = match hir.kind() { HirKind::Var(var) => Tir::from_hir(hir, env.lookup(*var)), + HirKind::MissingVar(var) => mkerr( + ErrorBuilder::new(format!("unbound variable `{}`", var)) + .span_err(hir.span(), "not found in this scope") + .format(), + )?, HirKind::Import(import) => { let typed = env.cx()[import].unwrap_result(); Tir::from_hir(hir, typed.ty.clone()) -- cgit v1.2.3 From 4473f3549f331c51a7df0e307d356a06c00d7288 Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Mon, 7 Dec 2020 19:02:07 +0000 Subject: Resolve imports and alternatives outside of the ast traversal --- dhall/src/ctxt.rs | 203 ++++++++++++++++++++++----------- dhall/src/lib.rs | 2 +- dhall/src/semantics/nze/normalize.rs | 8 ++ dhall/src/semantics/resolve/hir.rs | 14 ++- dhall/src/semantics/resolve/resolve.rs | 189 ++++++++++++++++++++---------- dhall/src/semantics/tck/typecheck.rs | 8 ++ 6 files changed, 294 insertions(+), 130 deletions(-) diff --git a/dhall/src/ctxt.rs b/dhall/src/ctxt.rs index 3809bc9..aad1a1b 100644 --- a/dhall/src/ctxt.rs +++ b/dhall/src/ctxt.rs @@ -3,29 +3,19 @@ use once_cell::sync::OnceCell; use std::marker::PhantomData; use std::ops::{Deref, Index}; -use crate::semantics::{Import, ImportLocation}; +use crate::semantics::{Import, ImportLocation, ImportNode}; use crate::syntax::Span; use crate::Typed; -#[derive(Debug, Copy, Clone, PartialEq, Eq, Hash)] -pub struct ImportId<'cx>(usize, PhantomData<&'cx ()>); -#[derive(Debug, Copy, Clone, PartialEq, Eq, Hash)] -pub struct ImportResultId<'cx>(usize, PhantomData<&'cx ()>); - -/// What's stored for each `ImportId`. Allows getting and setting a result for this import. -pub struct StoredImport<'cx> { - cx: Ctxt<'cx>, - pub base_location: ImportLocation, - pub import: Import, - pub span: Span, - result: OnceCell>, -} +///////////////////////////////////////////////////////////////////////////////////////////////////// +// Ctxt /// Implementation detail. Made public for the `Index` instances. #[derive(Default)] pub struct CtxtS<'cx> { imports: FrozenVec>>, - import_results: FrozenVec>>, + import_alternatives: FrozenVec>>, + import_results: FrozenVec>>, } /// Context for the dhall compiler. Stores various global maps. @@ -40,33 +30,46 @@ impl Ctxt<'_> { f(cx) } } -impl<'cx> Ctxt<'cx> { - /// Store an import and the location relative to which it must be resolved. - pub fn push_import( - self, - base_location: ImportLocation, - import: Import, - span: Span, - ) -> ImportId<'cx> { - let stored = StoredImport { - cx: self, - base_location, - import, - span, - result: OnceCell::new(), - }; - let id = self.0.imports.len(); - self.0.imports.push(Box::new(stored)); - ImportId(id, PhantomData) +impl<'cx> Deref for Ctxt<'cx> { + type Target = &'cx CtxtS<'cx>; + fn deref(&self) -> &&'cx CtxtS<'cx> { + &self.0 } - /// Store the result of fetching an import. - pub fn push_import_result(self, res: Typed<'cx>) -> ImportResultId<'cx> { - let id = self.0.import_results.len(); - self.0.import_results.push(Box::new(res)); - ImportResultId(id, PhantomData) +} +impl<'a, 'cx, T> Index<&'a T> for CtxtS<'cx> +where + Self: Index, + T: Copy, +{ + type Output = >::Output; + fn index(&self, id: &'a T) -> &Self::Output { + &self[*id] + } +} + +/// Empty impl, because `FrozenVec` does not implement `Debug` and I can't be bothered to do it +/// myself. +impl<'cx> std::fmt::Debug for Ctxt<'cx> { + fn fmt(&self, _: &mut std::fmt::Formatter) -> std::fmt::Result { + Ok(()) } } +///////////////////////////////////////////////////////////////////////////////////////////////////// +// Imports + +#[derive(Debug, Copy, Clone, PartialEq, Eq, Hash)] +pub struct ImportId<'cx>(usize, PhantomData<&'cx ()>); + +/// What's stored for each `ImportId`. Allows getting and setting a result for this import. +pub struct StoredImport<'cx> { + cx: Ctxt<'cx>, + pub base_location: ImportLocation, + pub import: Import, + pub span: Span, + result: OnceCell>, +} + impl<'cx> StoredImport<'cx> { /// Get the id of the result of fetching this import. Returns `None` if the result has not yet /// been fetched. @@ -79,58 +82,130 @@ impl<'cx> StoredImport<'cx> { } /// Get the result of fetching this import. Returns `None` if the result has not yet been /// fetched. - pub fn get_result(&self) -> Option<&'cx Typed<'cx>> { + pub fn get_result(&self) -> Option<&'cx StoredImportResult<'cx>> { let res = self.get_resultid()?; Some(&self.cx[res]) } /// Get the result of fetching this import. Panicx if the result has not yet been /// fetched. - pub fn unwrap_result(&self) -> &'cx Typed<'cx> { + pub fn unwrap_result(&self) -> &'cx StoredImportResult<'cx> { self.get_result() .expect("imports should all have been resolved at this stage") } /// Store the result of fetching this import. - pub fn set_result(&self, res: Typed<'cx>) -> ImportResultId<'cx> { + pub fn set_result( + &self, + res: StoredImportResult<'cx>, + ) -> ImportResultId<'cx> { let res = self.cx.push_import_result(res); self.set_resultid(res); res } } - -impl<'cx> Deref for Ctxt<'cx> { - type Target = &'cx CtxtS<'cx>; - fn deref(&self) -> &&'cx CtxtS<'cx> { - &self.0 +impl<'cx> Ctxt<'cx> { + /// Store an import and the location relative to which it must be resolved. + pub fn push_import( + self, + base_location: ImportLocation, + import: Import, + span: Span, + ) -> ImportId<'cx> { + let stored = StoredImport { + cx: self, + base_location, + import, + span, + result: OnceCell::new(), + }; + let id = self.0.imports.len(); + self.0.imports.push(Box::new(stored)); + ImportId(id, PhantomData) } } - impl<'cx> Index> for CtxtS<'cx> { type Output = StoredImport<'cx>; fn index(&self, id: ImportId<'cx>) -> &StoredImport<'cx> { &self.imports[id.0] } } -impl<'cx> Index> for CtxtS<'cx> { - type Output = Typed<'cx>; - fn index(&self, id: ImportResultId<'cx>) -> &Typed<'cx> { - &self.import_results[id.0] + +///////////////////////////////////////////////////////////////////////////////////////////////////// +// Import alternatives + +#[derive(Debug, Copy, Clone, PartialEq, Eq, Hash)] +pub struct ImportAlternativeId<'cx>(usize, PhantomData<&'cx ()>); + +/// What's stored for each `ImportAlternativeId`. +pub struct StoredImportAlternative<'cx> { + pub left_imports: Box<[ImportNode<'cx>]>, + pub right_imports: Box<[ImportNode<'cx>]>, + /// `true` for left, `false` for right. + selected: OnceCell, +} + +impl<'cx> StoredImportAlternative<'cx> { + /// Get which alternative got selected. `true` for left, `false` for right. + pub fn get_selected(&self) -> Option { + self.selected.get().copied() + } + /// Get which alternative got selected. `true` for left, `false` for right. + pub fn unwrap_selected(&self) -> bool { + self.get_selected() + .expect("imports should all have been resolved at this stage") + } + /// Set which alternative got selected. `true` for left, `false` for right. + pub fn set_selected(&self, selected: bool) { + let _ = self.selected.set(selected); } } -impl<'a, 'cx, T> Index<&'a T> for CtxtS<'cx> -where - Self: Index, - T: Copy, -{ - type Output = >::Output; - fn index(&self, id: &'a T) -> &Self::Output { - &self[*id] +impl<'cx> Ctxt<'cx> { + pub fn push_import_alternative( + self, + left_imports: Box<[ImportNode<'cx>]>, + right_imports: Box<[ImportNode<'cx>]>, + ) -> ImportAlternativeId<'cx> { + let stored = StoredImportAlternative { + left_imports, + right_imports, + selected: OnceCell::new(), + }; + let id = self.0.import_alternatives.len(); + self.0.import_alternatives.push(Box::new(stored)); + ImportAlternativeId(id, PhantomData) + } +} +impl<'cx> Index> for CtxtS<'cx> { + type Output = StoredImportAlternative<'cx>; + fn index( + &self, + id: ImportAlternativeId<'cx>, + ) -> &StoredImportAlternative<'cx> { + &self.import_alternatives[id.0] } } -/// Empty impl, because `FrozenVec` does not implement `Debug` and I can't be bothered to do it -/// myself. -impl<'cx> std::fmt::Debug for Ctxt<'cx> { - fn fmt(&self, _: &mut std::fmt::Formatter) -> std::fmt::Result { - Ok(()) +///////////////////////////////////////////////////////////////////////////////////////////////////// +// Import results + +#[derive(Debug, Copy, Clone, PartialEq, Eq, Hash)] +pub struct ImportResultId<'cx>(usize, PhantomData<&'cx ()>); + +type StoredImportResult<'cx> = Typed<'cx>; + +impl<'cx> Ctxt<'cx> { + /// Store the result of fetching an import. + pub fn push_import_result( + self, + res: StoredImportResult<'cx>, + ) -> ImportResultId<'cx> { + let id = self.0.import_results.len(); + self.0.import_results.push(Box::new(res)); + ImportResultId(id, PhantomData) + } +} +impl<'cx> Index> for CtxtS<'cx> { + type Output = StoredImportResult<'cx>; + fn index(&self, id: ImportResultId<'cx>) -> &StoredImportResult<'cx> { + &self.import_results[id.0] } } diff --git a/dhall/src/lib.rs b/dhall/src/lib.rs index c27b633..03d3931 100644 --- a/dhall/src/lib.rs +++ b/dhall/src/lib.rs @@ -26,7 +26,7 @@ use crate::semantics::resolve::ImportLocation; use crate::semantics::{typecheck, typecheck_with, Hir, Nir, Tir, Type}; use crate::syntax::Expr; -pub use ctxt::{Ctxt, ImportId, ImportResultId}; +pub use ctxt::*; #[derive(Debug, Clone)] pub struct Parsed(Expr, ImportLocation); diff --git a/dhall/src/semantics/nze/normalize.rs b/dhall/src/semantics/nze/normalize.rs index 0a09a80..59710d1 100644 --- a/dhall/src/semantics/nze/normalize.rs +++ b/dhall/src/semantics/nze/normalize.rs @@ -160,6 +160,14 @@ pub fn normalize_hir<'cx>(env: &NzEnv<'cx>, hir: &Hir<'cx>) -> NirKind<'cx> { let typed = env.cx()[import].unwrap_result(); normalize_hir(env, &typed.hir) } + HirKind::ImportAlternative(alt, left, right) => { + let hir = if env.cx()[alt].unwrap_selected() { + left + } else { + right + }; + normalize_hir(env, hir) + } HirKind::Expr(ExprKind::Lam(binder, annot, body)) => { let annot = annot.eval(env); NirKind::LamClosure { diff --git a/dhall/src/semantics/resolve/hir.rs b/dhall/src/semantics/resolve/hir.rs index 05a8550..8baad10 100644 --- a/dhall/src/semantics/resolve/hir.rs +++ b/dhall/src/semantics/resolve/hir.rs @@ -1,7 +1,7 @@ use crate::error::TypeError; use crate::semantics::{type_with, typecheck, NameEnv, Nir, NzEnv, Tir, TyEnv}; use crate::syntax::{Expr, ExprKind, Span, V}; -use crate::{Ctxt, ImportId, ToExprOptions}; +use crate::{Ctxt, ImportAlternativeId, ImportId, ToExprOptions}; /// Stores an alpha-normalized variable. #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] @@ -15,8 +15,10 @@ pub enum HirKind<'cx> { Var(AlphaVar), /// A variable that couldn't be resolved. Detected during resolution, but causes an error during typeck. MissingVar(V), - /// An import. It must have been resolved by the time we get to typechecking/normalization. + /// An import. It must have been resolved after resolution. Import(ImportId<'cx>), + /// An import alternative. It must have been decided after resolution. + ImportAlternative(ImportAlternativeId<'cx>, Hir<'cx>, Hir<'cx>), // Forbidden ExprKind variants: Var, Import, Completion Expr(ExprKind>), } @@ -111,6 +113,14 @@ fn hir_to_expr<'cx>( let typed = cx[import].unwrap_result(); return hir_to_expr(cx, &typed.hir, opts, &mut NameEnv::new()); } + HirKind::ImportAlternative(alt, left, right) => { + let hir = if cx[alt].unwrap_selected() { + left + } else { + right + }; + return hir_to_expr(cx, hir, opts, env); + } HirKind::Expr(e) => { let e = e.map_ref_maybe_binder(|l, hir| { if let Some(l) = l { diff --git a/dhall/src/semantics/resolve/resolve.rs b/dhall/src/semantics/resolve/resolve.rs index c4cd518..116e1a5 100644 --- a/dhall/src/semantics/resolve/resolve.rs +++ b/dhall/src/semantics/resolve/resolve.rs @@ -15,7 +15,10 @@ use crate::syntax::{ Expr, ExprKind, FilePath, FilePrefix, Hash, ImportMode, ImportTarget, Span, UnspannedExpr, URL, }; -use crate::{Ctxt, ImportId, ImportResultId, Parsed, Resolved, Typed}; +use crate::{ + Ctxt, ImportAlternativeId, ImportId, ImportResultId, Parsed, Resolved, + Typed, +}; // TODO: evaluate import headers pub type Import = syntax::Import<()>; @@ -252,7 +255,7 @@ impl ImportLocation { let typed = match self.mode { ImportMode::Code => { let parsed = self.kind.fetch_dhall()?; - let typed = resolve_with_env(env, parsed)?.typecheck(cx)?; + let typed = parsed.resolve_with_env(env)?.typecheck(cx)?; Typed { // TODO: manage to keep the Nir around. Will need fixing variables. hir: typed.normalize(cx).to_hir(), @@ -366,56 +369,6 @@ fn desugar(expr: &Expr) -> Cow<'_, Expr> { } } -/// Traverse the expression, handling import alternatives and passing -/// found imports to the provided function. Also resolving names. -fn traverse_resolve_expr<'cx>( - name_env: &mut NameEnv, - expr: &Expr, - f: &mut impl FnMut(Import, Span) -> Result, Error>, -) -> Result, Error> { - let expr = desugar(expr); - Ok(match expr.kind() { - ExprKind::Var(var) => match name_env.unlabel_var(&var) { - Some(v) => Hir::new(HirKind::Var(v), expr.span()), - None => Hir::new(HirKind::MissingVar(var.clone()), expr.span()), - }, - ExprKind::Op(OpKind::BinOp(BinOp::ImportAlt, l, r)) => { - match traverse_resolve_expr(name_env, l, f) { - Ok(l) => l, - Err(_) => { - match traverse_resolve_expr(name_env, r, f) { - Ok(r) => r, - // TODO: keep track of the other error too - Err(e) => return Err(e), - } - } - } - } - kind => { - let kind = kind.traverse_ref_maybe_binder(|l, e| { - if let Some(l) = l { - name_env.insert_mut(l); - } - let hir = traverse_resolve_expr(name_env, e, f)?; - if l.is_some() { - name_env.remove_mut(); - } - Ok::<_, Error>(hir) - })?; - let kind = match kind { - ExprKind::Import(import) => { - // TODO: evaluate import headers - let import = import.traverse_ref(|_| Ok::<_, Error>(()))?; - let import_id = f(import, expr.span())?; - HirKind::Import(import_id) - } - kind => HirKind::Expr(kind), - }; - Hir::new(kind, expr.span()) - } - }) -} - /// Fetch the import and store the result in the global context. fn fetch_import<'cx>( env: &mut ImportEnv<'cx>, @@ -469,22 +422,123 @@ fn fetch_import<'cx>( Ok(res_id) } +/// Part of a tree of imports. +#[derive(Debug, Clone, Copy)] +pub enum ImportNode<'cx> { + Import(ImportId<'cx>), + Alternative(ImportAlternativeId<'cx>), +} + +/// Traverse the expression and replace each import and import alternative by an id into the global +/// context. The ids are also accumulated into `nodes` so that we can resolve them afterwards. +fn traverse_accumulate<'cx>( + env: &mut ImportEnv<'cx>, + name_env: &mut NameEnv, + nodes: &mut Vec>, + base_location: &ImportLocation, + expr: &Expr, +) -> Hir<'cx> { + let cx = env.cx(); + let expr = desugar(expr); + let kind = match expr.kind() { + ExprKind::Var(var) => match name_env.unlabel_var(&var) { + Some(v) => HirKind::Var(v), + None => HirKind::MissingVar(var.clone()), + }, + ExprKind::Op(OpKind::BinOp(BinOp::ImportAlt, l, r)) => { + let mut imports_l = Vec::new(); + let l = traverse_accumulate( + env, + name_env, + &mut imports_l, + base_location, + l, + ); + let mut imports_r = Vec::new(); + let r = traverse_accumulate( + env, + name_env, + &mut imports_r, + base_location, + r, + ); + let alt = + cx.push_import_alternative(imports_l.into(), imports_r.into()); + nodes.push(ImportNode::Alternative(alt)); + HirKind::ImportAlternative(alt, l, r) + } + kind => { + let kind = kind.map_ref_maybe_binder(|l, e| { + if let Some(l) = l { + name_env.insert_mut(l); + } + let hir = + traverse_accumulate(env, name_env, nodes, base_location, e); + if l.is_some() { + name_env.remove_mut(); + } + hir + }); + match kind { + ExprKind::Import(import) => { + // TODO: evaluate import headers + let import = import.map_ref(|_| ()); + let import_id = cx.push_import( + base_location.clone(), + import, + expr.span(), + ); + nodes.push(ImportNode::Import(import_id)); + HirKind::Import(import_id) + } + kind => HirKind::Expr(kind), + } + } + }; + Hir::new(kind, expr.span()) +} + +/// Take a list of nodes and recursively resolve them. +fn resolve_nodes<'cx>( + env: &mut ImportEnv<'cx>, + nodes: &[ImportNode<'cx>], +) -> Result<(), Error> { + for &node in nodes { + match node { + ImportNode::Import(import) => { + let res_id = fetch_import(env, import)?; + env.cx()[import].set_resultid(res_id); + } + ImportNode::Alternative(alt) => { + let alt = &env.cx()[alt]; + if resolve_nodes(env, &alt.left_imports).is_ok() { + alt.set_selected(true); + } else { + resolve_nodes(env, &alt.right_imports)?; + alt.set_selected(false); + } + } + } + } + Ok(()) +} + fn resolve_with_env<'cx>( env: &mut ImportEnv<'cx>, parsed: Parsed, ) -> Result, Error> { let Parsed(expr, base_location) = parsed; - let resolved = traverse_resolve_expr( + let mut nodes = Vec::new(); + // First we collect all imports. + let resolved = traverse_accumulate( + env, &mut NameEnv::new(), + &mut nodes, + &base_location, &expr, - &mut |import, span| { - let import_id = - env.cx().push_import(base_location.clone(), import, span); - let res_id = fetch_import(env, import_id)?; - env.cx()[import_id].set_resultid(res_id); - Ok(import_id) - }, - )?; + ); + // Then we resolve them and choose sides for the alternatives. + resolve_nodes(env, &nodes)?; Ok(Resolved(resolved)) } @@ -494,10 +548,10 @@ pub fn resolve<'cx>( cx: Ctxt<'cx>, parsed: Parsed, ) -> Result, Error> { - resolve_with_env(&mut ImportEnv::new(cx), parsed) + parsed.resolve_with_env(&mut ImportEnv::new(cx)) } -/// Resolves names and errors if we find any imports. +/// Resolves names, and errors if we find any imports. pub fn skip_resolve<'cx>( cx: Ctxt<'cx>, parsed: Parsed, @@ -506,6 +560,15 @@ pub fn skip_resolve<'cx>( Ok(resolve(cx, parsed)?) } +impl Parsed { + fn resolve_with_env<'cx>( + self, + env: &mut ImportEnv<'cx>, + ) -> Result, Error> { + resolve_with_env(env, self) + } +} + pub trait Canonicalize { fn canonicalize(&self) -> Self; } diff --git a/dhall/src/semantics/tck/typecheck.rs b/dhall/src/semantics/tck/typecheck.rs index e14bcc6..23c2bd2 100644 --- a/dhall/src/semantics/tck/typecheck.rs +++ b/dhall/src/semantics/tck/typecheck.rs @@ -193,6 +193,14 @@ pub fn type_with<'cx, 'hir>( let typed = env.cx()[import].unwrap_result(); Tir::from_hir(hir, typed.ty.clone()) } + HirKind::ImportAlternative(alt, left, right) => { + let hir = if env.cx()[alt].unwrap_selected() { + left + } else { + right + }; + return type_with(env, hir, annot); + } HirKind::Expr(ExprKind::Var(_)) => { unreachable!("Hir should contain no unresolved variables") } -- cgit v1.2.3 From d0a61179addf838d33e7e7a4c6a13f06e871e9c5 Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Mon, 7 Dec 2020 19:30:04 +0000 Subject: Soothe clippy --- dhall/src/lib.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/dhall/src/lib.rs b/dhall/src/lib.rs index 03d3931..ad16a24 100644 --- a/dhall/src/lib.rs +++ b/dhall/src/lib.rs @@ -5,6 +5,7 @@ clippy::needless_lifetimes, clippy::new_ret_no_self, clippy::new_without_default, + clippy::try_err, clippy::useless_format )] -- cgit v1.2.3