Thread cx everywhere else imports are read

8 files changed, 80 insertions, 84 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<Parsed> for Expr {
         other.to_expr()
     }
 }
-impl<'cx> From<Normalized<'cx>> 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<Hash>, 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<Expr>) -> 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<Expr>) -> 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<Expr>) -> 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)?;
             }
         }