Skip to content

Commit

Permalink
feat: Add StructDefinition::set_fields (noir-lang/noir#5931)
Browse files Browse the repository at this point in the history
feat: Only check array bounds in brillig if index is unsafe (noir-lang/noir#5938)
chore: error on false constraint (noir-lang/noir#5890)
feat: warn on unused functions (noir-lang/noir#5892)
feat: add `fmtstr::contents` (noir-lang/noir#5928)
fix: collect functions generated by attributes (noir-lang/noir#5930)
fix: Support debug comptime flag for attributes (noir-lang/noir#5929)
feat: Allow inserting new structs and into programs from attributes (noir-lang/noir#5927)
feat: module attributes (noir-lang/noir#5888)
feat: unquote some value as tokens, not as unquote markers (noir-lang/noir#5924)
feat: check argument count and types on attribute function callback (noir-lang/noir#5921)
feat: LSP will now suggest private items if they are visible (noir-lang/noir#5923)
  • Loading branch information
AztecBot committed Sep 5, 2024
2 parents 76ca5d1 + de9b4bb commit f31f80d
Show file tree
Hide file tree
Showing 11 changed files with 148 additions and 6 deletions.
2 changes: 1 addition & 1 deletion .noir-sync-commit
Original file line number Diff line number Diff line change
@@ -1 +1 @@
af3db4bf2e8f7feba6d06c3095d7cdf17c8dde75
9d2629dd1bb28a8c2ecb4c33d26119da75d626c2
Original file line number Diff line number Diff line change
Expand Up @@ -648,7 +648,11 @@ impl<'block> BrilligBlock<'block> {
};

let index_variable = self.convert_ssa_single_addr_value(*index, dfg);
self.validate_array_index(array_variable, index_variable);

if !dfg.is_safe_index(*index, *array) {
self.validate_array_index(array_variable, index_variable);
}

self.retrieve_variable_from_array(
array_pointer,
index_variable,
Expand All @@ -667,7 +671,11 @@ impl<'block> BrilligBlock<'block> {
result_ids[0],
dfg,
);
self.validate_array_index(source_variable, index_register);

if !dfg.is_safe_index(*index, *array) {
self.validate_array_index(source_variable, index_register);
}

self.convert_ssa_array_set(
source_variable,
destination_variable,
Expand Down
3 changes: 3 additions & 0 deletions noir/noir-repo/compiler/noirc_evaluator/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@ impl From<SsaReport> for FileDiagnostic {
InternalBug::IndependentSubgraph { call_stack } => {
("There is no path from the output of this brillig call to either return values or inputs of the circuit, which creates an independent subgraph. This is quite likely a soundness vulnerability".to_string(),call_stack)
}
InternalBug::AssertFailed { call_stack } => ("As a result, the compiled circuit is ensured to fail. Other assertions may also fail during execution".to_string(), call_stack)
};
let call_stack = vecmap(call_stack, |location| location);
let file_id = call_stack.last().map(|location| location.file).unwrap_or_default();
Expand All @@ -111,6 +112,8 @@ pub enum InternalWarning {
pub enum InternalBug {
#[error("Input to brillig function is in a separate subgraph to output")]
IndependentSubgraph { call_stack: CallStack },
#[error("Assertion is always false")]
AssertFailed { call_stack: CallStack },
}

#[derive(Debug, PartialEq, Eq, Clone, Error)]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use super::big_int::BigIntContext;
use super::generated_acir::{BrilligStdlibFunc, GeneratedAcir, PLACEHOLDER_BRILLIG_INDEX};
use crate::brillig::brillig_gen::brillig_directive;
use crate::brillig::brillig_ir::artifact::GeneratedBrillig;
use crate::errors::{InternalError, RuntimeError, SsaReport};
use crate::errors::{InternalBug, InternalError, RuntimeError, SsaReport};
use crate::ssa::acir_gen::{AcirDynamicArray, AcirValue};
use crate::ssa::ir::dfg::CallStack;
use crate::ssa::ir::types::Type as SsaType;
Expand Down Expand Up @@ -126,6 +126,8 @@ pub(crate) struct AcirContext<F: AcirField> {
big_int_ctx: BigIntContext,

expression_width: ExpressionWidth,

pub(crate) warnings: Vec<SsaReport>,
}

impl<F: AcirField> AcirContext<F> {
Expand Down Expand Up @@ -518,6 +520,12 @@ impl<F: AcirField> AcirContext<F> {
self.mark_variables_equivalent(lhs, rhs)?;
return Ok(());
}
if diff_expr.is_const() {
// Constraint is always false
self.warnings.push(SsaReport::Bug(InternalBug::AssertFailed {
call_stack: self.get_call_stack(),
}));
}

self.acir_ir.assert_is_zero(diff_expr);
if let Some(payload) = assert_message {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -453,6 +453,7 @@ impl<'a> Context<'a> {
}

warnings.extend(return_warnings);
warnings.extend(self.acir_context.warnings.clone());

// Add the warnings from the alter Ssa passes
Ok(self.acir_context.finish(input_witness, return_witnesses, warnings))
Expand Down
13 changes: 13 additions & 0 deletions noir/noir-repo/compiler/noirc_frontend/src/hir/comptime/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -202,6 +202,11 @@ pub enum InterpreterError {
TypeAnnotationsNeededForMethodCall {
location: Location,
},
ExpectedIdentForStructField {
value: String,
index: usize,
location: Location,
},

// These cases are not errors, they are just used to prevent us from running more code
// until the loop can be resumed properly. These cases will never be displayed to users.
Expand Down Expand Up @@ -269,6 +274,7 @@ impl InterpreterError {
| InterpreterError::FailedToResolveTraitBound { location, .. }
| InterpreterError::FunctionAlreadyResolved { location, .. }
| InterpreterError::MultipleMatchingImpls { location, .. }
| InterpreterError::ExpectedIdentForStructField { location, .. }
| InterpreterError::TypeAnnotationsNeededForMethodCall { location } => *location,

InterpreterError::FailedToParseMacro { error, file, .. } => {
Expand Down Expand Up @@ -566,6 +572,13 @@ impl<'a> From<&'a InterpreterError> for CustomDiagnostic {
error.add_note(message.to_string());
error
}
InterpreterError::ExpectedIdentForStructField { value, index, location } => {
let msg = format!(
"Quoted value in index {index} of this slice is not a valid field name"
);
let secondary = format!("`{value}` is not a valid field name for `set_fields`");
CustomDiagnostic::simple_error(msg, secondary, location.span)
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ use crate::{
},
hir_def::function::FunctionBody,
lexer::Lexer,
macros_api::{HirExpression, HirLiteral, ModuleDefId, NodeInterner, Signedness},
macros_api::{HirExpression, HirLiteral, Ident, ModuleDefId, NodeInterner, Signedness},
node_interner::{DefinitionKind, TraitImplKind},
parser::{self},
token::Token,
Expand Down Expand Up @@ -133,6 +133,7 @@ impl<'local, 'context> Interpreter<'local, 'context> {
"struct_def_as_type" => struct_def_as_type(interner, arguments, location),
"struct_def_fields" => struct_def_fields(interner, arguments, location),
"struct_def_generics" => struct_def_generics(interner, arguments, location),
"struct_def_set_fields" => struct_def_set_fields(interner, arguments, location),
"to_le_radix" => to_le_radix(arguments, return_type, location),
"trait_constraint_eq" => trait_constraint_eq(interner, arguments, location),
"trait_constraint_hash" => trait_constraint_hash(interner, arguments, location),
Expand Down Expand Up @@ -326,6 +327,64 @@ fn struct_def_fields(
Ok(Value::Slice(fields, typ))
}

/// fn set_fields(self, new_fields: [(Quoted, Type)]) {}
/// Returns (name, type) pairs of each field of this StructDefinition
fn struct_def_set_fields(
interner: &mut NodeInterner,
arguments: Vec<(Value, Location)>,
location: Location,
) -> IResult<Value> {
let (the_struct, fields) = check_two_arguments(arguments, location)?;
let struct_id = get_struct(the_struct)?;

let struct_def = interner.get_struct(struct_id);
let mut struct_def = struct_def.borrow_mut();

let field_location = fields.1;
let fields = get_slice(interner, fields)?.0;

let new_fields = fields
.into_iter()
.flat_map(|field_pair| get_tuple(interner, (field_pair, field_location)))
.enumerate()
.map(|(index, mut field_pair)| {
if field_pair.len() == 2 {
let typ = field_pair.pop().unwrap();
let name_value = field_pair.pop().unwrap();

let name_tokens = get_quoted((name_value.clone(), field_location))?;
let typ = get_type((typ, field_location))?;

match name_tokens.first() {
Some(Token::Ident(name)) if name_tokens.len() == 1 => {
Ok((Ident::new(name.clone(), field_location.span), typ))
}
_ => {
let value = name_value.display(interner).to_string();
let location = field_location;
Err(InterpreterError::ExpectedIdentForStructField {
value,
index,
location,
})
}
}
} else {
let type_var = interner.next_type_variable();
let expected = Type::Tuple(vec![type_var.clone(), type_var]);

let actual =
Type::Tuple(vecmap(&field_pair, |value| value.get_type().into_owned()));

Err(InterpreterError::TypeMismatch { expected, actual, location })
}
})
.collect::<Result<Vec<_>, _>>()?;

struct_def.set_fields(new_fields);
Ok(Value::Unit)
}

fn slice_remove(
interner: &mut NodeInterner,
arguments: Vec<(Value, Location)>,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -259,7 +259,6 @@ impl StructType {
/// created. Therefore, this method is used to set the fields once they
/// become known.
pub fn set_fields(&mut self, fields: Vec<(Ident, Type)>) {
assert!(self.fields.is_empty());
self.fields = fields;
}

Expand Down
29 changes: 29 additions & 0 deletions noir/noir-repo/docs/docs/noir/standard_library/meta/struct_def.md
Original file line number Diff line number Diff line change
Expand Up @@ -43,3 +43,32 @@ comptime fn example(foo: StructDefinition) {
#include_code fields noir_stdlib/src/meta/struct_def.nr rust

Returns each field of this struct as a pair of (field name, field type).

### set_fields

#include_code set_fields noir_stdlib/src/meta/struct_def.nr rust

Sets the fields of this struct to the given fields list where each element
is a pair of the field's name and the field's type. Expects each field name
to be a single identifier. Note that this will override any previous fields
on this struct. If those should be preserved, use `.fields()` to retrieve the
current fields on the struct type and append the new fields from there.

Example:

```rust
// Change this struct to:
// struct Foo {
// a: u32,
// b: i8,
// }
#[mangle_fields]
struct Foo { x: Field }

comptime fn mangle_fields(s: StructDefinition) {
s.set_fields(&[
(quote { a }, quote { u32 }.as_type()),
(quote { b }, quote { i8 }.as_type()),
]);
}
```
9 changes: 9 additions & 0 deletions noir/noir-repo/noir_stdlib/src/meta/struct_def.nr
Original file line number Diff line number Diff line change
Expand Up @@ -18,4 +18,13 @@ impl StructDefinition {
// docs:start:fields
fn fields(self) -> [(Quoted, Type)] {}
// docs:end:fields

/// Sets the fields of this struct to the given fields list.
/// All existing fields of the struct will be overridden with the given fields.
/// Each element of the fields list corresponds to the name and type of a field.
/// Each name is expected to be a single identifier.
#[builtin(struct_def_set_fields)]
// docs:start:set_fields
fn set_fields(self, new_fields: [(Quoted, Type)]) {}
// docs:end:set_fields
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,21 @@ struct MyType<A, B, C> {
field2: (B, C),
}

#[mutate_struct_fields]
struct I32AndField {
z: i8,
}

comptime fn my_comptime_fn(typ: StructDefinition) {
let _ = typ.as_type();
assert_eq(typ.generics().len(), 3);
assert_eq(typ.fields().len(), 2);
}

comptime fn mutate_struct_fields(s: StructDefinition) {
let fields = &[
(quote[x], quote[i32].as_type()),
(quote[y], quote[Field].as_type())
];
s.set_fields(fields);
}

0 comments on commit f31f80d

Please sign in to comment.