Add assertions about exception formatting and consolidate exception f…

…ormatting code

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"

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<EvalErr> for CompileError {
+    fn from(e: EvalErr) -> Self {
+        CompileError::Classic(e.0, e.1)
+    }
+}
+
+impl From<CompileErr> for CompileError {
+    fn from(r: CompileErr) -> Self {
+        CompileError::Modern(r.0, r.1)
+    }
+}
+
+impl From<RunFailure> 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<dyn CompilerOpts>) -> 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<String, String>,
     path: &str,
@@ -44,7 +89,7 @@ pub fn compile_clvm_text_maybe_opt(
     text: &str,
     input_path: &str,
     classic_with_opts: bool,
-) -> Result<NodePtr, EvalErr> {
+) -> Result<NodePtr, CompileError> {
     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<NodePtr, EvalErr> {
+) -> Result<NodePtr, CompileError> {
     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(())
 }

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.

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<dyn CompilerOpts> = 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<dyn CompilerOpts> = 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
+    );
+}