mdebray/tvl-depot
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

refactor(tvix/eval): remove code and location from struct

Browse source 
Instead, it's passed in the evaluate/compile_only functions, which feels
more naturally. It lets us set up the Evaluation struct long before
we actually feed it with data to evaluate.

Now that Evaluation::new() would be accepting an empty list of
arguments, we can simply implement Default, making things a bit more
idiomatic.

Change-Id: I4369658634909a0c504fdffa18242a130daa0239
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10475
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>
Autosubmit: flokli <flokli@flokli.de>
This commit is contained in:
Florian Klink 2023-12-30 21:36:48 +01:00 committed by clbot
parent a5c5f1a29e
commit 4fba57c2c9
10 changed files with 72 additions and 69 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

4
corp/tvixbolt/src/main.rs
View file

@ -286,7 +286,7 @@ fn eval(model: &Model) -> Output {
return out;
}
let mut eval = tvix_eval::Evaluation::new(&model.code, Some("/nixbolt".into()));
let mut eval = tvix_eval::Evaluation::default();
let source = eval.source_map();
let result = {
@ -298,7 +298,7 @@ fn eval(model: &Model) -> Output {
eval.runtime_observer = Some(&mut runtime_observer);
}
eval.evaluate()
eval.evaluate(&model.code, Some("/nixbolt".into()))
};
if model.display_ast {

8
tvix/cli/src/main.rs
View file

@ -62,7 +62,7 @@ struct Args {
/// and the result itself. The return value indicates whether
/// evaluation succeeded.
fn interpret(code: &str, path: Option<PathBuf>, args: &Args, explain: bool) -> bool {
let mut eval = tvix_eval::Evaluation::new_impure(code, path);
let mut eval = tvix_eval::Evaluation::new_impure();
eval.strict = args.strict;
@ -101,7 +101,7 @@ fn interpret(code: &str, path: Option<PathBuf>, args: &Args, explain: bool) -> b
eval.runtime_observer = Some(&mut runtime_observer);
}
eval.evaluate()
eval.evaluate(code, path)
};
if args.display_ast {
@ -135,7 +135,7 @@ fn interpret(code: &str, path: Option<PathBuf>, args: &Args, explain: bool) -> b
/// Interpret the given code snippet, but only run the Tvix compiler
/// on it and return errors and warnings.
fn lint(code: &str, path: Option<PathBuf>, args: &Args) -> bool {
let mut eval = tvix_eval::Evaluation::new_impure(code, path);
let mut eval = tvix_eval::Evaluation::new_impure();
eval.strict = args.strict;
let source_map = eval.source_map();
@ -150,7 +150,7 @@ fn lint(code: &str, path: Option<PathBuf>, args: &Args) -> bool {
eprintln!("warning: --trace-runtime has no effect with --compile-only!");
}
let result = eval.compile_only();
let result = eval.compile_only(code, path);
if args.display_ast {
if let Some(ref expr) = result.expr {

89
tvix/eval/src/lib.rs
View file

@ -68,21 +68,10 @@ pub use crate::io::StdIO;
///
/// Public fields are intended to be set by the caller. Setting all
/// fields is optional.
pub struct Evaluation<'code, 'co, 'ro> {
/// The Nix source code to be evaluated.
code: &'code str,
/// Optional location of the source code (i.e. path to the file it was read
/// from). Used for error reporting, and for resolving relative paths in
/// impure functions.
location: Option<PathBuf>,
pub struct Evaluation<'co, 'ro> {
/// Source code map used for error reporting.
source_map: SourceCode,
/// Top-level file reference for this code inside the source map.
file: Arc<codemap::File>,
/// Set of all builtins that should be available during the
/// evaluation.
///
@ -142,27 +131,15 @@ pub struct EvaluationResult {
pub expr: Option<rnix::ast::Expr>,
}
impl<'code, 'co, 'ro> Evaluation<'code, 'co, 'ro> {
/// Initialise an `Evaluation` for the given Nix source code snippet, and
/// an optional code location.
pub fn new(code: &'code str, location: Option<PathBuf>) -> Self {
impl<'co, 'ro> Default for Evaluation<'co, 'ro> {
fn default() -> Self {
let source_map = SourceCode::default();
let location_str = location
.as_ref()
.map(|p| p.to_string_lossy().to_string())
.unwrap_or_else(|| "[code]".into());
let file = source_map.add_file(location_str, code.into());
let mut builtins = builtins::pure_builtins();
builtins.extend(builtins::placeholders()); // these are temporary
Evaluation {
code,
location,
Self {
source_map,
file,
builtins,
src_builtins: vec![],
io_handle: Box::new(DummyIO {}),
@ -173,15 +150,21 @@ impl<'code, 'co, 'ro> Evaluation<'code, 'co, 'ro> {
runtime_observer: None,
}
}
}
impl<'co, 'ro> Evaluation<'co, 'ro> {
#[cfg(feature = "impure")]
/// Initialise an `Evaluation` for the given snippet, with all
/// impure features turned on by default.
pub fn new_impure(code: &'code str, location: Option<PathBuf>) -> Self {
let mut eval = Self::new(code, location);
eval.enable_import = true;
pub fn new_impure() -> Self {
let mut eval = Self {
enable_import: true,
io_handle: Box::new(StdIO),
..Default::default()
};
eval.builtins.extend(builtins::impure_builtins());
eval.io_handle = Box::new(StdIO);
eval
}
@ -191,21 +174,30 @@ impl<'code, 'co, 'ro> Evaluation<'code, 'co, 'ro> {
self.source_map.clone()
}
/// Only compile the provided source code. This does not *run* the
/// code, it only provides analysis (errors and warnings) of the
/// compiler.
pub fn compile_only(mut self) -> EvaluationResult {
/// Only compile the provided source code, at an optional location of the
/// source code (i.e. path to the file it was read from; used for error
/// reporting, and for resolving relative paths in impure functions)
/// This does not *run* the code, it only provides analysis (errors and
/// warnings) of the compiler.
pub fn compile_only(mut self, code: &str, location: Option<PathBuf>) -> EvaluationResult {
let mut result = EvaluationResult::default();
let source = self.source_map();
let location_str = location
.as_ref()
.map(|p| p.to_string_lossy().to_string())
.unwrap_or_else(|| "[code]".into());
let file = source.add_file(location_str, code.into());
let mut noop_observer = observer::NoOpObserver::default();
let compiler_observer = self.compiler_observer.take().unwrap_or(&mut noop_observer);
parse_compile_internal(
&mut result,
self.code,
self.file.clone(),
self.location,
code,
file,
location,
source,
self.builtins,
self.src_builtins,
@ -216,11 +208,20 @@ impl<'code, 'co, 'ro> Evaluation<'code, 'co, 'ro> {
result
}
/// Evaluate the provided source code.
pub fn evaluate(mut self) -> EvaluationResult {
/// Evaluate the provided source code, at an optional location of the source
/// code (i.e. path to the file it was read from; used for error reporting,
/// and for resolving relative paths in impure functions)
pub fn evaluate(mut self, code: &str, location: Option<PathBuf>) -> EvaluationResult {
let mut result = EvaluationResult::default();
let source = self.source_map();
let location_str = location
.as_ref()
.map(|p| p.to_string_lossy().to_string())
.unwrap_or_else(|| "[code]".into());
let file = source.add_file(location_str, code.into());
let mut noop_observer = observer::NoOpObserver::default();
let compiler_observer = self.compiler_observer.take().unwrap_or(&mut noop_observer);
@ -231,9 +232,9 @@ impl<'code, 'co, 'ro> Evaluation<'code, 'co, 'ro> {
let (lambda, globals) = match parse_compile_internal(
&mut result,
self.code,
self.file.clone(),
self.location,
code,
file.clone(),
location,
source,
self.builtins,
self.src_builtins,
@ -255,7 +256,7 @@ impl<'code, 'co, 'ro> Evaluation<'code, 'co, 'ro> {
Err(err) => {
result.warnings.push(EvalWarning {
kind: WarningKind::InvalidNixPath(err.to_string()),
span: self.file.span,
span: file.span,
});
None
}

14
tvix/eval/src/tests/mod.rs
View file

@ -53,11 +53,11 @@ fn eval_test(code_path: &str, expect_success: bool) {
return;
}
let mut eval = crate::Evaluation::new_impure(&code, Some(code_path.into()));
let mut eval = crate::Evaluation::new_impure();
eval.strict = true;
eval.builtins.extend(mock_builtins::builtins());
let result = eval.evaluate();
let result = eval.evaluate(&code, Some(code_path.into()));
let failed = match result.value {
Some(Value::Catchable(_)) => true,
_ => !result.errors.is_empty(),
@ -106,11 +106,13 @@ fn eval_test(code_path: &str, expect_success: bool) {
fn identity(code_path: &str) {
let code = std::fs::read_to_string(code_path).expect("should be able to read test code");
let mut eval = crate::Evaluation::new(&code, None);
eval.strict = true;
eval.io_handle = Box::new(crate::StdIO);
let eval = crate::Evaluation {
strict: true,
io_handle: Box::new(crate::StdIO),
..Default::default()
};
let result = eval.evaluate();
let result = eval.evaluate(&code, None);
assert!(
result.errors.is_empty(),
"evaluation of identity test failed: {:?}",

6
tvix/eval/src/tests/one_offs.rs
View file

@ -5,10 +5,10 @@ fn test_source_builtin() {
// Test an evaluation with a source-only builtin. The test ensures
// that the artificially constructed thunking is correct.
let mut eval = Evaluation::new_impure("builtins.testSourceBuiltin", None);
let mut eval = Evaluation::new_impure();
eval.src_builtins.push(("testSourceBuiltin", "42"));
let result = eval.evaluate();
let result = eval.evaluate("builtins.testSourceBuiltin", None);
assert!(
result.errors.is_empty(),
"evaluation failed: {:?}",
@ -25,7 +25,7 @@ fn test_source_builtin() {
#[test]
fn skip_broken_bytecode() {
let result = Evaluation::new(/* code = */ "x", None).evaluate();
let result = Evaluation::default().evaluate(/* code = */ "x", None);
assert_eq!(result.errors.len(), 1);

4
tvix/eval/tests/nix_oracle.rs
View file

@ -51,12 +51,12 @@ fn nix_eval(expr: &str, strictness: Strictness) -> String {
#[track_caller]
fn compare_eval(expr: &str, strictness: Strictness) {
let nix_result = nix_eval(expr, strictness);
let mut eval = tvix_eval::Evaluation::new(expr, None);
let mut eval = tvix_eval::Evaluation::default();
eval.strict = matches!(strictness, Strictness::Strict);
eval.io_handle = Box::new(tvix_eval::StdIO);
let tvix_result = eval
.evaluate()
.evaluate(expr, None)
.value
.expect("tvix evaluation should succeed")
.to_string();

4
tvix/glue/benches/eval.rs
View file

@ -26,7 +26,7 @@ fn interpret(code: &str) {
// TODO: this is a bit annoying.
// It'd be nice if we could set this up once and then run evaluate() with a
// piece of code. b/262
let mut eval = tvix_eval::Evaluation::new_impure(code, None);
let mut eval = tvix_eval::Evaluation::new_impure();
let known_paths: Rc<RefCell<KnownPaths>> = Default::default();
add_derivation_builtins(&mut eval, known_paths.clone());
@ -47,7 +47,7 @@ fn interpret(code: &str) {
),
));
let result = eval.evaluate();
let result = eval.evaluate(code, None);
assert!(result.errors.is_empty());
}

4
tvix/glue/src/builtins/mod.rs
View file

@ -39,14 +39,14 @@ mod tests {
/// Takes care of setting up the evaluator so it knows about the
// `derivation` builtin.
fn eval(str: &str) -> EvaluationResult {
let mut eval = tvix_eval::Evaluation::new_impure(str, None);
let mut eval = tvix_eval::Evaluation::new_impure();
let known_paths: Rc<RefCell<KnownPaths>> = Default::default();
add_derivation_builtins(&mut eval, known_paths.clone());
// run the evaluation itself.
eval.evaluate()
eval.evaluate(str, None)
}
#[test]

4
tvix/glue/src/tvix_store_io.rs
View file

@ -375,7 +375,7 @@ mod tests {
/// Takes care of setting up the evaluator so it knows about the
// `derivation` builtin.
fn eval(str: &str) -> EvaluationResult {
let mut eval = tvix_eval::Evaluation::new_impure(str, None);
let mut eval = tvix_eval::Evaluation::new_impure();
let blob_service = Arc::new(MemoryBlobService::default());
let directory_service = Arc::new(MemoryDirectoryService::default());
@ -397,7 +397,7 @@ mod tests {
add_derivation_builtins(&mut eval, known_paths.clone());
// run the evaluation itself.
eval.evaluate()
eval.evaluate(str, None)
}
/// Helper function that takes a &Path, and invokes a tvix evaluator coercing that path to a string

4
tvix/serde/src/de.rs
View file

@ -46,12 +46,12 @@ where
F: FnOnce(&mut Evaluation),
{
// First step is to evaluate the Nix code ...
let mut eval = Evaluation::new(src, None);
let mut eval = Evaluation::default();
config(&mut eval);
eval.strict = true;
let source = eval.source_map();
let result = eval.evaluate();
let result = eval.evaluate(src, None);
if !result.errors.is_empty() {
return Err(Error::NixErrors {