From d2f4d70bdae2fb90459fe56be83e4dbe20257751 Mon Sep 17 00:00:00 2001 From: arty Date: Tue, 23 Jan 2024 16:16:52 -0800 Subject: [PATCH 01/10] Compile from text against dev re: https://github.com/richardkiss/clvm_tools_rs/commit/6f95cef9e049afff43801b7c86bb9972718dc913 --- resources/tests/test_compile_from_string.py | 45 ++++++++++++++++++ src/py/api.rs | 51 +++++++++++++++++++++ 2 files changed, 96 insertions(+) create mode 100644 resources/tests/test_compile_from_string.py diff --git a/resources/tests/test_compile_from_string.py b/resources/tests/test_compile_from_string.py new file mode 100644 index 00000000..9774bbcf --- /dev/null +++ b/resources/tests/test_compile_from_string.py @@ -0,0 +1,45 @@ +import clvm_tools_rs +import binascii + +classic_code = """ +(mod (N X) + (defun F (N X) (if N (sha256 (F (- N 1) X)) X)) + (F N X) + ) +""" +expected_classic = binascii.unhexlify("ff02ffff01ff02ff02ffff04ff02ffff04ff05ffff04ff0bff8080808080ffff04ffff01ff02ffff03ff05ffff01ff0bffff02ff02ffff04ff02ffff04ffff11ff05ffff010180ffff04ff0bff808080808080ffff010b80ff0180ff018080".encode("utf8")) + +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 = binascii.unhexlify("ff02ffff01ff02ff02ffff04ff02ffff04ff05ffff04ff0bff8080808080ffff04ffff01ff02ffff03ff05ffff01ff0bffff02ff02ffff04ff02ffff04ffff11ff05ffff010180ffff04ff0bff808080808080ffff010b80ff0180ff018080".encode("utf8")) + +compiled_code = clvm_tools_rs.compile( + classic_code, + ["."], + True +) +# Classic outputs symbols to the filesystem down all routes, which likely should +# be fixed. +assert bytes(compiled_code["output"]) == expected_classic + +compiled_code = clvm_tools_rs.compile( + classic_code, + ["."] +) +assert bytes(compiled_code) == expected_classic + +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 bytes(compiled_code["output"]) == expected_cl23 diff --git a/src/py/api.rs b/src/py/api.rs index 2a472254..7daf3438 100644 --- a/src/py/api.rs +++ b/src/py/api.rs @@ -15,9 +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::serialize::sexp_to_stream; +use crate::classic::clvm_tools::binutils::disassemble; use crate::classic::clvm_tools::clvmc; use crate::classic::clvm_tools::cmds; use crate::classic::clvm_tools::stages::stage_0::DefaultProgramRunner; @@ -93,6 +95,54 @@ fn compile_clvm( }) } +#[pyfunction(arg2 = "[]", arg3 = "None")] +fn compile( + source: String, + search_paths: Vec, + export_symbols: Option, +) -> PyResult { + let mut symbols = HashMap::new(); + + let mut allocator = Allocator::new(); + let input_name = "*inline*"; + let def_opts: Rc = + Rc::new(DefaultCompilerOpts::new(input_name)); + let opts = def_opts.set_search_paths(&search_paths); + let mut includes = Vec::new(); + + let compiled_node = clvmc::compile_clvm_text( + &mut allocator, + opts, + &mut symbols, + &mut includes, + &source, + input_name, + true, + ) + .map_err(|x| { + format!( + "error {} compiling {}", + x.1, + disassemble(&mut allocator, x.0, None) + ) + }) + .map_err(PyException::new_err)?; + + let blob = + node_to_bytes(&allocator, compiled_node).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(), blob.into_py(py)); + result_dict.insert("symbols".to_string(), symbols.into_py(py)); + Ok(result_dict.into_py(py)) + } else { + Ok(blob.into_py(py)) + } + }) +} + #[pyfunction(arg2 = "[]")] fn check_dependencies(input_path: &PyAny, search_paths: Vec) -> PyResult { let has_atom = input_path.hasattr("atom")?; @@ -413,6 +463,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)?)?; From 9702c5f4d4b022a8e2c48b8d28ed0197d6930513 Mon Sep 17 00:00:00 2001 From: arty Date: Tue, 23 Jan 2024 16:21:03 -0800 Subject: [PATCH 02/10] fmt + clippy + api drift --- .github/workflows/build-test.yml | 5 ++++- src/py/api.rs | 8 ++------ 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/.github/workflows/build-test.yml b/.github/workflows/build-test.yml index d21905ce..21413b41 100644 --- a/.github/workflows/build-test.yml +++ b/.github/workflows/build-test.yml @@ -152,7 +152,10 @@ 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_from_string.py - name: "Test step run with mandelbrot, setting print only" run: | diff --git a/src/py/api.rs b/src/py/api.rs index 7daf3438..877deafc 100644 --- a/src/py/api.rs +++ b/src/py/api.rs @@ -105,16 +105,13 @@ fn compile( let mut allocator = Allocator::new(); let input_name = "*inline*"; - let def_opts: Rc = - Rc::new(DefaultCompilerOpts::new(input_name)); + let def_opts: Rc = Rc::new(DefaultCompilerOpts::new(input_name)); let opts = def_opts.set_search_paths(&search_paths); - let mut includes = Vec::new(); let compiled_node = clvmc::compile_clvm_text( &mut allocator, opts, &mut symbols, - &mut includes, &source, input_name, true, @@ -128,8 +125,7 @@ fn compile( }) .map_err(PyException::new_err)?; - let blob = - node_to_bytes(&allocator, compiled_node).map_err(PyException::new_err)?; + let blob = node_to_bytes(&allocator, compiled_node).map_err(PyException::new_err)?; Python::with_gil(|py| { if export_symbols == Some(true) { From 2c35347e8e1d761a017a03039803bba79af0b1a9 Mon Sep 17 00:00:00 2001 From: arty Date: Wed, 24 Jan 2024 09:13:59 -0800 Subject: [PATCH 03/10] Correct test name --- .github/workflows/build-test.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/build-test.yml b/.github/workflows/build-test.yml index 21413b41..9371ed79 100644 --- a/.github/workflows/build-test.yml +++ b/.github/workflows/build-test.yml @@ -155,7 +155,7 @@ jobs: cd resources/tests && \ python test_clvm_step.py && \ python mandelbrot-cldb.py && \ - python test_compile_from_from_string.py + python test_compile_from_string.py - name: "Test step run with mandelbrot, setting print only" run: | From 42338a92b93def42e9beb196b561d035269e7008 Mon Sep 17 00:00:00 2001 From: arty Date: Wed, 24 Jan 2024 09:21:03 -0800 Subject: [PATCH 04/10] Execute this in bash so continued lines are correctly evaluated --- .github/workflows/build-test.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/build-test.yml b/.github/workflows/build-test.yml index 9371ed79..763d1f8f 100644 --- a/.github/workflows/build-test.yml +++ b/.github/workflows/build-test.yml @@ -142,6 +142,7 @@ jobs: # Test cldb output both run from python and via its command line tool. - name: "Run step run tests" + shell: bash run: | . ./activate cargo build From 6ff6c81838dbb6dec9bb2325c76f8a5e6779e6a5 Mon Sep 17 00:00:00 2001 From: arty Date: Wed, 24 Jan 2024 09:27:44 -0800 Subject: [PATCH 05/10] Keep this from getting complicated, just make a longer line for now --- .github/workflows/build-test.yml | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/.github/workflows/build-test.yml b/.github/workflows/build-test.yml index 763d1f8f..6fa0cc7f 100644 --- a/.github/workflows/build-test.yml +++ b/.github/workflows/build-test.yml @@ -142,7 +142,6 @@ jobs: # Test cldb output both run from python and via its command line tool. - name: "Run step run tests" - shell: bash run: | . ./activate cargo build @@ -153,10 +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 && \ - python test_compile_from_string.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: | From 9b1e321491c2fbc650c7b796c43359f099cc5762 Mon Sep 17 00:00:00 2001 From: arty Date: Wed, 24 Jan 2024 11:03:24 -0800 Subject: [PATCH 06/10] Refactor in py/api.rs to join up the various compilation entry points --- resources/tests/test_compile_from_string.py | 39 +++- src/py/api.rs | 228 +++++++++++--------- 2 files changed, 162 insertions(+), 105 deletions(-) diff --git a/resources/tests/test_compile_from_string.py b/resources/tests/test_compile_from_string.py index 9774bbcf..8bf4005c 100644 --- a/resources/tests/test_compile_from_string.py +++ b/resources/tests/test_compile_from_string.py @@ -1,5 +1,6 @@ -import clvm_tools_rs import binascii +import clvm_tools_rs +from pathlib import Path classic_code = """ (mod (N X) @@ -7,7 +8,7 @@ (F N X) ) """ -expected_classic = binascii.unhexlify("ff02ffff01ff02ff02ffff04ff02ffff04ff05ffff04ff0bff8080808080ffff04ffff01ff02ffff03ff05ffff01ff0bffff02ff02ffff04ff02ffff04ffff11ff05ffff010180ffff04ff0bff808080808080ffff010b80ff0180ff018080".encode("utf8")) +expected_classic = "ff02ffff01ff02ff02ffff04ff02ffff04ff05ffff04ff0bff8080808080ffff04ffff01ff02ffff03ff05ffff01ff0bffff02ff02ffff04ff02ffff04ffff11ff05ffff010180ffff04ff0bff808080808080ffff010b80ff0180ff018080" cl23_code = """ (mod (N X) @@ -16,7 +17,9 @@ (F N X) ) """ -expected_cl23 = binascii.unhexlify("ff02ffff01ff02ff02ffff04ff02ffff04ff05ffff04ff0bff8080808080ffff04ffff01ff02ffff03ff05ffff01ff0bffff02ff02ffff04ff02ffff04ffff11ff05ffff010180ffff04ff0bff808080808080ffff010b80ff0180ff018080".encode("utf8")) +expected_cl23 = "ff02ffff01ff02ff02ffff04ff02ffff04ff05ffff04ff0bff8080808080ffff04ffff01ff02ffff03ff05ffff01ff0bffff02ff02ffff04ff02ffff04ffff11ff05ffff010180ffff04ff0bff808080808080ffff010b80ff0180ff018080" + +expected_deinline = "ff02ffff01ff02ff02ffff04ff02ffff04ff03ffff04ffff10ff05ffff01830f424080ff8080808080ffff04ffff01ff10ff0bff0bff0bff0bff0bff0b80ff018080" compiled_code = clvm_tools_rs.compile( classic_code, @@ -25,14 +28,15 @@ ) # Classic outputs symbols to the filesystem down all routes, which likely should # be fixed. -assert bytes(compiled_code["output"]) == expected_classic +assert compiled_code["output"] == expected_classic compiled_code = clvm_tools_rs.compile( classic_code, ["."] ) -assert bytes(compiled_code) == expected_classic +assert compiled_code == expected_classic +# Verify modern compilation compiled_code = clvm_tools_rs.compile( cl23_code, ["."], @@ -42,4 +46,27 @@ assert symbols["__chia__main_arguments"] == "(N X)" assert symbols["30960d7f2ddc7188a6428a11d39a13ff70d308e6cc571ffb6ed5ec8dbe4376c0_arguments"] == "(N X)" assert symbols["30960d7f2ddc7188a6428a11d39a13ff70d308e6cc571ffb6ed5ec8dbe4376c0"] == "F" -assert bytes(compiled_code["output"]) == expected_cl23 +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' diff --git a/src/py/api.rs b/src/py/api.rs index 877deafc..305cea4d 100644 --- a/src/py/api.rs +++ b/src/py/api.rs @@ -17,9 +17,8 @@ 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::binutils::disassemble; use crate::classic::clvm_tools::clvmc; use crate::classic::clvm_tools::cmds; use crate::classic::clvm_tools::stages::stage_0::DefaultProgramRunner; @@ -52,6 +51,117 @@ 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(PyException::new_err)?; + 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, + &mut symbols, + &file_content, + &path_string, + true + ).map_err(|e| CompError::new_err(format!("{}", e)))?; + + // 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"; + fs::write(&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, @@ -59,40 +169,12 @@ 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"; - }; - - 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)) - } - }) + run_clvm_compilation( + CompileClvmSource::SourcePath(input_path), + CompileClvmAction::CompileCode(Some(output_path)), + search_paths, + export_symbols + ) } #[pyfunction(arg2 = "[]", arg3 = "None")] @@ -101,74 +183,22 @@ fn compile( search_paths: Vec, export_symbols: Option, ) -> PyResult { - let mut symbols = HashMap::new(); - - let mut allocator = Allocator::new(); - let input_name = "*inline*"; - let def_opts: Rc = Rc::new(DefaultCompilerOpts::new(input_name)); - let opts = def_opts.set_search_paths(&search_paths); - - let compiled_node = clvmc::compile_clvm_text( - &mut allocator, - opts, - &mut symbols, - &source, - input_name, - true, + run_clvm_compilation( + CompileClvmSource::SourceCode("*inline*".to_string(), source), + CompileClvmAction::CompileCode(None), + search_paths, + export_symbols ) - .map_err(|x| { - format!( - "error {} compiling {}", - x.1, - disassemble(&mut allocator, x.0, None) - ) - }) - .map_err(PyException::new_err)?; - - let blob = node_to_bytes(&allocator, compiled_node).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(), blob.into_py(py)); - result_dict.insert("symbols".to_string(), symbols.into_py(py)); - Ok(result_dict.into_py(py)) - } else { - Ok(blob.into_py(py)) - } - }) } #[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] From 1d2515832ebf7d65ed0e4601469c4555f6d0ade0 Mon Sep 17 00:00:00 2001 From: arty Date: Wed, 24 Jan 2024 11:04:52 -0800 Subject: [PATCH 07/10] fmt --- src/py/api.rs | 62 ++++++++++++++++++++++++++------------------------- 1 file changed, 32 insertions(+), 30 deletions(-) diff --git a/src/py/api.rs b/src/py/api.rs index 305cea4d..c6cc94d4 100644 --- a/src/py/api.rs +++ b/src/py/api.rs @@ -17,7 +17,9 @@ use std::thread; use clvm_rs::allocator::Allocator; use clvm_rs::serde::node_to_bytes; -use crate::classic::clvm::__type_compatibility__::{Bytes, BytesFromType, 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; @@ -58,7 +60,7 @@ enum CompileClvmSource<'a> { enum CompileClvmAction { CheckDependencies, - CompileCode(Option) + CompileCode(Option), } fn get_source_from_input(input_code: CompileClvmSource) -> PyResult<(String, String)> { @@ -87,9 +89,7 @@ fn get_source_from_input(input_code: CompileClvmSource) -> PyResult<(String, Str let file_data = fs::read_to_string(&path_string).map_err(PyException::new_err)?; Ok((path_string, file_data)) } - CompileClvmSource::SourceCode(name, code) => { - Ok((name.clone(), code.clone())) - } + CompileClvmSource::SourceCode(name, code) => Ok((name.clone(), code.clone())), } } @@ -103,8 +103,7 @@ fn run_clvm_compilation( 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 def_opts: Rc = Rc::new(DefaultCompilerOpts::new(&path_string)); let opts = def_opts.set_search_paths(&search_paths); match action { @@ -113,28 +112,31 @@ fn run_clvm_compilation( 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, - &mut symbols, - &file_content, - &path_string, - true - ).map_err(|e| CompError::new_err(format!("{}", e)))?; + let clvm_result = clvmc::compile_clvm_text( + &mut allocator, + opts, + &mut symbols, + &file_content, + &path_string, + true, + ) + .map_err(|e| CompError::new_err(format!("{}", e)))?; // 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"; - fs::write(&output_file, hex_text).map_err(PyException::new_err)?; - output_file.to_string() - } else { - hex_text - }; + 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"; + fs::write(&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. @@ -153,8 +155,8 @@ fn run_clvm_compilation( // 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())?; + .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))) @@ -173,7 +175,7 @@ fn compile_clvm( CompileClvmSource::SourcePath(input_path), CompileClvmAction::CompileCode(Some(output_path)), search_paths, - export_symbols + export_symbols, ) } @@ -187,7 +189,7 @@ fn compile( CompileClvmSource::SourceCode("*inline*".to_string(), source), CompileClvmAction::CompileCode(None), search_paths, - export_symbols + export_symbols, ) } From be2b8809d2d219e1961500bb716ebc0b6e14d491 Mon Sep 17 00:00:00 2001 From: arty Date: Wed, 24 Jan 2024 11:57:38 -0800 Subject: [PATCH 08/10] Refactor to increase sharing of gentle_overwrite and ensure that its behavior is shared between all producers of clvm code to disk --- src/classic/clvm_tools/clvmc.rs | 54 +++------------------------------ src/py/api.rs | 6 ++-- src/util/mod.rs | 54 +++++++++++++++++++++++++++++++++ 3 files changed, 61 insertions(+), 53 deletions(-) diff --git a/src/classic/clvm_tools/clvmc.rs b/src/classic/clvm_tools/clvmc.rs index 1ef267e4..384f8d51 100644 --- a/src/classic/clvm_tools/clvmc.rs +++ b/src/classic/clvm_tools/clvmc.rs @@ -1,15 +1,11 @@ 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; -use crate::classic::clvm::__type_compatibility__::Stream; +use crate::classic::clvm::__type_compatibility__::{Bytes, Stream, BytesFromType}; use crate::classic::clvm::serialize::sexp_to_stream; use crate::classic::clvm_tools::binutils::{assemble_from_ir, disassemble}; use crate::classic::clvm_tools::ir::reader::read_ir; @@ -26,6 +22,7 @@ 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::util::gentle_overwrite; pub fn write_sym_output( compiled_lookup: &HashMap, @@ -162,55 +159,12 @@ pub fn compile_clvm( false, )?; + result_stream.write(Bytes::new(Some(BytesFromType::Raw(b"\n".to_vec())))); 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(()) - }; - // 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 c6cc94d4..c9cf4269 100644 --- a/src/py/api.rs +++ b/src/py/api.rs @@ -38,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}; @@ -86,7 +86,7 @@ fn get_source_from_input(input_code: CompileClvmSource) -> PyResult<(String, Str path_string += ".clvm"; } - let file_data = fs::read_to_string(&path_string).map_err(PyException::new_err)?; + 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())), @@ -132,7 +132,7 @@ fn run_clvm_compilation( let compiled = if let Some(output_file) = output { // Write output with eol. hex_text += "\n"; - fs::write(&output_file, hex_text).map_err(PyException::new_err)?; + gentle_overwrite(&path_string, &output_file, &hex_text).map_err(PyException::new_err)?; output_file.to_string() } else { hex_text diff --git a/src/util/mod.rs b/src/util/mod.rs index e7b82c43..e9567356 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,53 @@ 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) +} From 0fc417f901237cd4a25e87956f0425a04b763473 Mon Sep 17 00:00:00 2001 From: arty Date: Wed, 24 Jan 2024 12:03:11 -0800 Subject: [PATCH 09/10] opps, ensure that we add the ending cr after hexifying, also fmt and clippy --- src/classic/clvm_tools/clvmc.rs | 6 +++--- src/py/api.rs | 6 ++++-- src/util/mod.rs | 23 +++++++++++++---------- 3 files changed, 20 insertions(+), 15 deletions(-) diff --git a/src/classic/clvm_tools/clvmc.rs b/src/classic/clvm_tools/clvmc.rs index 384f8d51..d3a07b9b 100644 --- a/src/classic/clvm_tools/clvmc.rs +++ b/src/classic/clvm_tools/clvmc.rs @@ -5,7 +5,7 @@ use std::rc::Rc; use clvm_rs::allocator::{Allocator, NodePtr}; use clvm_rs::reduction::EvalErr; -use crate::classic::clvm::__type_compatibility__::{Bytes, Stream, BytesFromType}; +use crate::classic::clvm::__type_compatibility__::Stream; use crate::classic::clvm::serialize::sexp_to_stream; use crate::classic::clvm_tools::binutils::{assemble_from_ir, disassemble}; use crate::classic::clvm_tools::ir::reader::read_ir; @@ -159,8 +159,8 @@ pub fn compile_clvm( false, )?; - result_stream.write(Bytes::new(Some(BytesFromType::Raw(b"\n".to_vec())))); - let target_data = result_stream.get_value().hex(); + 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. diff --git a/src/py/api.rs b/src/py/api.rs index c9cf4269..5d6a08fc 100644 --- a/src/py/api.rs +++ b/src/py/api.rs @@ -86,7 +86,8 @@ fn get_source_from_input(input_code: CompileClvmSource) -> PyResult<(String, Str path_string += ".clvm"; } - let file_data = fs::read_to_string(&path_string).map_err(|e| PyException::new_err(format!("error reading {path_string}: {e:?}")))?; + 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())), @@ -132,7 +133,8 @@ fn run_clvm_compilation( 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)?; + gentle_overwrite(&path_string, &output_file, &hex_text) + .map_err(PyException::new_err)?; output_file.to_string() } else { hex_text diff --git a/src/util/mod.rs b/src/util/mod.rs index e9567356..d075b9ab 100644 --- a/src/util/mod.rs +++ b/src/util/mod.rs @@ -150,7 +150,11 @@ where } } -pub fn atomic_write_file(input_path: &str, output_path: &str, target_data: &str) -> Result<(), String> { +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() @@ -159,9 +163,8 @@ pub fn atomic_write_file(input_path: &str, output_path: &str, target_data: &str) // 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 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(); @@ -170,9 +173,9 @@ pub fn atomic_write_file(input_path: &str, output_path: &str, target_data: &str) .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:?}") - })?; + temp_output_file + .persist(output_path) + .map_err(|e| format!("error persisting temporary compiler output {output_path}: {e:?}"))?; Ok(()) } @@ -180,7 +183,7 @@ pub fn atomic_write_file(input_path: &str, output_path: &str, target_data: &str) pub fn gentle_overwrite( input_path: &str, output_path: &str, - target_data: &str + target_data: &str, ) -> Result<(), String> { if let Ok(prev_content) = fs::read_to_string(output_path) { let prev_trimmed = prev_content.trim(); @@ -190,12 +193,12 @@ pub fn gentle_overwrite( // 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(); + 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) + atomic_write_file(input_path, output_path, target_data) } From 859a192674b4afc1c5cb72672546a16842b217c3 Mon Sep 17 00:00:00 2001 From: arty Date: Thu, 25 Jan 2024 12:39:56 -0800 Subject: [PATCH 10/10] Add assertions about exception formatting and consolidate exception formatting code --- resources/tests/test_compile_from_string.py | 16 +++++ src/classic/clvm_tools/clvmc.rs | 79 +++++++++++++++------ src/py/api.rs | 4 +- src/tests/classic/clvmc.rs | 58 ++++++++++++++- 4 files changed, 133 insertions(+), 24 deletions(-) diff --git a/resources/tests/test_compile_from_string.py b/resources/tests/test_compile_from_string.py index 8bf4005c..f067b11b 100644 --- a/resources/tests/test_compile_from_string.py +++ b/resources/tests/test_compile_from_string.py @@ -21,6 +21,9 @@ 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, ["."], @@ -70,3 +73,16 @@ ) 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 d3a07b9b..9db074e3 100644 --- a/src/classic/clvm_tools/clvmc.rs +++ b/src/classic/clvm_tools/clvmc.rs @@ -22,8 +22,53 @@ 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, path: &str, @@ -44,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))?; @@ -61,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())?; @@ -93,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, @@ -122,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(()) } diff --git a/src/py/api.rs b/src/py/api.rs index 5d6a08fc..e78b3deb 100644 --- a/src/py/api.rs +++ b/src/py/api.rs @@ -115,13 +115,13 @@ fn run_clvm_compilation( // Output is a program represented as clvm data in allocator. let clvm_result = clvmc::compile_clvm_text( &mut allocator, - opts, + opts.clone(), &mut symbols, &file_content, &path_string, true, ) - .map_err(|e| CompError::new_err(format!("{}", e)))?; + .map_err(|e| CompError::new_err(e.format(&allocator, opts)))?; // Get the text representation, which will go either to the output file // or result. diff --git a/src/tests/classic/clvmc.rs b/src/tests/classic/clvmc.rs index c62dc1a5..4f2d7aef 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 + ); +}