Merge pull request #63 from Chia-Network/20240123-add-compile-from-te…

…xt-base

20240123 add compile from text base

.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: |

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"

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<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>,
@@ -47,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))?;
 
@@ -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<NodePtr, EvalErr> {
+) -> Result<NodePtr, CompileError> {
     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())