Skip to content

Commit 460568e

Browse files
authored
fix: Assigning to tuple fields (#1318)
* Fix assigning to tuple fields * Cargo fmt * Formatting * Add regression test
1 parent d872890 commit 460568e

File tree

5 files changed

+63
-41
lines changed

5 files changed

+63
-41
lines changed

crates/nargo_cli/tests/test_data/tuples/src/main.nr

+15-5
Original file line numberDiff line numberDiff line change
@@ -9,11 +9,21 @@ fn main(x: Field, y: Field) {
99
assert(a == 0);
1010
assert(b == 1);
1111

12-
let (u,v) = if x as u32 <1 {
13-
(x,x+1)
12+
let (u,v) = if x as u32 < 1 {
13+
(x, x + 1)
1414
} else {
15-
(x+1,x)
15+
(x + 1, x)
1616
};
17-
assert(u==x+1);
18-
assert(v==x);
17+
assert(u == x+1);
18+
assert(v == x);
19+
20+
// Test mutating tuples
21+
let mut mutable = ((0, 0), 1, 2, 3);
22+
mutable.0 = pair;
23+
mutable.2 = 7;
24+
assert(mutable.0.0 == 1);
25+
assert(mutable.0.1 == 0);
26+
assert(mutable.1 == 1);
27+
assert(mutable.2 == 7);
28+
assert(mutable.3 == 3);
1929
}

crates/noirc_driver/src/contract.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
1+
use crate::program::{deserialize_circuit, serialize_circuit};
12
use acvm::acir::circuit::Circuit;
23
use noirc_abi::Abi;
34
use serde::{Deserialize, Serialize};
4-
use crate::program::{serialize_circuit, deserialize_circuit};
55

66
/// Describes the types of smart contract functions that are allowed.
77
/// Unlike the similar enum in noirc_frontend, 'open' and 'unconstrained'

crates/noirc_frontend/src/hir/type_check/expr.rs

+40-15
Original file line numberDiff line numberDiff line change
@@ -455,36 +455,61 @@ impl<'interner> TypeChecker<'interner> {
455455

456456
fn check_member_access(&mut self, access: expr::HirMemberAccess, expr_id: ExprId) -> Type {
457457
let lhs_type = self.check_expression(&access.lhs).follow_bindings();
458+
let span = self.interner.expr_span(&expr_id);
459+
460+
match self.check_field_access(&lhs_type, &access.rhs.0.contents, span) {
461+
Some((element_type, index)) => {
462+
self.interner.set_field_index(expr_id, index);
463+
element_type
464+
}
465+
None => Type::Error,
466+
}
467+
}
468+
469+
/// This will verify that an expression in the form `lhs.rhs_name` has the given field and will push
470+
/// a type error if it does not. If there is no error, the type of the struct/tuple field is returned
471+
/// along with the index of the field in question.
472+
///
473+
/// This function is abstracted from check_member_access so that it can be shared between
474+
/// there and the HirLValue::MemberAccess case of check_lvalue.
475+
pub(super) fn check_field_access(
476+
&mut self,
477+
lhs_type: &Type,
478+
field_name: &str,
479+
span: Span,
480+
) -> Option<(Type, usize)> {
481+
let lhs_type = lhs_type.follow_bindings();
458482

459483
if let Type::Struct(s, args) = &lhs_type {
460484
let s = s.borrow();
461-
if let Some((field, index)) = s.get_field(&access.rhs.0.contents, args) {
462-
self.interner.set_field_index(expr_id, index);
463-
return field;
485+
if let Some((field, index)) = s.get_field(field_name, args) {
486+
return Some((field, index));
464487
}
465488
} else if let Type::Tuple(elements) = &lhs_type {
466-
if let Ok(index) = access.rhs.0.contents.parse::<usize>() {
467-
if index < elements.len() {
468-
self.interner.set_field_index(expr_id, index);
469-
return elements[index].clone();
489+
if let Ok(index) = field_name.parse::<usize>() {
490+
let length = elements.len();
491+
if index < length {
492+
return Some((elements[index].clone(), index));
493+
} else {
494+
self.errors.push(TypeCheckError::Unstructured {
495+
msg: format!("Index {index} is out of bounds for this tuple {lhs_type} of length {length}"),
496+
span,
497+
});
498+
return None;
470499
}
471500
}
472501
}
473502

474503
// If we get here the type has no field named 'access.rhs'.
475504
// Now we specialize the error message based on whether we know the object type in question yet.
476505
if let Type::TypeVariable(..) = &lhs_type {
477-
self.errors.push(TypeCheckError::TypeAnnotationsNeeded {
478-
span: self.interner.expr_span(&access.lhs),
479-
});
506+
self.errors.push(TypeCheckError::TypeAnnotationsNeeded { span });
480507
} else if lhs_type != Type::Error {
481-
self.errors.push(TypeCheckError::Unstructured {
482-
msg: format!("Type {lhs_type} has no member named {}", access.rhs),
483-
span: self.interner.expr_span(&access.lhs),
484-
});
508+
let msg = format!("Type {lhs_type} has no member named {field_name}");
509+
self.errors.push(TypeCheckError::Unstructured { msg, span });
485510
}
486511

487-
Type::Error
512+
None
488513
}
489514

490515
fn comparator_operand_type_rules(

crates/noirc_frontend/src/hir/type_check/stmt.rs

+6-19
Original file line numberDiff line numberDiff line change
@@ -142,28 +142,15 @@ impl<'interner> TypeChecker<'interner> {
142142
(typ.clone(), HirLValue::Ident(ident, typ))
143143
}
144144
HirLValue::MemberAccess { object, field_name, .. } => {
145-
let (result, object) = self.check_lvalue(*object, assign_span);
145+
let (lhs_type, object) = self.check_lvalue(*object, assign_span);
146146
let object = Box::new(object);
147147

148-
let mut error = |typ| {
149-
self.errors.push(TypeCheckError::Unstructured {
150-
msg: format!("Type {typ} has no member named {field_name}"),
151-
span: field_name.span(),
152-
});
153-
(Type::Error, None)
154-
};
155-
156-
let (typ, field_index) = match result.follow_bindings() {
157-
Type::Struct(def, args) => {
158-
match def.borrow().get_field(&field_name.0.contents, &args) {
159-
Some((field, index)) => (field, Some(index)),
160-
None => error(Type::Struct(def.clone(), args)),
161-
}
162-
}
163-
Type::Error => (Type::Error, None),
164-
other => error(other),
165-
};
148+
let span = field_name.span();
149+
let (typ, field_index) = self
150+
.check_field_access(&lhs_type, &field_name.0.contents, span)
151+
.unwrap_or((Type::Error, 0));
166152

153+
let field_index = Some(field_index);
167154
(typ.clone(), HirLValue::MemberAccess { object, field_name, field_index, typ })
168155
}
169156
HirLValue::Index { array, index, .. } => {

crates/noirc_frontend/src/parser/parser.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -534,7 +534,7 @@ where
534534
{
535535
let l_ident = ident().map(LValue::Ident);
536536

537-
let l_member_rhs = just(Token::Dot).ignore_then(ident()).map(LValueRhs::MemberAccess);
537+
let l_member_rhs = just(Token::Dot).ignore_then(field_name()).map(LValueRhs::MemberAccess);
538538

539539
let l_index = expr_parser
540540
.delimited_by(just(Token::LeftBracket), just(Token::RightBracket))

0 commit comments

Comments
 (0)