From 859a192674b4afc1c5cb72672546a16842b217c3 Mon Sep 17 00:00:00 2001 From: arty Date: Thu, 25 Jan 2024 12:39:56 -0800 Subject: [PATCH] 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 8bf4005c2..f067b11ba 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 d3a07b9ba..9db074e35 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 5d6a08fc8..e78b3debb 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 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 + ); +}