From 4ad89938773080df5b5012f2ee653a2cd10b995f Mon Sep 17 00:00:00 2001 From: arty Date: Sat, 6 Jul 2024 00:55:33 -0700 Subject: [PATCH 01/10] Add favor hex flag (-X) for cldb --- resources/tests/cldb_tree/hex.json | 49 +++++++++++++++++ src/classic/clvm_tools/cmds.rs | 72 ++++++++++++++++--------- src/compiler/cldb.rs | 66 +++++++++++++++++++---- src/compiler/cldb_hierarchy.rs | 19 ++++++- src/tests/compiler/cldb.rs | 85 +++++++++++++++++++++++------- 5 files changed, 238 insertions(+), 53 deletions(-) create mode 100644 resources/tests/cldb_tree/hex.json diff --git a/resources/tests/cldb_tree/hex.json b/resources/tests/cldb_tree/hex.json new file mode 100644 index 000000000..be0039dc0 --- /dev/null +++ b/resources/tests/cldb_tree/hex.json @@ -0,0 +1,49 @@ +[ + { + "Compute": [ + { + "Function-Args": {}, + "Function-Name": "test_with_hex.clsp", + "Output": { + "Arguments": "(())", + "Operator": "4", + "Operator-Location": "test_with_hex.clsp(1):10-test_with_hex.clsp(1):16", + "Result-Location": "test_with_hex.clsp(1):10-test_with_hex.clsp(1):16", + "Function": "concat", + "Row": "0", + "Value": "()" + } + } + ] + }, + { + "Compute": [ + { + "Function-Args": {}, + "Function-Name": "test_with_hex.clsp", + "Output": { + "Arguments": "(1 0x42e576f7)", + "Function-Context": "()", + "Function": "concat", + "Operator": "14", + "Operator-Location": "test_with_hex.clsp(1):17", + "Result-Location": "test_with_hex.clsp(1):10-test_with_hex.clsp(1):16", + "Row": "1", + "Value": "0x0142e576f7" + } + } + ] + }, + { + "Compute": [ + { + "Function-Args": {}, + "Function-Name": "test_with_hex.clsp", + "Output": { + "Final": "0x0142e576f7", + "Final-Location": "test_with_hex.clsp(1):10-test_with_hex.clsp(1):16" + } + } + ] + } +] diff --git a/src/classic/clvm_tools/cmds.rs b/src/classic/clvm_tools/cmds.rs index 21a2dcc5d..0fc6043f8 100644 --- a/src/classic/clvm_tools/cmds.rs +++ b/src/classic/clvm_tools/cmds.rs @@ -49,7 +49,9 @@ use crate::classic::platform::argparse::{ TArgOptionAction, TArgumentParserProps, }; -use crate::compiler::cldb::{hex_to_modern_sexp, CldbNoOverride, CldbRun, CldbRunEnv}; +use crate::compiler::cldb::{ + hex_to_modern_sexp, hexize, CldbNoOverride, CldbRun, CldbRunEnv, FAVOR_HEX, +}; use crate::compiler::cldb_hierarchy::{HierarchialRunner, HierarchialStepResult, RunPurpose}; use crate::compiler::clvm::start_step; use crate::compiler::compiler::{compile_file, DefaultCompilerOpts}; @@ -383,25 +385,32 @@ fn yamlette_string(to_print: &[BTreeMap]) -> String { } } +pub struct CldbHierarchyArgs { + pub runner: Rc, + pub prim_map: Rc, Rc>>, + pub input_file_name: Option, + pub lines: Rc>, + pub symbol_table: Rc>, + pub prog: Rc, + pub args: Rc, + pub flags: u32, +} + pub fn cldb_hierarchy( - runner: Rc, - prim_map: Rc, Rc>>, - input_file_name: Option, - lines: Rc>, - symbol_table: Rc>, - prog: Rc, - args: Rc, + args: CldbHierarchyArgs ) -> Vec> { let mut runner = HierarchialRunner::new( - runner, - prim_map, - input_file_name, - lines, - symbol_table, - prog, - args, + args.runner, + args.prim_map, + args.input_file_name, + args.lines, + args.symbol_table, + args.prog, + args.args, ); + runner.set_flags(args.flags); + let mut output_stack = vec![Vec::new()]; loop { @@ -434,7 +443,10 @@ pub fn cldb_hierarchy( ); let mut arg_values = BTreeMap::new(); for (k, v) in runner.running[run_idx].named_args.iter() { - arg_values.insert(k.clone(), YamlElement::String(format!("{v}"))); + arg_values.insert( + k.clone(), + YamlElement::String(format!("{}", hexize(v.clone(), args.flags))), + ); } function_entry.insert( "Function-Args".to_string(), @@ -531,6 +543,12 @@ pub fn cldb(args: &[String]) { .set_type(Rc::new(PathOrCodeConv {})) .set_help("path to symbol file".to_string()), ); + parser.add_argument( + vec!["-X".to_string(), "--favor-hex".to_string()], + Argument::new() + .set_action(TArgOptionAction::StoreTrue) + .set_help("favor hex output to integer".to_string()), + ); parser.add_argument( vec!["-p".to_string(), "--only-print".to_string()], Argument::new() @@ -606,6 +624,7 @@ pub fn cldb(args: &[String]) { }); let only_print = parsed_args.get("only_print").map(|_| true).unwrap_or(false); + let favor_hex = parsed_args.get("favor_hex").map(|_| true).unwrap_or(false); let do_optimize = parsed_args .get("optimize") @@ -720,15 +739,16 @@ pub fn cldb(args: &[String]) { ); if parsed_args.contains_key("tree") { - let result = cldb_hierarchy( - runner, - Rc::new(prim_map), - input_file, - program_lines, - Rc::new(use_symbol_table), - program, + let result = cldb_hierarchy(CldbHierarchyArgs { + runner: runner, + prim_map: Rc::new(prim_map), + input_file_name: input_file, + lines: program_lines, + symbol_table: Rc::new(use_symbol_table), + prog: program, args, - ); + flags: if favor_hex { FAVOR_HEX } else { 0 }, + }); // Print the tree let string_result = yamlette_string(&result); @@ -738,6 +758,10 @@ pub fn cldb(args: &[String]) { let step = start_step(program, args); let mut cldbrun = CldbRun::new(runner, Rc::new(prim_map), Box::new(cldbenv), step); + if favor_hex { + cldbrun.set_flags(FAVOR_HEX); + } + let print_tree = |output: &mut Vec<_>, result: &BTreeMap| { let mut cvt_subtree = BTreeMap::new(); for (k, v) in result.iter() { diff --git a/src/compiler/cldb.rs b/src/compiler/cldb.rs index d8bb942cf..4e4882312 100644 --- a/src/compiler/cldb.rs +++ b/src/compiler/cldb.rs @@ -24,6 +24,8 @@ use crate::compiler::srcloc::Srcloc; use crate::util::u8_from_number; use crate::util::Number; +pub const FAVOR_HEX: u32 = 1; + fn print_atom() -> SExp { SExp::Atom(Srcloc::start("*print*"), b"$print$".to_vec()) } @@ -109,6 +111,7 @@ pub struct CldbRun { in_expr: bool, row: usize, print_only: bool, + flags: u32, outputs_to_step: HashMap, } @@ -133,6 +136,37 @@ fn humanize(a: Rc) -> Rc { } } +pub fn hexize(a: Rc, flags: u32) -> Rc { + match a.borrow() { + SExp::Atom(l, av) => { + if av.len() > 2 && (flags & FAVOR_HEX) != 0 { + Rc::new(SExp::QuotedString(l.clone(), b'x', av.clone())) + } else { + a.clone() + } + } + SExp::Integer(l, i) => { + // If it has a nice string representation then show that. + let bytes_of_int = u8_from_number(i.clone()); + if bytes_of_int.len() > 2 && (flags & FAVOR_HEX) != 0 { + Rc::new(SExp::QuotedString(l.clone(), b'x', bytes_of_int)) + } else { + a.clone() + } + } + SExp::Cons(l, a, b) => { + let new_a = hexize(a.clone(), flags); + let new_b = hexize(b.clone(), flags); + if Rc::as_ptr(&new_a) == Rc::as_ptr(a) && Rc::as_ptr(&new_b) == Rc::as_ptr(b) { + return a.clone(); + } + + Rc::new(SExp::Cons(l.clone(), new_a, new_b)) + } + _ => a.clone(), + } +} + fn is_print_request(a: &SExp) -> Option<(Srcloc, Rc)> { if let SExp::Cons(l, f, r) = a { if &print_atom() == f.borrow() { @@ -166,9 +200,14 @@ impl CldbRun { row: 0, outputs_to_step: HashMap::::new(), print_only: false, + flags: 0, } } + pub fn set_flags(&mut self, flags: u32) { + self.flags = flags; + } + pub fn set_print_only(&mut self, pronly: bool) { self.print_only = pronly; } @@ -207,7 +246,10 @@ impl CldbRun { if self.should_print_basic_output() { self.to_print .insert("Result-Location".to_string(), l.to_string()); - self.to_print.insert("Value".to_string(), x.to_string()); + self.to_print.insert( + "Value".to_string(), + hexize(x.clone(), self.flags).to_string(), + ); self.to_print .insert("Row".to_string(), self.row.to_string()); @@ -228,12 +270,14 @@ impl CldbRun { } } Ok(RunStep::Done(l, x)) => { + let final_value = hexize(x.clone(), self.flags); self.to_print .insert("Final-Location".to_string(), l.to_string()); - self.to_print.insert("Final".to_string(), x.to_string()); + self.to_print + .insert("Final".to_string(), final_value.to_string()); self.ended = true; - self.final_result = Some(x.clone()); + self.final_result = Some(final_value); swap(&mut self.to_print, &mut result); produce_result = true; } @@ -269,8 +313,8 @@ impl CldbRun { if should_print_basic_output { self.env.add_context( sexp.borrow(), - c.borrow(), - Some(a.clone()), + hexize(c.clone(), self.flags).borrow(), + Some(hexize(a.clone(), self.flags)), &mut self.to_print, ); self.env.add_function(sexp, &mut self.to_print); @@ -281,7 +325,10 @@ impl CldbRun { Err(RunFailure::RunExn(l, s)) => { self.to_print .insert("Throw-Location".to_string(), l.to_string()); - self.to_print.insert("Throw".to_string(), s.to_string()); + self.to_print.insert( + "Throw".to_string(), + hexize(s.clone(), self.flags).to_string(), + ); swap(&mut self.to_print, &mut result); self.ended = true; @@ -547,6 +594,7 @@ fn hex_to_modern_sexp_inner( symbol_table: &HashMap, loc: Srcloc, program: NodePtr, + flags: u32, ) -> Result, EvalErr> { let hash = sha256tree(allocator, program); let hash_str = hash.hex(); @@ -558,8 +606,8 @@ fn hex_to_modern_sexp_inner( match allocator.sexp(program) { allocator::SExp::Pair(a, b) => Ok(Rc::new(SExp::Cons( srcloc.clone(), - hex_to_modern_sexp_inner(allocator, symbol_table, srcloc.clone(), a)?, - hex_to_modern_sexp_inner(allocator, symbol_table, srcloc, b)?, + hex_to_modern_sexp_inner(allocator, symbol_table, srcloc.clone(), a, flags)?, + hex_to_modern_sexp_inner(allocator, symbol_table, srcloc, b, flags)?, ))), _ => convert_from_clvm_rs(allocator, srcloc, program).map_err(|_| { EvalErr( @@ -588,7 +636,7 @@ pub fn hex_to_modern_sexp( .map(|x| x.1) .map_err(|_| RunFailure::RunErr(loc.clone(), "Bad conversion from hex".to_string()))?; - hex_to_modern_sexp_inner(allocator, symbol_table, loc.clone(), sexp).map_err(|_| { + hex_to_modern_sexp_inner(allocator, symbol_table, loc.clone(), sexp, 0).map_err(|_| { RunFailure::RunErr(loc, "Failed to convert from classic to modern".to_string()) }) } diff --git a/src/compiler/cldb_hierarchy.rs b/src/compiler/cldb_hierarchy.rs index ce84f3e7d..2b6e79bad 100644 --- a/src/compiler/cldb_hierarchy.rs +++ b/src/compiler/cldb_hierarchy.rs @@ -57,6 +57,7 @@ pub struct HierarchialRunner { input_file: Option, program_lines: Rc>, prog: Rc, + flags: u32, pub running: Vec, } @@ -233,6 +234,7 @@ impl HierarchialRunner { program_lines, error: false, prog: prog.clone(), + flags: 0, running: vec![HierarchyFrame { purpose: RunPurpose::Main, @@ -258,6 +260,13 @@ impl HierarchialRunner { } } + pub fn set_flags(&mut self, flags: u32) { + self.flags = flags; + for r in self.running.iter_mut() { + r.run.set_flags(flags); + } + } + pub fn is_ended(&self) -> bool { self.running.is_empty() || self.error @@ -267,7 +276,7 @@ impl HierarchialRunner { fn push_synthetic_stack_frame(&mut self, current_env: Rc, info: &RunStepRelevantInfo) { let arg_step = clvm::start_step(info.prog.clone(), info.runtime_argument_values.clone()); - let arg_run = CldbRun::new( + let mut arg_run = CldbRun::new( self.runner.clone(), self.prim_map.clone(), Box::new(CldbRunEnv::new( @@ -278,6 +287,8 @@ impl HierarchialRunner { arg_step, ); + arg_run.set_flags(self.flags); + let mut named_args = HashMap::new(); get_args_from_env( &mut named_args, @@ -306,7 +317,7 @@ impl HierarchialRunner { // Make an empty frame to repopulate (maybe option here?). let step = clvm::start_step(info.prog.clone(), current_env.clone()); - let run = CldbRun::new( + let mut run = CldbRun::new( self.runner.clone(), self.prim_map.clone(), Box::new(CldbRunEnv::new( @@ -317,6 +328,8 @@ impl HierarchialRunner { step, ); + run.set_flags(self.flags); + self.running.push(HierarchyFrame { purpose: RunPurpose::Main, @@ -380,6 +393,8 @@ impl HierarchialRunner { step, ); + self.running[idx].run.set_flags(self.flags); + Ok(HierarchialStepResult::ShapeChange) } else if let Some(info) = relevant_run_step_info(&self.symbol_table, ¤t_step) { // Create a frame based on the last argument. diff --git a/src/tests/compiler/cldb.rs b/src/tests/compiler/cldb.rs index 47da4a32a..9835d1ce4 100644 --- a/src/tests/compiler/cldb.rs +++ b/src/tests/compiler/cldb.rs @@ -4,9 +4,9 @@ use std::rc::Rc; use clvmr::allocator::Allocator; -use crate::classic::clvm_tools::cmds::{cldb_hierarchy, YamlElement}; +use crate::classic::clvm_tools::cmds::{cldb_hierarchy, YamlElement, CldbHierarchyArgs}; use crate::classic::clvm_tools::stages::stage_0::DefaultProgramRunner; -use crate::compiler::cldb::{hex_to_modern_sexp, CldbNoOverride, CldbRun, CldbRunEnv}; +use crate::compiler::cldb::{hex_to_modern_sexp, CldbNoOverride, CldbRun, CldbRunEnv, FAVOR_HEX}; use crate::compiler::clvm::{start_step, RunStep}; use crate::compiler::compiler::{compile_file, DefaultCompilerOpts}; use crate::compiler::comptypes::CompilerOpts; @@ -49,6 +49,7 @@ fn run_clvm_in_cldb( symbols: HashMap, args: Rc, viewer: &mut V, + flags: u32, ) -> Option where V: StepOfCldbViewer, @@ -67,6 +68,8 @@ where Box::new(CldbNoOverride::new_symbols(symbols)), ); let mut cldbrun = CldbRun::new(runner, Rc::new(prim_map), Box::new(cldbenv), step); + cldbrun.set_flags(flags); + let mut output: BTreeMap = BTreeMap::new(); loop { @@ -116,6 +119,7 @@ fn test_run_clvm_in_cldb() { symbols, args, &mut DoesntWatchCldb {}, + 0, ), Some("120".to_string()) ); @@ -169,6 +173,7 @@ fn compile_and_run_program_with_tree( input_program_text: &str, args_text: &str, search_paths: &[String], + flags: u32, ) -> Vec> { let mut allocator = Allocator::new(); let runner = Rc::new(DefaultProgramRunner::new()); @@ -197,15 +202,16 @@ fn compile_and_run_program_with_tree( let program_lines: Rc> = Rc::new(input_program_text.lines().map(|x| x.to_string()).collect()); - cldb_hierarchy( + cldb_hierarchy(CldbHierarchyArgs { runner, - Rc::new(prim_map), - Some(input_file.to_owned()), - program_lines, - Rc::new(use_symbol_table), - Rc::new(program), + prim_map: Rc::new(prim_map), + input_file_name: Some(input_file.to_owned()), + lines: program_lines, + symbol_table: Rc::new(use_symbol_table), + prog: Rc::new(program), args, - ) + flags, + }) } fn run_program_as_tree_from_hex( @@ -240,15 +246,16 @@ fn run_program_as_tree_from_hex( } let program_lines = Rc::new(vec![]); let runner = Rc::new(DefaultProgramRunner::new()); - cldb_hierarchy( + cldb_hierarchy(CldbHierarchyArgs { runner, - Rc::new(prim_map), - Some(file_name.to_owned()), - program_lines, - Rc::new(symbol_table), - program, + prim_map: Rc::new(prim_map), + input_file_name: Some(file_name.to_owned()), + lines: program_lines, + symbol_table: Rc::new(symbol_table), + prog: program, args, - ) + flags: 0, + }) } fn compare_run_output( @@ -275,7 +282,8 @@ fn test_cldb_hierarchy_mode() { .expect("test resources should exist: test_rec_1.cl"); let input_file = "./test_rec_1.cl"; - let result = compile_and_run_program_with_tree(&input_file, &input_program, "(3 2)", &vec![]); + let result = + compile_and_run_program_with_tree(&input_file, &input_program, "(3 2)", &vec![], 0); compare_run_output(result, run_entries); } @@ -362,7 +370,8 @@ fn test_cldb_explicit_throw() { program, HashMap::new(), args, - &mut watcher + &mut watcher, + 0 ), None ); @@ -387,7 +396,47 @@ fn test_clvm_operator_with_weird_tail() { HashMap::new(), args, &mut DoesntWatchCldb {}, + 0, ), Some("8".to_string()) ); } + +#[test] +fn test_cldb_with_favor_hex() { + let filename = "favor_hex.clvm"; + let loc = Srcloc::start(filename); + let program = "(concat (1 . 1) (1 . 1122334455))"; + let parsed = parse_sexp(loc.clone(), program.as_bytes().iter().copied()).expect("should parse"); + let args = Rc::new(SExp::Nil(loc)); + let program_lines = Rc::new(vec![program.to_string()]); + + assert_eq!( + run_clvm_in_cldb( + filename, + program_lines, + parsed[0].clone(), + HashMap::new(), + args, + &mut DoesntWatchCldb {}, + FAVOR_HEX, + ), + Some("0x0142e576f7".to_string()) + ); +} + +#[test] +fn test_cldb_hierarchy_hex() { + let json_text = fs::read_to_string("resources/tests/cldb_tree/hex.json") + .expect("test resources should exist: test.json"); + let run_entries: Vec = + serde_json::from_str(&json_text).expect("should contain json"); + let input_program = "(mod () (concat 1 1122334455))".to_string(); + + let input_file = "test_with_hex.clsp"; + + let result = + compile_and_run_program_with_tree(&input_file, &input_program, "()", &vec![], FAVOR_HEX); + + compare_run_output(result, run_entries); +} From 2ed971542baebfaba913d4e0a534a29a9c1cf88a Mon Sep 17 00:00:00 2001 From: arty Date: Sat, 6 Jul 2024 00:56:43 -0700 Subject: [PATCH 02/10] fmt + clippy --- src/classic/clvm_tools/cmds.rs | 6 ++---- src/compiler/cldb.rs | 7 +++---- src/tests/compiler/cldb.rs | 2 +- 3 files changed, 6 insertions(+), 9 deletions(-) diff --git a/src/classic/clvm_tools/cmds.rs b/src/classic/clvm_tools/cmds.rs index 0fc6043f8..6d8b17f3d 100644 --- a/src/classic/clvm_tools/cmds.rs +++ b/src/classic/clvm_tools/cmds.rs @@ -396,9 +396,7 @@ pub struct CldbHierarchyArgs { pub flags: u32, } -pub fn cldb_hierarchy( - args: CldbHierarchyArgs -) -> Vec> { +pub fn cldb_hierarchy(args: CldbHierarchyArgs) -> Vec> { let mut runner = HierarchialRunner::new( args.runner, args.prim_map, @@ -740,7 +738,7 @@ pub fn cldb(args: &[String]) { if parsed_args.contains_key("tree") { let result = cldb_hierarchy(CldbHierarchyArgs { - runner: runner, + runner, prim_map: Rc::new(prim_map), input_file_name: input_file, lines: program_lines, diff --git a/src/compiler/cldb.rs b/src/compiler/cldb.rs index 4e4882312..3cdc6d6bf 100644 --- a/src/compiler/cldb.rs +++ b/src/compiler/cldb.rs @@ -594,7 +594,6 @@ fn hex_to_modern_sexp_inner( symbol_table: &HashMap, loc: Srcloc, program: NodePtr, - flags: u32, ) -> Result, EvalErr> { let hash = sha256tree(allocator, program); let hash_str = hash.hex(); @@ -606,8 +605,8 @@ fn hex_to_modern_sexp_inner( match allocator.sexp(program) { allocator::SExp::Pair(a, b) => Ok(Rc::new(SExp::Cons( srcloc.clone(), - hex_to_modern_sexp_inner(allocator, symbol_table, srcloc.clone(), a, flags)?, - hex_to_modern_sexp_inner(allocator, symbol_table, srcloc, b, flags)?, + hex_to_modern_sexp_inner(allocator, symbol_table, srcloc.clone(), a)?, + hex_to_modern_sexp_inner(allocator, symbol_table, srcloc, b)?, ))), _ => convert_from_clvm_rs(allocator, srcloc, program).map_err(|_| { EvalErr( @@ -636,7 +635,7 @@ pub fn hex_to_modern_sexp( .map(|x| x.1) .map_err(|_| RunFailure::RunErr(loc.clone(), "Bad conversion from hex".to_string()))?; - hex_to_modern_sexp_inner(allocator, symbol_table, loc.clone(), sexp, 0).map_err(|_| { + hex_to_modern_sexp_inner(allocator, symbol_table, loc.clone(), sexp).map_err(|_| { RunFailure::RunErr(loc, "Failed to convert from classic to modern".to_string()) }) } diff --git a/src/tests/compiler/cldb.rs b/src/tests/compiler/cldb.rs index 9835d1ce4..d43053da0 100644 --- a/src/tests/compiler/cldb.rs +++ b/src/tests/compiler/cldb.rs @@ -4,7 +4,7 @@ use std::rc::Rc; use clvmr::allocator::Allocator; -use crate::classic::clvm_tools::cmds::{cldb_hierarchy, YamlElement, CldbHierarchyArgs}; +use crate::classic::clvm_tools::cmds::{cldb_hierarchy, CldbHierarchyArgs, YamlElement}; use crate::classic::clvm_tools::stages::stage_0::DefaultProgramRunner; use crate::compiler::cldb::{hex_to_modern_sexp, CldbNoOverride, CldbRun, CldbRunEnv, FAVOR_HEX}; use crate::compiler::clvm::{start_step, RunStep}; From 83c95d9a721056b51d336800805d4d92fb9377bb Mon Sep 17 00:00:00 2001 From: arty Date: Sat, 6 Jul 2024 01:22:02 -0700 Subject: [PATCH 03/10] Fix mixup --- src/compiler/cldb.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/compiler/cldb.rs b/src/compiler/cldb.rs index 3cdc6d6bf..89ebc82da 100644 --- a/src/compiler/cldb.rs +++ b/src/compiler/cldb.rs @@ -154,14 +154,14 @@ pub fn hexize(a: Rc, flags: u32) -> Rc { a.clone() } } - SExp::Cons(l, a, b) => { - let new_a = hexize(a.clone(), flags); - let new_b = hexize(b.clone(), flags); - if Rc::as_ptr(&new_a) == Rc::as_ptr(a) && Rc::as_ptr(&new_b) == Rc::as_ptr(b) { + SExp::Cons(loc, l, r) => { + let new_l = hexize(l.clone(), flags); + let new_r = hexize(r.clone(), flags); + if Rc::as_ptr(&new_l) == Rc::as_ptr(l) && Rc::as_ptr(&new_r) == Rc::as_ptr(r) { return a.clone(); } - Rc::new(SExp::Cons(l.clone(), new_a, new_b)) + Rc::new(SExp::Cons(loc.clone(), new_l, new_r)) } _ => a.clone(), } From 468b2fd9b5192a6c9d48370fd941c55dcaff3d26 Mon Sep 17 00:00:00 2001 From: arty Date: Sat, 6 Jul 2024 09:27:34 -0700 Subject: [PATCH 04/10] Previous output was generated when the hexify function was not properly traversing --- resources/tests/cldb_tree/hex.json | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/resources/tests/cldb_tree/hex.json b/resources/tests/cldb_tree/hex.json index be0039dc0..c75c4ca61 100644 --- a/resources/tests/cldb_tree/hex.json +++ b/resources/tests/cldb_tree/hex.json @@ -5,13 +5,13 @@ "Function-Args": {}, "Function-Name": "test_with_hex.clsp", "Output": { - "Arguments": "(())", + "Arguments": "(() ())", "Operator": "4", "Operator-Location": "test_with_hex.clsp(1):10-test_with_hex.clsp(1):16", "Result-Location": "test_with_hex.clsp(1):10-test_with_hex.clsp(1):16", "Function": "concat", "Row": "0", - "Value": "()" + "Value": "(())" } } ] From 7617e02128dc62e037535c32d3fcb45a721f0463 Mon Sep 17 00:00:00 2001 From: arty Date: Sat, 6 Jul 2024 10:55:46 -0700 Subject: [PATCH 05/10] Change how string output is generated for atoms and strings that won't be canonical. Exclude '"', ' ' and "'" from atoms and '"' from strings. Changes the print representation of strings and atoms only. --- resources/tests/test_string_repr.clsp | 8 ++++++++ src/compiler/clvm.rs | 2 +- src/compiler/sexp.rs | 7 ++++--- src/tests/classic/run.rs | 14 ++++++++++++++ 4 files changed, 27 insertions(+), 4 deletions(-) create mode 100644 resources/tests/test_string_repr.clsp diff --git a/resources/tests/test_string_repr.clsp b/resources/tests/test_string_repr.clsp new file mode 100644 index 000000000..825bd305c --- /dev/null +++ b/resources/tests/test_string_repr.clsp @@ -0,0 +1,8 @@ +(mod () + (include *standard-cl-23*) + + (defmac M (X) (string-append "test" X)) + (defmac A (X) (c 1 (string->symbol (string-append "test" X)))) + + (list (M '"') (M "'") (M " hi") (A '"') (A "'") (A "_hi")) + ) \ No newline at end of file diff --git a/src/compiler/clvm.rs b/src/compiler/clvm.rs index 41a8b4cbf..3c912807a 100644 --- a/src/compiler/clvm.rs +++ b/src/compiler/clvm.rs @@ -305,7 +305,7 @@ pub fn convert_from_clvm_rs( } else { Ok(Rc::new(SExp::Integer(loc, integer))) } - } else if int_conv && !printable(atom_data, true) { + } else if int_conv && !printable(atom_data, false) { Ok(Rc::new(SExp::QuotedString(loc, b'x', atom_data.to_vec()))) } else { Ok(Rc::new(SExp::Atom(loc, atom_data.to_vec()))) diff --git a/src/compiler/sexp.rs b/src/compiler/sexp.rs index 46adc0137..6835c3d47 100644 --- a/src/compiler/sexp.rs +++ b/src/compiler/sexp.rs @@ -362,9 +362,10 @@ pub fn decode_string(v: &[u8]) -> String { pub fn printable(a: &[u8], quoted: bool) -> bool { !a.iter().any(|ch| { - (*ch as char).is_control() - || !(*ch as char).is_ascii() - || (!quoted && ch.is_ascii_whitespace()) + *ch < 32 + || *ch > 126 + || (!quoted && ((*ch as char).is_ascii_whitespace() || *ch == b'\'')) + || *ch == b'"' }) } diff --git a/src/tests/classic/run.rs b/src/tests/classic/run.rs index 57141d49c..d64369ee8 100644 --- a/src/tests/classic/run.rs +++ b/src/tests/classic/run.rs @@ -2428,3 +2428,17 @@ fn test_assign_cse_tricky_2() { let wanted_repr = "(2 (1 2 10 (4 2 (4 5 ()))) (4 (1 ((11 5 11) 2 8 (4 2 (4 5 (4 11 ())))) (2 22 (4 2 (4 3 (4 (18 5 (1 . 11)) (4 (16 5 (1 . 1)) ()))))) (2 30 (4 2 (4 3 (4 (1 . 121) ())))) 2 (3 (9 17 (1 . 13)) (1 2 12 (4 2 (4 45 (4 21 ())))) (1 2 (3 (9 17 (1 . 15)) (1 2 8 (4 2 (4 45 (4 21 ())))) (1 . 11)) 1)) 1) 1))"; assert_eq!(program, wanted_repr); } + +#[test] +fn test_quote_string_generation() { + let filename = "resources/tests/test_string_repr.clsp"; + let program = do_basic_run(&vec!["run".to_string(), filename.to_string()]) + .trim() + .to_string(); + let wanted_repr = "(4 (1 . 0x7465737422) (4 (1 . \"test'\") (4 (1 . \"test hi\") (4 (1 . 499918271522) (4 (1 . 499918271527) (4 (1 . test_hi) ()))))))"; + assert_eq!(program, wanted_repr); + let brun_result = do_basic_brun(&vec!["brun".to_string(), program]) + .trim() + .to_string(); + assert_eq!(brun_result, "(0x7465737422 \"test'\" \"test hi\" 0x7465737422 \"test'\" \"test_hi\")"); +} From da9b1a893df8a2a1989ebaebead831eba006be2c Mon Sep 17 00:00:00 2001 From: arty Date: Sat, 6 Jul 2024 10:58:02 -0700 Subject: [PATCH 06/10] Inadvertent change --- src/compiler/clvm.rs | 2 +- src/tests/classic/run.rs | 5 ++++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/src/compiler/clvm.rs b/src/compiler/clvm.rs index 3c912807a..41a8b4cbf 100644 --- a/src/compiler/clvm.rs +++ b/src/compiler/clvm.rs @@ -305,7 +305,7 @@ pub fn convert_from_clvm_rs( } else { Ok(Rc::new(SExp::Integer(loc, integer))) } - } else if int_conv && !printable(atom_data, false) { + } else if int_conv && !printable(atom_data, true) { Ok(Rc::new(SExp::QuotedString(loc, b'x', atom_data.to_vec()))) } else { Ok(Rc::new(SExp::Atom(loc, atom_data.to_vec()))) diff --git a/src/tests/classic/run.rs b/src/tests/classic/run.rs index d64369ee8..b38fc2a52 100644 --- a/src/tests/classic/run.rs +++ b/src/tests/classic/run.rs @@ -2440,5 +2440,8 @@ fn test_quote_string_generation() { let brun_result = do_basic_brun(&vec!["brun".to_string(), program]) .trim() .to_string(); - assert_eq!(brun_result, "(0x7465737422 \"test'\" \"test hi\" 0x7465737422 \"test'\" \"test_hi\")"); + assert_eq!( + brun_result, + "(0x7465737422 \"test'\" \"test hi\" 0x7465737422 \"test'\" \"test_hi\")" + ); } From ecd4338fb96f41889e6169fb560f648024ce8d48 Mon Sep 17 00:00:00 2001 From: arty Date: Sun, 7 Jul 2024 18:44:49 -0700 Subject: [PATCH 07/10] Add an exhaustive test and handle backslash heavy-handedly --- src/compiler/sexp.rs | 1 + src/tests/compiler/compiler.rs | 85 +++++++++++++++++++++++++++++++++- 2 files changed, 85 insertions(+), 1 deletion(-) diff --git a/src/compiler/sexp.rs b/src/compiler/sexp.rs index 6835c3d47..2bbd4fbbd 100644 --- a/src/compiler/sexp.rs +++ b/src/compiler/sexp.rs @@ -366,6 +366,7 @@ pub fn printable(a: &[u8], quoted: bool) -> bool { || *ch > 126 || (!quoted && ((*ch as char).is_ascii_whitespace() || *ch == b'\'')) || *ch == b'"' + || *ch == b'\\' }) } diff --git a/src/tests/compiler/compiler.rs b/src/tests/compiler/compiler.rs index 0e6ef1094..ba29d80f1 100644 --- a/src/tests/compiler/compiler.rs +++ b/src/tests/compiler/compiler.rs @@ -4,17 +4,20 @@ use std::rc::Rc; use clvm_rs::allocator::Allocator; use crate::classic::clvm::__type_compatibility__::bi_one; +use crate::classic::clvm_tools::binutils::disassemble; use crate::classic::clvm_tools::stages::stage_0::DefaultProgramRunner; use crate::compiler::clvm::run; use crate::compiler::compiler::{compile_file, DefaultCompilerOpts}; use crate::compiler::comptypes::{CompileErr, CompilerOpts}; -use crate::compiler::dialect::AcceptedDialect; +use crate::compiler::dialect::{AcceptedDialect, KNOWN_DIALECTS}; use crate::compiler::frontend::{collect_used_names_sexp, frontend}; use crate::compiler::rename::rename_in_cons; use crate::compiler::runtypes::RunFailure; use crate::compiler::sexp::{decode_string, enlist, parse_sexp, SExp}; use crate::compiler::srcloc::Srcloc; +use crate::tests::classic::run::do_basic_brun; + const TEST_TIMEOUT: usize = 1000000; fn compile_string(content: &String) -> Result { @@ -2448,3 +2451,83 @@ fn test_almost_empty_lambda_gives_error() { assert!(res.is_err()); assert!(format!("{res:?}").contains("Must provide at least arguments and body to lambda")); } + + +#[test] +fn test_exhaustive_chars() { + // Verify that we can create a program that gives the expected output using + // every byte value in the first, mid and last position of a value. + let mut substitute = vec![b'x', b'x', b'x']; + + let srcloc = Srcloc::start("*extest*"); + let make_test_program = |sub: Rc| { + // (mod () (include *standard-cl-23.1*) (q . )) + Rc::new(SExp::Cons( + srcloc.clone(), + Rc::new(SExp::Atom(srcloc.clone(), b"mod".to_vec())), + Rc::new(SExp::Cons( + srcloc.clone(), + Rc::new(SExp::Nil(srcloc.clone())), + Rc::new(SExp::Cons( + srcloc.clone(), + Rc::new(SExp::Cons( + srcloc.clone(), + Rc::new(SExp::Atom(srcloc.clone(), b"include".to_vec())), + Rc::new(SExp::Cons( + srcloc.clone(), + Rc::new(SExp::Atom(srcloc.clone(), b"*standard-cl-23.1*".to_vec())), + Rc::new(SExp::Nil(srcloc.clone())) + )), + )), + Rc::new(SExp::Cons( + srcloc.clone(), + Rc::new(SExp::Cons( + srcloc.clone(), + Rc::new(SExp::Integer(srcloc.clone(), bi_one())), + sub + )), + Rc::new(SExp::Nil(srcloc.clone())) + )) + )) + )) + )) + }; + + let runner = Rc::new(DefaultProgramRunner::new()); + + for i in 0..=2 { + for j in 0..=255 { + substitute[i] = j; + + let sub_qe = + Rc::new(SExp::QuotedString( + srcloc.clone(), + b'"', + substitute.clone() + )); + + let mut allocator = Allocator::new(); + let mut opts: Rc = Rc::new(DefaultCompilerOpts::new("*extest*")); + let dialect = KNOWN_DIALECTS["*standard-cl-23.1*"].accepted.clone(); + opts = opts.set_dialect(dialect); + + let compiled = opts.compile_program( + &mut allocator, + runner.clone(), + make_test_program(sub_qe), + &mut HashMap::new(), + ).expect("should compile"); + + let compiled_output = compiled.to_string(); + let result = do_basic_brun(&vec!["brun".to_string(), compiled_output]) + .trim() + .to_string(); + + let classic_atom = allocator.new_atom(&substitute).expect("should work"); + let disassembled = disassemble(&mut allocator, classic_atom, None); + assert_eq!(result, disassembled); + + substitute[i] = b'x'; + } + } +} From 97f1f9f74d6314477a8f1cc54b9dfc302ca57196 Mon Sep 17 00:00:00 2001 From: arty Date: Sun, 7 Jul 2024 18:50:12 -0700 Subject: [PATCH 08/10] fmt --- src/tests/compiler/compiler.rs | 34 +++++++++++++++------------------- 1 file changed, 15 insertions(+), 19 deletions(-) diff --git a/src/tests/compiler/compiler.rs b/src/tests/compiler/compiler.rs index ba29d80f1..9bca3b472 100644 --- a/src/tests/compiler/compiler.rs +++ b/src/tests/compiler/compiler.rs @@ -2452,7 +2452,6 @@ fn test_almost_empty_lambda_gives_error() { assert!(format!("{res:?}").contains("Must provide at least arguments and body to lambda")); } - #[test] fn test_exhaustive_chars() { // Verify that we can create a program that gives the expected output using @@ -2476,7 +2475,7 @@ fn test_exhaustive_chars() { Rc::new(SExp::Cons( srcloc.clone(), Rc::new(SExp::Atom(srcloc.clone(), b"*standard-cl-23.1*".to_vec())), - Rc::new(SExp::Nil(srcloc.clone())) + Rc::new(SExp::Nil(srcloc.clone())), )), )), Rc::new(SExp::Cons( @@ -2484,12 +2483,12 @@ fn test_exhaustive_chars() { Rc::new(SExp::Cons( srcloc.clone(), Rc::new(SExp::Integer(srcloc.clone(), bi_one())), - sub + sub, )), - Rc::new(SExp::Nil(srcloc.clone())) - )) - )) - )) + Rc::new(SExp::Nil(srcloc.clone())), + )), + )), + )), )) }; @@ -2499,24 +2498,21 @@ fn test_exhaustive_chars() { for j in 0..=255 { substitute[i] = j; - let sub_qe = - Rc::new(SExp::QuotedString( - srcloc.clone(), - b'"', - substitute.clone() - )); + let sub_qe = Rc::new(SExp::QuotedString(srcloc.clone(), b'"', substitute.clone())); let mut allocator = Allocator::new(); let mut opts: Rc = Rc::new(DefaultCompilerOpts::new("*extest*")); let dialect = KNOWN_DIALECTS["*standard-cl-23.1*"].accepted.clone(); opts = opts.set_dialect(dialect); - let compiled = opts.compile_program( - &mut allocator, - runner.clone(), - make_test_program(sub_qe), - &mut HashMap::new(), - ).expect("should compile"); + let compiled = opts + .compile_program( + &mut allocator, + runner.clone(), + make_test_program(sub_qe), + &mut HashMap::new(), + ) + .expect("should compile"); let compiled_output = compiled.to_string(); let result = do_basic_brun(&vec!["brun".to_string(), compiled_output]) From 273c3d8a11f3a63357121c0dba0aef6eacf75b20 Mon Sep 17 00:00:00 2001 From: arty Date: Wed, 17 Jul 2024 23:16:33 -0700 Subject: [PATCH 09/10] Add commentary --- resources/tests/test_string_repr.clsp | 20 ++++++++++++++++---- src/tests/classic/run.rs | 18 ++++++++++++++++++ 2 files changed, 34 insertions(+), 4 deletions(-) diff --git a/resources/tests/test_string_repr.clsp b/resources/tests/test_string_repr.clsp index 825bd305c..f094bb247 100644 --- a/resources/tests/test_string_repr.clsp +++ b/resources/tests/test_string_repr.clsp @@ -1,8 +1,20 @@ +;; +;; This program is used by the test_quote_string_generation test +;; which checks the representation of these outputs and ensures +;; that they're properly accepted by brun, which uses the classic +;; chialisp tokenizer/parser. +;; +;; the qs macro appends the given string onto "test" +;; the atom macro gives a quoted atom starting with test and +;; ending with the text given in the string. +;; +;; modern's macro system makes these different objects. +;; (mod () (include *standard-cl-23*) - (defmac M (X) (string-append "test" X)) - (defmac A (X) (c 1 (string->symbol (string-append "test" X)))) + (defmac qs (X) (string-append "test" X)) + (defmac atom (X) (c 1 (string->symbol (string-append "test" X)))) - (list (M '"') (M "'") (M " hi") (A '"') (A "'") (A "_hi")) - ) \ No newline at end of file + (list (qs '"') (qs "'") (qs " hi") (atom '"') (atom "'") (atom "_hi")) + ) diff --git a/src/tests/classic/run.rs b/src/tests/classic/run.rs index 09fcc9d0e..833358886 100644 --- a/src/tests/classic/run.rs +++ b/src/tests/classic/run.rs @@ -2431,15 +2431,33 @@ fn test_assign_cse_tricky_2() { #[test] fn test_quote_string_generation() { + // The program run here produces a list of strings and quoted atoms that have + // representations that must be flattened in various ways in this test. let filename = "resources/tests/test_string_repr.clsp"; let program = do_basic_run(&vec!["run".to_string(), filename.to_string()]) .trim() .to_string(); + // The proram produces this list + // (list (qs '"') (qs "'") (qs " hi") (atom '"') (atom "'") (atom "_hi")) + // + // where qs puts "test" in front of the given string and atom puts test in front of the + // given string, converts it to an atom and quotes it so that it won't be interpreted + // as an identifier. + // + // in other words + // (list 'test"' "test'" "test hi" + // (q . (string->symbol (string-append "test" '"'))) + // (q . (string->symbol (string-append "test" "'"))) + // (q . test_hi) + // ) + // + // The result below shows that the strings and atoms are reproduced as expected. let wanted_repr = "(4 (1 . 0x7465737422) (4 (1 . \"test'\") (4 (1 . \"test hi\") (4 (1 . 499918271522) (4 (1 . 499918271527) (4 (1 . test_hi) ()))))))"; assert_eq!(program, wanted_repr); let brun_result = do_basic_brun(&vec!["brun".to_string(), program]) .trim() .to_string(); + // This shows that brun interpreted and passed through the values successfully. assert_eq!( brun_result, "(0x7465737422 \"test'\" \"test hi\" 0x7465737422 \"test'\" \"test_hi\")" From cc69b5abf4c9b7ea099e91e94c329ce12e9b295c Mon Sep 17 00:00:00 2001 From: arty Date: Wed, 17 Jul 2024 23:49:02 -0700 Subject: [PATCH 10/10] Change hexize to improve_presentation because it takes flags and can respond to more types of formatting in the future. Add a pre-hex test to show how it differs --- resources/tests/cldb_tree/pre_hex.json | 49 ++++++++++++++++++++++++++ src/classic/clvm_tools/cmds.rs | 7 ++-- src/compiler/cldb.rs | 16 ++++----- src/tests/compiler/cldb.rs | 15 ++++++++ 4 files changed, 77 insertions(+), 10 deletions(-) create mode 100644 resources/tests/cldb_tree/pre_hex.json diff --git a/resources/tests/cldb_tree/pre_hex.json b/resources/tests/cldb_tree/pre_hex.json new file mode 100644 index 000000000..a1df3be66 --- /dev/null +++ b/resources/tests/cldb_tree/pre_hex.json @@ -0,0 +1,49 @@ +[ + { + "Compute": [ + { + "Function-Args": {}, + "Function-Name": "test_with_hex.clsp", + "Output": { + "Arguments": "(() ())", + "Operator": "4", + "Operator-Location": "test_with_hex.clsp(1):10-test_with_hex.clsp(1):16", + "Result-Location": "test_with_hex.clsp(1):10-test_with_hex.clsp(1):16", + "Function": "concat", + "Row": "0", + "Value": "(())" + } + } + ] + }, + { + "Compute": [ + { + "Function-Args": {}, + "Function-Name": "test_with_hex.clsp", + "Output": { + "Arguments": "(1 1122334455)", + "Function-Context": "()", + "Function": "concat", + "Operator": "14", + "Operator-Location": "test_with_hex.clsp(1):17", + "Result-Location": "test_with_hex.clsp(1):10-test_with_hex.clsp(1):16", + "Row": "1", + "Value": "5417301751" + } + } + ] + }, + { + "Compute": [ + { + "Function-Args": {}, + "Function-Name": "test_with_hex.clsp", + "Output": { + "Final": "5417301751", + "Final-Location": "test_with_hex.clsp(1):10-test_with_hex.clsp(1):16" + } + } + ] + } +] diff --git a/src/classic/clvm_tools/cmds.rs b/src/classic/clvm_tools/cmds.rs index 6d8b17f3d..af5073109 100644 --- a/src/classic/clvm_tools/cmds.rs +++ b/src/classic/clvm_tools/cmds.rs @@ -50,7 +50,7 @@ use crate::classic::platform::argparse::{ }; use crate::compiler::cldb::{ - hex_to_modern_sexp, hexize, CldbNoOverride, CldbRun, CldbRunEnv, FAVOR_HEX, + hex_to_modern_sexp, improve_presentation, CldbNoOverride, CldbRun, CldbRunEnv, FAVOR_HEX, }; use crate::compiler::cldb_hierarchy::{HierarchialRunner, HierarchialStepResult, RunPurpose}; use crate::compiler::clvm::start_step; @@ -443,7 +443,10 @@ pub fn cldb_hierarchy(args: CldbHierarchyArgs) -> Vec) -> Rc { } } -pub fn hexize(a: Rc, flags: u32) -> Rc { +pub fn improve_presentation(a: Rc, flags: u32) -> Rc { match a.borrow() { SExp::Atom(l, av) => { if av.len() > 2 && (flags & FAVOR_HEX) != 0 { @@ -155,8 +155,8 @@ pub fn hexize(a: Rc, flags: u32) -> Rc { } } SExp::Cons(loc, l, r) => { - let new_l = hexize(l.clone(), flags); - let new_r = hexize(r.clone(), flags); + let new_l = improve_presentation(l.clone(), flags); + let new_r = improve_presentation(r.clone(), flags); if Rc::as_ptr(&new_l) == Rc::as_ptr(l) && Rc::as_ptr(&new_r) == Rc::as_ptr(r) { return a.clone(); } @@ -248,7 +248,7 @@ impl CldbRun { .insert("Result-Location".to_string(), l.to_string()); self.to_print.insert( "Value".to_string(), - hexize(x.clone(), self.flags).to_string(), + improve_presentation(x.clone(), self.flags).to_string(), ); self.to_print .insert("Row".to_string(), self.row.to_string()); @@ -270,7 +270,7 @@ impl CldbRun { } } Ok(RunStep::Done(l, x)) => { - let final_value = hexize(x.clone(), self.flags); + let final_value = improve_presentation(x.clone(), self.flags); self.to_print .insert("Final-Location".to_string(), l.to_string()); self.to_print @@ -313,8 +313,8 @@ impl CldbRun { if should_print_basic_output { self.env.add_context( sexp.borrow(), - hexize(c.clone(), self.flags).borrow(), - Some(hexize(a.clone(), self.flags)), + improve_presentation(c.clone(), self.flags).borrow(), + Some(improve_presentation(a.clone(), self.flags)), &mut self.to_print, ); self.env.add_function(sexp, &mut self.to_print); @@ -327,7 +327,7 @@ impl CldbRun { .insert("Throw-Location".to_string(), l.to_string()); self.to_print.insert( "Throw".to_string(), - hexize(s.clone(), self.flags).to_string(), + improve_presentation(s.clone(), self.flags).to_string(), ); swap(&mut self.to_print, &mut result); diff --git a/src/tests/compiler/cldb.rs b/src/tests/compiler/cldb.rs index d43053da0..c22074a22 100644 --- a/src/tests/compiler/cldb.rs +++ b/src/tests/compiler/cldb.rs @@ -440,3 +440,18 @@ fn test_cldb_hierarchy_hex() { compare_run_output(result, run_entries); } + +#[test] +fn test_cldb_hierarchy_before_hex() { + let json_text = fs::read_to_string("resources/tests/cldb_tree/pre_hex.json") + .expect("test resources should exist: test.json"); + let run_entries: Vec = + serde_json::from_str(&json_text).expect("should contain json"); + let input_program = "(mod () (concat 1 1122334455))".to_string(); + + let input_file = "test_with_hex.clsp"; + + let result = compile_and_run_program_with_tree(&input_file, &input_program, "()", &vec![], 0); + + compare_run_output(result, run_entries); +}