From 41a1fe8c2f4256bc1942b3d7b842b5dd834258e5 Mon Sep 17 00:00:00 2001 From: Jake Fecher Date: Thu, 17 Jul 2025 15:09:58 -0500 Subject: [PATCH 1/9] Add a Shared wrapper for all struct and tuple fields --- .../src/hir/comptime/display.rs | 4 +- .../src/hir/comptime/interpreter.rs | 31 +-- .../src/hir/comptime/interpreter/builtin.rs | 195 +++++++++--------- .../interpreter/builtin/builtin_helpers.rs | 14 +- .../noirc_frontend/src/hir/comptime/value.rs | 24 ++- .../lsp/src/requests/hover/from_reference.rs | 2 +- .../execute__tests__stderr.snap.new | 56 +++++ .../execute__tests__stderr.snap.new | 28 +++ .../execute__tests__stderr.snap.new | 14 ++ .../execute__tests__stderr.snap.new | 14 ++ .../execute__tests__stderr.snap.new | 31 +++ tooling/nargo_expand/src/printer.rs | 4 +- 12 files changed, 292 insertions(+), 125 deletions(-) create mode 100644 tooling/nargo_cli/tests/snapshots/compile_failure/brillig_slice_to_acir/execute__tests__stderr.snap.new create mode 100644 tooling/nargo_cli/tests/snapshots/compile_failure/mutability_regression_2911/execute__tests__stderr.snap.new create mode 100644 tooling/nargo_cli/tests/snapshots/compile_failure/noirc_frontend_tests_cannot_determine_array_type/execute__tests__stderr.snap.new create mode 100644 tooling/nargo_cli/tests/snapshots/compile_failure/noirc_frontend_tests_cannot_determine_slice_type/execute__tests__stderr.snap.new create mode 100644 tooling/nargo_cli/tests/snapshots/compile_failure/noirc_frontend_tests_traits_errors_if_impl_trait_constraint_is_not_satisfied/execute__tests__stderr.snap.new diff --git a/compiler/noirc_frontend/src/hir/comptime/display.rs b/compiler/noirc_frontend/src/hir/comptime/display.rs index 70b11d18b0e..36ac164d433 100644 --- a/compiler/noirc_frontend/src/hir/comptime/display.rs +++ b/compiler/noirc_frontend/src/hir/comptime/display.rs @@ -386,7 +386,7 @@ impl Display for ValuePrinter<'_, '_> { Value::Function(..) => write!(f, "(function)"), Value::Closure(..) => write!(f, "(closure)"), Value::Tuple(fields) => { - let fields = vecmap(fields, |field| field.display(self.interner).to_string()); + let fields = vecmap(fields, |field| field.borrow().display(self.interner).to_string()); if fields.len() == 1 { write!(f, "({},)", fields[0]) } else { @@ -399,7 +399,7 @@ impl Display for ValuePrinter<'_, '_> { other => other.to_string(), }; let fields = vecmap(fields, |(name, value)| { - format!("{}: {}", name, value.display(self.interner)) + format!("{}: {}", name, value.borrow().display(self.interner)) }); write!(f, "{typename} {{ {} }}", fields.join(", ")) } diff --git a/compiler/noirc_frontend/src/hir/comptime/interpreter.rs b/compiler/noirc_frontend/src/hir/comptime/interpreter.rs index abbc78d3e32..564315be68a 100644 --- a/compiler/noirc_frontend/src/hir/comptime/interpreter.rs +++ b/compiler/noirc_frontend/src/hir/comptime/interpreter.rs @@ -422,6 +422,7 @@ impl<'local, 'interner> Interpreter<'local, 'interner> { for ((pattern, typ), argument) in pattern_fields.iter().zip(type_fields).zip(fields) { + let argument = argument.borrow().clone(); self.define_pattern(pattern, typ, argument, location)?; } Ok(()) @@ -450,6 +451,7 @@ impl<'local, 'interner> Interpreter<'local, 'interner> { } })?; + let field = field.borrow(); let field_type = field.get_type().into_owned(); let result = self.define_pattern( field_pattern, @@ -898,8 +900,8 @@ impl<'local, 'interner> Interpreter<'local, 'interner> { /// - `<=`: `ordering != Ordering::Less` fn evaluate_ordering(&self, ordering: Value, operator: BinaryOpKind) -> IResult { let ordering = match ordering { - Value::Struct(fields, _) => match fields.into_iter().next().unwrap().1 { - Value::Field(ordering) => ordering, + Value::Struct(fields, _) => match &*fields.into_iter().next().unwrap().1.borrow() { + Value::Field(ordering) => *ordering, _ => unreachable!("`cmp` should always return an Ordering value"), }, _ => unreachable!("`cmp` should always return an Ordering value"), @@ -938,7 +940,7 @@ impl<'local, 'interner> Interpreter<'local, 'interner> { .fields .into_iter() .map(|(name, expr)| { - let field_value = self.evaluate(expr)?; + let field_value = Shared::new(self.evaluate(expr)?); Ok((Rc::new(name.into_string()), field_value)) }) .collect::>()?; @@ -961,11 +963,11 @@ impl<'local, 'interner> Interpreter<'local, 'interner> { let (fields, struct_type) = match self.evaluate(access.lhs)? { Value::Struct(fields, typ) => (fields, typ), Value::Tuple(fields) => { - let (fields, field_types): (HashMap, Value>, Vec) = fields + let (fields, field_types): (HashMap, Shared>, Vec) = fields .into_iter() .enumerate() .map(|(i, field)| { - let field_type = field.get_type().into_owned(); + let field_type = field.borrow().get_type().into_owned(); let key_val_pair = (Rc::new(i.to_string()), field); (key_val_pair, field_type) }) @@ -979,13 +981,18 @@ impl<'local, 'interner> Interpreter<'local, 'interner> { } }; - fields.get(access.rhs.as_string()).cloned().ok_or_else(|| { + let field = fields.get(access.rhs.as_string()).cloned().ok_or_else(|| { let location = self.elaborator.interner.expr_location(&id); let value = Value::Struct(fields, struct_type); let field_name = access.rhs.into_string(); let typ = value.get_type().into_owned(); InterpreterError::ExpectedStructToHaveField { typ, field_name, location } - }) + })?; + + // Return a reference to the field so that `&mut foo.bar.baz` can use this reference. + // We set auto_deref to true so that when it is used elsewhere it is dereferenced + // automatically. + Ok(Value::Pointer(field, true, false)) } fn evaluate_call(&mut self, call: HirCallExpression, id: ExprId) -> IResult { @@ -1084,7 +1091,7 @@ impl<'local, 'interner> Interpreter<'local, 'interner> { } fn evaluate_tuple(&mut self, tuple: Vec) -> IResult { - let fields = try_vecmap(tuple, |field| self.evaluate(field))?; + let fields = try_vecmap(tuple, |field| Ok(Shared::new(self.evaluate(field)?)))?; Ok(Value::Tuple(fields)) } @@ -1186,11 +1193,11 @@ impl<'local, 'interner> Interpreter<'local, 'interner> { match object_value { Value::Tuple(mut fields) => { - fields[index] = rhs; + fields[index] = Shared::new(rhs); self.store_lvalue(*object, Value::Tuple(fields)) } Value::Struct(mut fields, typ) => { - fields.insert(Rc::new(field_name.into_string()), rhs); + fields.insert(Rc::new(field_name.into_string()), Shared::new(rhs)); self.store_lvalue(*object, Value::Struct(fields, typ.follow_bindings())) } value => { @@ -1244,8 +1251,8 @@ impl<'local, 'interner> Interpreter<'local, 'interner> { })?; match object_value { - Value::Tuple(mut values) => Ok(values.swap_remove(index)), - Value::Struct(fields, _) => Ok(fields[field_name.as_string()].clone()), + Value::Tuple(mut values) => Ok(values.swap_remove(index).unwrap_or_clone()), + Value::Struct(fields, _) => Ok(fields[field_name.as_string()].clone().unwrap_or_clone()), value => Err(InterpreterError::NonTupleOrStructInMemberAccess { typ: value.get_type().into_owned(), location: *location, diff --git a/compiler/noirc_frontend/src/hir/comptime/interpreter/builtin.rs b/compiler/noirc_frontend/src/hir/comptime/interpreter/builtin.rs index 84186cce80c..f94c9431dab 100644 --- a/compiler/noirc_frontend/src/hir/comptime/interpreter/builtin.rs +++ b/compiler/noirc_frontend/src/hir/comptime/interpreter/builtin.rs @@ -509,8 +509,8 @@ fn type_def_generics( Kind::Numeric(numeric_type) => Some(Value::Type(*numeric_type)), _ => None, }; - let numeric_type = option(option_typ.clone(), numeric_type, location); - Value::Tuple(vec![Value::Type(generic_as_named), numeric_type]) + let numeric_type = Shared::new(option(option_typ.clone(), numeric_type, location)); + Value::Tuple(vec![Shared::new(Value::Type(generic_as_named)), numeric_type]) }) .collect(); @@ -576,9 +576,10 @@ fn type_def_fields( if let Some(struct_fields) = struct_def.get_fields(&generic_args) { for (field_name, field_type, visibility) in struct_fields { let token = LocatedToken::new(Token::Ident(field_name), location); - let name = Value::Quoted(Rc::new(vec![token])); - let visibility = visibility_to_quoted(visibility, location); - fields.push_back(Value::Tuple(vec![name, Value::Type(field_type), visibility])); + let name = Shared::new(Value::Quoted(Rc::new(vec![token]))); + let field_type = Shared::new(Value::Type(field_type)); + let visibility = Shared::new(visibility_to_quoted(visibility, location)); + fields.push_back(Value::Tuple(vec![name, field_type, visibility])); } } @@ -609,9 +610,10 @@ fn type_def_fields_as_written( if let Some(struct_fields) = struct_def.get_fields_as_written() { for field in struct_fields { let token = LocatedToken::new(Token::Ident(field.name.to_string()), location); - let name = Value::Quoted(Rc::new(vec![token])); - let typ = Value::Type(field.typ); - let visibility = visibility_to_quoted(field.visibility, location); + let name = Shared::new(Value::Quoted(Rc::new(vec![token]))); + + let typ = Shared::new(Value::Type(field.typ)); + let visibility = Shared::new(visibility_to_quoted(field.visibility, location)); fields.push_back(Value::Tuple(vec![name, typ, visibility])); } } @@ -676,9 +678,9 @@ fn type_def_set_fields( for (index, mut field_pair) in fields { if field_pair.len() == 3 { - let visibility = field_pair.pop().unwrap(); - let typ = field_pair.pop().unwrap(); - let name_value = field_pair.pop().unwrap(); + let visibility = field_pair.pop().unwrap().unwrap_or_clone(); + let typ = field_pair.pop().unwrap().unwrap_or_clone(); + let name_value = field_pair.pop().unwrap().unwrap_or_clone(); let name_tokens = get_quoted((name_value.clone(), field_location))?; let typ = get_type((typ, field_location))?; @@ -709,7 +711,7 @@ fn type_def_set_fields( let type_var = elaborator.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())); + let actual = Type::Tuple(vecmap(&field_pair, |value| value.borrow().get_type().into_owned())); return Err(InterpreterError::TypeMismatch { expected, actual, location }); } @@ -742,8 +744,8 @@ fn slice_remove( return failing_constraint(message, location, call_stack); } - let element = values.remove(index); - Ok(Value::Tuple(vec![Value::Slice(values, typ), element])) + let element = Shared::new(values.remove(index)); + Ok(Value::Tuple(vec![Shared::new(Value::Slice(values, typ)), element])) } fn slice_push_front( @@ -768,7 +770,9 @@ fn slice_pop_front( let (mut values, typ) = get_slice(interner, argument)?; match values.pop_front() { - Some(element) => Ok(Value::Tuple(vec![element, Value::Slice(values, typ)])), + Some(element) => { + Ok(Value::Tuple(vec![Shared::new(element), Shared::new(Value::Slice(values, typ))])) + } None => failing_constraint("slice_pop_front called on empty slice", location, call_stack), } } @@ -783,7 +787,7 @@ fn slice_pop_back( let (mut values, typ) = get_slice(interner, argument)?; match values.pop_back() { - Some(element) => Ok(Value::Tuple(vec![Value::Slice(values, typ), element])), + Some(element) => Ok(Value::Tuple(vec![Shared::new(Value::Slice(values, typ)), Shared::new(element)])), None => failing_constraint("slice_pop_back called on empty slice", location, call_stack), } } @@ -1005,7 +1009,9 @@ fn type_as_array( ) -> IResult { type_as(arguments, return_type, location, |typ| { if let Type::Array(length, array_type) = typ { - Some(Value::Tuple(vec![Value::Type(*array_type), Value::Type(*length)])) + let array_type = Shared::new(Value::Type(*array_type)); + let length_type = Shared::new(Value::Type(*length)); + Some(Value::Tuple(vec![array_type, length_type])) } else { None } @@ -1045,7 +1051,9 @@ fn type_as_integer( ) -> IResult { type_as(arguments, return_type, location, |typ| { if let Type::Integer(sign, bits) = typ { - Some(Value::Tuple(vec![Value::Bool(sign.is_signed()), Value::U8(bits.bit_size())])) + let sign = Shared::new(Value::Bool(sign.is_signed())); + let bit_size = Shared::new(Value::U8(bits.bit_size())); + Some(Value::Tuple(vec![sign, bit_size])) } else { None } @@ -1094,11 +1102,11 @@ fn type_as_data_type( type_as(arguments, return_type, location, |typ| { if let Type::DataType(struct_type, generics) = typ { Some(Value::Tuple(vec![ - Value::TypeDefinition(struct_type.borrow().id), - Value::Slice( + Shared::new(Value::TypeDefinition(struct_type.borrow().id)), + Shared::new(Value::Slice( generics.into_iter().map(Value::Type).collect(), Type::Slice(Box::new(Type::Quoted(QuotedType::Type))), - ), + )), ])) } else { None @@ -1493,7 +1501,7 @@ fn zeroed(return_type: Type, location: Location) -> Value { } } Type::Unit => Value::Unit, - Type::Tuple(fields) => Value::Tuple(vecmap(fields, |field| zeroed(field, location))), + Type::Tuple(fields) => Value::Tuple(vecmap(fields, |field| Shared::new(zeroed(field, location)))), Type::DataType(data_type, generics) => { let typ = data_type.borrow(); @@ -1501,7 +1509,7 @@ fn zeroed(return_type: Type, location: Location) -> Value { let mut values = HashMap::default(); for (field_name, field_type, _) in fields { - let field_value = zeroed(field_type, location); + let field_value = Shared::new(zeroed(field_type, location)); values.insert(Rc::new(field_name), field_value); } @@ -1586,7 +1594,7 @@ fn expr_as_assert( } else { (Some(constrain.arguments.pop().unwrap()), constrain.arguments.pop().unwrap()) }; - let predicate = Value::expression(predicate.kind); + let predicate = Shared::new(Value::expression(predicate.kind)); let option_type = extract_option_generic_type(return_type); let Type::Tuple(mut tuple_types) = option_type else { @@ -1596,7 +1604,7 @@ fn expr_as_assert( let option_type = tuple_types.pop().unwrap(); let message = message.map(|msg| Value::expression(msg.kind)); - let message = option(option_type, message, location); + let message = Shared::new(option(option_type, message, location)); Some(Value::Tuple(vec![predicate, message])) } else { @@ -1631,8 +1639,8 @@ fn expr_as_assert_eq( ) }; - let lhs = Value::expression(lhs.kind); - let rhs = Value::expression(rhs.kind); + let lhs = Shared::new(Value::expression(lhs.kind)); + let rhs = Shared::new(Value::expression(rhs.kind)); let option_type = extract_option_generic_type(return_type); let Type::Tuple(mut tuple_types) = option_type else { @@ -1642,7 +1650,7 @@ fn expr_as_assert_eq( let option_type = tuple_types.pop().unwrap(); let message = message.map(|message| Value::expression(message.kind)); - let message = option(option_type, message, location); + let message = Shared::new(option(option_type, message, location)); Some(Value::Tuple(vec![lhs, rhs, message])) } else { @@ -1663,8 +1671,8 @@ fn expr_as_assign( ) -> IResult { expr_as(interner, arguments, return_type, location, |expr| { if let ExprValue::Statement(StatementKind::Assign(assign)) = expr { - let lhs = Value::lvalue(assign.lvalue); - let rhs = Value::expression(assign.expression.kind); + let lhs = Shared::new(Value::lvalue(assign.lvalue)); + let rhs = Shared::new(Value::expression(assign.expression.kind)); Some(Value::Tuple(vec![lhs, rhs])) } else { None @@ -1689,9 +1697,9 @@ fn expr_as_binary_op( tuple_types.pop().unwrap(); let binary_op_type = tuple_types.pop().unwrap(); - let binary_op = new_binary_op(infix_expr.operator, binary_op_type); - let lhs = Value::expression(infix_expr.lhs.kind); - let rhs = Value::expression(infix_expr.rhs.kind); + let binary_op = Shared::new(new_binary_op(infix_expr.operator, binary_op_type)); + let lhs = Shared::new(Value::expression(infix_expr.lhs.kind)); + let rhs = Shared::new(Value::expression(infix_expr.rhs.kind)); Some(Value::Tuple(vec![lhs, binary_op, rhs])) } else { None @@ -1740,8 +1748,8 @@ fn expr_as_cast( ) -> IResult { expr_as(interner, arguments, return_type, location, |expr| { if let ExprValue::Expression(ExpressionKind::Cast(cast)) = expr { - let lhs = Value::expression(cast.lhs.kind); - let typ = Value::UnresolvedType(cast.r#type.typ); + let lhs = Shared::new(Value::expression(cast.lhs.kind)); + let typ = Shared::new(Value::UnresolvedType(cast.r#type.typ)); Some(Value::Tuple(vec![lhs, typ])) } else { None @@ -1795,17 +1803,19 @@ fn expr_as_constructor( let option_value = if let ExprValue::Expression(ExpressionKind::Constructor(constructor)) = expr_value { - let typ = Value::UnresolvedType(constructor.typ.typ); + let typ = Shared::new(Value::UnresolvedType(constructor.typ.typ)); let fields = constructor.fields.into_iter(); let fields = fields.map(|(name, value)| { - Value::Tuple(vec![quote_ident(&name, location), Value::expression(value.kind)]) + let ident = Shared::new(quote_ident(&name, location)); + let expr = Shared::new(Value::expression(value.kind)); + Value::Tuple(vec![ident, expr]) }); let fields = fields.collect(); let fields_type = Type::Slice(Box::new(Type::Tuple(vec![ Type::Quoted(QuotedType::Quoted), Type::Quoted(QuotedType::Expr), ]))); - let fields = Value::Slice(fields, fields_type); + let fields = Shared::new(Value::Slice(fields, fields_type)); Some(Value::Tuple(vec![typ, fields])) } else { None @@ -1826,9 +1836,9 @@ fn expr_as_for( if let ForRange::Array(array) = for_statement.range { let token = Token::Ident(for_statement.identifier.into_string()); let token = LocatedToken::new(token, location); - let identifier = Value::Quoted(Rc::new(vec![token])); - let array = Value::expression(array.kind); - let body = Value::expression(for_statement.block.kind); + let identifier = Shared::new(Value::Quoted(Rc::new(vec![token]))); + let array = Shared::new(Value::expression(array.kind)); + let body = Shared::new(Value::expression(for_statement.block.kind)); Some(Value::Tuple(vec![identifier, array, body])) } else { None @@ -1852,10 +1862,10 @@ fn expr_as_for_range( let (from, to) = bounds.into_half_open(); let token = Token::Ident(for_statement.identifier.into_string()); let token = LocatedToken::new(token, location); - let identifier = Value::Quoted(Rc::new(vec![token])); - let from = Value::expression(from.kind); - let to = Value::expression(to.kind); - let body = Value::expression(for_statement.block.kind); + let identifier = Shared::new(Value::Quoted(Rc::new(vec![token]))); + let from = Shared::new(Value::expression(from.kind)); + let to = Shared::new(Value::expression(to.kind)); + let body = Shared::new(Value::expression(for_statement.block.kind)); Some(Value::Tuple(vec![identifier, from, to, body])) } else { None @@ -1875,11 +1885,11 @@ fn expr_as_function_call( ) -> IResult { expr_as(interner, arguments, return_type, location, |expr| { if let ExprValue::Expression(ExpressionKind::Call(call_expression)) = expr { - let function = Value::expression(call_expression.func.kind); + let function = Shared::new(Value::expression(call_expression.func.kind)); let arguments = call_expression.arguments.into_iter(); let arguments = arguments.map(|argument| Value::expression(argument.kind)).collect(); let arguments = - Value::Slice(arguments, Type::Slice(Box::new(Type::Quoted(QuotedType::Expr)))); + Shared::new(Value::Slice(arguments, Type::Slice(Box::new(Type::Quoted(QuotedType::Expr))))); Some(Value::Tuple(vec![function, arguments])) } else { None @@ -1911,9 +1921,9 @@ fn expr_as_if( ); Some(Value::Tuple(vec![ - Value::expression(if_expr.condition.kind), - Value::expression(if_expr.consequence.kind), - alternative, + Shared::new(Value::expression(if_expr.condition.kind)), + Shared::new(Value::expression(if_expr.consequence.kind)), + Shared::new(alternative), ])) } else { None @@ -1931,8 +1941,8 @@ fn expr_as_index( expr_as(interner, arguments, return_type, location, |expr| { if let ExprValue::Expression(ExpressionKind::Index(index_expr)) = expr { Some(Value::Tuple(vec![ - Value::expression(index_expr.collection.kind), - Value::expression(index_expr.index.kind), + Shared::new(Value::expression(index_expr.collection.kind)), + Shared::new(Value::expression(index_expr.index.kind)), ])) } else { None @@ -1950,15 +1960,15 @@ fn expr_as_integer( expr_as(interner, arguments, return_type.clone(), location, |expr| match expr { ExprValue::Expression(ExpressionKind::Literal(Literal::Integer(field, _suffix))) => { Some(Value::Tuple(vec![ - Value::Field(SignedField::positive(field.absolute_value())), - Value::Bool(field.is_negative()), + Shared::new(Value::Field(SignedField::positive(field.absolute_value()))), + Shared::new(Value::Bool(field.is_negative())), ])) } ExprValue::Expression(ExpressionKind::Resolved(id)) => { if let HirExpression::Literal(HirLiteral::Integer(field)) = interner.expression(&id) { Some(Value::Tuple(vec![ - Value::Field(SignedField::positive(field.absolute_value())), - Value::Bool(field.is_negative()), + Shared::new(Value::Field(SignedField::positive(field.absolute_value()))), + Shared::new(Value::Bool(field.is_negative())), ])) } else { None @@ -1994,23 +2004,23 @@ fn expr_as_lambda( .parameters .into_iter() .map(|(pattern, typ)| { - let pattern = Value::pattern(pattern); + let pattern = Shared::new(Value::pattern(pattern)); let typ = if let UnresolvedTypeData::Unspecified = typ.typ { None } else { Some(Value::UnresolvedType(typ.typ)) }; - let typ = option(option_unresolved_type.clone(), typ, location); + let typ = Shared::new(option(option_unresolved_type.clone(), typ, location)); Value::Tuple(vec![pattern, typ]) }) .collect(); - let parameters = Value::Slice( + let parameters = Shared::new(Value::Slice( parameters, Type::Slice(Box::new(Type::Tuple(vec![ Type::Quoted(QuotedType::Expr), Type::Quoted(QuotedType::UnresolvedType), ]))), - ); + )); let return_type = lambda.return_type.typ; let return_type = if let UnresolvedTypeData::Unspecified = return_type { @@ -2019,9 +2029,9 @@ fn expr_as_lambda( Some(return_type) }; let return_type = return_type.map(Value::UnresolvedType); - let return_type = option(option_unresolved_type, return_type, location); + let return_type = Shared::new(option(option_unresolved_type, return_type, location)); - let body = Value::expression(lambda.body.kind); + let body = Shared::new(Value::expression(lambda.body.kind)); Some(Value::Tuple(vec![parameters, return_type, body])) } else { @@ -2056,9 +2066,9 @@ fn expr_as_let( let typ = option(option_type, typ, location); Some(Value::Tuple(vec![ - Value::pattern(let_statement.pattern), - typ, - Value::expression(let_statement.expression.kind), + Shared::new(Value::pattern(let_statement.pattern)), + Shared::new(typ), + Shared::new(Value::expression(let_statement.expression.kind)), ])) } _ => None, @@ -2075,12 +2085,12 @@ fn expr_as_member_access( expr_as(interner, arguments, return_type, location, |expr| match expr { ExprValue::Expression(ExpressionKind::MemberAccess(member_access)) => { Some(Value::Tuple(vec![ - Value::expression(member_access.lhs.kind), - quote_ident(&member_access.rhs, location), + Shared::new(Value::expression(member_access.lhs.kind)), + Shared::new(quote_ident(&member_access.rhs, location)), ])) } ExprValue::LValue(crate::ast::LValue::MemberAccess { object, field_name, location: _ }) => { - Some(Value::Tuple(vec![Value::lvalue(*object), quote_ident(&field_name, location)])) + Some(Value::Tuple(vec![Shared::new(Value::lvalue(*object)), Shared::new(quote_ident(&field_name, location))])) } _ => None, }) @@ -2095,21 +2105,21 @@ fn expr_as_method_call( ) -> IResult { expr_as(interner, arguments, return_type, location, |expr| { if let ExprValue::Expression(ExpressionKind::MethodCall(method_call)) = expr { - let object = Value::expression(method_call.object.kind); + let object = Shared::new(Value::expression(method_call.object.kind)); - let name = quote_ident(&method_call.method_name, location); + let name = Shared::new(quote_ident(&method_call.method_name, location)); let generics = method_call.generics.unwrap_or_default().into_iter(); let generics = generics.map(|generic| Value::UnresolvedType(generic.typ)).collect(); - let generics = Value::Slice( + let generics = Shared::new(Value::Slice( generics, Type::Slice(Box::new(Type::Quoted(QuotedType::UnresolvedType))), - ); + )); let arguments = method_call.arguments.into_iter(); let arguments = arguments.map(|argument| Value::expression(argument.kind)).collect(); let arguments = - Value::Slice(arguments, Type::Slice(Box::new(Type::Quoted(QuotedType::Expr)))); + Shared::new(Value::Slice(arguments, Type::Slice(Box::new(Type::Quoted(QuotedType::Expr))))); Some(Value::Tuple(vec![object, name, generics, arguments])) } else { @@ -2131,8 +2141,8 @@ fn expr_as_repeated_element_array( ))) = expr { Some(Value::Tuple(vec![ - Value::expression(repeated_element.kind), - Value::expression(length.kind), + Shared::new(Value::expression(repeated_element.kind)), + Shared::new(Value::expression(length.kind)), ])) } else { None @@ -2153,8 +2163,8 @@ fn expr_as_repeated_element_slice( ))) = expr { Some(Value::Tuple(vec![ - Value::expression(repeated_element.kind), - Value::expression(length.kind), + Shared::new(Value::expression(repeated_element.kind)), + Shared::new(Value::expression(length.kind)), ])) } else { None @@ -2219,8 +2229,8 @@ fn expr_as_unary_op( tuple_types.pop().unwrap(); let unary_op_type = tuple_types.pop().unwrap(); - let unary_op = new_unary_op(prefix_expr.operator, unary_op_type)?; - let rhs = Value::expression(prefix_expr.rhs.kind); + let unary_op = Shared::new(new_unary_op(prefix_expr.operator, unary_op_type)?); + let rhs = Shared::new(Value::expression(prefix_expr.rhs.kind)); Some(Value::Tuple(vec![unary_op, rhs])) } else { None @@ -2313,16 +2323,16 @@ fn expr_resolve( }; let is_some = fields.get(&Rc::new("_is_some".to_string())).unwrap(); - let Value::Bool(is_some) = is_some else { + let Value::Bool(is_some) = is_some.borrow().clone() else { panic!("Expected is_some to be a boolean"); }; - let function_to_resolve_in = if *is_some { + let function_to_resolve_in = if is_some { let value = fields.get(&Rc::new("_value".to_string())).unwrap(); - let Value::FunctionDefinition(func_id) = value else { + let Value::FunctionDefinition(func_id) = value.borrow().clone() else { panic!("Expected option value to be a FunctionDefinition"); }; - Some(*func_id) + Some(func_id) } else { interpreter.current_function }; @@ -2599,8 +2609,8 @@ fn function_def_parameters( .map(|(hir_pattern, typ, _visibility)| { let tokens = hir_pattern_to_tokens(interner, hir_pattern); let tokens = vecmap(tokens, |token| LocatedToken::new(token, location)); - let name = Value::Quoted(Rc::new(tokens)); - let typ = Value::Type(typ.clone()); + let name = Shared::new(Value::Quoted(Rc::new(tokens))); + let typ = Shared::new(Value::Type(typ.clone())); Value::Tuple(vec![name, typ]) }) .collect(); @@ -2692,10 +2702,11 @@ fn function_def_set_parameters( interpreter.elaborator.interner, (input_parameter, parameters_argument_location), )?; - let parameter_type = get_type((tuple.pop().unwrap(), parameters_argument_location))?; + let parameter = tuple.pop().unwrap().unwrap_or_clone(); + let parameter_type = get_type((parameter, parameters_argument_location))?; let parameter_pattern = parse( interpreter.elaborator, - (tuple.pop().unwrap(), parameters_argument_location), + (tuple.pop().unwrap().unwrap_or_clone(), parameters_argument_location), Parser::parse_pattern_or_error, "a pattern", )?; @@ -3030,8 +3041,8 @@ pub(crate) fn option(option_type: Type, value: Option, location: Location }; let mut fields = HashMap::default(); - fields.insert(Rc::new("_is_some".to_string()), is_some); - fields.insert(Rc::new("_value".to_string()), value); + fields.insert(Rc::new("_is_some".to_string()), Shared::new(is_some)); + fields.insert(Rc::new("_value".to_string()), Shared::new(value)); Value::Struct(fields, option_type) } @@ -3098,12 +3109,12 @@ fn derive_generators( let y = FieldElement::from_be_bytes_reduce(&y_big.to_bytes_be()); let mut embedded_curve_point_fields = HashMap::default(); embedded_curve_point_fields - .insert(x_field_name.clone(), Value::Field(SignedField::positive(x))); + .insert(x_field_name.clone(), Shared::new(Value::Field(SignedField::positive(x)))); embedded_curve_point_fields - .insert(y_field_name.clone(), Value::Field(SignedField::positive(y))); + .insert(y_field_name.clone(), Shared::new(Value::Field(SignedField::positive(y)))); embedded_curve_point_fields.insert( is_infinite_field_name.clone(), - Value::Field(SignedField::positive(is_infinite)), + Shared::new(Value::Field(SignedField::positive(is_infinite))), ); let embedded_curve_point_struct = Value::Struct(embedded_curve_point_fields, *elements.clone()); diff --git a/compiler/noirc_frontend/src/hir/comptime/interpreter/builtin/builtin_helpers.rs b/compiler/noirc_frontend/src/hir/comptime/interpreter/builtin/builtin_helpers.rs index b151c099c46..943556a24b9 100644 --- a/compiler/noirc_frontend/src/hir/comptime/interpreter/builtin/builtin_helpers.rs +++ b/compiler/noirc_frontend/src/hir/comptime/interpreter/builtin/builtin_helpers.rs @@ -108,7 +108,7 @@ pub(crate) fn get_array( pub(crate) fn get_struct_fields( name: &str, (value, location): (Value, Location), -) -> IResult<(HashMap, Value>, Type)> { +) -> IResult<(HashMap, Shared>, Type)> { match value { Value::Struct(fields, typ) => Ok((fields, typ)), _ => { @@ -128,7 +128,7 @@ pub(crate) fn get_struct_fields( /// Get a specific field of a struct and apply a decoder function on it. pub(crate) fn get_struct_field( field_name: &str, - struct_fields: &HashMap, Value>, + struct_fields: &HashMap, Shared>, struct_type: &Type, location: Location, f: impl Fn((Value, Location)) -> IResult, @@ -141,7 +141,7 @@ pub(crate) fn get_struct_field( location, }); }; - f((value.clone(), location)) + f((value.borrow().clone(), location)) } pub(crate) fn get_bool((value, location): (Value, Location)) -> IResult { @@ -231,7 +231,7 @@ pub(crate) fn get_ctstring((value, location): (Value, Location)) -> IResult IResult> { +) -> IResult>> { match value { Value::Tuple(values) => Ok(values), value => { @@ -681,7 +681,7 @@ pub(crate) fn to_struct( fields: impl IntoIterator, typ: Type, ) -> Value { - let fields = fields.into_iter().map(|(k, v)| (Rc::new(k.to_string()), v)).collect(); + let fields = fields.into_iter().map(|(k, v)| (Rc::new(k.to_string()), Shared::new(v))).collect(); Value::Struct(fields, typ) } @@ -699,7 +699,7 @@ pub(crate) fn new_unary_op(operator: UnaryOp, typ: Type) -> Option { }; let mut fields = HashMap::default(); - fields.insert(Rc::new("op".to_string()), Value::Field(SignedField::positive(unary_op_value))); + fields.insert(Rc::new("op".to_string()), Shared::new(Value::Field(SignedField::positive(unary_op_value)))); Some(Value::Struct(fields, typ)) } @@ -709,7 +709,7 @@ pub(crate) fn new_binary_op(operator: BinaryOp, typ: Type) -> Value { let binary_op_value = operator.contents as u128; let mut fields = HashMap::default(); - fields.insert(Rc::new("op".to_string()), Value::Field(SignedField::positive(binary_op_value))); + fields.insert(Rc::new("op".to_string()), Shared::new(Value::Field(SignedField::positive(binary_op_value)))); Value::Struct(fields, typ) } diff --git a/compiler/noirc_frontend/src/hir/comptime/value.rs b/compiler/noirc_frontend/src/hir/comptime/value.rs index fd777a88d87..f15f7fdcb06 100644 --- a/compiler/noirc_frontend/src/hir/comptime/value.rs +++ b/compiler/noirc_frontend/src/hir/comptime/value.rs @@ -59,8 +59,14 @@ pub enum Value { // in case they use functions such as `Quoted::as_type` which require them. Closure(Box), - Tuple(Vec), - Struct(HashMap, Value>, Type), + /// Tuple elements are automatically shared to support projection into a tuple: + /// `let elem = &mut tuple.0` should mutate the original element. + Tuple(Vec>), + + /// Struct elements are automatically shared to support projection: + /// `let elem = &mut my_struct.field` should mutate the original element. + Struct(HashMap, Shared>, Type), + Enum(/*tag*/ usize, /*args*/ Vec, Type), Pointer(Shared, /* auto_deref */ bool, /* mutable */ bool), Array(Vector, Type), @@ -145,7 +151,7 @@ impl Value { Value::Function(_, typ, _) => return Cow::Borrowed(typ), Value::Closure(closure) => return Cow::Borrowed(&closure.typ), Value::Tuple(fields) => { - Type::Tuple(vecmap(fields, |field| field.get_type().into_owned())) + Type::Tuple(vecmap(fields, |field| field.borrow().get_type().into_owned())) } Value::Struct(_, typ) => return Cow::Borrowed(typ), Value::Enum(_, _, typ) => return Cow::Borrowed(typ), @@ -245,12 +251,12 @@ impl Value { } Value::Tuple(fields) => { let fields = - try_vecmap(fields, |field| field.into_expression(elaborator, location))?; + try_vecmap(fields, |field| field.unwrap_or_clone().into_expression(elaborator, location))?; ExpressionKind::Tuple(fields) } Value::Struct(fields, typ) => { let fields = try_vecmap(fields, |(name, field)| { - let field = field.into_expression(elaborator, location)?; + let field = field.unwrap_or_clone().into_expression(elaborator, location)?; Ok((Ident::new(unwrap_rc(name), location), field)) })?; @@ -413,12 +419,12 @@ impl Value { } Value::Tuple(fields) => { let fields = - try_vecmap(fields, |field| field.into_hir_expression(interner, location))?; + try_vecmap(fields, |field| field.unwrap_or_clone().into_hir_expression(interner, location))?; HirExpression::Tuple(fields) } Value::Struct(fields, typ) => { let fields = try_vecmap(fields, |(name, field)| { - let field = field.into_hir_expression(interner, location)?; + let field = field.unwrap_or_clone().into_hir_expression(interner, location)?; Ok((Ident::new(unwrap_rc(name), location), field)) })?; @@ -615,9 +621,9 @@ impl Value { Value::Slice(values, _) => { values.iter().any(|value| value.contains_function_or_closure()) } - Value::Tuple(values) => values.iter().any(|value| value.contains_function_or_closure()), + Value::Tuple(values) => values.iter().any(|value| value.borrow().contains_function_or_closure()), Value::Struct(fields, _) => { - fields.values().any(|value| value.contains_function_or_closure()) + fields.values().any(|value| value.borrow().contains_function_or_closure()) } Value::Enum(_, values, _) => { values.iter().any(|value| value.contains_function_or_closure()) diff --git a/tooling/lsp/src/requests/hover/from_reference.rs b/tooling/lsp/src/requests/hover/from_reference.rs index 23a6098b940..cfccd72a9be 100644 --- a/tooling/lsp/src/requests/hover/from_reference.rs +++ b/tooling/lsp/src/requests/hover/from_reference.rs @@ -905,7 +905,7 @@ fn append_value_to_string(value: &Value, string: &mut String) -> Option<()> { if index > 0 { string.push_str(", "); } - append_value_to_string(value, string)?; + append_value_to_string(&value.borrow(), string)?; } if len == 1 { string.push(','); diff --git a/tooling/nargo_cli/tests/snapshots/compile_failure/brillig_slice_to_acir/execute__tests__stderr.snap.new b/tooling/nargo_cli/tests/snapshots/compile_failure/brillig_slice_to_acir/execute__tests__stderr.snap.new new file mode 100644 index 00000000000..d00744bc447 --- /dev/null +++ b/tooling/nargo_cli/tests/snapshots/compile_failure/brillig_slice_to_acir/execute__tests__stderr.snap.new @@ -0,0 +1,56 @@ +--- +source: tooling/nargo_cli/tests/execute.rs +assertion_line: 339 +expression: stderr +--- +error3 +warning: unused variable i + ┌─ src/main.nr:10:9 + │ +10 │ for i in 0..y { + │ - unused variable + │ + +warning: unused variable x + ┌─ src/main.nr:8:24 + │ +8 │ unconstrained fn clear(x: [u32; DEPTH], y: u32) -> [u32] { + │ - unused variable + │ + +error: Call to unconstrained function is unsafe and must be in an unconstrained function or unsafe block + ┌─ src/main.nr:5:13 + │ +5 │ new_x = clear(x, y); + │ ----------- + │ + +error: Slices cannot be returned from an unconstrained runtime to a constrained runtime + ┌─ src/main.nr:5:13 + │ +5 │ new_x = clear(x, y); + │ ----------- + │ + +error: Cannot assign an expression of type [u32] to a value of type [_; 0] + ┌─ src/main.nr:5:13 + │ +5 │ new_x = clear(x, y); + │ ----------- + │ + +error: Type annotation needed + ┌─ src/main.nr:4:21 + │ +4 │ let mut new_x = []; + │ -- Could not determine the type of the array + │ + +error: No method named 'push_back' found for type '[u32; 0]' + ┌─ src/main.nr:11:13 + │ +11 │ a = a.push_back(x[i]); + │ ----------------- + │ + +Aborting due to 5 previous errors diff --git a/tooling/nargo_cli/tests/snapshots/compile_failure/mutability_regression_2911/execute__tests__stderr.snap.new b/tooling/nargo_cli/tests/snapshots/compile_failure/mutability_regression_2911/execute__tests__stderr.snap.new new file mode 100644 index 00000000000..447eed873f0 --- /dev/null +++ b/tooling/nargo_cli/tests/snapshots/compile_failure/mutability_regression_2911/execute__tests__stderr.snap.new @@ -0,0 +1,28 @@ +--- +source: tooling/nargo_cli/tests/execute.rs +assertion_line: 339 +expression: stderr +--- +error3 +error: Expected type &mut [Field], found type &mut [_; 0] + ┌─ src/main.nr:3:32 + │ +3 │ let slice : &mut [Field] = &mut []; + │ ------- + │ + +error: Variable `slice` must be mutable to be assigned to + ┌─ src/main.nr:4:5 + │ +4 │ slice = &mut (*slice).push_back(1); + │ ----- + │ + +error: Type annotation needed + ┌─ src/main.nr:3:37 + │ +3 │ let slice : &mut [Field] = &mut []; + │ -- Could not determine the type of the array + │ + +Aborting due to 3 previous errors diff --git a/tooling/nargo_cli/tests/snapshots/compile_failure/noirc_frontend_tests_cannot_determine_array_type/execute__tests__stderr.snap.new b/tooling/nargo_cli/tests/snapshots/compile_failure/noirc_frontend_tests_cannot_determine_array_type/execute__tests__stderr.snap.new new file mode 100644 index 00000000000..2e7541f2eb1 --- /dev/null +++ b/tooling/nargo_cli/tests/snapshots/compile_failure/noirc_frontend_tests_cannot_determine_array_type/execute__tests__stderr.snap.new @@ -0,0 +1,14 @@ +--- +source: tooling/nargo_cli/tests/execute.rs +assertion_line: 339 +expression: stderr +--- +error3 +error: Type annotation needed + ┌─ src/main.nr:3:17 + │ +3 │ let _ = []; + │ -- Could not determine the type of the array + │ + +Aborting due to 1 previous error diff --git a/tooling/nargo_cli/tests/snapshots/compile_failure/noirc_frontend_tests_cannot_determine_slice_type/execute__tests__stderr.snap.new b/tooling/nargo_cli/tests/snapshots/compile_failure/noirc_frontend_tests_cannot_determine_slice_type/execute__tests__stderr.snap.new new file mode 100644 index 00000000000..fc4eabb018d --- /dev/null +++ b/tooling/nargo_cli/tests/snapshots/compile_failure/noirc_frontend_tests_cannot_determine_slice_type/execute__tests__stderr.snap.new @@ -0,0 +1,14 @@ +--- +source: tooling/nargo_cli/tests/execute.rs +assertion_line: 339 +expression: stderr +--- +error3 +error: Type annotation needed + ┌─ src/main.nr:3:17 + │ +3 │ let _ = &[]; + │ --- Could not determine the type of the slice + │ + +Aborting due to 1 previous error diff --git a/tooling/nargo_cli/tests/snapshots/compile_failure/noirc_frontend_tests_traits_errors_if_impl_trait_constraint_is_not_satisfied/execute__tests__stderr.snap.new b/tooling/nargo_cli/tests/snapshots/compile_failure/noirc_frontend_tests_traits_errors_if_impl_trait_constraint_is_not_satisfied/execute__tests__stderr.snap.new new file mode 100644 index 00000000000..55644d972da --- /dev/null +++ b/tooling/nargo_cli/tests/snapshots/compile_failure/noirc_frontend_tests_traits_errors_if_impl_trait_constraint_is_not_satisfied/execute__tests__stderr.snap.new @@ -0,0 +1,31 @@ +--- +source: tooling/nargo_cli/tests/execute.rs +assertion_line: 339 +expression: stderr +--- +warning: function `greet` cannot return without recursing + ┌─ src/main.nr:10:16 + │ +10 │ fn greet(object: U) + │ ----- function cannot return without recursing + │ + +error: Object type is unknown in method call + ┌─ src/main.nr:14:17 + │ +14 │ object.greet(); + │ ------------ Type must be known by this point to know which method to call + │ + = Try adding a type annotation for the object type before this method call + +error: The trait bound `SomeGreeter: Greeter` is not satisfied + ┌─ src/main.nr:8:16 + │ + 8 │ T: Greeter, + │ ------- required by this bound in `Foo` + · +22 │ impl Foo for Bar {} + │ --- The trait `Greeter` is not implemented for `SomeGreeter` + │ + +Aborting due to 2 previous errors diff --git a/tooling/nargo_expand/src/printer.rs b/tooling/nargo_expand/src/printer.rs index 4cb4ce2957e..ad0a2d33085 100644 --- a/tooling/nargo_expand/src/printer.rs +++ b/tooling/nargo_expand/src/printer.rs @@ -849,7 +849,7 @@ impl<'context, 'string> ItemPrinter<'context, 'string> { if index != 0 { self.push_str(", "); } - self.show_value(value); + self.show_value(&value.borrow()); } if values.len() == 1 { self.push(','); @@ -868,7 +868,7 @@ impl<'context, 'string> ItemPrinter<'context, 'string> { self.write_indent(); self.push_str(name); self.push_str(": "); - self.show_value(value); + self.show_value(&value.borrow()); self.push_str(",\n"); } self.decrease_indent(); From b14e1f0c018a3e64f08c0cd0d4ebbdefa37a895b Mon Sep 17 00:00:00 2001 From: Jake Fecher Date: Fri, 18 Jul 2025 10:18:06 -0500 Subject: [PATCH 2/9] Fix issue and find new issue --- .../src/hir/comptime/display.rs | 3 +- .../src/hir/comptime/interpreter.rs | 41 +++++++++++++++++-- .../src/hir/comptime/interpreter/builtin.rs | 28 +++++++++---- .../interpreter/builtin/builtin_helpers.rs | 13 ++++-- .../noirc_frontend/src/hir/comptime/value.rs | 14 ++++--- .../Nargo.toml | 4 ++ .../src/main.nr | 28 +++++++++++++ 7 files changed, 111 insertions(+), 20 deletions(-) create mode 100644 test_programs/execution_success/struct_assignment_with_shared_ref_to_field/Nargo.toml create mode 100644 test_programs/execution_success/struct_assignment_with_shared_ref_to_field/src/main.nr diff --git a/compiler/noirc_frontend/src/hir/comptime/display.rs b/compiler/noirc_frontend/src/hir/comptime/display.rs index 36ac164d433..63537b1f27c 100644 --- a/compiler/noirc_frontend/src/hir/comptime/display.rs +++ b/compiler/noirc_frontend/src/hir/comptime/display.rs @@ -386,7 +386,8 @@ impl Display for ValuePrinter<'_, '_> { Value::Function(..) => write!(f, "(function)"), Value::Closure(..) => write!(f, "(closure)"), Value::Tuple(fields) => { - let fields = vecmap(fields, |field| field.borrow().display(self.interner).to_string()); + let fields = + vecmap(fields, |field| field.borrow().display(self.interner).to_string()); if fields.len() == 1 { write!(f, "({},)", fields[0]) } else { diff --git a/compiler/noirc_frontend/src/hir/comptime/interpreter.rs b/compiler/noirc_frontend/src/hir/comptime/interpreter.rs index 564315be68a..a71befece45 100644 --- a/compiler/noirc_frontend/src/hir/comptime/interpreter.rs +++ b/compiler/noirc_frontend/src/hir/comptime/interpreter.rs @@ -494,7 +494,11 @@ impl<'local, 'interner> Interpreter<'local, 'interner> { if let Entry::Occupied(mut entry) = scope.entry(id) { match entry.get() { Value::Pointer(reference, true, _) => { - *reference.borrow_mut() = argument; + // We can't store to the reference directly, we need to check if the value + // is a struct or tuple to store to each field instead. This is so any + // references to these fields are also updated. + let reference = reference.clone(); + self.store_flattened(reference, argument); } _ => { entry.insert(argument); @@ -1172,7 +1176,7 @@ impl<'local, 'interner> Interpreter<'local, 'interner> { HirLValue::Dereference { lvalue, element_type: _, location, implicitly_added: _ } => { match self.evaluate_lvalue(&lvalue)? { Value::Pointer(value, _, _) => { - *value.borrow_mut() = rhs; + self.store_flattened(value, rhs); Ok(()) } value => { @@ -1224,6 +1228,35 @@ impl<'local, 'interner> Interpreter<'local, 'interner> { } } + /// When we store to a struct such as in + /// ```noir + /// let mut a = (false,); + /// let b = &mut a.0; + /// a = (true,); + /// ``` + /// we must flatten the store to store to each individual field so that any existing + /// references, such as `b` above, will also reflect the mutation. + fn store_flattened(&mut self, lvalue: Shared, rvalue: Value) { + let lvalue_ref = lvalue.borrow(); + match (&*lvalue_ref, rvalue) { + (Value::Struct(lvalue_fields, _), Value::Struct(mut rvalue_fields, _)) => { + for (name, lvalue) in lvalue_fields.iter() { + let Some(rvalue) = rvalue_fields.remove(name) else { continue }; + self.store_flattened(lvalue.clone(), rvalue.unwrap_or_clone()); + } + } + (Value::Tuple(lvalue_fields), Value::Tuple(rvalue_fields)) => { + for (lvalue, rvalue) in lvalue_fields.iter().zip(rvalue_fields) { + self.store_flattened(lvalue.clone(), rvalue.unwrap_or_clone()); + } + } + (_, rvalue) => { + drop(lvalue_ref); + *lvalue.borrow_mut() = rvalue; + } + } + } + fn evaluate_lvalue(&mut self, lvalue: &HirLValue) -> IResult { match lvalue { HirLValue::Ident(ident, _) => match self.lookup(ident)? { @@ -1252,7 +1285,9 @@ impl<'local, 'interner> Interpreter<'local, 'interner> { match object_value { Value::Tuple(mut values) => Ok(values.swap_remove(index).unwrap_or_clone()), - Value::Struct(fields, _) => Ok(fields[field_name.as_string()].clone().unwrap_or_clone()), + Value::Struct(fields, _) => { + Ok(fields[field_name.as_string()].clone().unwrap_or_clone()) + } value => Err(InterpreterError::NonTupleOrStructInMemberAccess { typ: value.get_type().into_owned(), location: *location, diff --git a/compiler/noirc_frontend/src/hir/comptime/interpreter/builtin.rs b/compiler/noirc_frontend/src/hir/comptime/interpreter/builtin.rs index f94c9431dab..31a2903c99f 100644 --- a/compiler/noirc_frontend/src/hir/comptime/interpreter/builtin.rs +++ b/compiler/noirc_frontend/src/hir/comptime/interpreter/builtin.rs @@ -711,7 +711,8 @@ fn type_def_set_fields( let type_var = elaborator.interner.next_type_variable(); let expected = Type::Tuple(vec![type_var.clone(), type_var]); - let actual = Type::Tuple(vecmap(&field_pair, |value| value.borrow().get_type().into_owned())); + let actual = + Type::Tuple(vecmap(&field_pair, |value| value.borrow().get_type().into_owned())); return Err(InterpreterError::TypeMismatch { expected, actual, location }); } @@ -787,7 +788,9 @@ fn slice_pop_back( let (mut values, typ) = get_slice(interner, argument)?; match values.pop_back() { - Some(element) => Ok(Value::Tuple(vec![Shared::new(Value::Slice(values, typ)), Shared::new(element)])), + Some(element) => { + Ok(Value::Tuple(vec![Shared::new(Value::Slice(values, typ)), Shared::new(element)])) + } None => failing_constraint("slice_pop_back called on empty slice", location, call_stack), } } @@ -1501,7 +1504,9 @@ fn zeroed(return_type: Type, location: Location) -> Value { } } Type::Unit => Value::Unit, - Type::Tuple(fields) => Value::Tuple(vecmap(fields, |field| Shared::new(zeroed(field, location)))), + Type::Tuple(fields) => { + Value::Tuple(vecmap(fields, |field| Shared::new(zeroed(field, location)))) + } Type::DataType(data_type, generics) => { let typ = data_type.borrow(); @@ -1888,8 +1893,10 @@ fn expr_as_function_call( let function = Shared::new(Value::expression(call_expression.func.kind)); let arguments = call_expression.arguments.into_iter(); let arguments = arguments.map(|argument| Value::expression(argument.kind)).collect(); - let arguments = - Shared::new(Value::Slice(arguments, Type::Slice(Box::new(Type::Quoted(QuotedType::Expr))))); + let arguments = Shared::new(Value::Slice( + arguments, + Type::Slice(Box::new(Type::Quoted(QuotedType::Expr))), + )); Some(Value::Tuple(vec![function, arguments])) } else { None @@ -2090,7 +2097,10 @@ fn expr_as_member_access( ])) } ExprValue::LValue(crate::ast::LValue::MemberAccess { object, field_name, location: _ }) => { - Some(Value::Tuple(vec![Shared::new(Value::lvalue(*object)), Shared::new(quote_ident(&field_name, location))])) + Some(Value::Tuple(vec![ + Shared::new(Value::lvalue(*object)), + Shared::new(quote_ident(&field_name, location)), + ])) } _ => None, }) @@ -2118,8 +2128,10 @@ fn expr_as_method_call( let arguments = method_call.arguments.into_iter(); let arguments = arguments.map(|argument| Value::expression(argument.kind)).collect(); - let arguments = - Shared::new(Value::Slice(arguments, Type::Slice(Box::new(Type::Quoted(QuotedType::Expr))))); + let arguments = Shared::new(Value::Slice( + arguments, + Type::Slice(Box::new(Type::Quoted(QuotedType::Expr))), + )); Some(Value::Tuple(vec![object, name, generics, arguments])) } else { diff --git a/compiler/noirc_frontend/src/hir/comptime/interpreter/builtin/builtin_helpers.rs b/compiler/noirc_frontend/src/hir/comptime/interpreter/builtin/builtin_helpers.rs index 943556a24b9..93db8c4eb73 100644 --- a/compiler/noirc_frontend/src/hir/comptime/interpreter/builtin/builtin_helpers.rs +++ b/compiler/noirc_frontend/src/hir/comptime/interpreter/builtin/builtin_helpers.rs @@ -681,7 +681,8 @@ pub(crate) fn to_struct( fields: impl IntoIterator, typ: Type, ) -> Value { - let fields = fields.into_iter().map(|(k, v)| (Rc::new(k.to_string()), Shared::new(v))).collect(); + let fields = + fields.into_iter().map(|(k, v)| (Rc::new(k.to_string()), Shared::new(v))).collect(); Value::Struct(fields, typ) } @@ -699,7 +700,10 @@ pub(crate) fn new_unary_op(operator: UnaryOp, typ: Type) -> Option { }; let mut fields = HashMap::default(); - fields.insert(Rc::new("op".to_string()), Shared::new(Value::Field(SignedField::positive(unary_op_value)))); + fields.insert( + Rc::new("op".to_string()), + Shared::new(Value::Field(SignedField::positive(unary_op_value))), + ); Some(Value::Struct(fields, typ)) } @@ -709,7 +713,10 @@ pub(crate) fn new_binary_op(operator: BinaryOp, typ: Type) -> Value { let binary_op_value = operator.contents as u128; let mut fields = HashMap::default(); - fields.insert(Rc::new("op".to_string()), Shared::new(Value::Field(SignedField::positive(binary_op_value)))); + fields.insert( + Rc::new("op".to_string()), + Shared::new(Value::Field(SignedField::positive(binary_op_value))), + ); Value::Struct(fields, typ) } diff --git a/compiler/noirc_frontend/src/hir/comptime/value.rs b/compiler/noirc_frontend/src/hir/comptime/value.rs index f15f7fdcb06..e54c080ab76 100644 --- a/compiler/noirc_frontend/src/hir/comptime/value.rs +++ b/compiler/noirc_frontend/src/hir/comptime/value.rs @@ -250,8 +250,9 @@ impl Value { ExpressionKind::Resolved(expr_id) } Value::Tuple(fields) => { - let fields = - try_vecmap(fields, |field| field.unwrap_or_clone().into_expression(elaborator, location))?; + let fields = try_vecmap(fields, |field| { + field.unwrap_or_clone().into_expression(elaborator, location) + })?; ExpressionKind::Tuple(fields) } Value::Struct(fields, typ) => { @@ -418,8 +419,9 @@ impl Value { return Ok(expr_id); } Value::Tuple(fields) => { - let fields = - try_vecmap(fields, |field| field.unwrap_or_clone().into_hir_expression(interner, location))?; + let fields = try_vecmap(fields, |field| { + field.unwrap_or_clone().into_hir_expression(interner, location) + })?; HirExpression::Tuple(fields) } Value::Struct(fields, typ) => { @@ -621,7 +623,9 @@ impl Value { Value::Slice(values, _) => { values.iter().any(|value| value.contains_function_or_closure()) } - Value::Tuple(values) => values.iter().any(|value| value.borrow().contains_function_or_closure()), + Value::Tuple(values) => { + values.iter().any(|value| value.borrow().contains_function_or_closure()) + } Value::Struct(fields, _) => { fields.values().any(|value| value.borrow().contains_function_or_closure()) } diff --git a/test_programs/execution_success/struct_assignment_with_shared_ref_to_field/Nargo.toml b/test_programs/execution_success/struct_assignment_with_shared_ref_to_field/Nargo.toml new file mode 100644 index 00000000000..201866c02ed --- /dev/null +++ b/test_programs/execution_success/struct_assignment_with_shared_ref_to_field/Nargo.toml @@ -0,0 +1,4 @@ +[package] +name = "struct_assignment_with_shared_ref_to_field" +type = "bin" +authors = [""] diff --git a/test_programs/execution_success/struct_assignment_with_shared_ref_to_field/src/main.nr b/test_programs/execution_success/struct_assignment_with_shared_ref_to_field/src/main.nr new file mode 100644 index 00000000000..0bb65270d86 --- /dev/null +++ b/test_programs/execution_success/struct_assignment_with_shared_ref_to_field/src/main.nr @@ -0,0 +1,28 @@ +fn main() { + let (a, b) = comptime { + (plain_store(), store_through_pointer()) + }; + let (c, d) = (plain_store(), store_through_pointer()); + assert(a); + assert(b); + assert(c); + + // Bug! + assert(!d); +} + +fn plain_store() -> bool { + let mut a = (false,); + let b = &mut a.0; + a = (true,); + println(*b); + *b +} + +fn store_through_pointer() -> bool { + let a = &mut (false,); + let b = &mut a.0; + *a = (true,); + println(*b); + *b +} From 8bf8cd84dae1d636b7342c631ca5a17c67a42a91 Mon Sep 17 00:00:00 2001 From: Jake Fecher Date: Fri, 18 Jul 2025 10:43:51 -0500 Subject: [PATCH 3/9] Format --- .../src/hir/comptime/interpreter.rs | 11 ++-- .../interpreter/builtin/builtin_helpers.rs | 3 +- .../noirc_frontend/src/hir/comptime/value.rs | 4 +- .../src/main.nr | 4 +- .../execute__tests__stderr.snap.new | 56 ------------------- .../execute__tests__stderr.snap.new | 28 ---------- .../execute__tests__stderr.snap.new | 14 ----- .../execute__tests__stderr.snap.new | 14 ----- .../execute__tests__stderr.snap.new | 31 ---------- .../execute__tests__expanded.snap | 26 +++++++++ .../execute__tests__stdout.snap | 5 ++ 11 files changed, 42 insertions(+), 154 deletions(-) delete mode 100644 tooling/nargo_cli/tests/snapshots/compile_failure/brillig_slice_to_acir/execute__tests__stderr.snap.new delete mode 100644 tooling/nargo_cli/tests/snapshots/compile_failure/mutability_regression_2911/execute__tests__stderr.snap.new delete mode 100644 tooling/nargo_cli/tests/snapshots/compile_failure/noirc_frontend_tests_cannot_determine_array_type/execute__tests__stderr.snap.new delete mode 100644 tooling/nargo_cli/tests/snapshots/compile_failure/noirc_frontend_tests_cannot_determine_slice_type/execute__tests__stderr.snap.new delete mode 100644 tooling/nargo_cli/tests/snapshots/compile_failure/noirc_frontend_tests_traits_errors_if_impl_trait_constraint_is_not_satisfied/execute__tests__stderr.snap.new create mode 100644 tooling/nargo_cli/tests/snapshots/execution_success/struct_assignment_with_shared_ref_to_field/execute__tests__expanded.snap create mode 100644 tooling/nargo_cli/tests/snapshots/execution_success/struct_assignment_with_shared_ref_to_field/execute__tests__stdout.snap diff --git a/compiler/noirc_frontend/src/hir/comptime/interpreter.rs b/compiler/noirc_frontend/src/hir/comptime/interpreter.rs index a71befece45..500c59e5672 100644 --- a/compiler/noirc_frontend/src/hir/comptime/interpreter.rs +++ b/compiler/noirc_frontend/src/hir/comptime/interpreter.rs @@ -497,8 +497,7 @@ impl<'local, 'interner> Interpreter<'local, 'interner> { // We can't store to the reference directly, we need to check if the value // is a struct or tuple to store to each field instead. This is so any // references to these fields are also updated. - let reference = reference.clone(); - self.store_flattened(reference, argument); + Self::store_flattened(reference.clone(), argument); } _ => { entry.insert(argument); @@ -1176,7 +1175,7 @@ impl<'local, 'interner> Interpreter<'local, 'interner> { HirLValue::Dereference { lvalue, element_type: _, location, implicitly_added: _ } => { match self.evaluate_lvalue(&lvalue)? { Value::Pointer(value, _, _) => { - self.store_flattened(value, rhs); + Self::store_flattened(value, rhs); Ok(()) } value => { @@ -1236,18 +1235,18 @@ impl<'local, 'interner> Interpreter<'local, 'interner> { /// ``` /// we must flatten the store to store to each individual field so that any existing /// references, such as `b` above, will also reflect the mutation. - fn store_flattened(&mut self, lvalue: Shared, rvalue: Value) { + fn store_flattened(lvalue: Shared, rvalue: Value) { let lvalue_ref = lvalue.borrow(); match (&*lvalue_ref, rvalue) { (Value::Struct(lvalue_fields, _), Value::Struct(mut rvalue_fields, _)) => { for (name, lvalue) in lvalue_fields.iter() { let Some(rvalue) = rvalue_fields.remove(name) else { continue }; - self.store_flattened(lvalue.clone(), rvalue.unwrap_or_clone()); + Self::store_flattened(lvalue.clone(), rvalue.unwrap_or_clone()); } } (Value::Tuple(lvalue_fields), Value::Tuple(rvalue_fields)) => { for (lvalue, rvalue) in lvalue_fields.iter().zip(rvalue_fields) { - self.store_flattened(lvalue.clone(), rvalue.unwrap_or_clone()); + Self::store_flattened(lvalue.clone(), rvalue.unwrap_or_clone()); } } (_, rvalue) => { diff --git a/compiler/noirc_frontend/src/hir/comptime/interpreter/builtin/builtin_helpers.rs b/compiler/noirc_frontend/src/hir/comptime/interpreter/builtin/builtin_helpers.rs index 93db8c4eb73..194228bccdf 100644 --- a/compiler/noirc_frontend/src/hir/comptime/interpreter/builtin/builtin_helpers.rs +++ b/compiler/noirc_frontend/src/hir/comptime/interpreter/builtin/builtin_helpers.rs @@ -4,6 +4,7 @@ use std::{hash::Hasher, rc::Rc}; use iter_extended::{try_vecmap, vecmap}; use noirc_errors::Location; +use crate::hir::comptime::value::StructFields; use crate::ast::{BinaryOp, ItemVisibility, UnaryOp}; use crate::elaborator::Elaborator; use crate::hir::comptime::display::tokens_to_string; @@ -108,7 +109,7 @@ pub(crate) fn get_array( pub(crate) fn get_struct_fields( name: &str, (value, location): (Value, Location), -) -> IResult<(HashMap, Shared>, Type)> { +) -> IResult<(StructFields, Type)> { match value { Value::Struct(fields, typ) => Ok((fields, typ)), _ => { diff --git a/compiler/noirc_frontend/src/hir/comptime/value.rs b/compiler/noirc_frontend/src/hir/comptime/value.rs index e54c080ab76..1b66d957489 100644 --- a/compiler/noirc_frontend/src/hir/comptime/value.rs +++ b/compiler/noirc_frontend/src/hir/comptime/value.rs @@ -65,7 +65,7 @@ pub enum Value { /// Struct elements are automatically shared to support projection: /// `let elem = &mut my_struct.field` should mutate the original element. - Struct(HashMap, Shared>, Type), + Struct(StructFields, Type), Enum(/*tag*/ usize, /*args*/ Vec, Type), Pointer(Shared, /* auto_deref */ bool, /* mutable */ bool), @@ -85,6 +85,8 @@ pub enum Value { UnresolvedType(UnresolvedTypeData), } +pub type StructFields = HashMap, Shared>; + #[derive(Debug, Clone, PartialEq, Eq)] pub struct Closure { pub lambda: HirLambda, diff --git a/test_programs/execution_success/struct_assignment_with_shared_ref_to_field/src/main.nr b/test_programs/execution_success/struct_assignment_with_shared_ref_to_field/src/main.nr index 0bb65270d86..b369d9b33bc 100644 --- a/test_programs/execution_success/struct_assignment_with_shared_ref_to_field/src/main.nr +++ b/test_programs/execution_success/struct_assignment_with_shared_ref_to_field/src/main.nr @@ -7,7 +7,7 @@ fn main() { assert(b); assert(c); - // Bug! + // Bug! https://github.com/noir-lang/noir/issues/9251 assert(!d); } @@ -15,7 +15,6 @@ fn plain_store() -> bool { let mut a = (false,); let b = &mut a.0; a = (true,); - println(*b); *b } @@ -23,6 +22,5 @@ fn store_through_pointer() -> bool { let a = &mut (false,); let b = &mut a.0; *a = (true,); - println(*b); *b } diff --git a/tooling/nargo_cli/tests/snapshots/compile_failure/brillig_slice_to_acir/execute__tests__stderr.snap.new b/tooling/nargo_cli/tests/snapshots/compile_failure/brillig_slice_to_acir/execute__tests__stderr.snap.new deleted file mode 100644 index d00744bc447..00000000000 --- a/tooling/nargo_cli/tests/snapshots/compile_failure/brillig_slice_to_acir/execute__tests__stderr.snap.new +++ /dev/null @@ -1,56 +0,0 @@ ---- -source: tooling/nargo_cli/tests/execute.rs -assertion_line: 339 -expression: stderr ---- -error3 -warning: unused variable i - ┌─ src/main.nr:10:9 - │ -10 │ for i in 0..y { - │ - unused variable - │ - -warning: unused variable x - ┌─ src/main.nr:8:24 - │ -8 │ unconstrained fn clear(x: [u32; DEPTH], y: u32) -> [u32] { - │ - unused variable - │ - -error: Call to unconstrained function is unsafe and must be in an unconstrained function or unsafe block - ┌─ src/main.nr:5:13 - │ -5 │ new_x = clear(x, y); - │ ----------- - │ - -error: Slices cannot be returned from an unconstrained runtime to a constrained runtime - ┌─ src/main.nr:5:13 - │ -5 │ new_x = clear(x, y); - │ ----------- - │ - -error: Cannot assign an expression of type [u32] to a value of type [_; 0] - ┌─ src/main.nr:5:13 - │ -5 │ new_x = clear(x, y); - │ ----------- - │ - -error: Type annotation needed - ┌─ src/main.nr:4:21 - │ -4 │ let mut new_x = []; - │ -- Could not determine the type of the array - │ - -error: No method named 'push_back' found for type '[u32; 0]' - ┌─ src/main.nr:11:13 - │ -11 │ a = a.push_back(x[i]); - │ ----------------- - │ - -Aborting due to 5 previous errors diff --git a/tooling/nargo_cli/tests/snapshots/compile_failure/mutability_regression_2911/execute__tests__stderr.snap.new b/tooling/nargo_cli/tests/snapshots/compile_failure/mutability_regression_2911/execute__tests__stderr.snap.new deleted file mode 100644 index 447eed873f0..00000000000 --- a/tooling/nargo_cli/tests/snapshots/compile_failure/mutability_regression_2911/execute__tests__stderr.snap.new +++ /dev/null @@ -1,28 +0,0 @@ ---- -source: tooling/nargo_cli/tests/execute.rs -assertion_line: 339 -expression: stderr ---- -error3 -error: Expected type &mut [Field], found type &mut [_; 0] - ┌─ src/main.nr:3:32 - │ -3 │ let slice : &mut [Field] = &mut []; - │ ------- - │ - -error: Variable `slice` must be mutable to be assigned to - ┌─ src/main.nr:4:5 - │ -4 │ slice = &mut (*slice).push_back(1); - │ ----- - │ - -error: Type annotation needed - ┌─ src/main.nr:3:37 - │ -3 │ let slice : &mut [Field] = &mut []; - │ -- Could not determine the type of the array - │ - -Aborting due to 3 previous errors diff --git a/tooling/nargo_cli/tests/snapshots/compile_failure/noirc_frontend_tests_cannot_determine_array_type/execute__tests__stderr.snap.new b/tooling/nargo_cli/tests/snapshots/compile_failure/noirc_frontend_tests_cannot_determine_array_type/execute__tests__stderr.snap.new deleted file mode 100644 index 2e7541f2eb1..00000000000 --- a/tooling/nargo_cli/tests/snapshots/compile_failure/noirc_frontend_tests_cannot_determine_array_type/execute__tests__stderr.snap.new +++ /dev/null @@ -1,14 +0,0 @@ ---- -source: tooling/nargo_cli/tests/execute.rs -assertion_line: 339 -expression: stderr ---- -error3 -error: Type annotation needed - ┌─ src/main.nr:3:17 - │ -3 │ let _ = []; - │ -- Could not determine the type of the array - │ - -Aborting due to 1 previous error diff --git a/tooling/nargo_cli/tests/snapshots/compile_failure/noirc_frontend_tests_cannot_determine_slice_type/execute__tests__stderr.snap.new b/tooling/nargo_cli/tests/snapshots/compile_failure/noirc_frontend_tests_cannot_determine_slice_type/execute__tests__stderr.snap.new deleted file mode 100644 index fc4eabb018d..00000000000 --- a/tooling/nargo_cli/tests/snapshots/compile_failure/noirc_frontend_tests_cannot_determine_slice_type/execute__tests__stderr.snap.new +++ /dev/null @@ -1,14 +0,0 @@ ---- -source: tooling/nargo_cli/tests/execute.rs -assertion_line: 339 -expression: stderr ---- -error3 -error: Type annotation needed - ┌─ src/main.nr:3:17 - │ -3 │ let _ = &[]; - │ --- Could not determine the type of the slice - │ - -Aborting due to 1 previous error diff --git a/tooling/nargo_cli/tests/snapshots/compile_failure/noirc_frontend_tests_traits_errors_if_impl_trait_constraint_is_not_satisfied/execute__tests__stderr.snap.new b/tooling/nargo_cli/tests/snapshots/compile_failure/noirc_frontend_tests_traits_errors_if_impl_trait_constraint_is_not_satisfied/execute__tests__stderr.snap.new deleted file mode 100644 index 55644d972da..00000000000 --- a/tooling/nargo_cli/tests/snapshots/compile_failure/noirc_frontend_tests_traits_errors_if_impl_trait_constraint_is_not_satisfied/execute__tests__stderr.snap.new +++ /dev/null @@ -1,31 +0,0 @@ ---- -source: tooling/nargo_cli/tests/execute.rs -assertion_line: 339 -expression: stderr ---- -warning: function `greet` cannot return without recursing - ┌─ src/main.nr:10:16 - │ -10 │ fn greet(object: U) - │ ----- function cannot return without recursing - │ - -error: Object type is unknown in method call - ┌─ src/main.nr:14:17 - │ -14 │ object.greet(); - │ ------------ Type must be known by this point to know which method to call - │ - = Try adding a type annotation for the object type before this method call - -error: The trait bound `SomeGreeter: Greeter` is not satisfied - ┌─ src/main.nr:8:16 - │ - 8 │ T: Greeter, - │ ------- required by this bound in `Foo` - · -22 │ impl Foo for Bar {} - │ --- The trait `Greeter` is not implemented for `SomeGreeter` - │ - -Aborting due to 2 previous errors diff --git a/tooling/nargo_cli/tests/snapshots/execution_success/struct_assignment_with_shared_ref_to_field/execute__tests__expanded.snap b/tooling/nargo_cli/tests/snapshots/execution_success/struct_assignment_with_shared_ref_to_field/execute__tests__expanded.snap new file mode 100644 index 00000000000..54da29923f7 --- /dev/null +++ b/tooling/nargo_cli/tests/snapshots/execution_success/struct_assignment_with_shared_ref_to_field/execute__tests__expanded.snap @@ -0,0 +1,26 @@ +--- +source: tooling/nargo_cli/tests/execute.rs +expression: expanded_code +--- +fn main() { + let (a, b): (bool, bool) = (true, true); + let (c, d): (bool, bool) = (plain_store(), store_through_pointer()); + assert(a); + assert(b); + assert(c); + assert(!d); +} + +fn plain_store() -> bool { + let mut a: (bool,) = (false,); + let b: &mut bool = &mut a.0; + a = (true,); + *b +} + +fn store_through_pointer() -> bool { + let a: &mut (bool,) = &mut (false,); + let b: &mut bool = &mut a.0; + *(a) = (true,); + *b +} diff --git a/tooling/nargo_cli/tests/snapshots/execution_success/struct_assignment_with_shared_ref_to_field/execute__tests__stdout.snap b/tooling/nargo_cli/tests/snapshots/execution_success/struct_assignment_with_shared_ref_to_field/execute__tests__stdout.snap new file mode 100644 index 00000000000..e86e3de90e1 --- /dev/null +++ b/tooling/nargo_cli/tests/snapshots/execution_success/struct_assignment_with_shared_ref_to_field/execute__tests__stdout.snap @@ -0,0 +1,5 @@ +--- +source: tooling/nargo_cli/tests/execute.rs +expression: stdout +--- + From d05d63b7be76e0e4056695dd1905adee45615632 Mon Sep 17 00:00:00 2001 From: Jake Fecher Date: Fri, 18 Jul 2025 13:03:54 -0500 Subject: [PATCH 4/9] Thought I already updated snaps --- ...ig_false_inliner_-9223372036854775808.snap | 26 ++++++++++++++ ..._tests__force_brillig_false_inliner_0.snap | 26 ++++++++++++++ ...lig_false_inliner_9223372036854775807.snap | 26 ++++++++++++++ ...lig_true_inliner_-9223372036854775808.snap | 36 +++++++++++++++++++ ...__tests__force_brillig_true_inliner_0.snap | 36 +++++++++++++++++++ ...llig_true_inliner_9223372036854775807.snap | 36 +++++++++++++++++++ 6 files changed, 186 insertions(+) create mode 100644 tooling/nargo_cli/tests/snapshots/execution_success/struct_assignment_with_shared_ref_to_field/execute__tests__force_brillig_false_inliner_-9223372036854775808.snap create mode 100644 tooling/nargo_cli/tests/snapshots/execution_success/struct_assignment_with_shared_ref_to_field/execute__tests__force_brillig_false_inliner_0.snap create mode 100644 tooling/nargo_cli/tests/snapshots/execution_success/struct_assignment_with_shared_ref_to_field/execute__tests__force_brillig_false_inliner_9223372036854775807.snap create mode 100644 tooling/nargo_cli/tests/snapshots/execution_success/struct_assignment_with_shared_ref_to_field/execute__tests__force_brillig_true_inliner_-9223372036854775808.snap create mode 100644 tooling/nargo_cli/tests/snapshots/execution_success/struct_assignment_with_shared_ref_to_field/execute__tests__force_brillig_true_inliner_0.snap create mode 100644 tooling/nargo_cli/tests/snapshots/execution_success/struct_assignment_with_shared_ref_to_field/execute__tests__force_brillig_true_inliner_9223372036854775807.snap diff --git a/tooling/nargo_cli/tests/snapshots/execution_success/struct_assignment_with_shared_ref_to_field/execute__tests__force_brillig_false_inliner_-9223372036854775808.snap b/tooling/nargo_cli/tests/snapshots/execution_success/struct_assignment_with_shared_ref_to_field/execute__tests__force_brillig_false_inliner_-9223372036854775808.snap new file mode 100644 index 00000000000..7de940827c8 --- /dev/null +++ b/tooling/nargo_cli/tests/snapshots/execution_success/struct_assignment_with_shared_ref_to_field/execute__tests__force_brillig_false_inliner_-9223372036854775808.snap @@ -0,0 +1,26 @@ +--- +source: tooling/nargo_cli/tests/execute.rs +expression: artifact +--- +{ + "noir_version": "[noir_version]", + "hash": "[hash]", + "abi": { + "parameters": [], + "return_type": null, + "error_types": {} + }, + "bytecode": [ + "func 0", + "current witness index : _0", + "private parameters indices : []", + "public parameters indices : []", + "return value indices : []" + ], + "debug_symbols": "XY5BCsQwCEXv4rqLWfcqw1BsaosgJtikMITefWyYQOlK/3/6tcJCc9km1jXuML4rzMYivE0SA2aO6m49B+hyykbkFty4byU00gyjFpEBDpTShvaE2mpGc/oagHTx6oErC13d+XGBge158UBjnIX+ci0abjR/Uyf942Qx0FKMrqTGPPsH", + "file_map": {}, + "names": [ + "main" + ], + "brillig_names": [] +} diff --git a/tooling/nargo_cli/tests/snapshots/execution_success/struct_assignment_with_shared_ref_to_field/execute__tests__force_brillig_false_inliner_0.snap b/tooling/nargo_cli/tests/snapshots/execution_success/struct_assignment_with_shared_ref_to_field/execute__tests__force_brillig_false_inliner_0.snap new file mode 100644 index 00000000000..7de940827c8 --- /dev/null +++ b/tooling/nargo_cli/tests/snapshots/execution_success/struct_assignment_with_shared_ref_to_field/execute__tests__force_brillig_false_inliner_0.snap @@ -0,0 +1,26 @@ +--- +source: tooling/nargo_cli/tests/execute.rs +expression: artifact +--- +{ + "noir_version": "[noir_version]", + "hash": "[hash]", + "abi": { + "parameters": [], + "return_type": null, + "error_types": {} + }, + "bytecode": [ + "func 0", + "current witness index : _0", + "private parameters indices : []", + "public parameters indices : []", + "return value indices : []" + ], + "debug_symbols": "XY5BCsQwCEXv4rqLWfcqw1BsaosgJtikMITefWyYQOlK/3/6tcJCc9km1jXuML4rzMYivE0SA2aO6m49B+hyykbkFty4byU00gyjFpEBDpTShvaE2mpGc/oagHTx6oErC13d+XGBge158UBjnIX+ci0abjR/Uyf942Qx0FKMrqTGPPsH", + "file_map": {}, + "names": [ + "main" + ], + "brillig_names": [] +} diff --git a/tooling/nargo_cli/tests/snapshots/execution_success/struct_assignment_with_shared_ref_to_field/execute__tests__force_brillig_false_inliner_9223372036854775807.snap b/tooling/nargo_cli/tests/snapshots/execution_success/struct_assignment_with_shared_ref_to_field/execute__tests__force_brillig_false_inliner_9223372036854775807.snap new file mode 100644 index 00000000000..7de940827c8 --- /dev/null +++ b/tooling/nargo_cli/tests/snapshots/execution_success/struct_assignment_with_shared_ref_to_field/execute__tests__force_brillig_false_inliner_9223372036854775807.snap @@ -0,0 +1,26 @@ +--- +source: tooling/nargo_cli/tests/execute.rs +expression: artifact +--- +{ + "noir_version": "[noir_version]", + "hash": "[hash]", + "abi": { + "parameters": [], + "return_type": null, + "error_types": {} + }, + "bytecode": [ + "func 0", + "current witness index : _0", + "private parameters indices : []", + "public parameters indices : []", + "return value indices : []" + ], + "debug_symbols": "XY5BCsQwCEXv4rqLWfcqw1BsaosgJtikMITefWyYQOlK/3/6tcJCc9km1jXuML4rzMYivE0SA2aO6m49B+hyykbkFty4byU00gyjFpEBDpTShvaE2mpGc/oagHTx6oErC13d+XGBge158UBjnIX+ci0abjR/Uyf942Qx0FKMrqTGPPsH", + "file_map": {}, + "names": [ + "main" + ], + "brillig_names": [] +} diff --git a/tooling/nargo_cli/tests/snapshots/execution_success/struct_assignment_with_shared_ref_to_field/execute__tests__force_brillig_true_inliner_-9223372036854775808.snap b/tooling/nargo_cli/tests/snapshots/execution_success/struct_assignment_with_shared_ref_to_field/execute__tests__force_brillig_true_inliner_-9223372036854775808.snap new file mode 100644 index 00000000000..41e0c42011c --- /dev/null +++ b/tooling/nargo_cli/tests/snapshots/execution_success/struct_assignment_with_shared_ref_to_field/execute__tests__force_brillig_true_inliner_-9223372036854775808.snap @@ -0,0 +1,36 @@ +--- +source: tooling/nargo_cli/tests/execute.rs +expression: artifact +--- +{ + "noir_version": "[noir_version]", + "hash": "[hash]", + "abi": { + "parameters": [], + "return_type": null, + "error_types": { + "17843811134343075018": { + "error_kind": "string", + "string": "Stack too deep" + } + } + }, + "bytecode": [ + "func 0", + "current witness index : _0", + "private parameters indices : []", + "public parameters indices : []", + "return value indices : []", + "BRILLIG CALL func 0: inputs: [], outputs: []", + "unconstrained func 0", + "[Const { destination: Direct(2), bit_size: Integer(U32), value: 1 }, Const { destination: Direct(1), bit_size: Integer(U32), value: 32836 }, Const { destination: Direct(0), bit_size: Integer(U32), value: 3 }, Const { destination: Relative(1), bit_size: Integer(U32), value: 0 }, Const { destination: Relative(2), bit_size: Integer(U32), value: 0 }, CalldataCopy { destination_address: Direct(32836), size_address: Relative(1), offset_address: Relative(2) }, Call { location: 11 }, Call { location: 12 }, Const { destination: Relative(1), bit_size: Integer(U32), value: 32836 }, Const { destination: Relative(2), bit_size: Integer(U32), value: 0 }, Stop { return_data: HeapVector { pointer: Relative(1), size: Relative(2) } }, Return, Call { location: 14 }, Return, Const { destination: Direct(32772), bit_size: Integer(U32), value: 30720 }, BinaryIntOp { destination: Direct(32771), op: LessThan, bit_size: U32, lhs: Direct(0), rhs: Direct(32772) }, JumpIf { condition: Direct(32771), location: 19 }, IndirectConst { destination_pointer: Direct(1), bit_size: Integer(U64), value: 17843811134343075018 }, Trap { revert_data: HeapVector { pointer: Direct(1), size: Direct(2) } }, Return]" + ], + "debug_symbols": "XY7BCoRACIbfxfMcCvayvUpE2GQxIM5gM8ESvfs6sUHsRf391N8DZprKOgZZ4gZdf8CkgTmsI0ePOUSx7nE6uOWYlcha8OC2lVBJMnRSmB3syOUa2hLKlTOq0cYByWzZDi6BqVbnYAJ90H/HHTXgxPSTSxH/oPmTbnJ/nDR6motSvVQZNDW0Fvv25dr3cFa3Lw==", + "file_map": {}, + "names": [ + "main" + ], + "brillig_names": [ + "main" + ] +} diff --git a/tooling/nargo_cli/tests/snapshots/execution_success/struct_assignment_with_shared_ref_to_field/execute__tests__force_brillig_true_inliner_0.snap b/tooling/nargo_cli/tests/snapshots/execution_success/struct_assignment_with_shared_ref_to_field/execute__tests__force_brillig_true_inliner_0.snap new file mode 100644 index 00000000000..41e0c42011c --- /dev/null +++ b/tooling/nargo_cli/tests/snapshots/execution_success/struct_assignment_with_shared_ref_to_field/execute__tests__force_brillig_true_inliner_0.snap @@ -0,0 +1,36 @@ +--- +source: tooling/nargo_cli/tests/execute.rs +expression: artifact +--- +{ + "noir_version": "[noir_version]", + "hash": "[hash]", + "abi": { + "parameters": [], + "return_type": null, + "error_types": { + "17843811134343075018": { + "error_kind": "string", + "string": "Stack too deep" + } + } + }, + "bytecode": [ + "func 0", + "current witness index : _0", + "private parameters indices : []", + "public parameters indices : []", + "return value indices : []", + "BRILLIG CALL func 0: inputs: [], outputs: []", + "unconstrained func 0", + "[Const { destination: Direct(2), bit_size: Integer(U32), value: 1 }, Const { destination: Direct(1), bit_size: Integer(U32), value: 32836 }, Const { destination: Direct(0), bit_size: Integer(U32), value: 3 }, Const { destination: Relative(1), bit_size: Integer(U32), value: 0 }, Const { destination: Relative(2), bit_size: Integer(U32), value: 0 }, CalldataCopy { destination_address: Direct(32836), size_address: Relative(1), offset_address: Relative(2) }, Call { location: 11 }, Call { location: 12 }, Const { destination: Relative(1), bit_size: Integer(U32), value: 32836 }, Const { destination: Relative(2), bit_size: Integer(U32), value: 0 }, Stop { return_data: HeapVector { pointer: Relative(1), size: Relative(2) } }, Return, Call { location: 14 }, Return, Const { destination: Direct(32772), bit_size: Integer(U32), value: 30720 }, BinaryIntOp { destination: Direct(32771), op: LessThan, bit_size: U32, lhs: Direct(0), rhs: Direct(32772) }, JumpIf { condition: Direct(32771), location: 19 }, IndirectConst { destination_pointer: Direct(1), bit_size: Integer(U64), value: 17843811134343075018 }, Trap { revert_data: HeapVector { pointer: Direct(1), size: Direct(2) } }, Return]" + ], + "debug_symbols": "XY7BCoRACIbfxfMcCvayvUpE2GQxIM5gM8ESvfs6sUHsRf391N8DZprKOgZZ4gZdf8CkgTmsI0ePOUSx7nE6uOWYlcha8OC2lVBJMnRSmB3syOUa2hLKlTOq0cYByWzZDi6BqVbnYAJ90H/HHTXgxPSTSxH/oPmTbnJ/nDR6motSvVQZNDW0Fvv25dr3cFa3Lw==", + "file_map": {}, + "names": [ + "main" + ], + "brillig_names": [ + "main" + ] +} diff --git a/tooling/nargo_cli/tests/snapshots/execution_success/struct_assignment_with_shared_ref_to_field/execute__tests__force_brillig_true_inliner_9223372036854775807.snap b/tooling/nargo_cli/tests/snapshots/execution_success/struct_assignment_with_shared_ref_to_field/execute__tests__force_brillig_true_inliner_9223372036854775807.snap new file mode 100644 index 00000000000..41e0c42011c --- /dev/null +++ b/tooling/nargo_cli/tests/snapshots/execution_success/struct_assignment_with_shared_ref_to_field/execute__tests__force_brillig_true_inliner_9223372036854775807.snap @@ -0,0 +1,36 @@ +--- +source: tooling/nargo_cli/tests/execute.rs +expression: artifact +--- +{ + "noir_version": "[noir_version]", + "hash": "[hash]", + "abi": { + "parameters": [], + "return_type": null, + "error_types": { + "17843811134343075018": { + "error_kind": "string", + "string": "Stack too deep" + } + } + }, + "bytecode": [ + "func 0", + "current witness index : _0", + "private parameters indices : []", + "public parameters indices : []", + "return value indices : []", + "BRILLIG CALL func 0: inputs: [], outputs: []", + "unconstrained func 0", + "[Const { destination: Direct(2), bit_size: Integer(U32), value: 1 }, Const { destination: Direct(1), bit_size: Integer(U32), value: 32836 }, Const { destination: Direct(0), bit_size: Integer(U32), value: 3 }, Const { destination: Relative(1), bit_size: Integer(U32), value: 0 }, Const { destination: Relative(2), bit_size: Integer(U32), value: 0 }, CalldataCopy { destination_address: Direct(32836), size_address: Relative(1), offset_address: Relative(2) }, Call { location: 11 }, Call { location: 12 }, Const { destination: Relative(1), bit_size: Integer(U32), value: 32836 }, Const { destination: Relative(2), bit_size: Integer(U32), value: 0 }, Stop { return_data: HeapVector { pointer: Relative(1), size: Relative(2) } }, Return, Call { location: 14 }, Return, Const { destination: Direct(32772), bit_size: Integer(U32), value: 30720 }, BinaryIntOp { destination: Direct(32771), op: LessThan, bit_size: U32, lhs: Direct(0), rhs: Direct(32772) }, JumpIf { condition: Direct(32771), location: 19 }, IndirectConst { destination_pointer: Direct(1), bit_size: Integer(U64), value: 17843811134343075018 }, Trap { revert_data: HeapVector { pointer: Direct(1), size: Direct(2) } }, Return]" + ], + "debug_symbols": "XY7BCoRACIbfxfMcCvayvUpE2GQxIM5gM8ESvfs6sUHsRf391N8DZprKOgZZ4gZdf8CkgTmsI0ePOUSx7nE6uOWYlcha8OC2lVBJMnRSmB3syOUa2hLKlTOq0cYByWzZDi6BqVbnYAJ90H/HHTXgxPSTSxH/oPmTbnJ/nDR6motSvVQZNDW0Fvv25dr3cFa3Lw==", + "file_map": {}, + "names": [ + "main" + ], + "brillig_names": [ + "main" + ] +} From fcaad3952cf0d2b17cb445fe41085f3067533929 Mon Sep 17 00:00:00 2001 From: Jake Fecher Date: Fri, 18 Jul 2025 13:56:58 -0500 Subject: [PATCH 5/9] Format --- .../src/hir/comptime/interpreter/builtin/builtin_helpers.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/noirc_frontend/src/hir/comptime/interpreter/builtin/builtin_helpers.rs b/compiler/noirc_frontend/src/hir/comptime/interpreter/builtin/builtin_helpers.rs index 194228bccdf..369348e355b 100644 --- a/compiler/noirc_frontend/src/hir/comptime/interpreter/builtin/builtin_helpers.rs +++ b/compiler/noirc_frontend/src/hir/comptime/interpreter/builtin/builtin_helpers.rs @@ -4,10 +4,10 @@ use std::{hash::Hasher, rc::Rc}; use iter_extended::{try_vecmap, vecmap}; use noirc_errors::Location; -use crate::hir::comptime::value::StructFields; use crate::ast::{BinaryOp, ItemVisibility, UnaryOp}; use crate::elaborator::Elaborator; use crate::hir::comptime::display::tokens_to_string; +use crate::hir::comptime::value::StructFields; use crate::hir::comptime::value::unwrap_rc; use crate::hir::def_collector::dc_crate::CompilationError; use crate::lexer::Lexer; From 6b2293e225066f11bcb40861220f3b7c660e4d99 Mon Sep 17 00:00:00 2001 From: Jake Fecher Date: Fri, 18 Jul 2025 13:57:13 -0500 Subject: [PATCH 6/9] Format test --- .../src/main.nr | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/test_programs/compile_failure/noirc_frontend_tests_struct_numeric_generic_in_struct/src/main.nr b/test_programs/compile_failure/noirc_frontend_tests_struct_numeric_generic_in_struct/src/main.nr index d1ff69ced46..6327c6ad949 100644 --- a/test_programs/compile_failure/noirc_frontend_tests_struct_numeric_generic_in_struct/src/main.nr +++ b/test_programs/compile_failure/noirc_frontend_tests_struct_numeric_generic_in_struct/src/main.nr @@ -1,7 +1,5 @@ +pub struct Foo { + inner: u64, +} - pub struct Foo { - inner: u64 - } - - pub struct Bar { } - \ No newline at end of file +pub struct Bar {} From cfe5fd69c9c413b1a160faab028752e5f4c393e3 Mon Sep 17 00:00:00 2001 From: Jake Fecher Date: Fri, 18 Jul 2025 13:58:21 -0500 Subject: [PATCH 7/9] Format test --- .../struct_assignment_with_shared_ref_to_field/src/main.nr | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/test_programs/execution_success/struct_assignment_with_shared_ref_to_field/src/main.nr b/test_programs/execution_success/struct_assignment_with_shared_ref_to_field/src/main.nr index b369d9b33bc..2c6ad52155d 100644 --- a/test_programs/execution_success/struct_assignment_with_shared_ref_to_field/src/main.nr +++ b/test_programs/execution_success/struct_assignment_with_shared_ref_to_field/src/main.nr @@ -1,7 +1,5 @@ fn main() { - let (a, b) = comptime { - (plain_store(), store_through_pointer()) - }; + let (a, b) = comptime { (plain_store(), store_through_pointer()) }; let (c, d) = (plain_store(), store_through_pointer()); assert(a); assert(b); From 8bd4cc10ccc1016043662ec9f378f4ca61e223c1 Mon Sep 17 00:00:00 2001 From: Jake Fecher Date: Fri, 18 Jul 2025 14:20:48 -0500 Subject: [PATCH 8/9] Formatted test needs updated snap --- .../execute__tests__stderr.snap | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tooling/nargo_cli/tests/snapshots/compile_failure/noirc_frontend_tests_struct_numeric_generic_in_struct/execute__tests__stderr.snap b/tooling/nargo_cli/tests/snapshots/compile_failure/noirc_frontend_tests_struct_numeric_generic_in_struct/execute__tests__stderr.snap index 0dad40641bb..6da7f11db2f 100644 --- a/tooling/nargo_cli/tests/snapshots/compile_failure/noirc_frontend_tests_struct_numeric_generic_in_struct/execute__tests__stderr.snap +++ b/tooling/nargo_cli/tests/snapshots/compile_failure/noirc_frontend_tests_struct_numeric_generic_in_struct/execute__tests__stderr.snap @@ -3,10 +3,10 @@ source: tooling/nargo_cli/tests/execute.rs expression: stderr --- error: N has a type of Foo. The only supported numeric generic types are `u1`, `u8`, `u16`, and `u32`. - ┌─ src/main.nr:6:27 + ┌─ src/main.nr:5:23 │ -6 │ pub struct Bar { } - │ --- Unsupported numeric generic type +5 │ pub struct Bar {} + │ --- Unsupported numeric generic type │ Aborting due to 1 previous error From 1c840b3810b55fa7c0b1850ab771caa9d1a68f8f Mon Sep 17 00:00:00 2001 From: Jake Fecher Date: Mon, 21 Jul 2025 09:53:03 -0500 Subject: [PATCH 9/9] Copy values --- .../src/hir/comptime/interpreter.rs | 3 ++- .../noirc_frontend/src/hir/comptime/value.rs | 16 ++++++++++++++++ 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/compiler/noirc_frontend/src/hir/comptime/interpreter.rs b/compiler/noirc_frontend/src/hir/comptime/interpreter.rs index 500c59e5672..5f97421774e 100644 --- a/compiler/noirc_frontend/src/hir/comptime/interpreter.rs +++ b/compiler/noirc_frontend/src/hir/comptime/interpreter.rs @@ -1001,7 +1001,8 @@ impl<'local, 'interner> Interpreter<'local, 'interner> { fn evaluate_call(&mut self, call: HirCallExpression, id: ExprId) -> IResult { let function = self.evaluate(call.func)?; let arguments = try_vecmap(call.arguments, |arg| { - Ok((self.evaluate(arg)?, self.elaborator.interner.expr_location(&arg))) + let value = self.evaluate(arg)?.copy(); + Ok((value, self.elaborator.interner.expr_location(&arg))) })?; let location = self.elaborator.interner.expr_location(&id); diff --git a/compiler/noirc_frontend/src/hir/comptime/value.rs b/compiler/noirc_frontend/src/hir/comptime/value.rs index 1b66d957489..f9e8c1192de 100644 --- a/compiler/noirc_frontend/src/hir/comptime/value.rs +++ b/compiler/noirc_frontend/src/hir/comptime/value.rs @@ -719,6 +719,22 @@ impl Value { _ => false, // Unsigned or Field types are never negative } } + + /// Copies an argument ensuring any references to struct/tuple fields are replaced with fresh ones. + pub(crate) fn copy(self) -> Value { + match self { + Value::Tuple(fields) => { + Value::Tuple(vecmap(fields, |field| Shared::new(field.unwrap_or_clone().copy()))) + } + Value::Struct(fields, typ) => { + let fields = fields + .into_iter() + .map(|(name, field)| (name, Shared::new(field.unwrap_or_clone().copy()))); + Value::Struct(fields.collect(), typ) + } + other => other, + } + } } /// Unwraps an Rc value without cloning the inner value if the reference count is 1. Clones otherwise.