Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Basic strings #622

Merged
merged 34 commits into from
Jan 25, 2023
Merged
Show file tree
Hide file tree
Changes from 14 commits
Commits
Show all changes
34 commits
Select commit Hold shift + click to select a range
19bd86b
preliminary strings work, type added to Hir and abi
vezenovm Jan 6, 2023
4a2d6da
nargo build working for simple str types
vezenovm Jan 6, 2023
a627b02
initial strings work. can input to abi and compare string literal. st…
vezenovm Jan 9, 2023
c6f40b5
little cleanup and test addition
vezenovm Jan 9, 2023
b4f9e5c
cargo clippy and fixed decoding of public string values in toml
vezenovm Jan 9, 2023
fdc3058
Merge branch 'master' into mv/strings
vezenovm Jan 10, 2023
a1eaba3
some pr comments, moving to use str keyword
vezenovm Jan 11, 2023
8aeab6a
clarifying comment for string toml type to input value
vezenovm Jan 11, 2023
8780311
pr comments
vezenovm Jan 11, 2023
82af973
update display for string types
vezenovm Jan 11, 2023
92a6b69
merge conflicts
vezenovm Jan 17, 2023
9828d63
merge conflicts with master
vezenovm Jan 17, 2023
279d266
fix convert type for new string type
vezenovm Jan 17, 2023
dcb3a0a
fix up some minor PR nitpicks
vezenovm Jan 17, 2023
deeabd3
parsing toml now follow abi
vezenovm Jan 19, 2023
c11a1bc
additional parser error and some comments
vezenovm Jan 19, 2023
7168c4b
merge conflicts
vezenovm Jan 19, 2023
755417f
fix usage of to_bytes with master merge
vezenovm Jan 19, 2023
31ded35
working string inputs inside struct inputs to main
vezenovm Jan 19, 2023
2715eb0
cargo clippy cleanup
vezenovm Jan 19, 2023
09998b9
remove unused err
vezenovm Jan 19, 2023
cf2eb44
additional test case for type directed toml parsing
vezenovm Jan 19, 2023
d6e2270
merge conflicts
vezenovm Jan 20, 2023
6bfcfda
using less and greater than for str type
vezenovm Jan 23, 2023
ed44404
Merge branch 'master' into mv/strings
vezenovm Jan 23, 2023
0917038
cargo clippy
vezenovm Jan 23, 2023
41e6a80
fix type def for str fields in struct_inputs test
vezenovm Jan 23, 2023
7bea868
Merge branch 'master' into mv/strings
vezenovm Jan 23, 2023
1722c2d
merge conflicts
vezenovm Jan 23, 2023
861da20
uncommit accidental prover input to test and cargo fmt
vezenovm Jan 24, 2023
7d0b3e4
switch to using abi BTreeMap instead of flattened vec
vezenovm Jan 24, 2023
4019440
comments
vezenovm Jan 24, 2023
099828a
pr comments, mainly style
vezenovm Jan 25, 2023
fed1292
Merge branch 'master' into mv/strings
vezenovm Jan 25, 2023
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
247 changes: 100 additions & 147 deletions Cargo.lock

Large diffs are not rendered by default.

5 changes: 5 additions & 0 deletions crates/nargo/tests/test_data/strings/Nargo.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
[package]
authors = [""]
compiler_version = "0.1"

[dependencies]
1 change: 1 addition & 0 deletions crates/nargo/tests/test_data/strings/Prover.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
message = "hello world"
8 changes: 8 additions & 0 deletions crates/nargo/tests/test_data/strings/src/main.nr
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
fn main(message : pub str[11]) {
let mut bad_message = "hello world";

constrain message == "hello world";
bad_message = "helld world";

constrain message != bad_message;
}
5 changes: 5 additions & 0 deletions crates/noirc_abi/src/input_parser/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ use crate::AbiType;
pub enum InputValue {
Field(FieldElement),
Vec(Vec<FieldElement>),
String(String),
Struct(BTreeMap<String, InputValue>),
Undefined,
}
Expand All @@ -38,6 +39,10 @@ impl InputValue {
.all(|field_element| Self::Field(*field_element).matches_abi(typ))
}

(InputValue::String(string), AbiType::String { length }) => {
string.len() == *length as usize
}

(InputValue::Struct(map), AbiType::Struct { fields, .. }) => {
if map.len() != fields.len() {
return false;
Expand Down
16 changes: 11 additions & 5 deletions crates/noirc_abi/src/input_parser/toml.rs
Original file line number Diff line number Diff line change
Expand Up @@ -55,9 +55,14 @@ fn toml_map_to_field(
for (parameter, value) in toml_map {
let mapped_value = match value {
TomlTypes::String(string) => {
let new_value = parse_str(&string)?;

if let Some(field_element) = new_value {
let new_value = try_str_to_field(&string);

// We accept UTF-8 strings as input as well as hex strings representing field elements.
// We still want developers to be able to pass in hex strings for FieldElement inputs.
// Thus, we first try to parse the string into a field element, and if that fails we assume that the input is meant to be a plain string
jfecher marked this conversation as resolved.
Show resolved Hide resolved
if new_value.is_err() {
InputValue::String(string)
} else if let Ok(Some(field_element)) = new_value {
InputValue::Field(field_element)
} else {
InputValue::Undefined
Expand All @@ -84,7 +89,7 @@ fn toml_map_to_field(
TomlTypes::ArrayString(arr_str) => {
let array_elements: Vec<_> = arr_str
.into_iter()
.map(|elem_str| parse_str(&elem_str).unwrap().unwrap())
.map(|elem_str| try_str_to_field(&elem_str).unwrap().unwrap())
.collect();

InputValue::Vec(array_elements)
Expand Down Expand Up @@ -116,6 +121,7 @@ fn toml_remap(map: &BTreeMap<String, InputValue>) -> BTreeMap<String, TomlTypes>
let array = v.iter().map(|i| format!("0x{}", i.to_hex())).collect();
TomlTypes::ArrayString(array)
}
InputValue::String(s) => TomlTypes::String(s.clone()),
InputValue::Struct(map) => {
let map_with_toml_types = toml_remap(map);
TomlTypes::Table(map_with_toml_types)
Expand Down Expand Up @@ -145,7 +151,7 @@ enum TomlTypes {
Table(BTreeMap<String, TomlTypes>),
}

fn parse_str(value: &str) -> Result<Option<FieldElement>, InputParserError> {
fn try_str_to_field(value: &str) -> Result<Option<FieldElement>, InputParserError> {
if value.is_empty() {
Ok(None)
} else if value.starts_with("0x") {
Expand Down
30 changes: 29 additions & 1 deletion crates/noirc_abi/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use std::{collections::BTreeMap, convert::TryInto};
use std::{collections::BTreeMap, convert::TryInto, str};

use acvm::FieldElement;
use errors::AbiError;
Expand Down Expand Up @@ -46,6 +46,9 @@ pub enum AbiType {
)]
jfecher marked this conversation as resolved.
Show resolved Hide resolved
fields: BTreeMap<String, AbiType>,
},
String {
length: u64,
},
}

#[derive(Clone, Copy, Debug, PartialEq, Eq, Serialize, Deserialize)]
Expand Down Expand Up @@ -80,6 +83,7 @@ impl AbiType {
AbiType::Field | AbiType::Integer { .. } => 1,
AbiType::Array { length, typ: _ } => *length as usize,
AbiType::Struct { fields, .. } => fields.len(),
AbiType::String { length } => *length as usize,
}
}

Expand All @@ -91,6 +95,7 @@ impl AbiType {
AbiType::Struct { fields, .. } => {
fields.iter().fold(0, |acc, (_, field_type)| acc + field_type.field_count())
}
AbiType::String { length } => *length as u32,
}
}
}
Expand Down Expand Up @@ -195,6 +200,11 @@ impl Abi {
match value {
InputValue::Field(elem) => encoded_value.push(elem),
InputValue::Vec(vec_elem) => encoded_value.extend(vec_elem),
InputValue::String(string) => {
let str_as_fields =
string.bytes().map(|byte| FieldElement::from_be_bytes_reduce(&[byte]));
encoded_value.extend(str_as_fields)
jfecher marked this conversation as resolved.
Show resolved Hide resolved
}
InputValue::Struct(object) => {
for (field_name, value) in object {
let new_name = format!("{param_name}.{field_name}");
Expand Down Expand Up @@ -253,6 +263,24 @@ impl Abi {
index += *length as usize;
InputValue::Vec(field_elements.to_vec())
}
AbiType::String { length } => {
let field_elements = &encoded_inputs[index..index + (*length as usize)];

let string_as_slice = field_elements
.iter()
.map(|e| {
let mut field_as_bytes = e.to_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::<Vec<_>>();

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

index += *length as usize;
InputValue::String(final_string.to_owned())
}
AbiType::Struct { fields, .. } => {
let mut struct_map = BTreeMap::new();

Expand Down
11 changes: 11 additions & 0 deletions crates/noirc_evaluator/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,11 @@ impl Evaluator {
self.generate_struct_witnesses(&mut struct_witnesses, visibility, &new_fields)?;
igen.abi_struct(name, Some(def), fields, struct_witnesses);
}
AbiType::String { length } => {
let typ = AbiType::Integer { sign: noirc_abi::Sign::Unsigned, width: 8 };
let witnesses = self.generate_array_witnesses(visibility, length, &typ)?;
igen.abi_array(name, Some(def), &typ, *length, witnesses);
}
}
Ok(())
}
Expand Down Expand Up @@ -192,6 +197,12 @@ impl Evaluator {
}
self.generate_struct_witnesses(struct_witnesses, visibility, &new_fields)?
}
AbiType::String { length } => {
let typ = AbiType::Integer { sign: noirc_abi::Sign::Unsigned, width: 8 };
let internal_str_witnesses =
self.generate_array_witnesses(visibility, length, &typ)?;
struct_witnesses.insert(name.clone(), internal_str_witnesses);
}
}
}
Ok(())
Expand Down
24 changes: 24 additions & 0 deletions crates/noirc_evaluator/src/ssa/code_gen.rs
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,9 @@ impl IRGenerator {
noirc_abi::AbiType::Struct { .. } => {
unreachable!("array of structs are not supported for now")
}
noirc_abi::AbiType::String { .. } => {
unreachable!("array of strings are not supported for now")
}
}
}

Expand Down Expand Up @@ -544,6 +547,27 @@ impl IRGenerator {
}
Ok(Value::Single(new_var))
}
Expression::Literal(Literal::Str(string)) => {
let string_as_integers = string
.bytes()
.map(|byte| {
jfecher marked this conversation as resolved.
Show resolved Hide resolved
let f = FieldElement::from_be_bytes_reduce(&[byte]);
Expression::Literal(Literal::Integer(
f,
Type::Integer(noirc_frontend::Signedness::Unsigned, 8),
))
})
.collect::<Vec<_>>();
jfecher marked this conversation as resolved.
Show resolved Hide resolved

let string_arr_literal = ArrayLiteral {
contents: string_as_integers,
element_type: Type::Integer(noirc_frontend::Signedness::Unsigned, 8),
};

let new_value = self
.codegen_expression(&Expression::Literal(Literal::Array(string_arr_literal)))?;
Ok(new_value)
}
Expression::Ident(ident) => self.codegen_identifier(ident),
Expression::Binary(binary) => {
// Note: we disallows structs/tuples in infix expressions.
Expand Down
1 change: 1 addition & 0 deletions crates/noirc_evaluator/src/ssa/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1127,6 +1127,7 @@ impl SsaContext {
Type::Unit => ObjectType::NotAnObject,
Type::Function(..) => ObjectType::Function,
Type::Tuple(_) => todo!("Conversion to ObjectType is unimplemented for tuples"),
Type::String(_) => todo!("Conversion to ObjectType is unimplemented for strings"),
}
}

Expand Down
5 changes: 5 additions & 0 deletions crates/noirc_frontend/src/ast/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ pub enum UnresolvedType {
Array(Option<Expression>, Box<UnresolvedType>), // [4]Witness = Array(4, Witness)
Integer(Comptime, Signedness, u32), // u32 = Integer(unsigned, 32)
Bool(Comptime),
String(Option<Expression>),
Unit,

/// A Named UnresolvedType can be a struct type or a type variable
Expand Down Expand Up @@ -70,6 +71,10 @@ impl std::fmt::Display for UnresolvedType {
write!(f, "({})", elements.join(", "))
}
Bool(is_const) => write!(f, "{is_const}bool"),
String(len) => match len {
None => write!(f, "str[]"),
Some(len) => write!(f, "str[{}]", len),
},
Function(args, ret) => {
let args = vecmap(args, ToString::to_string);
write!(f, "fn({}) -> {ret}", args.join(", "))
Expand Down
44 changes: 28 additions & 16 deletions crates/noirc_frontend/src/hir/resolution/resolver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -286,27 +286,16 @@ impl<'a> Resolver<'a> {
match typ {
UnresolvedType::FieldElement(comptime) => Type::FieldElement(comptime),
UnresolvedType::Array(size, elem) => {
let resolved_size = match &size {
None => {
let id = self.interner.next_type_variable_id();
let typevar = Shared::new(TypeBinding::Unbound(id));
new_variables.push((id, typevar.clone()));

// 'Named'Generic is a bit of a misnomer here, we want a type variable that
// wont be bound over but this one has no name since we do not currently
// require users to explicitly be generic over array lengths.
Type::NamedGeneric(typevar, Rc::new("".into()))
}
Some(expr) => {
let len = self.eval_array_length(expr);
Type::ArrayLength(len)
}
};
let resolved_size = self.resolve_array_size(size, new_variables);
let elem = Box::new(self.resolve_type_inner(*elem, new_variables));
Type::Array(Box::new(resolved_size), elem)
}
UnresolvedType::Integer(comptime, sign, bits) => Type::Integer(comptime, sign, bits),
UnresolvedType::Bool(comptime) => Type::Bool(comptime),
UnresolvedType::String(size) => {
let resolved_size = self.resolve_array_size(size, new_variables);
Type::String(Box::new(resolved_size))
}
UnresolvedType::Unit => Type::Unit,
UnresolvedType::Unspecified => Type::Error,
UnresolvedType::Error => Type::Error,
Expand Down Expand Up @@ -339,6 +328,29 @@ impl<'a> Resolver<'a> {
}
}

fn resolve_array_size(
&mut self,
size: Option<Expression>,
new_variables: &mut Generics,
) -> Type {
match &size {
None => {
let id = self.interner.next_type_variable_id();
let typevar = Shared::new(TypeBinding::Unbound(id));
new_variables.push((id, typevar.clone()));

// 'Named'Generic is a bit of a misnomer here, we want a type variable that
// wont be bound over but this one has no name since we do not currently
// require users to explicitly be generic over array lengths.
Type::NamedGeneric(typevar, Rc::new("".into()))
}
Some(expr) => {
let len = self.eval_array_length(expr);
Type::ArrayLength(len)
}
}
}

fn get_ident_from_path(&mut self, path: Path) -> HirIdent {
let location = Location::new(path.span(), self.file);

Expand Down
27 changes: 20 additions & 7 deletions crates/noirc_frontend/src/hir/type_check/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -71,9 +71,9 @@ pub(crate) fn type_check_expression(
Shared::new(TypeBinding::Unbound(id)),
)
}
HirLiteral::Str(_) => unimplemented!(
"[Coming Soon] : Currently string literal types have not been implemented"
),
HirLiteral::Str(string) => {
Type::String(Box::new(Type::ArrayLength(string.len() as u64)))
}
}
}
HirExpression::Infix(infix_expr) => {
Expand Down Expand Up @@ -718,9 +718,12 @@ pub fn comparator_operand_type_rules(
}
});

if x_size != y_size {
return Err(format!("Can only compare arrays of the same length. Here LHS is of length {x_size}, and RHS is {y_size} "));
}
x_size.unify(y_size, op.location.span, errors, || {
TypeCheckError::Unstructured {
msg: format!("Can only compare arrays of the same length. Here LHS is of length {}, and RHS is {} ", x_size, y_size),
span: op.location.span,
}
});

// We could check if all elements of all arrays are comptime but I am lazy
Ok(Bool(Comptime::No(Some(op.location.span))))
Expand All @@ -731,6 +734,16 @@ pub fn comparator_operand_type_rules(
}
Err(format!("Unsupported types for comparison: {name_a} and {name_b}"))
}
(lhs, rhs) => Err(format!("Unsupported types for comparison: {lhs} and {rhs}")),
(String(x_size), String(y_size)) => {
x_size.unify(y_size, op.location.span, errors, || {
TypeCheckError::Unstructured {
msg: format!("Can only compare strings of the same length. Here LHS is of length {}, and RHS is {} ", x_size, y_size),
span: op.location.span,
}
});

Ok(Bool(Comptime::No(Some(op.location.span))))
}
(lhs, rhs) => Err(format!("Unsupported types for comparison: {} and {}", lhs, rhs)),
}
}
Loading