From 39c5c62028fd1a3d6a43ca59de426a243455d4a5 Mon Sep 17 00:00:00 2001 From: Leandro Pacheco Date: Wed, 31 Jan 2024 14:05:58 -0300 Subject: [PATCH 1/2] AST parsing and impl Display fixes - `instr x = y` statements must end with semicolon - fixed Display implementation for AST objects such that source->AST->source->AST works properly - tests for the above --- analysis/README.md | 8 +- ast/src/parsed/display.rs | 33 ++++++- parser/Cargo.toml | 1 + parser/src/lib.rs | 88 +++++++++++-------- parser/src/powdr.lalrpop | 3 +- riscv/src/coprocessors.rs | 14 +-- test_data/asm/block_machine_cache_miss.asm | 4 +- test_data/asm/book/modules.asm | 6 +- test_data/asm/different_signatures.asm | 6 +- test_data/asm/sqrt.asm | 2 +- test_data/asm/vm_to_block_array.asm | 4 +- .../asm/vm_to_block_multiple_interfaces.asm | 4 +- test_data/asm/vm_to_block_to_block.asm | 2 +- .../asm/vm_to_block_unique_interface.asm | 8 +- test_data/asm/vm_to_vm.asm | 4 +- .../asm/vm_to_vm_dynamic_trace_length.asm | 2 +- test_data/asm/vm_to_vm_to_block.asm | 6 +- test_data/asm/vm_to_vm_to_vm.asm | 6 +- test_data/std/arith_test.asm | 2 +- test_data/std/poseidon_bn254_test.asm | 2 +- test_data/std/poseidon_gl_test.asm | 2 +- test_data/std/split_bn254_test.asm | 2 +- test_data/std/split_gl_test.asm | 2 +- 23 files changed, 124 insertions(+), 87 deletions(-) diff --git a/analysis/README.md b/analysis/README.md index 482ffedea..30616a847 100644 --- a/analysis/README.md +++ b/analysis/README.md @@ -36,9 +36,9 @@ machine Main { reg Y[<=]; reg A; - instr identity X -> Y = sub.identity - instr one -> Y = sub.one - instr nothing = sub.nothing + instr identity X -> Y = sub.identity; + instr one -> Y = sub.one; + instr nothing = sub.nothing; function main { start: @@ -662,4 +662,4 @@ pol constant _block_enforcer_last_step = [0]* + [1]; pol commit _operation_id_no_change; _operation_id_no_change = ((1 - _block_enforcer_last_step) * (1 - instr_return)); (_operation_id_no_change * (_operation_id' - _operation_id)) = 0; -``` \ No newline at end of file +``` diff --git a/ast/src/parsed/display.rs b/ast/src/parsed/display.rs index d00c633c1..27119beaf 100644 --- a/ast/src/parsed/display.rs +++ b/ast/src/parsed/display.rs @@ -37,9 +37,12 @@ impl Display for ModuleStatement { write!(f, "machine {name} {m}") } SymbolValue::Import(i) => { - write!(f, "{i} as {name}") + write!(f, "{i} as {name};") } - SymbolValue::Module(m) => { + SymbolValue::Module(m @ Module::External(_)) => { + write!(f, "mod {m}") + } + SymbolValue::Module(m @ Module::Local(_)) => { write!(f, "mod {name} {m}") } SymbolValue::Expression(e) => { @@ -80,12 +83,34 @@ impl Display for Machine { impl Display for InstructionBody { fn fmt(&self, f: &mut Formatter<'_>) -> Result { match self { - InstructionBody::Local(elements) => write!(f, "{{ {} }}", elements.iter().format(", ")), + InstructionBody::Local(elements) => write!( + f, + "{{ {} }}", + elements + .iter() + .map(format_instruction_statement) + .format(", ") + ), InstructionBody::CallableRef(r) => write!(f, " = {r};"), } } } +fn format_instruction_statement(stmt: &PilStatement) -> String { + match stmt { + PilStatement::PolynomialIdentity(_, _) + | PilStatement::PlookupIdentity(_, _, _) + | PilStatement::PermutationIdentity(_, _, _) + | PilStatement::ConnectIdentity(_, _, _) => { + // statements inside instruction definition don't end in semicolon + let mut s = format!("{stmt}"); + assert_eq!(s.pop(), Some(';')); + s + } + _ => panic!("invalid statement inside instruction body: {}", stmt), + } +} + impl Display for Instruction { fn fmt(&self, f: &mut Formatter<'_>) -> Result { write!( @@ -119,7 +144,7 @@ impl Display for MachineStatement { fn fmt(&self, f: &mut Formatter<'_>) -> Result { match self { MachineStatement::Degree(_, degree) => write!(f, "degree {};", degree), - MachineStatement::Pil(_, statement) => write!(f, "{statement};"), + MachineStatement::Pil(_, statement) => write!(f, "{statement}"), MachineStatement::Submachine(_, ty, name) => write!(f, "{ty} {name};"), MachineStatement::RegisterDeclaration(_, name, flag) => write!( f, diff --git a/parser/Cargo.toml b/parser/Cargo.toml index 20715d972..f9f4b5572 100644 --- a/parser/Cargo.toml +++ b/parser/Cargo.toml @@ -24,6 +24,7 @@ regex-syntax = { version = "0.6", default_features = false, features = ["unicode pretty_assertions = "1.3.0" test-log = "0.2.12" env_logger = "0.10.0" +walkdir = "2.4.0" [build-dependencies] lalrpop = "^0.19" diff --git a/parser/src/lib.rs b/parser/src/lib.rs index 14aa9776a..5b4f45d72 100644 --- a/parser/src/lib.rs +++ b/parser/src/lib.rs @@ -96,10 +96,11 @@ mod test { use powdr_ast::parsed::{ build::direct_reference, PILFile, PilStatement, PolynomialName, SelectedExpressions, }; + use powdr_number::Bn254Field; use powdr_number::GoldilocksField; use powdr_parser_util::UnwrapErrToStderr; - use std::fs; use test_log::test; + use walkdir::WalkDir; #[test] fn empty() { @@ -191,49 +192,60 @@ mod test { ); } - fn parse_file(name: &str) -> PILFile { - let file = std::path::PathBuf::from(format!( - "{}/../test_data/{name}", - env!("CARGO_MANIFEST_DIR") - )); - - let input = fs::read_to_string(file).unwrap(); - parse(Some(name), &input).unwrap_err_to_stderr() - } - - fn parse_asm_file(name: &str) -> ASMProgram { - let file = std::path::PathBuf::from(format!( - "{}/../test_data/{name}", - env!("CARGO_MANIFEST_DIR") - )); - - let input = fs::read_to_string(file).unwrap(); - parse_asm(Some(name), &input).unwrap_err_to_stderr() + fn find_files_with_ext( + dir: std::path::PathBuf, + ext: String, + ) -> impl Iterator { + WalkDir::new(&dir).into_iter().filter_map(move |e| { + let entry = e.unwrap(); + let path = entry.path(); + match path.extension() { + Some(path_ext) if path_ext.to_str() == Some(&ext) => Some(( + path.to_str().unwrap().into(), + std::fs::read_to_string(path).unwrap(), + )), + _ => None, + } + }) } #[test] - fn parse_example_files() { - parse_file("polygon-hermez/arith.pil"); - parse_file("polygon-hermez/binary.pil"); - parse_file("polygon-hermez/byte4.pil"); - parse_file("polygon-hermez/config.pil"); - parse_file("polygon-hermez/global.pil"); - parse_file("polygon-hermez/keccakf.pil"); - parse_file("polygon-hermez/main.pil"); - parse_file("polygon-hermez/mem_align.pil"); - parse_file("polygon-hermez/mem.pil"); - parse_file("polygon-hermez/nine2one.pil"); - parse_file("polygon-hermez/padding_kk.pil"); - parse_file("polygon-hermez/padding_kkbit.pil"); - parse_file("polygon-hermez/padding_pg.pil"); - parse_file("polygon-hermez/poseidong.pil"); - parse_file("polygon-hermez/rom.pil"); - parse_file("polygon-hermez/storage.pil"); + /// Test that (source -> AST -> source -> AST) works properly + fn parse_write_reparse_asm() { + let crate_dir = env!("CARGO_MANIFEST_DIR"); + let basedir = std::path::PathBuf::from(format!("{crate_dir}/../test_data/")); + let asm_files = find_files_with_ext(basedir, "asm".into()); + for (file, orig_string) in asm_files { + let orig_asm = + parse_asm::(Some(&file), &orig_string).unwrap_err_to_stderr(); + let orig_asm_to_string = format!("{}", orig_asm); + let reparsed_asm = + parse_asm::(Some((file + " reparsed").as_ref()), &orig_asm_to_string) + .unwrap_err_to_stderr(); + // TODO: we can't directly assert ASTs because SourceRefs will differ + // assert_eq!(orig_asm, reparsed_asm); + let reparsed_asm_to_string = format!("{}", reparsed_asm); + assert_eq!(reparsed_asm_to_string, orig_asm_to_string); + } } #[test] - fn parse_example_asm_files() { - parse_asm_file("asm/simple_sum.asm"); + /// Test that (source -> AST -> source -> AST) works properly + fn parse_write_reparse_pil() { + let crate_dir = env!("CARGO_MANIFEST_DIR"); + let basedir = std::path::PathBuf::from(format!("{crate_dir}/../test_data/")); + let pil_files = find_files_with_ext(basedir, "pil".into()); + for (file, orig_string) in pil_files { + let orig_pil = parse::(Some(&file), &orig_string).unwrap_err_to_stderr(); + let orig_pil_to_string = format!("{}", orig_pil); + let reparsed_pil = + parse::(Some((file + " reparsed").as_ref()), &orig_pil_to_string) + .unwrap_err_to_stderr(); + // TODO: we can't directly assert ASTs because SourceRefs will differ + // assert_eq!(orig_pil, reparsed_pil); + let reparsed_pil_to_string = format!("{}", reparsed_pil); + assert_eq!(reparsed_pil_to_string, orig_pil_to_string); + } } mod display { diff --git a/parser/src/powdr.lalrpop b/parser/src/powdr.lalrpop index 4952c2e57..15d26d929 100644 --- a/parser/src/powdr.lalrpop +++ b/parser/src/powdr.lalrpop @@ -254,7 +254,7 @@ pub LinkDeclaration: MachineStatement = { pub InstructionBody: InstructionBody = { "{}" => InstructionBody::Local(vec![]), "{" "}" => InstructionBody::Local(<>), - "=" => InstructionBody::CallableRef(f_ref), + "=" ";" => InstructionBody::CallableRef(f_ref), } pub CallableRef: CallableRef = { @@ -566,7 +566,6 @@ SpecialIdentifier: &'input str = { ConstantIdentifier: String = { // TODO it seems the lexer splits the token after % - "%N" => <>.to_string(), r"%[a-zA-Z_][a-zA-Z$_0-9@]*" => <>.to_string(), } diff --git a/riscv/src/coprocessors.rs b/riscv/src/coprocessors.rs index d06db4708..e3f818c07 100644 --- a/riscv/src/coprocessors.rs +++ b/riscv/src/coprocessors.rs @@ -20,9 +20,9 @@ static BINARY_COPROCESSOR: CoProcessor = CoProcessor { import: "use std::binary::Binary;", instructions: r#" // ================= binary/bitwise instructions ================= - instr and Y, Z -> X = binary.and - instr or Y, Z -> X = binary.or - instr xor Y, Z -> X = binary.xor + instr and Y, Z -> X = binary.and; + instr or Y, Z -> X = binary.or; + instr xor Y, Z -> X = binary.xor; "#, runtime_function_impl: None, @@ -34,8 +34,8 @@ static SHIFT_COPROCESSOR: CoProcessor = CoProcessor { import: "use std::shift::Shift;", instructions: r#" // ================= shift instructions ================= - instr shl Y, Z -> X = shift.shl - instr shr Y, Z -> X = shift.shr + instr shl Y, Z -> X = shift.shl; + instr shr Y, Z -> X = shift.shr; "#, runtime_function_impl: None, @@ -47,7 +47,7 @@ static SPLIT_GL_COPROCESSOR: CoProcessor = CoProcessor { import: "use std::split::split_gl::SplitGL;", instructions: r#" // ================== wrapping instructions ============== -instr split_gl Z -> X, Y = split_gl.split +instr split_gl Z -> X, Y = split_gl.split; "#, runtime_function_impl: None, @@ -59,7 +59,7 @@ static POSEIDON_GL_COPROCESSOR: CoProcessor = CoProcessor { import: "use std::hash::poseidon_gl::PoseidonGL;", instructions: r#" // ================== hashing instructions ============== -instr poseidon_gl A0, A1, A2, A3, A4, A5, A6, A7, A8, A9, A10, A11 -> X, Y, Z, W = poseidon_gl.poseidon_permutation +instr poseidon_gl A0, A1, A2, A3, A4, A5, A6, A7, A8, A9, A10, A11 -> X, Y, Z, W = poseidon_gl.poseidon_permutation; "#, runtime_function_impl: Some(("poseidon_gl_coprocessor", poseidon_gl_call)), diff --git a/test_data/asm/block_machine_cache_miss.asm b/test_data/asm/block_machine_cache_miss.asm index 22e467e38..8860e7b6d 100644 --- a/test_data/asm/block_machine_cache_miss.asm +++ b/test_data/asm/block_machine_cache_miss.asm @@ -38,8 +38,8 @@ machine Main { reg Y[<=]; reg A; - instr double X -> Y = arith.double - instr square X -> Y = arith.square + instr double X -> Y = arith.double; + instr square X -> Y = arith.square; instr assert_eq X, Y { X = Y } function main { diff --git a/test_data/asm/book/modules.asm b/test_data/asm/book/modules.asm index de57a57c8..0ba560073 100644 --- a/test_data/asm/book/modules.asm +++ b/test_data/asm/book/modules.asm @@ -35,9 +35,9 @@ machine Main { reg pc[@pc]; - instr nothing = a.nothing - instr also_nothing = b.nothing - instr still_nothing = c.nothing + instr nothing = a.nothing; + instr also_nothing = b.nothing; + instr still_nothing = c.nothing; function main { nothing; diff --git a/test_data/asm/different_signatures.asm b/test_data/asm/different_signatures.asm index 84ac2ec8f..a3b4b43b7 100644 --- a/test_data/asm/different_signatures.asm +++ b/test_data/asm/different_signatures.asm @@ -9,9 +9,9 @@ machine Main { reg Y[<=]; reg A; - instr identity X -> Y = sub.identity - instr one -> Y = sub.one - instr nothing = sub.nothing + instr identity X -> Y = sub.identity; + instr one -> Y = sub.one; + instr nothing = sub.nothing; function main { start: diff --git a/test_data/asm/sqrt.asm b/test_data/asm/sqrt.asm index e3915843b..c74ddffe7 100644 --- a/test_data/asm/sqrt.asm +++ b/test_data/asm/sqrt.asm @@ -41,7 +41,7 @@ machine Main { instr assert_zero X { XIsZero = 1 } - instr sqrt X -> Y = sqrt.sqrt + instr sqrt X -> Y = sqrt.sqrt; function main { diff --git a/test_data/asm/vm_to_block_array.asm b/test_data/asm/vm_to_block_array.asm index a5f59961b..770690555 100644 --- a/test_data/asm/vm_to_block_array.asm +++ b/test_data/asm/vm_to_block_array.asm @@ -10,8 +10,8 @@ machine Main { reg Z[<=]; reg A; - instr add X, Y -> Z = arith.add - instr mul X, Y -> Z = arith.mul + instr add X, Y -> Z = arith.add; + instr mul X, Y -> Z = arith.mul; instr assert_eq X, Y { X = Y } function main { diff --git a/test_data/asm/vm_to_block_multiple_interfaces.asm b/test_data/asm/vm_to_block_multiple_interfaces.asm index c570e4b94..34d1c1959 100644 --- a/test_data/asm/vm_to_block_multiple_interfaces.asm +++ b/test_data/asm/vm_to_block_multiple_interfaces.asm @@ -28,8 +28,8 @@ machine Main { reg Z[<=]; reg A; - instr add X, Y -> Z = arith.add - instr sub X, Y -> Z = arith.sub + instr add X, Y -> Z = arith.add; + instr sub X, Y -> Z = arith.sub; instr assert_eq X, Y { X = Y } function main { diff --git a/test_data/asm/vm_to_block_to_block.asm b/test_data/asm/vm_to_block_to_block.asm index 58f1d829c..0ca61d6c3 100644 --- a/test_data/asm/vm_to_block_to_block.asm +++ b/test_data/asm/vm_to_block_to_block.asm @@ -40,7 +40,7 @@ machine Main { reg X[<=]; reg A; - instr assert1 X -> = assert1.assert1 + instr assert1 X -> = assert1.assert1; instr loop { pc' = pc diff --git a/test_data/asm/vm_to_block_unique_interface.asm b/test_data/asm/vm_to_block_unique_interface.asm index 20c7a4e16..6eae771e9 100644 --- a/test_data/asm/vm_to_block_unique_interface.asm +++ b/test_data/asm/vm_to_block_unique_interface.asm @@ -43,10 +43,10 @@ machine Main { reg Z[<=]; reg A; - instr add X, Y -> Z = arith.add - instr sub X, Y -> Z = arith.sub - instr and X, Y -> Z = binary.and - instr or X, Y -> Z = binary.or + instr add X, Y -> Z = arith.add; + instr sub X, Y -> Z = arith.sub; + instr and X, Y -> Z = binary.and; + instr or X, Y -> Z = binary.or; instr assert_eq X, Y { X = Y } function main { diff --git a/test_data/asm/vm_to_vm.asm b/test_data/asm/vm_to_vm.asm index 9cd7e217a..048b6e445 100644 --- a/test_data/asm/vm_to_vm.asm +++ b/test_data/asm/vm_to_vm.asm @@ -10,8 +10,8 @@ machine Main { reg Z[<=]; reg A; - instr add X, Y -> Z = vm.add - instr sub X, Y -> Z = vm.sub + instr add X, Y -> Z = vm.add; + instr sub X, Y -> Z = vm.sub; instr assert_eq X, Y { X = Y } function main { diff --git a/test_data/asm/vm_to_vm_dynamic_trace_length.asm b/test_data/asm/vm_to_vm_dynamic_trace_length.asm index d5f317d53..1982a0e98 100644 --- a/test_data/asm/vm_to_vm_dynamic_trace_length.asm +++ b/test_data/asm/vm_to_vm_dynamic_trace_length.asm @@ -10,7 +10,7 @@ machine Main { reg Z[<=]; reg A; - instr pow X, Y -> Z = pow.pow + instr pow X, Y -> Z = pow.pow; instr assert_eq X, Y { X = Y } function main { diff --git a/test_data/asm/vm_to_vm_to_block.asm b/test_data/asm/vm_to_vm_to_block.asm index 214a70f05..f6793ac2f 100644 --- a/test_data/asm/vm_to_vm_to_block.asm +++ b/test_data/asm/vm_to_vm_to_block.asm @@ -10,7 +10,7 @@ machine Main { reg Z[<=]; reg A; - instr pythagoras X, Y -> Z = pythagoras.pythagoras + instr pythagoras X, Y -> Z = pythagoras.pythagoras; instr assert_eq X, Y { X = Y } function main { @@ -40,8 +40,8 @@ machine Pythagoras { reg B; - instr add X, Y -> Z = arith.add - instr mul X, Y -> Z = arith.mul + instr add X, Y -> Z = arith.add; + instr mul X, Y -> Z = arith.mul; function pythagoras a: field, b: field -> field { A <== mul(a, a); diff --git a/test_data/asm/vm_to_vm_to_vm.asm b/test_data/asm/vm_to_vm_to_vm.asm index 7033c8aae..f2971d4da 100644 --- a/test_data/asm/vm_to_vm_to_vm.asm +++ b/test_data/asm/vm_to_vm_to_vm.asm @@ -10,7 +10,7 @@ machine Main { reg Z[<=]; reg A; - instr pythagoras X, Y -> Z = pythagoras.pythagoras + instr pythagoras X, Y -> Z = pythagoras.pythagoras; instr assert_eq X, Y { X = Y } function main { @@ -39,8 +39,8 @@ machine Pythagoras { reg B; - instr add X, Y -> Z = arith.add - instr mul X, Y -> Z = arith.mul + instr add X, Y -> Z = arith.add; + instr mul X, Y -> Z = arith.mul; function pythagoras a: field, b: field -> field { A <== mul(a, a); diff --git a/test_data/std/arith_test.asm b/test_data/std/arith_test.asm index b726080f5..24737d6b5 100644 --- a/test_data/std/arith_test.asm +++ b/test_data/std/arith_test.asm @@ -64,7 +64,7 @@ machine Main{ Arith arith; - instr eq0 A0, A1, A2, A3, A4, A5, A6, A7, B0, B1, B2, B3, B4, B5, B6, B7, C0, C1, C2, C3, C4, C5, C6, C7 -> D0, D1, D2, D3, D4, D5, D6, D7, E0, E1, E2, E3, E4, E5, E6, E7 = arith.eq0 + instr eq0 A0, A1, A2, A3, A4, A5, A6, A7, B0, B1, B2, B3, B4, B5, B6, B7, C0, C1, C2, C3, C4, C5, C6, C7 -> D0, D1, D2, D3, D4, D5, D6, D7, E0, E1, E2, E3, E4, E5, E6, E7 = arith.eq0; instr assert_eq A0, A1, A2, A3, A4, A5, A6, A7, B0, B1, B2, B3, B4, B5, B6, B7 { A0 = B0, diff --git a/test_data/std/poseidon_bn254_test.asm b/test_data/std/poseidon_bn254_test.asm index 4abd0838b..e2096183c 100644 --- a/test_data/std/poseidon_bn254_test.asm +++ b/test_data/std/poseidon_bn254_test.asm @@ -12,7 +12,7 @@ machine Main { PoseidonBN254 poseidon; - instr poseidon X0, X1, X2 -> X3 = poseidon.poseidon_permutation + instr poseidon X0, X1, X2 -> X3 = poseidon.poseidon_permutation; instr assert_eq X0, X1 { X0 = X1 diff --git a/test_data/std/poseidon_gl_test.asm b/test_data/std/poseidon_gl_test.asm index 3264db755..dfd1819f3 100644 --- a/test_data/std/poseidon_gl_test.asm +++ b/test_data/std/poseidon_gl_test.asm @@ -27,7 +27,7 @@ machine Main { PoseidonGL poseidon; - instr poseidon X0, X1, X2, X3, X4, X5, X6, X7, X8, X9, X10, X11 -> X12, X13, X14, X15 = poseidon.poseidon_permutation + instr poseidon X0, X1, X2, X3, X4, X5, X6, X7, X8, X9, X10, X11 -> X12, X13, X14, X15 = poseidon.poseidon_permutation; instr assert_eq X0, X1 { X0 = X1 diff --git a/test_data/std/split_bn254_test.asm b/test_data/std/split_bn254_test.asm index 3bff0805c..a6d71bacd 100644 --- a/test_data/std/split_bn254_test.asm +++ b/test_data/std/split_bn254_test.asm @@ -25,7 +25,7 @@ machine Main { SplitBN254 split_machine; - instr split X0 -> X1, X2, X3, X4, X5, X6, X7, X8 = split_machine.split + instr split X0 -> X1, X2, X3, X4, X5, X6, X7, X8 = split_machine.split; instr assert_eq X0, X1 { X0 = X1 diff --git a/test_data/std/split_gl_test.asm b/test_data/std/split_gl_test.asm index 239051b55..7bca9ed66 100644 --- a/test_data/std/split_gl_test.asm +++ b/test_data/std/split_gl_test.asm @@ -13,7 +13,7 @@ machine Main { SplitGL split_machine; - instr split X0 -> X1, X2 = split_machine.split + instr split X0 -> X1, X2 = split_machine.split; instr assert_eq X0, X1 { X0 = X1 From cbec66e13064bf4576558a30db4bbf99cd678f9c Mon Sep 17 00:00:00 2001 From: Leandro Pacheco Date: Fri, 2 Feb 2024 19:16:01 -0300 Subject: [PATCH 2/2] fix impl Display for ASMProgram - have `FieldElement` printed as non-negative - print constrained machine arguments - fix related tests --- ast/src/parsed/display.rs | 18 ++++- linker/src/lib.rs | 4 +- number/src/macros.rs | 8 +- parser/Cargo.toml | 1 + parser/src/lib.rs | 163 +++++++++++++++++++++++++++++++++----- parser/src/powdr.lalrpop | 1 - pipeline/tests/pil.rs | 2 +- 7 files changed, 163 insertions(+), 34 deletions(-) diff --git a/ast/src/parsed/display.rs b/ast/src/parsed/display.rs index 27119beaf..928753ac9 100644 --- a/ast/src/parsed/display.rs +++ b/ast/src/parsed/display.rs @@ -33,9 +33,21 @@ impl Display for ModuleStatement { fn fmt(&self, f: &mut Formatter<'_>) -> Result { match self { ModuleStatement::SymbolDefinition(SymbolDefinition { name, value }) => match value { - SymbolValue::Machine(m) => { - write!(f, "machine {name} {m}") - } + SymbolValue::Machine( + m @ Machine { + arguments: + MachineArguments { + latch, + operation_id, + }, + .. + }, + ) => match (latch, operation_id) { + (None, None) => write!(f, "machine {name} {m}"), + (Some(latch), None) => write!(f, "machine {name}({latch}, _) {m}"), + (None, Some(op_id)) => write!(f, "machine {name}(_, {op_id}) {m}"), + (Some(latch), Some(op_id)) => write!(f, "machine {name}({latch}, {op_id}) {m}"), + }, SymbolValue::Import(i) => { write!(f, "{i} as {name};") } diff --git a/linker/src/lib.rs b/linker/src/lib.rs index 33d708474..862f7d97f 100644 --- a/linker/src/lib.rs +++ b/linker/src/lib.rs @@ -446,7 +446,7 @@ namespace main_sub(16); pol constant p_line = [0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10] + [10]*; pol commit X_free_value(i) query match pc(i) { 2 => ("input", 1), 4 => ("input", (std::convert::int(CNT(i)) + 1)), 7 => ("input", 0), }; pol constant p_X_const = [0]*; - pol constant p_X_read_free = [0, 0, 1, 0, 1, 0, 0, -1, 0, 0, 0] + [0]*; + pol constant p_X_read_free = [0, 0, 1, 0, 1, 0, 0, 18446744069414584320, 0, 0, 0] + [0]*; pol constant p_instr__jump_to_operation = [0, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0] + [0]*; pol constant p_instr__loop = [0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1] + [1]*; pol constant p_instr__reset = [1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0] + [0]*; @@ -519,7 +519,7 @@ machine Machine { pol constant p_instr__loop = [0, 0, 0, 0, 1] + [1]*; pol constant p_instr__reset = [1, 0, 0, 0, 0] + [0]*; pol constant p_instr_adjust_fp = [0, 0, 0, 1, 0] + [0]*; - pol constant p_instr_adjust_fp_param_amount = [0, 0, 0, -2, 0] + [0]*; + pol constant p_instr_adjust_fp_param_amount = [0, 0, 0, 18446744069414584319, 0] + [0]*; pol constant p_instr_adjust_fp_param_t = [0, 0, 0, 3, 0] + [0]*; pol constant p_instr_inc_fp = [0, 0, 1, 0, 0] + [0]*; pol constant p_instr_inc_fp_param_amount = [0, 0, 7, 0, 0] + [0]*; diff --git a/number/src/macros.rs b/number/src/macros.rs index c78fefd48..457a79112 100644 --- a/number/src/macros.rs +++ b/number/src/macros.rs @@ -433,13 +433,7 @@ macro_rules! powdr_field { impl fmt::Display for $name { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { let value = self.to_integer().value; - if self.is_in_lower_half() { - write!(f, "{value}") - } else { - let mut res = Self::modulus(); - assert!(!res.value.sub_with_borrow(&value)); - write!(f, "-{}", res) - } + write!(f, "{value}") } } }; diff --git a/parser/Cargo.toml b/parser/Cargo.toml index f9f4b5572..93fc3b8de 100644 --- a/parser/Cargo.toml +++ b/parser/Cargo.toml @@ -25,6 +25,7 @@ pretty_assertions = "1.3.0" test-log = "0.2.12" env_logger = "0.10.0" walkdir = "2.4.0" +similar = "2.4" [build-dependencies] lalrpop = "^0.19" diff --git a/parser/src/lib.rs b/parser/src/lib.rs index 5b4f45d72..e6b9f85b3 100644 --- a/parser/src/lib.rs +++ b/parser/src/lib.rs @@ -94,11 +94,13 @@ pub fn unescape_string(s: &str) -> String { mod test { use super::*; use powdr_ast::parsed::{ - build::direct_reference, PILFile, PilStatement, PolynomialName, SelectedExpressions, + asm::ASMProgram, build::direct_reference, PILFile, PilStatement, PolynomialName, + SelectedExpressions, }; use powdr_number::Bn254Field; use powdr_number::GoldilocksField; use powdr_parser_util::UnwrapErrToStderr; + use similar::TextDiff; use test_log::test; use walkdir::WalkDir; @@ -201,7 +203,11 @@ mod test { let path = entry.path(); match path.extension() { Some(path_ext) if path_ext.to_str() == Some(&ext) => Some(( - path.to_str().unwrap().into(), + std::fs::canonicalize(path) + .unwrap() + .to_str() + .unwrap() + .into(), std::fs::read_to_string(path).unwrap(), )), _ => None, @@ -209,42 +215,159 @@ mod test { }) } + // helper function to clear SourceRef's inside the AST so we can compare for equality + fn pil_clear_source_refs(ast: &mut PILFile) { + ast.0.iter_mut().for_each(pil_statement_clear_source_ref); + } + + fn pil_statement_clear_source_ref(stmt: &mut PilStatement) { + match stmt { + PilStatement::Include(s, _) + | PilStatement::Namespace(s, _, _) + | PilStatement::LetStatement(s, _, _) + | PilStatement::PolynomialDefinition(s, _, _) + | PilStatement::PublicDeclaration(s, _, _, _, _) + | PilStatement::PolynomialConstantDeclaration(s, _) + | PilStatement::PolynomialConstantDefinition(s, _, _) + | PilStatement::PolynomialCommitDeclaration(s, _, _) + | PilStatement::PolynomialIdentity(s, _) + | PilStatement::PlookupIdentity(s, _, _) + | PilStatement::PermutationIdentity(s, _, _) + | PilStatement::ConnectIdentity(s, _, _) + | PilStatement::ConstantDefinition(s, _, _) + | PilStatement::Expression(s, _) => *s = SourceRef::unknown(), + } + } + + // helper function to clear SourceRef's inside the AST so we can compare for equality + fn asm_clear_source_refs(ast: &mut ASMProgram) { + use powdr_ast::parsed::asm::{ + ASMModule, FunctionStatement, Instruction, InstructionBody, LinkDeclaration, Machine, + MachineStatement, Module, ModuleStatement, SymbolDefinition, SymbolValue, + }; + + fn clear_machine_stmt(stmt: &mut MachineStatement) { + match stmt { + MachineStatement::Degree(s, _) + | MachineStatement::Submachine(s, _, _) + | MachineStatement::RegisterDeclaration(s, _, _) + | MachineStatement::OperationDeclaration(s, _, _, _) + | MachineStatement::LinkDeclaration(LinkDeclaration { source: s, .. }) => { + *s = SourceRef::unknown(); + } + MachineStatement::Pil(s, stmt) => { + *s = SourceRef::unknown(); + pil_statement_clear_source_ref(stmt) + } + MachineStatement::InstructionDeclaration(s, _, Instruction { body, .. }) => { + *s = SourceRef::unknown(); + if let InstructionBody::Local(statements) = body { + statements + .iter_mut() + .for_each(pil_statement_clear_source_ref) + } + } + MachineStatement::FunctionDeclaration(s, _, _, statements) => { + *s = SourceRef::unknown(); + for fstmt in statements { + match fstmt { + FunctionStatement::Assignment(s, _, _, _) + | FunctionStatement::Instruction(s, _, _) + | FunctionStatement::Label(s, _) + | FunctionStatement::DebugDirective(s, _) + | FunctionStatement::Return(s, _) => *s = SourceRef::unknown(), + } + } + } + } + } + + fn clear_module_stmt(stmt: &mut ModuleStatement) { + let ModuleStatement::SymbolDefinition(SymbolDefinition { value, .. }) = stmt; + match value { + SymbolValue::Machine(Machine { statements, .. }) => { + statements.iter_mut().for_each(clear_machine_stmt) + } + SymbolValue::Module(Module::Local(ASMModule { statements })) => { + statements.iter_mut().for_each(clear_module_stmt); + } + SymbolValue::Module(Module::External(_)) + | SymbolValue::Import(_) + | SymbolValue::Expression(_) => (), + } + } + + ast.main.statements.iter_mut().for_each(clear_module_stmt); + } + #[test] - /// Test that (source -> AST -> source -> AST) works properly + /// Test that (source -> AST -> source -> AST) works properly for asm files fn parse_write_reparse_asm() { let crate_dir = env!("CARGO_MANIFEST_DIR"); let basedir = std::path::PathBuf::from(format!("{crate_dir}/../test_data/")); let asm_files = find_files_with_ext(basedir, "asm".into()); for (file, orig_string) in asm_files { - let orig_asm = + let mut orig_asm = parse_asm::(Some(&file), &orig_string).unwrap_err_to_stderr(); let orig_asm_to_string = format!("{}", orig_asm); - let reparsed_asm = - parse_asm::(Some((file + " reparsed").as_ref()), &orig_asm_to_string) - .unwrap_err_to_stderr(); - // TODO: we can't directly assert ASTs because SourceRefs will differ - // assert_eq!(orig_asm, reparsed_asm); - let reparsed_asm_to_string = format!("{}", reparsed_asm); - assert_eq!(reparsed_asm_to_string, orig_asm_to_string); + let mut reparsed_asm = parse_asm::( + Some((file.clone() + " reparsed").as_ref()), + &orig_asm_to_string, + ) + .unwrap_err_to_stderr(); + asm_clear_source_refs(&mut orig_asm); + asm_clear_source_refs(&mut reparsed_asm); + if orig_asm != reparsed_asm { + let orig_ast = format!("{orig_asm:#?}"); + let reparsed_ast = format!("{reparsed_asm:#?}"); + let diff = TextDiff::from_lines(&orig_ast, &reparsed_ast); + eprintln!("parsed and re-parsed ASTs differ:"); + for change in diff.iter_all_changes() { + let sign = match change.tag() { + similar::ChangeTag::Delete => "-", + similar::ChangeTag::Insert => "+", + similar::ChangeTag::Equal => " ", + }; + eprint!("\t{}{}", sign, change); + } + panic!("parsed and re-parsed ASTs differ for file: {file}"); + } } } #[test] - /// Test that (source -> AST -> source -> AST) works properly + /// Test that (source -> AST -> source -> AST) works properly for pil files fn parse_write_reparse_pil() { let crate_dir = env!("CARGO_MANIFEST_DIR"); let basedir = std::path::PathBuf::from(format!("{crate_dir}/../test_data/")); let pil_files = find_files_with_ext(basedir, "pil".into()); for (file, orig_string) in pil_files { - let orig_pil = parse::(Some(&file), &orig_string).unwrap_err_to_stderr(); + let mut orig_pil = + parse::(Some(&file), &orig_string).unwrap_err_to_stderr(); let orig_pil_to_string = format!("{}", orig_pil); - let reparsed_pil = - parse::(Some((file + " reparsed").as_ref()), &orig_pil_to_string) - .unwrap_err_to_stderr(); - // TODO: we can't directly assert ASTs because SourceRefs will differ - // assert_eq!(orig_pil, reparsed_pil); - let reparsed_pil_to_string = format!("{}", reparsed_pil); - assert_eq!(reparsed_pil_to_string, orig_pil_to_string); + let mut reparsed_pil = parse::( + Some((file.clone() + " reparsed").as_ref()), + &orig_pil_to_string, + ) + .unwrap_err_to_stderr(); + pil_clear_source_refs(&mut orig_pil); + pil_clear_source_refs(&mut reparsed_pil); + assert_eq!(orig_pil, reparsed_pil); + if orig_pil != reparsed_pil { + let orig_ast = format!("{orig_pil:#?}"); + let reparsed_ast = format!("{reparsed_pil:#?}"); + let diff = TextDiff::from_lines(&orig_ast, &reparsed_ast); + eprintln!("parsed and re-parsed ASTs differ:"); + for change in diff.iter_all_changes() { + let sign = match change.tag() { + similar::ChangeTag::Delete => "-", + similar::ChangeTag::Insert => "+", + similar::ChangeTag::Equal => " ", + }; + eprint!("\t{}{}", sign, change); + } + panic!("parsed and re-parsed ASTs differ for file: {file}"); + } } } diff --git a/parser/src/powdr.lalrpop b/parser/src/powdr.lalrpop index 15d26d929..1cd55a1d8 100644 --- a/parser/src/powdr.lalrpop +++ b/parser/src/powdr.lalrpop @@ -565,7 +565,6 @@ SpecialIdentifier: &'input str = { } ConstantIdentifier: String = { - // TODO it seems the lexer splits the token after % r"%[a-zA-Z_][a-zA-Z$_0-9@]*" => <>.to_string(), } diff --git a/pipeline/tests/pil.rs b/pipeline/tests/pil.rs index bee8706f5..6c19e9f00 100644 --- a/pipeline/tests/pil.rs +++ b/pipeline/tests/pil.rs @@ -63,7 +63,7 @@ fn test_external_witgen_both_provided() { } #[test] -#[should_panic = "called `Result::unwrap()` on an `Err` value: Generic(\"main.b = (main.a + 1);:\\n Linear constraint is not satisfiable: -1 != 0\")"] +#[should_panic = "called `Result::unwrap()` on an `Err` value: Generic(\"main.b = (main.a + 1);:\\n Linear constraint is not satisfiable: 18446744069414584320 != 0\")"] fn test_external_witgen_fails_on_conflicting_external_witness() { let f = "pil/external_witgen.pil"; let external_witness = vec![