diff --git a/.github/workflows/build-test.yml b/.github/workflows/build-test.yml index d21905ce2..6fa0cc7fd 100644 --- a/.github/workflows/build-test.yml +++ b/.github/workflows/build-test.yml @@ -152,7 +152,7 @@ jobs: pip install --no-index --find-links target/wheels/ clvm_tools_rs pip install clvm_rs pip install clvm_tools - cd resources/tests && python test_clvm_step.py && python mandelbrot-cldb.py + cd resources/tests && python test_clvm_step.py && python mandelbrot-cldb.py && python test_compile_from_string.py - name: "Test step run with mandelbrot, setting print only" run: | diff --git a/resources/tests/test_compile_from_string.py b/resources/tests/test_compile_from_string.py new file mode 100644 index 000000000..f067b11ba --- /dev/null +++ b/resources/tests/test_compile_from_string.py @@ -0,0 +1,88 @@ +import binascii +import clvm_tools_rs +from pathlib import Path + +classic_code = """ +(mod (N X) + (defun F (N X) (if N (sha256 (F (- N 1) X)) X)) + (F N X) + ) +""" +expected_classic = "ff02ffff01ff02ff02ffff04ff02ffff04ff05ffff04ff0bff8080808080ffff04ffff01ff02ffff03ff05ffff01ff0bffff02ff02ffff04ff02ffff04ffff11ff05ffff010180ffff04ff0bff808080808080ffff010b80ff0180ff018080" + +cl23_code = """ +(mod (N X) + (include *standard-cl-23*) + (defun F (N X) (if N (sha256 (F (- N 1) X)) X)) + (F N X) + ) +""" +expected_cl23 = "ff02ffff01ff02ff02ffff04ff02ffff04ff05ffff04ff0bff8080808080ffff04ffff01ff02ffff03ff05ffff01ff0bffff02ff02ffff04ff02ffff04ffff11ff05ffff010180ffff04ff0bff808080808080ffff010b80ff0180ff018080" + +expected_deinline = "ff02ffff01ff02ff02ffff04ff02ffff04ff03ffff04ffff10ff05ffff01830f424080ff8080808080ffff04ffff01ff10ff0bff0bff0bff0bff0bff0b80ff018080" + +classic_error = "(mod (X) (xxx X))" +cl23_error = "(mod (X) (include *standard-cl-23*) (+ X X1))" + +compiled_code = clvm_tools_rs.compile( + classic_code, + ["."], + True +) +# Classic outputs symbols to the filesystem down all routes, which likely should +# be fixed. +assert compiled_code["output"] == expected_classic + +compiled_code = clvm_tools_rs.compile( + classic_code, + ["."] +) +assert compiled_code == expected_classic + +# Verify modern compilation +compiled_code = clvm_tools_rs.compile( + cl23_code, + ["."], + True +) +symbols = compiled_code["symbols"] +assert symbols["__chia__main_arguments"] == "(N X)" +assert symbols["30960d7f2ddc7188a6428a11d39a13ff70d308e6cc571ffb6ed5ec8dbe4376c0_arguments"] == "(N X)" +assert symbols["30960d7f2ddc7188a6428a11d39a13ff70d308e6cc571ffb6ed5ec8dbe4376c0"] == "F" +assert compiled_code["output"] == expected_cl23 + +# Check compilation with a path +test_path = Path(__file__).parent + +output_file = "simple_deinline_case_23.hex" +compiled_code = clvm_tools_rs.compile_clvm( + str(test_path / "simple_deinline_case_23.clsp"), + output_file, + ["."], + True +) +assert compiled_code["symbols"]["d623cecd87575189eb1518b50cecc8944a51aa6f4bb4cf6419f70e4aa34f5a20"].startswith("letbinding") +assert compiled_code["output"] == output_file +assert open(output_file).read().strip() == expected_deinline + +# Check dependency output +game_referee = Path(__file__).parent / "game-referee-in-cl23" +dependencies = clvm_tools_rs.check_dependencies( + str(game_referee / "test_reverse.clsp"), + [str(game_referee)] +) +assert len(dependencies) == 1 +assert Path(dependencies[0]) == game_referee / 'reverse.clinc' + +# Better error reporting +try: + clvm_tools_rs.compile(classic_error, []) + assert False +except Exception as e: + assert e.args[0] == "error can't compile (\"xxx\" 88), unknown operator compiling (\"xxx\" 88)" + +try: + clvm_tools_rs.compile(cl23_error, []) + assert False +except Exception as e: + assert e.args[0] == "*inline*(1):42-*inline*(1):44: Unbound use of X1 as a variable name" diff --git a/src/classic/clvm_tools/clvmc.rs b/src/classic/clvm_tools/clvmc.rs index 1ef267e4a..9db074e35 100644 --- a/src/classic/clvm_tools/clvmc.rs +++ b/src/classic/clvm_tools/clvmc.rs @@ -1,11 +1,7 @@ use std::collections::HashMap; use std::fs; -use std::io::Write; -use std::path::Path; use std::rc::Rc; -use tempfile::NamedTempFile; - use clvm_rs::allocator::{Allocator, NodePtr}; use clvm_rs::reduction::EvalErr; @@ -26,6 +22,52 @@ use crate::compiler::comptypes::{CompileErr, CompilerOpts}; use crate::compiler::dialect::detect_modern; use crate::compiler::optimize::maybe_finalize_program_via_classic_optimizer; use crate::compiler::runtypes::RunFailure; +use crate::compiler::srcloc::Srcloc; +use crate::util::gentle_overwrite; + +#[derive(Debug, Clone)] +pub enum CompileError { + Modern(Srcloc, String), + Classic(NodePtr, String), +} + +impl From for CompileError { + fn from(e: EvalErr) -> Self { + CompileError::Classic(e.0, e.1) + } +} + +impl From for CompileError { + fn from(r: CompileErr) -> Self { + CompileError::Modern(r.0, r.1) + } +} + +impl From for CompileError { + fn from(r: RunFailure) -> Self { + match r { + RunFailure::RunErr(l, x) => CompileError::Modern(l, x), + RunFailure::RunExn(l, x) => CompileError::Modern(l, x.to_string()), + } + } +} + +impl CompileError { + pub fn format(&self, allocator: &Allocator, opts: Rc) -> String { + match self { + CompileError::Classic(node, message) => { + format!( + "error {} compiling {}", + message, + disassemble(allocator, *node, opts.disassembly_ver()) + ) + } + CompileError::Modern(loc, message) => { + format!("{}: {}", loc, message) + } + } + } +} pub fn write_sym_output( compiled_lookup: &HashMap, @@ -47,7 +89,7 @@ pub fn compile_clvm_text_maybe_opt( text: &str, input_path: &str, classic_with_opts: bool, -) -> Result { +) -> Result { let ir_src = read_ir(text).map_err(|s| EvalErr(allocator.null(), s.to_string()))?; let assembled_sexp = assemble_from_ir(allocator, Rc::new(ir_src))?; @@ -64,18 +106,16 @@ pub fn compile_clvm_text_maybe_opt( .set_optimize(do_optimize || stepping > 22) // Would apply to cl23 .set_frontend_opt(stepping == 22); - let unopt_res = compile_file(allocator, runner.clone(), opts.clone(), text, symbol_table); - let res = unopt_res.and_then(|x| { - maybe_finalize_program_via_classic_optimizer(allocator, runner, opts, do_optimize, &x) - }); - - res.and_then(|x| { - convert_to_clvm_rs(allocator, x).map_err(|r| match r { - RunFailure::RunErr(l, x) => CompileErr(l, x), - RunFailure::RunExn(l, x) => CompileErr(l, x.to_string()), - }) - }) - .map_err(|s| EvalErr(allocator.null(), s.1)) + let unopt_res = compile_file(allocator, runner.clone(), opts.clone(), text, symbol_table)?; + let res = maybe_finalize_program_via_classic_optimizer( + allocator, + runner, + opts, + do_optimize, + &unopt_res, + )?; + + Ok(convert_to_clvm_rs(allocator, res)?) } else { let compile_invoke_code = run(allocator); let input_sexp = allocator.new_pair(assembled_sexp, allocator.null())?; @@ -96,7 +136,7 @@ pub fn compile_clvm_text( text: &str, input_path: &str, classic_with_opts: bool, -) -> Result { +) -> Result { compile_clvm_text_maybe_opt( allocator, true, @@ -125,13 +165,7 @@ pub fn compile_clvm_inner( filename, classic_with_opts, ) - .map_err(|x| { - format!( - "error {} compiling {}", - x.1, - disassemble(allocator, x.0, opts.disassembly_ver()) - ) - })?; + .map_err(|e| e.format(allocator, opts))?; sexp_to_stream(allocator, result, result_stream); Ok(()) } @@ -162,55 +196,12 @@ pub fn compile_clvm( false, )?; - let target_data = result_stream.get_value().hex(); - - let write_file = |output_path: &str, target_data: &str| -> Result<(), String> { - let output_path_obj = Path::new(output_path); - let output_dir = output_path_obj - .parent() - .map(Ok) - .unwrap_or_else(|| Err("could not get parent of output path"))?; - - // Make the contents appear atomically so that other test processes - // won't mistake an empty file for intended output. - let mut temp_output_file = NamedTempFile::new_in(output_dir).map_err(|e| { - format!("error creating temporary compiler output for {input_path}: {e:?}") - })?; - - let err_text = format!("failed to write to {:?}", temp_output_file.path()); - let translate_err = |_| err_text.clone(); - - temp_output_file - .write_all(target_data.as_bytes()) - .map_err(translate_err)?; - - temp_output_file.write_all(b"\n").map_err(translate_err)?; - - temp_output_file.persist(output_path).map_err(|e| { - format!("error persisting temporary compiler output {output_path}: {e:?}") - })?; - - Ok(()) - }; + let mut target_data = result_stream.get_value().hex(); + target_data += "\n"; // Try to detect whether we'd put the same output in the output file. // Don't proceed if true. - if let Ok(prev_content) = fs::read_to_string(output_path) { - let prev_trimmed = prev_content.trim(); - let trimmed = target_data.trim(); - if prev_trimmed == trimmed { - // We should try to overwrite here, but not fail if it doesn't - // work. This will accomodate both the read only scenario and - // the scenario where a target file is newer and people want the - // date to be updated. - write_file(output_path, &target_data).ok(); - - // It's the same program, bail regardless. - return Ok(output_path.to_string()); - } - } - - write_file(output_path, &target_data)?; + gentle_overwrite(input_path, output_path, &target_data)?; } Ok(output_path.to_string()) diff --git a/src/py/api.rs b/src/py/api.rs index 2a4722548..e78b3debb 100644 --- a/src/py/api.rs +++ b/src/py/api.rs @@ -15,8 +15,11 @@ use std::sync::mpsc::{Receiver, Sender}; use std::thread; use clvm_rs::allocator::Allocator; +use clvm_rs::serde::node_to_bytes; -use crate::classic::clvm::__type_compatibility__::{Bytes, Stream, UnvalidatedBytesFromType}; +use crate::classic::clvm::__type_compatibility__::{ + Bytes, BytesFromType, Stream, UnvalidatedBytesFromType, +}; use crate::classic::clvm::serialize::sexp_to_stream; use crate::classic::clvm_tools::clvmc; use crate::classic::clvm_tools::cmds; @@ -35,7 +38,7 @@ use crate::compiler::runtypes::RunFailure; use crate::compiler::sexp::{decode_string, SExp}; use crate::compiler::srcloc::Srcloc; -use crate::util::version; +use crate::util::{gentle_overwrite, version}; use crate::py::pyval::{clvm_value_to_python, python_value_to_clvm}; @@ -50,6 +53,119 @@ fn get_version() -> PyResult { Ok(version()) } +enum CompileClvmSource<'a> { + SourcePath(&'a PyAny), + SourceCode(String, String), +} + +enum CompileClvmAction { + CheckDependencies, + CompileCode(Option), +} + +fn get_source_from_input(input_code: CompileClvmSource) -> PyResult<(String, String)> { + match input_code { + CompileClvmSource::SourcePath(input_path) => { + let has_atom = input_path.hasattr("atom")?; + let has_pair = input_path.hasattr("pair")?; + + let real_input_path = if has_atom { + input_path.getattr("atom").and_then(|x| x.str()) + } else if has_pair { + input_path + .getattr("pair") + .and_then(|x| x.get_item(0)) + .and_then(|x| x.str()) + } else { + input_path.extract() + }?; + + let mut path_string = real_input_path.to_string(); + + if !std::path::Path::new(&path_string).exists() && !path_string.ends_with(".clvm") { + path_string += ".clvm"; + } + + let file_data = fs::read_to_string(&path_string) + .map_err(|e| PyException::new_err(format!("error reading {path_string}: {e:?}")))?; + Ok((path_string, file_data)) + } + CompileClvmSource::SourceCode(name, code) => Ok((name.clone(), code.clone())), + } +} + +fn run_clvm_compilation( + input_code: CompileClvmSource, + action: CompileClvmAction, + search_paths: Vec, + export_symbols: Option, +) -> PyResult { + // Resolve the input, get the indicated path and content. + let (path_string, file_content) = get_source_from_input(input_code)?; + + // Load up our compiler opts. + let def_opts: Rc = Rc::new(DefaultCompilerOpts::new(&path_string)); + let opts = def_opts.set_search_paths(&search_paths); + + match action { + CompileClvmAction::CompileCode(output) => { + let mut allocator = Allocator::new(); + let mut symbols = HashMap::new(); + + // Output is a program represented as clvm data in allocator. + let clvm_result = clvmc::compile_clvm_text( + &mut allocator, + opts.clone(), + &mut symbols, + &file_content, + &path_string, + true, + ) + .map_err(|e| CompError::new_err(e.format(&allocator, opts)))?; + + // Get the text representation, which will go either to the output file + // or result. + let mut hex_text = Bytes::new(Some(BytesFromType::Raw(node_to_bytes( + &allocator, + clvm_result, + )?))) + .hex(); + let compiled = if let Some(output_file) = output { + // Write output with eol. + hex_text += "\n"; + gentle_overwrite(&path_string, &output_file, &hex_text) + .map_err(PyException::new_err)?; + output_file.to_string() + } else { + hex_text + }; + + // Produce compiled output according to whether output with symbols + // or just the standard result is required. + Python::with_gil(|py| { + if export_symbols == Some(true) { + let mut result_dict = HashMap::new(); + result_dict.insert("output".to_string(), compiled.into_py(py)); + result_dict.insert("symbols".to_string(), symbols.into_py(py)); + Ok(result_dict.into_py(py)) + } else { + Ok(compiled.into_py(py)) + } + }) + } + CompileClvmAction::CheckDependencies => { + // Produce dependency results. + let result_deps: Vec = + gather_dependencies(opts, &path_string.to_string(), &file_content) + .map_err(|e| CompError::new_err(format!("{}: {}", e.0, e.1))) + .map(|rlist| rlist.iter().map(|i| decode_string(&i.name)).collect())?; + + // Return all visited files. + Python::with_gil(|py| Ok(result_deps.into_py(py))) + } + } +} + #[pyfunction(arg3 = "[]", arg4 = "None")] fn compile_clvm( input_path: &PyAny, @@ -57,72 +173,36 @@ fn compile_clvm( search_paths: Vec, export_symbols: Option, ) -> PyResult { - let has_atom = input_path.hasattr("atom")?; - let has_pair = input_path.hasattr("pair")?; - - let real_input_path = if has_atom { - input_path.getattr("atom").and_then(|x| x.str()) - } else if has_pair { - input_path - .getattr("pair") - .and_then(|x| x.get_item(0)) - .and_then(|x| x.str()) - } else { - input_path.extract() - }?; - - let mut path_string = real_input_path.to_string(); - - if !std::path::Path::new(&path_string).exists() && !path_string.ends_with(".clvm") { - path_string += ".clvm"; - }; + run_clvm_compilation( + CompileClvmSource::SourcePath(input_path), + CompileClvmAction::CompileCode(Some(output_path)), + search_paths, + export_symbols, + ) +} - let mut symbols = HashMap::new(); - let compiled = clvmc::compile_clvm(&path_string, &output_path, &search_paths, &mut symbols) - .map_err(PyException::new_err)?; - - Python::with_gil(|py| { - if export_symbols == Some(true) { - let mut result_dict = HashMap::new(); - result_dict.insert("output".to_string(), compiled.into_py(py)); - result_dict.insert("symbols".to_string(), symbols.into_py(py)); - Ok(result_dict.into_py(py)) - } else { - Ok(compiled.into_py(py)) - } - }) +#[pyfunction(arg2 = "[]", arg3 = "None")] +fn compile( + source: String, + search_paths: Vec, + export_symbols: Option, +) -> PyResult { + run_clvm_compilation( + CompileClvmSource::SourceCode("*inline*".to_string(), source), + CompileClvmAction::CompileCode(None), + search_paths, + export_symbols, + ) } #[pyfunction(arg2 = "[]")] fn check_dependencies(input_path: &PyAny, search_paths: Vec) -> PyResult { - let has_atom = input_path.hasattr("atom")?; - let has_pair = input_path.hasattr("pair")?; - - let real_input_path = if has_atom { - input_path.getattr("atom").and_then(|x| x.str()) - } else if has_pair { - input_path - .getattr("pair") - .and_then(|x| x.get_item(0)) - .and_then(|x| x.str()) - } else { - input_path.extract() - }?; - - let file_content = fs::read_to_string(&real_input_path.to_string()) - .map_err(|_| CompError::new_err("failed to read file"))?; - - let def_opts: Rc = - Rc::new(DefaultCompilerOpts::new(&real_input_path.to_string())); - let opts = def_opts.set_search_paths(&search_paths); - - let result_deps: Vec = - gather_dependencies(opts, &real_input_path.to_string(), &file_content) - .map_err(|e| CompError::new_err(format!("{}: {}", e.0, e.1))) - .map(|rlist| rlist.iter().map(|i| decode_string(&i.name)).collect())?; - - // Return all visited files. - Python::with_gil(|py| Ok(result_deps.into_py(py))) + run_clvm_compilation( + CompileClvmSource::SourcePath(input_path), + CompileClvmAction::CheckDependencies, + search_paths, + None, + ) } #[pyclass] @@ -413,6 +493,7 @@ fn clvm_tools_rs(py: Python, m: &PyModule) -> PyResult<()> { m.add("CompError", py.get_type::())?; m.add_function(wrap_pyfunction!(compile_clvm, m)?)?; + m.add_function(wrap_pyfunction!(compile, m)?)?; m.add_function(wrap_pyfunction!(get_version, m)?)?; m.add_function(wrap_pyfunction!(start_clvm_program, m)?)?; m.add_function(wrap_pyfunction!(launch_tool, m)?)?; diff --git a/src/tests/classic/clvmc.rs b/src/tests/classic/clvmc.rs index c62dc1a51..4f2d7aef7 100644 --- a/src/tests/classic/clvmc.rs +++ b/src/tests/classic/clvmc.rs @@ -1,7 +1,12 @@ use std::collections::HashMap; use std::fs; +use std::rc::Rc; -use crate::classic::clvm_tools::clvmc::compile_clvm; +use clvmr::Allocator; + +use crate::classic::clvm_tools::clvmc::{compile_clvm, compile_clvm_text}; +use crate::compiler::compiler::DefaultCompilerOpts; +use crate::compiler::comptypes::CompilerOpts; use crate::tests::classic::run::read_json_from_file; #[test] @@ -50,3 +55,54 @@ fn test_compile_clvm_with_previous_data() { .expect("should compile"); fs::remove_file(bridge_hex_file).expect("should have existed"); } + +#[test] +fn test_classic_compile_error_output() { + let mut allocator = Allocator::new(); + let mut symbols = HashMap::new(); + let path = "*test*"; + let content = "(mod (X) (xxx X))"; + let opts: Rc = Rc::new(DefaultCompilerOpts::new(path)); + let res = compile_clvm_text( + &mut allocator, + opts.clone(), + &mut symbols, + &content, + &path, + true, + ) + .map_err(|e| e.format(&allocator, opts)); + assert_eq!( + Err( + "error can't compile (\"xxx\" 88), unknown operator compiling (\"xxx\" 88)".to_string() + ), + res + ); +} + +#[test] +fn test_modern_compile_error_output() { + let mut allocator = Allocator::new(); + let mut symbols = HashMap::new(); + let path = "*test*"; + let content = indoc! { + "(mod (X) + (include *standard-cl-23*) + (+ X1 X) + ) + "}; + let opts: Rc = Rc::new(DefaultCompilerOpts::new(path)); + let res = compile_clvm_text( + &mut allocator, + opts.clone(), + &mut symbols, + &content, + &path, + true, + ) + .map_err(|e| e.format(&allocator, opts)); + assert_eq!( + Err("*test*(3):4-*test*(3):6: Unbound use of X1 as a variable name".to_string()), + res + ); +} diff --git a/src/util/mod.rs b/src/util/mod.rs index e7b82c43c..d075b9aba 100644 --- a/src/util/mod.rs +++ b/src/util/mod.rs @@ -1,6 +1,10 @@ use num_bigint::BigInt; use std::collections::HashSet; +use std::fs; +use std::io::Write; use std::mem::swap; +use std::path::Path; +use tempfile::NamedTempFile; use unicode_segmentation::UnicodeSegmentation; pub type Number = BigInt; @@ -145,3 +149,56 @@ where self.map_err(|e| e.into()) } } + +pub fn atomic_write_file( + input_path: &str, + output_path: &str, + target_data: &str, +) -> Result<(), String> { + let output_path_obj = Path::new(output_path); + let output_dir = output_path_obj + .parent() + .map(Ok) + .unwrap_or_else(|| Err("could not get parent of output path"))?; + + // Make the contents appear atomically so that other test processes + // won't mistake an empty file for intended output. + let mut temp_output_file = NamedTempFile::new_in(output_dir) + .map_err(|e| format!("error creating temporary compiler output for {input_path}: {e:?}"))?; + + let err_text = format!("failed to write to {:?}", temp_output_file.path()); + let translate_err = |_| err_text.clone(); + + temp_output_file + .write_all(target_data.as_bytes()) + .map_err(translate_err)?; + + temp_output_file + .persist(output_path) + .map_err(|e| format!("error persisting temporary compiler output {output_path}: {e:?}"))?; + + Ok(()) +} + +pub fn gentle_overwrite( + input_path: &str, + output_path: &str, + target_data: &str, +) -> Result<(), String> { + if let Ok(prev_content) = fs::read_to_string(output_path) { + let prev_trimmed = prev_content.trim(); + let trimmed = target_data.trim(); + if prev_trimmed == trimmed { + // We should try to overwrite here, but not fail if it doesn't + // work. This will accomodate both the read only scenario and + // the scenario where a target file is newer and people want the + // date to be updated. + atomic_write_file(input_path, output_path, target_data).ok(); + + // It's the same program, bail regardless. + return Ok(()); + } + } + + atomic_write_file(input_path, output_path, target_data) +}