From 8a2de7537986b1d60b9e8ce3bc4c08e9ec6c489a Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Sun, 1 Nov 2020 14:53:38 +0000 Subject: Untangle ImportEnv::handle_import --- dhall/src/semantics/resolve/resolve.rs | 23 ++++++++++++++++------- 1 file changed, 16 insertions(+), 7 deletions(-) (limited to 'dhall/src/semantics/resolve/resolve.rs') diff --git a/dhall/src/semantics/resolve/resolve.rs b/dhall/src/semantics/resolve/resolve.rs index 2b401dc..f6d664e 100644 --- a/dhall/src/semantics/resolve/resolve.rs +++ b/dhall/src/semantics/resolve/resolve.rs @@ -224,12 +224,10 @@ fn resolve_one_import( env: &mut ImportEnv, cache: &Cache, import: &Import, - location: &ImportLocation, + location: ImportLocation, span: Span, ) -> Result { - let do_sanity_check = import.mode != ImportMode::Location; - let location = location.chain(&import.location, do_sanity_check)?; - env.handle_import(location.clone(), |env| match import.mode { + match import.mode { ImportMode::Code => { let (hir, ty) = cache.caching_import( import, @@ -278,7 +276,7 @@ fn resolve_one_import( let ty = hir.typecheck_noenv()?.ty().clone(); Ok((hir, ty)) } - }) + } } /// Desugar a `with` expression. @@ -396,12 +394,23 @@ fn resolve_with_env( cache: &Cache, parsed: Parsed, ) -> Result { - let Parsed(expr, location) = parsed; + let Parsed(expr, base_location) = parsed; let resolved = traverse_resolve_expr( &mut NameEnv::new(), &expr, &mut |import, span| { - resolve_one_import(env, cache, &import, &location, span) + let do_sanity_check = import.mode != ImportMode::Location; + let location = + base_location.chain(&import.location, do_sanity_check)?; + if let Some(expr) = env.get_from_cache(&location) { + return Ok(expr.clone()); + } + let expr = env.with_cycle_detection(location.clone(), |env| { + resolve_one_import(env, cache, &import, location.clone(), span) + })?; + // Add the resolved import to the cache + env.set_cache(location, expr.clone()); + Ok(expr) }, )?; Ok(Resolved(resolved)) -- cgit v1.2.3 From 2c245e7f5ce4019381e0fa47a1e2caf6276106be Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Sun, 1 Nov 2020 15:17:42 +0000 Subject: Store file cache in ImportEnv --- dhall/src/semantics/resolve/resolve.rs | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) (limited to 'dhall/src/semantics/resolve/resolve.rs') diff --git a/dhall/src/semantics/resolve/resolve.rs b/dhall/src/semantics/resolve/resolve.rs index f6d664e..8f8a9fe 100644 --- a/dhall/src/semantics/resolve/resolve.rs +++ b/dhall/src/semantics/resolve/resolve.rs @@ -10,7 +10,7 @@ use crate::builtins::Builtin; use crate::error::ErrorBuilder; use crate::error::{Error, ImportError}; use crate::operations::{BinOp, OpKind}; -use crate::semantics::{mkerr, Cache, Hir, HirKind, ImportEnv, NameEnv, Type}; +use crate::semantics::{mkerr, Hir, HirKind, ImportEnv, NameEnv, Type}; use crate::syntax; use crate::syntax::{ Expr, ExprKind, FilePath, FilePrefix, Hash, ImportMode, ImportTarget, @@ -222,19 +222,17 @@ fn make_aslocation_uniontype() -> Expr { fn resolve_one_import( env: &mut ImportEnv, - cache: &Cache, import: &Import, location: ImportLocation, span: Span, ) -> Result { match import.mode { ImportMode::Code => { - let (hir, ty) = cache.caching_import( + let (hir, ty) = env.file_cache.clone().caching_import( import, || location.fetch_dhall(), |parsed| { - let typed = - resolve_with_env(env, cache, parsed)?.typecheck()?; + let typed = resolve_with_env(env, parsed)?.typecheck()?; let hir = typed.normalize().to_hir(); Ok((hir, typed.ty)) }, @@ -391,7 +389,6 @@ fn traverse_resolve_expr( fn resolve_with_env( env: &mut ImportEnv, - cache: &Cache, parsed: Parsed, ) -> Result { let Parsed(expr, base_location) = parsed; @@ -406,7 +403,7 @@ fn resolve_with_env( return Ok(expr.clone()); } let expr = env.with_cycle_detection(location.clone(), |env| { - resolve_one_import(env, cache, &import, location.clone(), span) + resolve_one_import(env, &import, location.clone(), span) })?; // Add the resolved import to the cache env.set_cache(location, expr.clone()); @@ -417,8 +414,7 @@ fn resolve_with_env( } pub fn resolve(parsed: Parsed) -> Result { - let cache = Cache::new(); - resolve_with_env(&mut ImportEnv::new(), &cache, parsed) + resolve_with_env(&mut ImportEnv::new(), parsed) } pub fn skip_resolve_expr(expr: &Expr) -> Result { -- cgit v1.2.3 From 66ea301fc25a07485286560c434a9fdaf460c431 Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Sun, 1 Nov 2020 16:10:00 +0000 Subject: Untangle caching code --- dhall/src/semantics/resolve/resolve.rs | 86 +++++++++++++++++----------------- 1 file changed, 44 insertions(+), 42 deletions(-) (limited to 'dhall/src/semantics/resolve/resolve.rs') diff --git a/dhall/src/semantics/resolve/resolve.rs b/dhall/src/semantics/resolve/resolve.rs index 8f8a9fe..d6bc4cd 100644 --- a/dhall/src/semantics/resolve/resolve.rs +++ b/dhall/src/semantics/resolve/resolve.rs @@ -220,6 +220,32 @@ fn make_aslocation_uniontype() -> Expr { mkexpr(ExprKind::UnionType(union)) } +fn check_hash( + import: &Import, + typed: &TypedHir, + span: Span, +) -> Result<(), Error> { + match (import.mode, &import.hash) { + (ImportMode::Code, Some(Hash::SHA256(hash))) => { + let actual_hash = typed.0.to_expr_alpha().sha256_hash()?; + if hash[..] != actual_hash[..] { + mkerr( + ErrorBuilder::new("hash mismatch") + .span_err(span, "hash mismatch") + .note(format!("Expected sha256:{}", hex::encode(hash))) + .note(format!( + "Found sha256:{}", + hex::encode(actual_hash) + )) + .format(), + )? + } + } + _ => {} + } + Ok(()) +} + fn resolve_one_import( env: &mut ImportEnv, import: &Import, @@ -228,44 +254,15 @@ fn resolve_one_import( ) -> Result { match import.mode { ImportMode::Code => { - let (hir, ty) = env.file_cache.clone().caching_import( - import, - || location.fetch_dhall(), - |parsed| { - let typed = resolve_with_env(env, parsed)?.typecheck()?; - let hir = typed.normalize().to_hir(); - Ok((hir, typed.ty)) - }, - )?; - match &import.hash { - Some(Hash::SHA256(hash)) => { - let actual_hash = hir.to_expr_alpha().hash()?; - if hash[..] != actual_hash[..] { - mkerr( - ErrorBuilder::new("hash mismatch") - .span_err(span, "hash mismatch") - .note(format!( - "Expected sha256:{}", - hex::encode(hash) - )) - .note(format!( - "Found sha256:{}", - hex::encode(actual_hash) - )) - .format(), - )? - } - } - None => {} - } - Ok((hir, ty)) + let parsed = location.fetch_dhall()?; + let typed = resolve_with_env(env, parsed)?.typecheck()?; + let hir = typed.normalize().to_hir(); + Ok((hir, typed.ty)) } ImportMode::RawText => { let text = location.fetch_text()?; - let hir = Hir::new( - HirKind::Expr(ExprKind::TextLit(text.into())), - Span::Artificial, - ); + let hir = + Hir::new(HirKind::Expr(ExprKind::TextLit(text.into())), span); Ok((hir, Type::from_builtin(Builtin::Text))) } ImportMode::Location => { @@ -399,15 +396,20 @@ fn resolve_with_env( let do_sanity_check = import.mode != ImportMode::Location; let location = base_location.chain(&import.location, do_sanity_check)?; - if let Some(expr) = env.get_from_cache(&location) { - return Ok(expr.clone()); + // 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(resolved) = + env.get_from_cache(&location, import.hash.as_ref()) + { + return Ok(resolved); } - let expr = env.with_cycle_detection(location.clone(), |env| { - resolve_one_import(env, &import, location.clone(), span) + let typed = env.with_cycle_detection(location.clone(), |env| { + resolve_one_import(env, &import, location.clone(), span.clone()) })?; - // Add the resolved import to the cache - env.set_cache(location, expr.clone()); - Ok(expr) + check_hash(&import, &typed, span)?; + // Add the resolved import to the caches + env.set_cache(location, import.hash.as_ref(), typed.clone()); + Ok(typed) }, )?; Ok(Resolved(resolved)) -- cgit v1.2.3 From a4332cfbd99a5a00563090a080e9bcb0cde9e963 Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Sun, 1 Nov 2020 17:45:12 +0000 Subject: Typed and TypedHir are the same --- dhall/src/semantics/resolve/resolve.rs | 30 ++++++++++++------------------ 1 file changed, 12 insertions(+), 18 deletions(-) (limited to 'dhall/src/semantics/resolve/resolve.rs') diff --git a/dhall/src/semantics/resolve/resolve.rs b/dhall/src/semantics/resolve/resolve.rs index d6bc4cd..002490b 100644 --- a/dhall/src/semantics/resolve/resolve.rs +++ b/dhall/src/semantics/resolve/resolve.rs @@ -16,14 +16,11 @@ use crate::syntax::{ Expr, ExprKind, FilePath, FilePrefix, Hash, ImportMode, ImportTarget, Label, Span, UnspannedExpr, URL, }; -use crate::{Parsed, Resolved}; +use crate::{Parsed, Resolved, Typed}; // TODO: evaluate import headers pub type Import = syntax::Import<()>; -/// Owned Hir with a type. Different from Tir because the Hir is owned. -pub type TypedHir = (Hir, Type); - /// The location of some data, usually some dhall code. #[derive(Debug, Clone, PartialEq, Eq, Hash)] pub enum ImportLocation { @@ -220,14 +217,10 @@ fn make_aslocation_uniontype() -> Expr { mkexpr(ExprKind::UnionType(union)) } -fn check_hash( - import: &Import, - typed: &TypedHir, - span: Span, -) -> Result<(), Error> { +fn check_hash(import: &Import, typed: &Typed, span: Span) -> Result<(), Error> { match (import.mode, &import.hash) { (ImportMode::Code, Some(Hash::SHA256(hash))) => { - let actual_hash = typed.0.to_expr_alpha().sha256_hash()?; + let actual_hash = typed.hir.to_expr_alpha().sha256_hash()?; if hash[..] != actual_hash[..] { mkerr( ErrorBuilder::new("hash mismatch") @@ -251,27 +244,28 @@ fn resolve_one_import( import: &Import, location: ImportLocation, span: Span, -) -> Result { - match import.mode { +) -> Result { + let (hir, ty) = match import.mode { ImportMode::Code => { let parsed = location.fetch_dhall()?; let typed = resolve_with_env(env, parsed)?.typecheck()?; let hir = typed.normalize().to_hir(); - Ok((hir, typed.ty)) + (hir, typed.ty) } ImportMode::RawText => { let text = location.fetch_text()?; let hir = Hir::new(HirKind::Expr(ExprKind::TextLit(text.into())), span); - Ok((hir, Type::from_builtin(Builtin::Text))) + (hir, Type::from_builtin(Builtin::Text)) } ImportMode::Location => { let expr = location.into_location(); let hir = skip_resolve_expr(&expr)?; let ty = hir.typecheck_noenv()?.ty().clone(); - Ok((hir, ty)) + (hir, ty) } - } + }; + Ok(Typed { hir, ty }) } /// Desugar a `with` expression. @@ -335,7 +329,7 @@ fn desugar(expr: &Expr) -> Cow<'_, Expr> { fn traverse_resolve_expr( name_env: &mut NameEnv, expr: &Expr, - f: &mut impl FnMut(Import, Span) -> Result, + f: &mut impl FnMut(Import, Span) -> Result, ) -> Result { let expr = desugar(expr); Ok(match expr.kind() { @@ -375,7 +369,7 @@ fn traverse_resolve_expr( // TODO: evaluate import headers let import = import.traverse_ref(|_| Ok::<_, Error>(()))?; let imported = f(import, expr.span())?; - HirKind::Import(imported.0, imported.1) + HirKind::Import(imported.hir, imported.ty) } kind => HirKind::Expr(kind), }; -- cgit v1.2.3 From e5381c9b76f1d88dedb4a453cd026c8e98be5533 Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Sun, 1 Nov 2020 18:32:33 +0000 Subject: Ensure that the hash always gets checked --- dhall/src/semantics/resolve/resolve.rs | 28 ++++++++++++++++++++++------ 1 file changed, 22 insertions(+), 6 deletions(-) (limited to 'dhall/src/semantics/resolve/resolve.rs') diff --git a/dhall/src/semantics/resolve/resolve.rs b/dhall/src/semantics/resolve/resolve.rs index 002490b..614ea22 100644 --- a/dhall/src/semantics/resolve/resolve.rs +++ b/dhall/src/semantics/resolve/resolve.rs @@ -390,19 +390,35 @@ fn resolve_with_env( let do_sanity_check = import.mode != ImportMode::Location; let location = base_location.chain(&import.location, do_sanity_check)?; + // 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(resolved) = - env.get_from_cache(&location, import.hash.as_ref()) - { - return Ok(resolved); + if let Some(typed) = env.get_from_mem_cache(&location) { + // The same location may be used with different or no hashes. Thus we need to check + // the hashes every time. + check_hash(&import, &typed, span)?; + env.write_to_disk_cache(&import.hash, &typed); + return Ok(typed); + } + 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). + // This actually means that importing many times a same hashed import will take + // longer than importing many times a same non-hashed import. + return Ok(typed); } + + // Resolve this import, making sure that recursive imports don't cycle back to the + // current one. let typed = env.with_cycle_detection(location.clone(), |env| { resolve_one_import(env, &import, location.clone(), span.clone()) })?; - check_hash(&import, &typed, span)?; + // Add the resolved import to the caches - env.set_cache(location, import.hash.as_ref(), typed.clone()); + check_hash(&import, &typed, span)?; + env.write_to_disk_cache(&import.hash, &typed); + env.write_to_mem_cache(location, typed.clone()); Ok(typed) }, )?; -- cgit v1.2.3 From 8cfb6d563d26abb2fa80124eda2231eb05c7f3e6 Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Sun, 1 Nov 2020 18:59:01 +0000 Subject: feat: add location to import error messages This is crude but helpful. See #40 --- dhall/src/semantics/resolve/resolve.rs | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) (limited to 'dhall/src/semantics/resolve/resolve.rs') diff --git a/dhall/src/semantics/resolve/resolve.rs b/dhall/src/semantics/resolve/resolve.rs index 614ea22..68b899c 100644 --- a/dhall/src/semantics/resolve/resolve.rs +++ b/dhall/src/semantics/resolve/resolve.rs @@ -411,9 +411,17 @@ fn resolve_with_env( // Resolve this import, making sure that recursive imports don't cycle back to the // current one. - let typed = env.with_cycle_detection(location.clone(), |env| { + let res = env.with_cycle_detection(location.clone(), |env| { resolve_one_import(env, &import, location.clone(), span.clone()) - })?; + }); + let typed = match res { + Ok(typed) => typed, + Err(e) => mkerr( + ErrorBuilder::new("error") + .span_err(span.clone(), e.to_string()) + .format(), + )?, + }; // Add the resolved import to the caches check_hash(&import, &typed, span)?; -- cgit v1.2.3 From 34d92560a0a2124e5eadea4832795874505b6cc5 Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Sun, 1 Nov 2020 19:03:21 +0000 Subject: fix: clippy --- dhall/src/semantics/resolve/resolve.rs | 31 +++++++++++++++---------------- 1 file changed, 15 insertions(+), 16 deletions(-) (limited to 'dhall/src/semantics/resolve/resolve.rs') diff --git a/dhall/src/semantics/resolve/resolve.rs b/dhall/src/semantics/resolve/resolve.rs index 68b899c..572df25 100644 --- a/dhall/src/semantics/resolve/resolve.rs +++ b/dhall/src/semantics/resolve/resolve.rs @@ -218,23 +218,22 @@ fn make_aslocation_uniontype() -> Expr { } fn check_hash(import: &Import, typed: &Typed, span: Span) -> Result<(), Error> { - match (import.mode, &import.hash) { - (ImportMode::Code, Some(Hash::SHA256(hash))) => { - let actual_hash = typed.hir.to_expr_alpha().sha256_hash()?; - if hash[..] != actual_hash[..] { - mkerr( - ErrorBuilder::new("hash mismatch") - .span_err(span, "hash mismatch") - .note(format!("Expected sha256:{}", hex::encode(hash))) - .note(format!( - "Found sha256:{}", - hex::encode(actual_hash) - )) - .format(), - )? - } + if let (ImportMode::Code, Some(Hash::SHA256(hash))) = + (import.mode, &import.hash) + { + let actual_hash = typed.hir.to_expr_alpha().sha256_hash()?; + if hash[..] != actual_hash[..] { + mkerr( + ErrorBuilder::new("hash mismatch") + .span_err(span, "hash mismatch") + .note(format!("Expected sha256:{}", hex::encode(hash))) + .note(format!( + "Found sha256:{}", + hex::encode(actual_hash) + )) + .format(), + )? } - _ => {} } Ok(()) } -- cgit v1.2.3 From 6987b275e4bf5f545d823d186ce08a2fe9a3eb44 Mon Sep 17 00:00:00 2001 From: Basile Henry Date: Sun, 1 Nov 2020 22:46:35 +0100 Subject: Implement type checking for With op --- dhall/src/semantics/resolve/resolve.rs | 32 ++------------------------------ 1 file changed, 2 insertions(+), 30 deletions(-) (limited to 'dhall/src/semantics/resolve/resolve.rs') diff --git a/dhall/src/semantics/resolve/resolve.rs b/dhall/src/semantics/resolve/resolve.rs index 572df25..dc1951e 100644 --- a/dhall/src/semantics/resolve/resolve.rs +++ b/dhall/src/semantics/resolve/resolve.rs @@ -2,7 +2,6 @@ use itertools::Itertools; use std::borrow::Cow; use std::collections::BTreeMap; use std::env; -use std::iter::once; use std::path::PathBuf; use url::Url; @@ -13,8 +12,8 @@ use crate::operations::{BinOp, OpKind}; use crate::semantics::{mkerr, Hir, HirKind, ImportEnv, NameEnv, Type}; use crate::syntax; use crate::syntax::{ - Expr, ExprKind, FilePath, FilePrefix, Hash, ImportMode, ImportTarget, - Label, Span, UnspannedExpr, URL, + Expr, ExprKind, FilePath, FilePrefix, Hash, ImportMode, ImportTarget, Span, + UnspannedExpr, URL, }; use crate::{Parsed, Resolved, Typed}; @@ -267,30 +266,6 @@ fn resolve_one_import( Ok(Typed { hir, ty }) } -/// Desugar a `with` expression. -fn desugar_with(x: Expr, path: &[Label], y: Expr, span: Span) -> Expr { - use crate::operations::BinOp::RightBiasedRecordMerge; - use ExprKind::{Op, RecordLit}; - use OpKind::{BinOp, Field}; - let expr = |k| Expr::new(k, span.clone()); - match path { - [] => y, - [l, rest @ ..] => { - let res = desugar_with( - expr(Op(Field(x.clone(), l.clone()))), - rest, - y, - span.clone(), - ); - expr(Op(BinOp( - RightBiasedRecordMerge, - x, - expr(RecordLit(once((l.clone(), res)).collect())), - ))) - } - } -} - /// Desugar the first level of the expression. fn desugar(expr: &Expr) -> Cow<'_, Expr> { match expr.kind() { @@ -316,9 +291,6 @@ fn desugar(expr: &Expr) -> Cow<'_, Expr> { expr.span(), )) } - ExprKind::Op(OpKind::With(x, path, y)) => { - Cow::Owned(desugar_with(x.clone(), path, y.clone(), expr.span())) - } _ => Cow::Borrowed(expr), } } -- cgit v1.2.3