Skip to content

Commit

Permalink
feat(std_lib): println statements (#630)
Browse files Browse the repository at this point in the history
* preliminary strings work, type added to Hir and abi

* nargo build working for simple str types

* initial strings work. can input to abi and compare string literal. strings currently mock array functionality

* little cleanup and test addition

* cargo clippy and fixed decoding of public string values in toml

* some pr comments, moving to use str keyword

* clarifying comment for string toml type to input value

* pr comments

* starting log work

* update missing type from monomorphisation ast

* update display for string types

* preliminary logging work, can log strings and arrays stored in idents. need to create Log directive for more accurate logging logic

* switch to using Log directive

* switch to using builtin function for println rather than having a new opcode in ssa

* some missing cleanup to remove Log operation from SSA

* fix convert type for new string type

* cargo fmt

* fix up some minor PR nitpicks

* decode method for string of field into abi string

* small cleanuo

* println for blackbox func output now working. need to still do some final testing and cleanup

* little cleanup and additional comments

* parsing toml now follow abi

* additional parser error and some comments

* fix usage of to_bytes with master merge

* working string inputs inside struct inputs to main

* cargo clippy cleanup

* remove unused err

* additional test case for type directed toml parsing

* use remote acvm repo, do not merge as need to wait for acvm to be updated first

* update cargo lock

* clippy

* more cargo clippy fixes

* chore: remove unwanted dependency to old acir crate

* remove old comments

* add nocapture flag to test and start println output on new line

* use refactored SsaContext methods

* switc to show-logs over nocapture flag for printing test output

* cargo clippy changes

* remove comment

* PR comments

* some more pr comments, prepend 0x to hex strings and removed prepended inputs

* format_field_str method so that we have even number of digits in hex str

* added comment to format_field_string

* cargo clippy

* bring back cached_witness() method on InternalVar

* cargo fmt

---------

Co-authored-by: TomAFrench <tom@tomfren.ch>
  • Loading branch information
vezenovm and TomAFrench authored Feb 14, 2023
1 parent 0108ac6 commit d5d1be2
Show file tree
Hide file tree
Showing 17 changed files with 312 additions and 67 deletions.
18 changes: 9 additions & 9 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

7 changes: 6 additions & 1 deletion crates/nargo/src/cli/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,12 @@ pub fn start_cli() {
Arg::with_name("test_name")
.help("If given, only tests with names containing this string will be run"),
)
.arg(allow_warnings.clone()),
.arg(allow_warnings.clone())
.arg(
Arg::with_name("show-logs")
.long("show-logs")
.help("Display output of println statements during tests"),
),
)
.subcommand(
App::new("compile")
Expand Down
17 changes: 12 additions & 5 deletions crates/nargo/src/cli/test_cmd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,19 @@ pub(crate) fn run(args: ArgMatches) -> Result<(), CliError> {
let args = args.subcommand_matches("test").unwrap();
let test_name = args.value_of("test_name").unwrap_or("");
let allow_warnings = args.is_present("allow-warnings");
let show_output = args.is_present("show-logs");
let program_dir =
args.value_of("path").map_or_else(|| std::env::current_dir().unwrap(), PathBuf::from);

run_tests(&program_dir, test_name, allow_warnings)
run_tests(&program_dir, test_name, allow_warnings, show_output)
}

fn run_tests(program_dir: &Path, test_name: &str, allow_warnings: bool) -> Result<(), CliError> {
fn run_tests(
program_dir: &Path,
test_name: &str,
allow_warnings: bool,
show_output: bool,
) -> Result<(), CliError> {
let backend = crate::backends::ConcreteBackend;

let mut driver = Resolver::resolve_root_config(program_dir, backend.np_language())?;
Expand All @@ -43,10 +49,10 @@ fn run_tests(program_dir: &Path, test_name: &str, allow_warnings: bool) -> Resul

for test_function in test_functions {
let test_name = driver.function_name(test_function);
write!(writer, "Testing {test_name}... ").expect("Failed to write to stdout");
writeln!(writer, "Testing {test_name}...").expect("Failed to write to stdout");
writer.flush().ok();

match run_test(test_name, test_function, &driver, allow_warnings) {
match run_test(test_name, test_function, &driver, allow_warnings, show_output) {
Ok(_) => {
writer.set_color(ColorSpec::new().set_fg(Some(Color::Green))).ok();
writeln!(writer, "ok").ok();
Expand Down Expand Up @@ -76,12 +82,13 @@ fn run_test(
main: FuncId,
driver: &Driver,
allow_warnings: bool,
show_output: bool,
) -> Result<(), CliError> {
let backend = crate::backends::ConcreteBackend;
let language = backend.np_language();

let program = driver
.compile_no_check(language, false, allow_warnings, Some(main))
.compile_no_check(language, false, allow_warnings, Some(main), show_output)
.map_err(|_| CliError::Generic(format!("Test '{test_name}' failed to compile")))?;

let mut solved_witness = BTreeMap::new();
Expand Down
3 changes: 2 additions & 1 deletion crates/nargo/tests/test_data/strings/Prover.toml
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
message = "hello world"
y = 5
hex_as_string = "0x41"
hex_as_field = "0x41"
hex_as_field = "0x41"
46 changes: 45 additions & 1 deletion crates/nargo/tests/test_data/strings/src/main.nr
Original file line number Diff line number Diff line change
@@ -1,12 +1,56 @@
fn main(message : pub str<11>, hex_as_string : str<4>, hex_as_field : Field) {
use dep::std;

fn main(message : pub str<11>, y : Field, hex_as_string : str<4>, hex_as_field : Field) {
let mut bad_message = "hello world";

constrain message == "hello world";
bad_message = "helld world";
let x = 10;
let z = x * 5;
std::println(10);

std::println(z); // x * 5 in println not yet supported
std::println(x);

let array = [1, 2, 3, 5, 8];
constrain y == 5; // Change to y != 5 to see how the later print statements are not called
std::println(array);

std::println(bad_message);
constrain message != bad_message;

let hash = std::hash::pedersen([x]);
std::println(hash);

constrain hex_as_string == "0x41";
// constrain hex_as_string != 0x41; This will fail with a type mismatch between str[4] and Field
constrain hex_as_field == 0x41;
}

#[test]
fn test_prints_strings() {
let message = "hello world!";

std::println(message);
std::println("goodbye world");
}

#[test]
fn test_prints_array() {
let array = [1, 2, 3, 5, 8];

// TODO: Printing structs currently not supported
// let s = Test { a: 1, b: 2, c: [3, 4] };
// std::println(s);

std::println(array);

let hash = std::hash::pedersen(array);
std::println(hash);
}

struct Test {
a: Field,
b: Field,
c: [Field; 2],
}
31 changes: 17 additions & 14 deletions crates/noirc_abi/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@ use std::{collections::BTreeMap, convert::TryInto, str};
use acvm::FieldElement;
use errors::AbiError;
use input_parser::InputValue;
use iter_extended::vecmap;
use serde::{Deserialize, Serialize};

// This is the ABI used to bridge the different TOML formats for the initial
// witness, the partial witness generator and the interpreter.
//
Expand Down Expand Up @@ -260,19 +260,10 @@ impl Abi {
InputValue::Vec(field_elements)
}
AbiType::String { length } => {
let string_as_slice: Vec<u8> = field_iterator
.take(*length as usize)
.map(|e| {
let mut field_as_bytes = e.to_be_bytes();
let char_byte = field_as_bytes.pop().unwrap(); // A character in a string is represented by a u8, thus we just want the last byte of the element
assert!(field_as_bytes.into_iter().all(|b| b == 0)); // Assert that the rest of the field element's bytes are empty
char_byte
})
.collect();

let final_string = str::from_utf8(&string_as_slice).unwrap();

InputValue::String(final_string.to_owned())
let field_elements: Vec<FieldElement> =
field_iterator.take(*length as usize).collect();

InputValue::String(decode_string_value(&field_elements))
}
AbiType::Struct { fields, .. } => {
let mut struct_map = BTreeMap::new();
Expand All @@ -290,3 +281,15 @@ impl Abi {
Ok(value)
}
}

pub fn decode_string_value(field_elements: &[FieldElement]) -> String {
let string_as_slice = vecmap(field_elements, |e| {
let mut field_as_bytes = e.to_be_bytes();
let char_byte = field_as_bytes.pop().unwrap(); // A character in a string is represented by a u8, thus we just want the last byte of the element
assert!(field_as_bytes.into_iter().all(|b| b == 0)); // Assert that the rest of the field element's bytes are empty
char_byte
});

let final_string = str::from_utf8(&string_as_slice).unwrap();
final_string.to_owned()
}
5 changes: 3 additions & 2 deletions crates/noirc_driver/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ impl Driver {
allow_warnings: bool,
) -> Result<CompiledProgram, ReportedError> {
self.check_crate(allow_warnings)?;
self.compile_no_check(np_language, show_ssa, allow_warnings, None)
self.compile_no_check(np_language, show_ssa, allow_warnings, None, true)
}

/// Compile the current crate. Assumes self.check_crate is called beforehand!
Expand All @@ -163,6 +163,7 @@ impl Driver {
allow_warnings: bool,
// Optional override to provide a different `main` function to start execution
main_function: Option<FuncId>,
show_output: bool,
) -> Result<CompiledProgram, ReportedError> {
// Find the local crate, one should always be present
let local_crate = self.context.def_map(LOCAL_CRATE).unwrap();
Expand All @@ -187,7 +188,7 @@ impl Driver {
let program = monomorphize(main_function, &self.context.def_interner);

let blackbox_supported = acvm::default_is_black_box_supported(np_language.clone());
match create_circuit(program, np_language, blackbox_supported, show_ssa) {
match create_circuit(program, np_language, blackbox_supported, show_ssa, show_output) {
Ok(circuit) => Ok(CompiledProgram { circuit, abi: Some(abi) }),
Err(err) => {
// The FileId here will be the file id of the file with the main file
Expand Down
6 changes: 4 additions & 2 deletions crates/noirc_evaluator/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,11 +39,12 @@ pub fn create_circuit(
np_language: Language,
is_blackbox_supported: IsBlackBoxSupported,
enable_logging: bool,
show_output: bool,
) -> Result<Circuit, RuntimeError> {
let mut evaluator = Evaluator::new();

// First evaluate the main function
evaluator.evaluate_main_alt(program, enable_logging)?;
evaluator.evaluate_main_alt(program, enable_logging, show_output)?;

let witness_index = evaluator.current_witness_index();

Expand Down Expand Up @@ -110,6 +111,7 @@ impl Evaluator {
&mut self,
program: Program,
enable_logging: bool,
show_output: bool,
) -> Result<(), RuntimeError> {
let mut ir_gen = IrGenerator::new(program);
self.parse_abi_alt(&mut ir_gen);
Expand All @@ -118,7 +120,7 @@ impl Evaluator {
ir_gen.ssa_gen_main()?;

//Generates ACIR representation:
ir_gen.context.ir_to_acir(self, enable_logging)?;
ir_gen.context.ir_to_acir(self, enable_logging, show_output)?;
Ok(())
}

Expand Down
13 changes: 12 additions & 1 deletion crates/noirc_evaluator/src/ssa/acir_gen.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
use crate::ssa::{
builtin,
context::SsaContext,
node::{Instruction, Operation},
};
Expand Down Expand Up @@ -34,6 +35,7 @@ impl Acir {
ins: &Instruction,
evaluator: &mut Evaluator,
ctx: &SsaContext,
show_output: bool,
) -> Result<(), RuntimeErrorKind> {
use operations::{
binary, condition, constrain, intrinsics, load, not, r#return, store, truncate,
Expand All @@ -57,7 +59,16 @@ impl Acir {
truncate::evaluate(value, *bit_size, *max_bit_size, var_cache, evaluator, ctx)
}
Operation::Intrinsic(opcode, args) => {
intrinsics::evaluate(args, ins, *opcode, var_cache, acir_mem, ctx, evaluator)
let opcode = match opcode {
builtin::Opcode::Println(print_info) => {
builtin::Opcode::Println(builtin::PrintlnInfo {
is_string_output: print_info.is_string_output,
show_output,
})
}
_ => *opcode,
};
intrinsics::evaluate(args, ins, opcode, var_cache, acir_mem, ctx, evaluator)
}
Operation::Return(node_ids) => {
r#return::evaluate(node_ids, acir_mem, var_cache, evaluator, ctx)?
Expand Down
3 changes: 3 additions & 0 deletions crates/noirc_evaluator/src/ssa/acir_gen/internal_var.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,9 @@ impl InternalVar {
pub(crate) fn set_id(&mut self, id: NodeId) {
self.id = Some(id)
}
pub(crate) fn cached_witness(&self) -> &Option<Witness> {
&self.cached_witness
}

pub fn to_expression(&self) -> Expression {
if let Some(w) = self.cached_witness {
Expand Down
Loading

0 comments on commit d5d1be2

Please sign in to comment.