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 --- dhall/src/semantics/resolve/resolve.rs | 113 ++++++++++++++++++++------------- 1 file changed, 69 insertions(+), 44 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 16987de..da17c0a 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::{Parsed, Resolved, Typed}; +use crate::{Ctxt, ImportId, Parsed, Resolved, Typed}; // TODO: evaluate import headers pub type Import = syntax::Import<()>; @@ -227,7 +227,11 @@ impl ImportLocation { } /// Fetches the expression corresponding to this location. - fn fetch(&self, env: &mut ImportEnv, span: Span) -> Result { + fn fetch<'cx>( + &self, + env: &mut ImportEnv<'cx>, + span: Span, + ) -> Result { let (hir, ty) = match self.mode { ImportMode::Code => { let parsed = self.kind.fetch_dhall()?; @@ -386,8 +390,63 @@ fn traverse_resolve_expr( }) } -fn resolve_with_env( - env: &mut ImportEnv, +/// Fetch the import and store the result in the global context. +fn fetch_import<'cx>( + env: &mut ImportEnv<'cx>, + import_id: ImportId, +) -> Result<(), Error> { + let base_location = env.cx().get_import_base_location(import_id); + let import = env.cx().get_import(import_id); + let span = env.cx().get_import_span(import_id); + let location = base_location.chain(&import)?; + + // 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) { + env.cx().set_resultid_of_import(import_id, res_id); + // The same location may be used with different or no hashes. Thus we need to check + // the hashes every time. + let typed = env.cx().get_import_result(res_id); + check_hash(import, typed, span)?; + env.write_to_disk_cache(&import.hash, typed); + 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). + // This actually means that importing many times a same hashed import will take + // longer than importing many times a same non-hashed import. + env.cx().set_result_of_import(import_id, typed); + return Ok(()); + } + + // Resolve this import, making sure that recursive imports don't cycle back to the + // current one. + let res = env.with_cycle_detection(location.clone(), |env| { + location.fetch(env, 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 + let import = env.cx().get_import(import_id); + check_hash(import, &typed, span)?; + env.write_to_disk_cache(&import.hash, &typed); + let res_id = env.cx().set_result_of_import(import_id, typed); + env.write_to_mem_cache(location, res_id); + + Ok(()) +} + +fn resolve_with_env<'cx>( + env: &mut ImportEnv<'cx>, parsed: Parsed, ) -> Result { let Parsed(expr, base_location) = parsed; @@ -395,52 +454,18 @@ fn resolve_with_env( &mut NameEnv::new(), &expr, &mut |import, span| { - let location = base_location.chain(&import)?; - - // 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(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 res = env.with_cycle_detection(location.clone(), |env| { - location.fetch(env, 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)?; - env.write_to_disk_cache(&import.hash, &typed); - env.write_to_mem_cache(location, typed.clone()); - Ok(typed) + 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().get_result_of_import(import_id).unwrap().clone()) }, )?; Ok(Resolved(resolved)) } pub fn resolve(parsed: Parsed) -> Result { - resolve_with_env(&mut ImportEnv::new(), parsed) + Ctxt::with_new(|cx| resolve_with_env(&mut ImportEnv::new(cx), parsed)) } pub fn skip_resolve_expr(expr: &Expr) -> Result { -- cgit v1.2.3 From 9991bd4891774c4dd598decae02ee860554d2ab7 Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Sun, 6 Dec 2020 22:52:50 +0000 Subject: Improve ergonomics of `Ctxt` --- dhall/src/semantics/resolve/resolve.rs | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 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 da17c0a..955e2fa 100644 --- a/dhall/src/semantics/resolve/resolve.rs +++ b/dhall/src/semantics/resolve/resolve.rs @@ -395,18 +395,18 @@ fn fetch_import<'cx>( env: &mut ImportEnv<'cx>, import_id: ImportId, ) -> Result<(), Error> { - let base_location = env.cx().get_import_base_location(import_id); - let import = env.cx().get_import(import_id); - let span = env.cx().get_import_span(import_id); - let location = base_location.chain(&import)?; + let cx = env.cx(); + let import = &cx[import_id].import; + let span = cx[import_id].span.clone(); + let location = cx[import_id].base_location.chain(import)?; // 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) { - env.cx().set_resultid_of_import(import_id, res_id); + 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 = env.cx().get_import_result(res_id); + let typed = &cx[res_id]; check_hash(import, typed, span)?; env.write_to_disk_cache(&import.hash, typed); return Ok(()); @@ -417,7 +417,7 @@ fn fetch_import<'cx>( // 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. - env.cx().set_result_of_import(import_id, typed); + cx[import_id].set_result(typed); return Ok(()); } @@ -436,10 +436,9 @@ fn fetch_import<'cx>( }; // Add the resolved import to the caches - let import = env.cx().get_import(import_id); check_hash(import, &typed, span)?; env.write_to_disk_cache(&import.hash, &typed); - let res_id = env.cx().set_result_of_import(import_id, typed); + let res_id = cx[import_id].set_result(typed); env.write_to_mem_cache(location, res_id); Ok(()) @@ -458,7 +457,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().get_result_of_import(import_id).unwrap().clone()) + Ok(env.cx()[import_id].get_result().unwrap().clone()) }, )?; Ok(Resolved(resolved)) -- cgit v1.2.3 From 6287b7a7f9e421877ee13fefa586395fec844c99 Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Sun, 6 Dec 2020 21:41:03 +0000 Subject: Thread cx through typecheck --- dhall/src/semantics/resolve/resolve.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 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 955e2fa..cf72f80 100644 --- a/dhall/src/semantics/resolve/resolve.rs +++ b/dhall/src/semantics/resolve/resolve.rs @@ -250,7 +250,7 @@ impl ImportLocation { ImportMode::Location => { let expr = self.kind.to_location(); let hir = skip_resolve_expr(&expr)?; - let ty = hir.typecheck_noenv()?.ty().clone(); + let ty = hir.typecheck_noenv(env.cx())?.ty().clone(); (hir, ty) } }; @@ -463,8 +463,8 @@ fn resolve_with_env<'cx>( Ok(Resolved(resolved)) } -pub fn resolve(parsed: Parsed) -> Result { - Ctxt::with_new(|cx| resolve_with_env(&mut ImportEnv::new(cx), parsed)) +pub fn resolve(cx: Ctxt<'_>, parsed: Parsed) -> Result { + resolve_with_env(&mut ImportEnv::new(cx), parsed) } pub fn skip_resolve_expr(expr: &Expr) -> Result { -- cgit v1.2.3 From c785b7c0c6cd8b3b1cc15eb79caf982a757020ba Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Sun, 6 Dec 2020 23:55:21 +0000 Subject: Thread cx through normalization --- dhall/src/semantics/resolve/resolve.rs | 32 ++++++++++++++++++++------------ 1 file changed, 20 insertions(+), 12 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 cf72f80..488c516 100644 --- a/dhall/src/semantics/resolve/resolve.rs +++ b/dhall/src/semantics/resolve/resolve.rs @@ -231,12 +231,13 @@ impl ImportLocation { &self, env: &mut ImportEnv<'cx>, span: Span, - ) -> Result { + ) -> Result, Error> { let (hir, ty) = match self.mode { ImportMode::Code => { let parsed = self.kind.fetch_dhall()?; - let typed = resolve_with_env(env, parsed)?.typecheck()?; - let hir = typed.normalize().to_hir(); + let typed = + resolve_with_env(env, parsed)?.typecheck(env.cx())?; + let hir = typed.normalize(env.cx()).to_hir(); (hir, typed.ty) } ImportMode::RawText => { @@ -245,7 +246,7 @@ impl ImportLocation { HirKind::Expr(ExprKind::TextLit(text.into())), span, ); - (hir, Type::from_builtin(Builtin::Text)) + (hir, Type::from_builtin(env.cx(), Builtin::Text)) } ImportMode::Location => { let expr = self.kind.to_location(); @@ -286,7 +287,11 @@ fn make_aslocation_uniontype() -> Expr { mkexpr(ExprKind::UnionType(union)) } -fn check_hash(import: &Import, typed: &Typed, span: Span) -> Result<(), Error> { +fn check_hash<'cx>( + import: &Import, + typed: &Typed<'cx>, + span: Span, +) -> Result<(), Error> { if let (ImportMode::Code, Some(Hash::SHA256(hash))) = (import.mode, &import.hash) { @@ -338,11 +343,11 @@ 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( +fn traverse_resolve_expr<'cx>( name_env: &mut NameEnv, expr: &Expr, - f: &mut impl FnMut(Import, Span) -> Result, -) -> Result { + 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) { @@ -447,7 +452,7 @@ fn fetch_import<'cx>( fn resolve_with_env<'cx>( env: &mut ImportEnv<'cx>, parsed: Parsed, -) -> Result { +) -> Result, Error> { let Parsed(expr, base_location) = parsed; let resolved = traverse_resolve_expr( &mut NameEnv::new(), @@ -463,17 +468,20 @@ fn resolve_with_env<'cx>( Ok(Resolved(resolved)) } -pub fn resolve(cx: Ctxt<'_>, parsed: Parsed) -> Result { +pub fn resolve<'cx>( + cx: Ctxt<'cx>, + parsed: Parsed, +) -> Result, Error> { resolve_with_env(&mut ImportEnv::new(cx), parsed) } -pub fn skip_resolve_expr(expr: &Expr) -> Result { +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(parsed: Parsed) -> Result { +pub fn skip_resolve<'cx>(parsed: Parsed) -> Result, Error> { let Parsed(expr, _) = parsed; let resolved = skip_resolve_expr(&expr)?; Ok(Resolved(resolved)) -- 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/semantics/resolve/resolve.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (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 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/semantics/resolve/resolve.rs | 36 ++++++++++++++++++---------------- 1 file changed, 19 insertions(+), 17 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 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)) -- 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/resolve/resolve.rs | 9 ++++----- 1 file changed, 4 insertions(+), 5 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 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)) -- 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/semantics/resolve/resolve.rs | 69 ++++++++++++++++++++++------------ 1 file changed, 44 insertions(+), 25 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 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 { -- 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/resolve.rs | 30 +++++++++++++++--------------- 1 file changed, 15 insertions(+), 15 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 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/resolve/resolve.rs | 9 ++++----- 1 file changed, 4 insertions(+), 5 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 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, -- 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/semantics/resolve/resolve.rs | 189 ++++++++++++++++++++++----------- 1 file changed, 126 insertions(+), 63 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 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; } -- cgit v1.2.3