From 06a1ff48422db37ac6f93302d29ab0edfd21a19c Mon Sep 17 00:00:00 2001 From: arty Date: Wed, 22 May 2024 05:48:36 -0700 Subject: [PATCH 1/3] local tests + fmt + clippy --- src/classic/clvm_tools/binutils.rs | 2 +- src/compiler/clvm.rs | 44 +++++++++- src/compiler/compiler.rs | 4 +- src/compiler/dialect.rs | 18 +++++ src/tests/classic/smoke.rs | 41 ++++++++++ src/tests/compiler/clvm.rs | 81 ++++++++++++++++++- src/tests/compiler/compiler.rs | 2 + src/tests/compiler/optimizer/cse.rs | 2 + src/tests/compiler/optimizer/cse_fuzz.rs | 1 + .../compiler/optimizer/cse_regression.rs | 1 + src/tests/compiler/optimizer/depgraph.rs | 1 + src/tests/compiler/optimizer/output.rs | 2 + src/tests/compiler/preprocessor.rs | 2 + 13 files changed, 194 insertions(+), 7 deletions(-) diff --git a/src/classic/clvm_tools/binutils.rs b/src/classic/clvm_tools/binutils.rs index dbb2afc46..e06192a36 100644 --- a/src/classic/clvm_tools/binutils.rs +++ b/src/classic/clvm_tools/binutils.rs @@ -98,7 +98,7 @@ pub fn ir_for_atom( // Determine whether the bytes identity an integer in canonical form. // It's not canonical if there is oversized sign extension. - if !has_oversized_sign_extension(atom) { + if atom.data() != &[0] && !has_oversized_sign_extension(atom) { return IRRepr::Int(atom.clone(), true); } } diff --git a/src/compiler/clvm.rs b/src/compiler/clvm.rs index 3e14cb9fe..a0b7765bf 100644 --- a/src/compiler/clvm.rs +++ b/src/compiler/clvm.rs @@ -1,5 +1,7 @@ use std::borrow::Borrow; +use std::cell::RefCell; use std::collections::HashMap; +use std::mem::swap; use std::rc::Rc; use clvm_rs::allocator; @@ -15,11 +17,36 @@ use crate::classic::clvm_tools::stages::stage_0::TRunProgram; use crate::compiler::prims; use crate::compiler::runtypes::RunFailure; -use crate::compiler::sexp::{parse_sexp, SExp}; +use crate::compiler::sexp::{parse_sexp, printable, SExp}; use crate::compiler::srcloc::Srcloc; use crate::util::{number_from_u8, u8_from_number, Number}; +thread_local! { + static NEW_COMPILATION_LEVEL_INT: RefCell = const { RefCell::new(true) }; +} + +pub struct NewStyleIntConversion(bool); + +impl NewStyleIntConversion { + pub fn new(mut new_val: bool) -> NewStyleIntConversion { + NewStyleIntConversion(NEW_COMPILATION_LEVEL_INT.with(|v| { + let mut val_ref = v.borrow_mut(); + swap(&mut new_val, &mut val_ref); + new_val + })) + } + fn setting() -> bool { + NEW_COMPILATION_LEVEL_INT.with(|v| *v.borrow()) + } +} + +impl Drop for NewStyleIntConversion { + fn drop(&mut self) { + NEW_COMPILATION_LEVEL_INT.with(|v| *v.borrow_mut() = self.0) + } +} + /// Provide a way of intercepting and running new primitives. pub trait PrimOverride { fn try_handle( @@ -236,7 +263,7 @@ pub fn convert_to_clvm_rs( .new_atom(x) .map_err(|_e| RunFailure::RunErr(head.loc(), format!("failed to alloc string {head}"))), SExp::Integer(_, i) => { - if *i == bi_zero() { + if NewStyleIntConversion::setting() && *i == bi_zero() { Ok(allocator.null()) } else { allocator @@ -264,6 +291,7 @@ pub fn convert_from_clvm_rs( ) -> Result, RunFailure> { match allocator.sexp(head) { allocator::SExp::Atom => { + let int_conv = NewStyleIntConversion::setting(); let atom_data = allocator.atom(head); if atom_data.is_empty() { Ok(Rc::new(SExp::Nil(loc))) @@ -272,11 +300,13 @@ pub fn convert_from_clvm_rs( // Ensure that atom values that don't evaluate equal to integers // are represented faithfully as atoms. if u8_from_number(integer.clone()) == atom_data { - if atom_data == [0] { + if int_conv && atom_data == [0] { Ok(Rc::new(SExp::QuotedString(loc, b'x', atom_data.to_vec()))) } else { Ok(Rc::new(SExp::Integer(loc, integer))) } + } 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()))) } @@ -782,7 +812,13 @@ pub fn sha256tree(s: Rc) -> Vec { hasher.finalize().to_vec() } SExp::Nil(_) => sha256tree_from_atom(&[]), - SExp::Integer(_, i) => sha256tree_from_atom(&u8_from_number(i.clone())), + SExp::Integer(_, i) => { + if NewStyleIntConversion::setting() && *i == bi_zero() { + sha256tree_from_atom(&[]) + } else { + sha256tree_from_atom(&u8_from_number(i.clone())) + } + } SExp::QuotedString(_, _, v) => sha256tree_from_atom(v), SExp::Atom(_, v) => sha256tree_from_atom(v), } diff --git a/src/compiler/compiler.rs b/src/compiler/compiler.rs index 7b8a9bb27..8209ac9d0 100644 --- a/src/compiler/compiler.rs +++ b/src/compiler/compiler.rs @@ -10,7 +10,7 @@ use clvm_rs::allocator::Allocator; use crate::classic::clvm::__type_compatibility__::{bi_one, bi_zero}; use crate::classic::clvm_tools::stages::stage_0::TRunProgram; -use crate::compiler::clvm::sha256tree; +use crate::compiler::clvm::{sha256tree, NewStyleIntConversion}; use crate::compiler::codegen::{codegen, hoist_body_let_binding, process_helper_let_bindings}; use crate::compiler::comptypes::{CompileErr, CompileForm, CompilerOpts, PrimaryCodegen}; use crate::compiler::dialect::{AcceptedDialect, KNOWN_DIALECTS}; @@ -163,6 +163,7 @@ pub fn compile_file( content: &str, symbol_table: &mut HashMap, ) -> Result { + let _int_conversion_bug = NewStyleIntConversion::new(opts.dialect().int_fix); let srcloc = Srcloc::start(&opts.filename()); let pre_forms = parse_sexp(srcloc.clone(), content.bytes())?; let mut context_wrapper = CompileContextWrapper::new( @@ -318,6 +319,7 @@ impl CompilerOpts for DefaultCompilerOpts { sexp: Rc, symbol_table: &mut HashMap, ) -> Result { + let _int_conversion_bug = NewStyleIntConversion::new(self.dialect.int_fix); let me = Rc::new(self.clone()); let optimizer = get_optimizer(&sexp.loc(), me.clone())?; let mut context_wrapper = diff --git a/src/compiler/dialect.rs b/src/compiler/dialect.rs index e3f876e93..4345558c0 100644 --- a/src/compiler/dialect.rs +++ b/src/compiler/dialect.rs @@ -11,6 +11,7 @@ use crate::compiler::sexp::decode_string; pub struct AcceptedDialect { pub stepping: Option, pub strict: bool, + pub int_fix: bool, } /// A package containing the content we should insert when a dialect include is @@ -44,6 +45,7 @@ lazy_static! { accepted: AcceptedDialect { stepping: Some(21), strict: true, + int_fix: false, }, content: indoc! {"( (defconstant *chialisp-version* 22) @@ -57,6 +59,7 @@ lazy_static! { accepted: AcceptedDialect { stepping: Some(22), strict: false, + int_fix: false, }, content: indoc! {"( (defconstant *chialisp-version* 22) @@ -70,6 +73,21 @@ lazy_static! { accepted: AcceptedDialect { stepping: Some(23), strict: true, + int_fix: false, + }, + content: indoc! {"( + (defconstant *chialisp-version* 23) + )"} + .to_string(), + }, + ), + ( + "*standard-cl-23.1*", + DialectDescription { + accepted: AcceptedDialect { + stepping: Some(23), + strict: true, + int_fix: true, }, content: indoc! {"( (defconstant *chialisp-version* 23) diff --git a/src/tests/classic/smoke.rs b/src/tests/classic/smoke.rs index 4930957ee..86987ab8b 100644 --- a/src/tests/classic/smoke.rs +++ b/src/tests/classic/smoke.rs @@ -64,6 +64,15 @@ fn large_odd_sized_neg_opd() { assert_eq!(result.rest(), "(0xfde1e61f36454dc00001)"); } +#[test] +fn mid_negative_value_opd_00() { + let mut allocator = Allocator::new(); + let result = opd_conversion() + .invoke(&mut allocator, &"00".to_string()) + .unwrap(); + assert_eq!(result.rest(), "0x00"); +} + #[test] fn mid_negative_value_opd_m1() { let mut allocator = Allocator::new(); @@ -827,3 +836,35 @@ fn test_sub_args() { "(\"body\" (f (\"test1\" \"test2\")) (f (r (\"test1\" \"test2\"))))" ); } + +#[test] +fn test_smoke_cl23_program_with_zero_folding() { + let mut s = Stream::new(None); + launch_tool( + &mut s, + &vec![ + "run".to_string(), + "(mod () (include *standard-cl-23*) (defconst X (concat 0x00 0x00)) X)".to_string(), + ], + &"run".to_string(), + 2, + ); + let result = s.get_value().decode().trim().to_string(); + assert_eq!(result, "(2 (1 . 2) (4 (1 . 0) 1))"); +} + +#[test] +fn test_smoke_cl23_program_without_zero_folding() { + let mut s = Stream::new(None); + launch_tool( + &mut s, + &vec![ + "run".to_string(), + "(mod () (include *standard-cl-23.1*) (defconst X (concat 0x00 0x00)) X)".to_string(), + ], + &"run".to_string(), + 2, + ); + let result = s.get_value().decode().trim().to_string(); + assert_eq!(result, "(2 (1 . 2) (4 (1 . 0x0000) 1))"); +} diff --git a/src/tests/compiler/clvm.rs b/src/tests/compiler/clvm.rs index f71c1536e..af4595f9c 100644 --- a/src/tests/compiler/clvm.rs +++ b/src/tests/compiler/clvm.rs @@ -15,7 +15,7 @@ use crate::classic::clvm::__type_compatibility__::{bi_one, bi_zero, Bytes, Bytes use crate::classic::clvm::casts::{bigint_to_bytes_clvm, bigint_to_bytes_unsigned}; use crate::classic::clvm_tools::stages::stage_0::DefaultProgramRunner; -use crate::compiler::clvm::{parse_and_run, sha256tree}; +use crate::compiler::clvm::{convert_to_clvm_rs, parse_and_run, sha256tree, NewStyleIntConversion}; use crate::compiler::runtypes::RunFailure; use crate::compiler::sexp::{parse_sexp, SExp}; use crate::compiler::srcloc::Srcloc; @@ -231,3 +231,82 @@ fn test_sha256_tree_hash() { "156e86309040ed6bbfee805c9c6ca7eebc140490bd1b97d6d18fb8ebc91fd05a" ); } + +#[test] +fn test_sha256_integer_0_hash() { + let srcloc = Srcloc::start("*test*"); + let value = Rc::new(SExp::Integer(srcloc, bi_zero())); + let hash_result = Bytes::new(Some(BytesFromType::Raw(sha256tree(value)))).hex(); + assert_eq!( + hash_result, + "4bf5122f344554c53bde2ebb8cd2b7e3d1600ad631c385a5d7cce23c7785459a" + ); +} + +#[test] +fn test_sha256_integer_0_hash_pre_fix() { + let srcloc = Srcloc::start("*test*"); + let value = Rc::new(SExp::Integer(srcloc, bi_zero())); + let _old_style = NewStyleIntConversion::new(false); + let hash_result = Bytes::new(Some(BytesFromType::Raw(sha256tree(value)))).hex(); + assert_eq!( + hash_result, + "47dc540c94ceb704a23875c11273e16bb0b8a87aed84de911f2133568115f254" + ); +} + +#[test] +fn test_integer_0_to_clvm() { + let srcloc = Srcloc::start("*test*"); + let value = Rc::new(SExp::Integer(srcloc, bi_zero())); + let mut allocator = Allocator::new(); + let clvm_target = convert_to_clvm_rs(&mut allocator, value).expect("ok"); + let empty: [u8; 0] = []; + assert_eq!(allocator.atom(clvm_target), &empty); +} + +#[test] +fn test_integer_hash_255() { + let srcloc = Srcloc::start("*test*"); + let value = Rc::new(SExp::Integer(srcloc, 255_u32.to_bigint().unwrap())); + let hash_result = Bytes::new(Some(BytesFromType::Raw(sha256tree(value)))).hex(); + assert_eq!( + hash_result, + "b7ae7729555ec6829c579c2602edc6cb94b4ed3d820ddda0a45ac54030f8a53d" + ); +} + +#[test] +fn test_integer_hash_255_pre_fix() { + let srcloc = Srcloc::start("*test*"); + let value = Rc::new(SExp::Integer(srcloc, 255_u32.to_bigint().unwrap())); + let _old_style = NewStyleIntConversion::new(false); + let hash_result = Bytes::new(Some(BytesFromType::Raw(sha256tree(value)))).hex(); + assert_eq!( + hash_result, + "b7ae7729555ec6829c579c2602edc6cb94b4ed3d820ddda0a45ac54030f8a53d" + ); +} + +#[test] +fn test_integer_hash_m129() { + let srcloc = Srcloc::start("*test*"); + let value = Rc::new(SExp::Integer(srcloc, -129_i32.to_bigint().unwrap())); + let hash_result = Bytes::new(Some(BytesFromType::Raw(sha256tree(value)))).hex(); + assert_eq!( + hash_result, + "5a0c1fec64751e82c0d4861d0bc19c7580525d2f47667956bbd9d79e260aae00" + ); +} + +#[test] +fn test_integer_hash_m129_pre_fix() { + let srcloc = Srcloc::start("*test*"); + let value = Rc::new(SExp::Integer(srcloc, -129_i32.to_bigint().unwrap())); + let _old_style = NewStyleIntConversion::new(false); + let hash_result = Bytes::new(Some(BytesFromType::Raw(sha256tree(value)))).hex(); + assert_eq!( + hash_result, + "5a0c1fec64751e82c0d4861d0bc19c7580525d2f47667956bbd9d79e260aae00" + ); +} diff --git a/src/tests/compiler/compiler.rs b/src/tests/compiler/compiler.rs index 1f426f1fa..0e6ef1094 100644 --- a/src/tests/compiler/compiler.rs +++ b/src/tests/compiler/compiler.rs @@ -43,6 +43,7 @@ fn run_string_maybe_opt( opts = opts.set_dialect(AcceptedDialect { stepping: Some(21), strict: true, + int_fix: false, }); } @@ -2396,6 +2397,7 @@ fn test_handle_explicit_empty_atom() { let opts = Rc::new(DefaultCompilerOpts::new(filename)).set_dialect(AcceptedDialect { stepping: Some(21), strict: true, + int_fix: false, }); let atom = |s: &str| Rc::new(SExp::Atom(srcloc.clone(), s.as_bytes().to_vec())); diff --git a/src/tests/compiler/optimizer/cse.rs b/src/tests/compiler/optimizer/cse.rs index ee6ed9e8a..cb518d4f3 100644 --- a/src/tests/compiler/optimizer/cse.rs +++ b/src/tests/compiler/optimizer/cse.rs @@ -852,11 +852,13 @@ fn test_generated_cse(n: u32) { let opts21 = opts.set_dialect(AcceptedDialect { stepping: Some(21), strict: true, + int_fix: false, }); let opts23 = opts .set_dialect(AcceptedDialect { stepping: Some(23), strict: true, + int_fix: false, }) .set_optimize(true); let mut allocator = Allocator::new(); diff --git a/src/tests/compiler/optimizer/cse_fuzz.rs b/src/tests/compiler/optimizer/cse_fuzz.rs index 39cc8309c..8202a35c0 100644 --- a/src/tests/compiler/optimizer/cse_fuzz.rs +++ b/src/tests/compiler/optimizer/cse_fuzz.rs @@ -246,6 +246,7 @@ impl PropertyTestState for TrickyAssignExpectation { opts.set_dialect(AcceptedDialect { stepping: Some(23), strict: true, + int_fix: false, }) .set_optimize(true), ) diff --git a/src/tests/compiler/optimizer/cse_regression.rs b/src/tests/compiler/optimizer/cse_regression.rs index bf9dee738..9096d64c8 100644 --- a/src/tests/compiler/optimizer/cse_regression.rs +++ b/src/tests/compiler/optimizer/cse_regression.rs @@ -105,6 +105,7 @@ fn test_cse_merge_regression() { let dialect = AcceptedDialect { stepping: Some(23), strict: true, + int_fix: false, }; let new_opts: Rc = Rc::new(DefaultCompilerOpts::new("test.clsp")) .set_dialect(dialect.clone()) diff --git a/src/tests/compiler/optimizer/depgraph.rs b/src/tests/compiler/optimizer/depgraph.rs index 977762caa..6eb045927 100644 --- a/src/tests/compiler/optimizer/depgraph.rs +++ b/src/tests/compiler/optimizer/depgraph.rs @@ -14,6 +14,7 @@ fn get_depgraph_for_program(prog: &str) -> FunctionDependencyGraph { let opts = DefaultCompilerOpts::new(filename).set_dialect(AcceptedDialect { stepping: Some(21), strict: true, + int_fix: false, }); let compileform = frontend(opts.clone(), &forms).expect("should frontend"); diff --git a/src/tests/compiler/optimizer/output.rs b/src/tests/compiler/optimizer/output.rs index 0c2b1ba78..6ac41c882 100644 --- a/src/tests/compiler/optimizer/output.rs +++ b/src/tests/compiler/optimizer/output.rs @@ -115,6 +115,7 @@ fn run_string_get_program_and_output_with_includes( dialect: AcceptedDialect { stepping: Some(23), strict: false, + int_fix: false, }, optimize: false, fe_opt, @@ -313,6 +314,7 @@ const SPEC_23: OptimizationRunSpec = OptimizationRunSpec { dialect: AcceptedDialect { stepping: Some(23), strict: true, + int_fix: false, }, optimize: true, fe_opt: true, diff --git a/src/tests/compiler/preprocessor.rs b/src/tests/compiler/preprocessor.rs index e2a40e515..4e627ec60 100644 --- a/src/tests/compiler/preprocessor.rs +++ b/src/tests/compiler/preprocessor.rs @@ -526,6 +526,7 @@ fn test_preprocess_basic_list() { Rc::new(DefaultCompilerOpts::new(file)).set_dialect(AcceptedDialect { stepping: Some(21), strict: true, + int_fix: false, }); let mut includes = Vec::new(); let pp = preprocess(opts.clone(), &mut includes, parsed_forms[0].clone()) @@ -564,6 +565,7 @@ fn test_preprocessor_tours_includes_properly() { .set_dialect(AcceptedDialect { stepping: Some(21), strict: true, + int_fix: false, }); let parsed = parse_sexp(Srcloc::start(pname), prog.bytes()).expect("should parse"); let mut includes = Vec::new(); From fa3bd2ee441efa829aa7ba0134797d1918f8910f Mon Sep 17 00:00:00 2001 From: arty Date: Fri, 24 May 2024 09:10:35 -0700 Subject: [PATCH 2/3] Add a set of value tests between old and new --- src/tests/classic/mod.rs | 1 + src/tests/classic/zero_constant_generation.rs | 47 +++++++++++++++++++ 2 files changed, 48 insertions(+) create mode 100644 src/tests/classic/zero_constant_generation.rs diff --git a/src/tests/classic/mod.rs b/src/tests/classic/mod.rs index 3f13d1767..645823aac 100644 --- a/src/tests/classic/mod.rs +++ b/src/tests/classic/mod.rs @@ -5,3 +5,4 @@ mod optimize; pub mod run; mod smoke; mod stage_2; +mod zero_constant_generation; diff --git a/src/tests/classic/zero_constant_generation.rs b/src/tests/classic/zero_constant_generation.rs new file mode 100644 index 000000000..b580f1e56 --- /dev/null +++ b/src/tests/classic/zero_constant_generation.rs @@ -0,0 +1,47 @@ +use crate::tests::classic::run::{do_basic_brun, do_basic_run}; + +#[test] +fn test_fuzz_constant_gen() { + let test_program = "(mod X %1 (defconst C %c) (concat C X))"; + + let test_parallel_run = |test| { + let final_program_classic = test_program.replace("%1", "").replace("%c", test); + let final_program_23_1 = test_program.replace("%1", "(include *standard-cl-23.1*)").replace("%c", test); + let classic = do_basic_run(&vec![ + "run".to_string(), + final_program_classic + ]); + let new_ver = do_basic_run(&vec![ + "run".to_string(), + final_program_23_1 + ]); + let classic_output = do_basic_brun(&vec![ + "brun".to_string(), + classic, + "0x01".to_string(), + ]); + let new_output = do_basic_brun(&vec![ + "brun".to_string(), + new_ver, + "0x01".to_string(), + ]); + assert_eq!(classic_output, new_output); + }; + + for test in [ + "0x00", + "(concat 0x00 0x00)", + "(concat 0 0x00)", + "(concat 1 0x00)", + "(concat -1 0x00)", + "(concat -129 0x00)", + "(concat 0x00 0)", + "(concat 0x00 1)", + "(concat 0x00 -1)", + "(concat 0x00 -129)", + "(concat 0x00 (sha256 0x00 0))", + "(concat 0x00 (sha256 0 0x00 0))", + ].iter() { + test_parallel_run(test); + } +} From c5a2a285c6ba10bd926522d16d012f0ba877f195 Mon Sep 17 00:00:00 2001 From: arty Date: Fri, 24 May 2024 09:11:59 -0700 Subject: [PATCH 3/3] fmt + clippy --- src/tests/classic/zero_constant_generation.rs | 30 +++++++------------ 1 file changed, 10 insertions(+), 20 deletions(-) diff --git a/src/tests/classic/zero_constant_generation.rs b/src/tests/classic/zero_constant_generation.rs index b580f1e56..f14e44e8b 100644 --- a/src/tests/classic/zero_constant_generation.rs +++ b/src/tests/classic/zero_constant_generation.rs @@ -6,25 +6,13 @@ fn test_fuzz_constant_gen() { let test_parallel_run = |test| { let final_program_classic = test_program.replace("%1", "").replace("%c", test); - let final_program_23_1 = test_program.replace("%1", "(include *standard-cl-23.1*)").replace("%c", test); - let classic = do_basic_run(&vec![ - "run".to_string(), - final_program_classic - ]); - let new_ver = do_basic_run(&vec![ - "run".to_string(), - final_program_23_1 - ]); - let classic_output = do_basic_brun(&vec![ - "brun".to_string(), - classic, - "0x01".to_string(), - ]); - let new_output = do_basic_brun(&vec![ - "brun".to_string(), - new_ver, - "0x01".to_string(), - ]); + let final_program_23_1 = test_program + .replace("%1", "(include *standard-cl-23.1*)") + .replace("%c", test); + let classic = do_basic_run(&vec!["run".to_string(), final_program_classic]); + let new_ver = do_basic_run(&vec!["run".to_string(), final_program_23_1]); + let classic_output = do_basic_brun(&vec!["brun".to_string(), classic, "0x01".to_string()]); + let new_output = do_basic_brun(&vec!["brun".to_string(), new_ver, "0x01".to_string()]); assert_eq!(classic_output, new_output); }; @@ -41,7 +29,9 @@ fn test_fuzz_constant_gen() { "(concat 0x00 -129)", "(concat 0x00 (sha256 0x00 0))", "(concat 0x00 (sha256 0 0x00 0))", - ].iter() { + ] + .iter() + { test_parallel_run(test); } }