Use contract elision for annotations as well

core/src/eval/merge.rs

@@ -32,7 +32,7 @@ use crate::label::{Label, MergeLabel};
 use crate::position::TermPos;
 use crate::term::{
     record::{self, Field, FieldDeps, FieldMetadata, RecordAttrs, RecordData},
-    BinaryOp, IndexMap, RichTerm, Term,
+    BinaryOp, IndexMap, RichTerm, Term, TypeAnnotation,
 };
 use crate::transform::Closurizable;
 
@@ -415,13 +415,19 @@ fn merge_fields<'a, C: Cache, I: DoubleEndedIterator<Item = &'a LocIdent> + Clon
     let mut pending_contracts = pending_contracts1.revert_closurize(cache, env_final, env1.clone());
 
     for ctr2 in pending_contracts2.revert_closurize(cache, env_final, env2.clone()) {
-        RuntimeContract::push_elide(initial_env, &mut pending_contracts, &env_final, ctr2, &env_final);
+        RuntimeContract::push_elide(
+            initial_env,
+            &mut pending_contracts,
+            &env_final,
+            ctr2,
+            &env_final,
+        );
     }
 
     Ok(Field {
         metadata: FieldMetadata {
             doc: merge_doc(metadata1.doc, metadata2.doc),
-            annotation: Combine::combine(metadata1.annotation, metadata2.annotation),
+            annotation: TypeAnnotation::combine_elide(metadata1.annotation, metadata2.annotation),
             // If one of the record requires this field, then it musn't be optional. The
             // resulting field is optional iff both are.
             opt: metadata1.opt && metadata2.opt,

core/src/eval/mod.rs

@@ -175,45 +175,43 @@ impl<R: ImportResolver, C: Cache> VirtualMachine<R, C> {
 
     /// Fully evaluate a Nickel term: the result is not a WHNF but to a value with all variables
     /// substituted.
-    pub fn eval_full(
-        &mut self,
-        t0: RichTerm,
-    ) -> Result<RichTerm, EvalError> {
-        self.eval_full_closure(Closure::atomic_closure(t0)).map(|(term, _)| term)
+    pub fn eval_full(&mut self, t0: RichTerm) -> Result<RichTerm, EvalError> {
+        self.eval_full_closure(Closure::atomic_closure(t0))
+            .map(|(term, _)| term)
     }
 
-    pub fn eval_full_closure(
-        &mut self,
-        t0: Closure,
-    ) -> Result<(RichTerm, Environment), EvalError> {
+    pub fn eval_full_closure(&mut self, t0: Closure) -> Result<(RichTerm, Environment), EvalError> {
         self.eval_deep_closure(t0, false)
             .map(|(term, env)| (subst(&self.cache, term, &self.initial_env, &env), env))
     }
 
     /// Like `eval_full`, but skips evaluating record fields marked `not_exported`.
-    pub fn eval_full_for_export(
-        &mut self,
-        t0: RichTerm,
-    ) -> Result<RichTerm, EvalError> {
+    pub fn eval_full_for_export(&mut self, t0: RichTerm) -> Result<RichTerm, EvalError> {
         self.eval_deep_closure(Closure::atomic_closure(t0), true)
             .map(|(term, env)| subst(&self.cache, term, &self.initial_env, &env))
     }
 
     /// Fully evaluates a Nickel term like `eval_full`, but does not substitute all variables.
-    pub fn eval_deep(
-        &mut self,
-        t0: RichTerm,
-    ) -> Result<RichTerm, EvalError> {
+    pub fn eval_deep(&mut self, t0: RichTerm) -> Result<RichTerm, EvalError> {
         self.eval_deep_closure(Closure::atomic_closure(t0), false)
             .map(|(term, _)| term)
     }
 
+    /// Use a specific initial environment for evaluation. Usually, [VirtualMachine::prepare_eval]
+    /// is populating the initial environment. But in some cases, such as testing or benchmarks, we
+    /// might want to use a different one.
+    ///
+    /// Return the new virtual machine with the updated initial environment.
+    pub fn with_initial_env(mut self, env: Environment) -> Self {
+        self.initial_env = env;
+        self
+    }
+
     fn eval_deep_closure(
         &mut self,
         mut closure: Closure,
         for_export: bool,
     ) -> Result<(RichTerm, Environment), EvalError> {
-
         closure.body = mk_term::op1(
             UnaryOp::Force {
                 ignore_not_exported: for_export,
@@ -230,14 +228,15 @@ impl<R: ImportResolver, C: Cache> VirtualMachine<R, C> {
     /// `baz`. The content of `baz` is evaluated as well, and variables are substituted, in order
     /// to obtain a value that can be printed. The metadata and the evaluated value are returned as
     /// a new field.
-    pub fn query(
-        &mut self,
-        t: RichTerm,
-        path: QueryPath,
-        initial_env: &Environment,
-    ) -> Result<Field, EvalError> {
-        let mut prev_pos = t.pos;
-        let (rt, mut env) = self.eval_closure(Closure::atomic_closure(t))?;
+    pub fn query(&mut self, t: RichTerm, path: QueryPath) -> Result<Field, EvalError> {
+        self.query_closure(Closure::atomic_closure(t), path)
+    }
+
+    /// Same as [VirtualMachine::query], but starts from a closure instead of a term in an empty
+    /// environment.
+    pub fn query_closure(&mut self, closure: Closure, path: QueryPath) -> Result<Field, EvalError> {
+        let mut prev_pos = closure.body.pos;
+        let (rt, mut env) = self.eval_closure(closure)?;
 
         let mut field: Field = rt.into();
 
@@ -277,12 +276,10 @@ impl<R: ImportResolver, C: Cache> VirtualMachine<R, C> {
                                     pending_contracts.into_iter(),
                                     prev_pos,
                                 );
-                                let (new_value, new_env) = self.eval_closure(
-                                    Closure {
-                                        body: value_with_ctr,
-                                        env: env.clone(),
-                                    }
-                                )?;
+                                let (new_value, new_env) = self.eval_closure(Closure {
+                                    body: value_with_ctr,
+                                    env: env.clone(),
+                                })?;
                                 env = new_env;
 
                                 Ok(new_value)
@@ -827,18 +824,29 @@ impl<R: ImportResolver, C: Cache> VirtualMachine<R, C> {
 }
 
 impl<C: Cache> VirtualMachine<ImportCache, C> {
+    /// Prepare the underlying program for evaluation (load the stdlib, typecheck, transform,
+    /// etc.). Sets the initial environment of the virtual machine.
     pub fn prepare_eval(&mut self, main_id: FileId) -> Result<RichTerm, Error> {
         let Envs {
             eval_env,
             type_ctxt,
-        }: Envs = self.import_resolver.prepare_stdlib(&mut self.cache)?;
+        } = self.import_resolver.prepare_stdlib(&mut self.cache)?;
         self.import_resolver.prepare(main_id, &type_ctxt)?;
         self.initial_env = eval_env;
         Ok(self.import_resolver().get(main_id).unwrap())
     }
 
+    /// Prepare the stdlib for evaluation. Sets the initial environment of the virtual machine. As
+    /// opposed to [VirtualMachine::prepare_eval], [VirtualMachine::prepare_stdlib] doesn't prepare
+    /// the main program yet (typechecking, transformations, etc.).
+    ///
+    /// # Returns
+    ///
+    /// The initial evaluation and typing environments, containing the stdlib items.
     pub fn prepare_stdlib(&mut self) -> Result<Envs, Error> {
-        self.import_resolver.prepare_stdlib(&mut self.cache)
+        let envs = self.import_resolver.prepare_stdlib(&mut self.cache)?;
+        self.initial_env = envs.eval_env.clone();
+        Ok(envs)
     }
 }
 

core/src/eval/tests.rs

@@ -14,7 +14,7 @@ use codespan::Files;
 /// Evaluate a term without import support.
 fn eval_no_import(t: RichTerm) -> Result<Term, EvalError> {
     VirtualMachine::<_, CacheImpl>::new(DummyResolver {}, std::io::sink())
-        .eval(t, &Environment::new())
+        .eval(t)
         .map(Term::from)
 }
 
@@ -169,9 +169,7 @@ fn imports() {
     let mk_import_two = mk_import("x", "two", mk_term::var("x"), &mut vm).unwrap();
     vm.reset();
     assert_eq!(
-        vm.eval(mk_import_two, &Environment::new(),)
-            .map(RichTerm::without_pos)
-            .unwrap(),
+        vm.eval(mk_import_two).map(RichTerm::without_pos).unwrap(),
         mk_term::integer(2)
     );
 
@@ -187,9 +185,7 @@ fn imports() {
     );
     vm.reset();
     assert_eq!(
-        vm.eval(mk_import_lib.unwrap(), &Environment::new(),)
-            .map(Term::from)
-            .unwrap(),
+        vm.eval(mk_import_lib.unwrap()).map(Term::from).unwrap(),
         Term::Bool(true)
     );
 }
@@ -269,15 +265,17 @@ fn initial_env() {
     let t = mk_term::let_in("x", mk_term::integer(2), mk_term::var("x"));
     assert_eq!(
         VirtualMachine::new_with_cache(DummyResolver {}, eval_cache.clone(), std::io::sink())
-            .eval(t, &initial_env)
+            .with_initial_env(initial_env.clone())
+            .eval(t)
             .map(RichTerm::without_pos),
         Ok(mk_term::integer(2))
     );
 
     let t = mk_term::let_in("x", mk_term::integer(2), mk_term::var("g"));
     assert_eq!(
         VirtualMachine::new_with_cache(DummyResolver {}, eval_cache.clone(), std::io::sink())
-            .eval(t, &initial_env)
+            .with_initial_env(initial_env.clone())
+            .eval(t)
             .map(RichTerm::without_pos),
         Ok(mk_term::integer(1))
     );
@@ -286,7 +284,8 @@ fn initial_env() {
     let t = mk_term::let_in("g", mk_term::integer(2), mk_term::var("g"));
     assert_eq!(
         VirtualMachine::new_with_cache(DummyResolver {}, eval_cache.clone(), std::io::sink())
-            .eval(t, &initial_env)
+            .with_initial_env(initial_env.clone())
+            .eval(t)
             .map(RichTerm::without_pos),
         Ok(mk_term::integer(2))
     );

core/src/program.rs

@@ -191,7 +191,7 @@ impl<EC: EvalCache> Program<EC> {
             .clone())
     }
 
-    /// Retrieve the parsed term and typecheck it, and generate a fresh initial environment. Return
+    /// Retrieve the parsed term, typecheck it, and generate a fresh initial environment. Return
     /// both.
     fn prepare_eval(&mut self) -> Result<RichTerm, Error> {
         self.vm.prepare_eval(self.main_id)
@@ -262,9 +262,7 @@ impl<EC: EvalCache> Program<EC> {
             }
         };
         self.vm.reset();
-        self.vm
-            .eval_full_for_export(t)
-            .map_err(|e| e.into())
+        self.vm.eval_full_for_export(t).map_err(|e| e.into())
     }
 
     /// Same as `eval_full`, but does not substitute all variables.
@@ -276,9 +274,10 @@ impl<EC: EvalCache> Program<EC> {
 
     /// Wrapper for [`query`].
     pub fn query(&mut self, path: Option<String>) -> Result<Field, Error> {
-        let initial_env = self.vm.prepare_stdlib()?;
+        let rt = self.prepare_eval()?;
         let query_path = QueryPath::parse_opt(self.vm.import_resolver_mut(), path)?;
-        query(&mut self.vm, self.main_id, &initial_env, query_path)
+
+        Ok(self.vm.query(rt, query_path)?)
     }
 
     /// Load, parse, and typecheck the program and the standard library, if not already done.
@@ -411,12 +410,10 @@ impl<EC: EvalCache> Program<EC> {
             current_env: Environment,
         ) -> Result<RichTerm, Error> {
             vm.reset();
-            let result = vm.eval_closure(
-                Closure {
-                    body: t.clone(),
-                    env: current_env,
-                },
-            );
+            let result = vm.eval_closure(Closure {
+                body: t.clone(),
+                env: current_env,
+            });
 
             // We expect to hit `MissingFieldDef` errors. When a configuration
             // contains undefined record fields they most likely will be used
@@ -488,7 +485,7 @@ impl<EC: EvalCache> Program<EC> {
         out: &mut impl std::io::Write,
         apply_transforms: bool,
     ) -> Result<(), Error> {
-        use crate::pretty::*;
+        use crate::{pretty::*, transform::transform};
         use pretty::BoxAllocator;
 
         let Program {
@@ -498,7 +495,7 @@ impl<EC: EvalCache> Program<EC> {
 
         let rt = vm.import_resolver().parse_nocache(*main_id)?.0;
         let rt = if apply_transforms {
-            crate::transform::transform(rt, None).unwrap()
+            transform(rt, None).unwrap()
         } else {
             rt
         };
@@ -508,29 +505,6 @@ impl<EC: EvalCache> Program<EC> {
     }
 }
 
-/// Query the metadata of a path of a term in the cache.
-///
-/// The path is a list of dot separated identifiers. For example, querying `{a = {b  = ..}}` (call
-/// it `exp`) with path `a.b` will evaluate `exp.a` and retrieve the `b` field. `b` is forced as
-/// well, in order to print its value (note that forced just means evaluated to a WHNF, it isn't
-/// deeply - or recursively - evaluated).
-//TODO: also gather type information, such that `query a.b.c <<< '{ ... } : {a: {b: {c: Num}}}`
-//would additionally report `type: Num` for example. Maybe use the LSP infrastructure?
-//TODO: not sure where this should go. It seems to embed too much logic to be in `Cache`, but is
-//common to both `Program` and `Repl`. Leaving it here as a stand-alone function for now
-pub fn query<EC: EvalCache>(
-    vm: &mut VirtualMachine<Cache, EC>,
-    file_id: FileId,
-    initial_env: &Envs,
-    path: QueryPath,
-) -> Result<Field, Error> {
-    vm.import_resolver_mut()
-        .prepare(file_id, &initial_env.type_ctxt)?;
-
-    let rt = vm.import_resolver().get_owned(file_id).unwrap();
-    Ok(vm.query(rt, path, &initial_env.eval_env)?)
-}
-
 #[cfg(feature = "doc")]
 mod doc {
     use crate::error::{Error, ExportError, IOError};

core/src/repl/mod.rs

@@ -189,7 +189,15 @@ impl<EC: EvalCache> ReplImpl<EC> {
         match term {
             ExtendedTerm::RichTerm(t) => {
                 let t = self.prepare(None, t)?;
-                Ok(eval_function(&mut self.vm, Closure { body: t, env: self.env.eval_env.clone() })?.0.into())
+                Ok(eval_function(
+                    &mut self.vm,
+                    Closure {
+                        body: t,
+                        env: self.env.eval_env.clone(),
+                    },
+                )?
+                .0
+                .into())
             }
             ExtendedTerm::ToplevelLet(id, t) => {
                 let t = self.prepare(Some(id), t)?;
@@ -223,9 +231,10 @@ impl<EC: EvalCache> Repl for ReplImpl<EC> {
 
         let term = self.prepare(None, term)?;
 
-        let (term, new_env) = self
-            .vm
-            .eval_closure(Closure { body: term, env: self.env.eval_env.clone()})?;
+        let (term, new_env) = self.vm.eval_closure(Closure {
+            body: term,
+            env: self.env.eval_env.clone(),
+        })?;
 
         if !matches!(term.as_ref(), Term::Record(..) | Term::RecRecord(..)) {
             return Err(Error::EvalError(EvalError::Other(
@@ -295,8 +304,6 @@ impl<EC: EvalCache> Repl for ReplImpl<EC> {
     }
 
     fn query(&mut self, path: String) -> Result<Field, Error> {
-        use crate::program;
-
         let mut query_path = QueryPath::parse(self.vm.import_resolver_mut(), path)?;
 
         // remove(): this is safe because there is no such thing as an empty field path. If `path`
@@ -309,7 +316,13 @@ impl<EC: EvalCache> Repl for ReplImpl<EC> {
             .import_resolver_mut()
             .replace_string(SourcePath::ReplQuery, target.label().into());
 
-        program::query(&mut self.vm, file_id, &self.env, query_path)
+        Ok(self.vm.query_closure(
+            Closure {
+                body: self.vm.import_resolver().get_owned(file_id).unwrap(),
+                env: self.env.eval_env.clone(),
+            },
+            query_path,
+        )?)
     }
 
     fn cache_mut(&mut self) -> &mut Cache {