Skip to content

Commit

Permalink
Fix mut type check
Browse files Browse the repository at this point in the history
  • Loading branch information
@jfecher
jfecher committed Oct 5, 2023
1 parent 6796bec commit 066d8d9
Show file tree
Hide file tree
Showing 2 changed files with 46 additions and 24 deletions.
2 changes: 1 addition & 1 deletion compiler/noirc_frontend/src/hir/type_check/expr.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -409,7 +409,7 @@ impl<'interner> TypeChecker<'interner> {
});

let lhs_type = self.check_expression(&index_expr.collection);
match lhs_type {
match lhs_type.follow_bindings() {
// XXX: We can check the array bounds here also, but it may be better to constant fold first
// and have ConstId instead of ExprId for constants
Type::Array(_, base_type) => *base_type,
Expand Down
68 changes: 45 additions & 23 deletions compiler/noirc_frontend/src/hir/type_check/stmt.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,12 @@ impl<'interner> TypeChecker<'interner> {
fn check_assign_stmt(&mut self, assign_stmt: HirAssignStatement, stmt_id: &StmtId) {
let expr_type = self.check_expression(&assign_stmt.expression);
let span = self.interner.expr_span(&assign_stmt.expression);
let (lvalue_type, new_lvalue) = self.check_lvalue(assign_stmt.lvalue, span);
let (lvalue_type, new_lvalue, mutable) = self.check_lvalue(&assign_stmt.lvalue, span);

if !mutable {
let (name, span) = self.get_lvalue_name_and_span(&assign_stmt.lvalue);
self.errors.push(TypeCheckError::VariableMustBeMutable { name, span });
}

// Must push new lvalue to the interner, we've resolved any field indices
self.interner.update_statement(stmt_id, |stmt| match stmt {
Expand All @@ -159,39 +164,52 @@ impl<'interner> TypeChecker<'interner> {
});
}

fn get_lvalue_name_and_span(&self, lvalue: &HirLValue) -> (String, Span) {
match lvalue {
HirLValue::Ident(name, _) => {
let span = name.location.span;

if let Some(definition) = self.interner.try_definition(name.id) {
(definition.name.clone(), span)
} else {
("(undeclared variable)".into(), span)
}
}
HirLValue::MemberAccess { object, .. } => self.get_lvalue_name_and_span(object),
HirLValue::Index { array, .. } => self.get_lvalue_name_and_span(array),
HirLValue::Dereference { lvalue, .. } => self.get_lvalue_name_and_span(lvalue),
}
}

/// Type check an lvalue - the left hand side of an assignment statement.
fn check_lvalue(&mut self, lvalue: HirLValue, assign_span: Span) -> (Type, HirLValue) {
fn check_lvalue(&mut self, lvalue: &HirLValue, assign_span: Span) -> (Type, HirLValue, bool) {
match lvalue {
HirLValue::Ident(ident, _) => {
let mut mutable = true;

let typ = if ident.id == DefinitionId::dummy_id() {
Type::Error
} else {
// Do we need to store TypeBindings here?
let typ = self.interner.id_type(ident.id).instantiate(self.interner).0;
let typ = typ.follow_bindings();

if let Some(definition) = self.interner.try_definition(ident.id) {
if !definition.mutable && !matches!(typ, Type::MutableReference(_)) {
self.errors.push(TypeCheckError::VariableMustBeMutable {
name: definition.name.clone(),
span: ident.location.span,
});
}
mutable = definition.mutable;
}

typ
let typ = self.interner.id_type(ident.id).instantiate(self.interner).0;
typ.follow_bindings()
};

(typ.clone(), HirLValue::Ident(ident, typ))
(typ.clone(), HirLValue::Ident(*ident, typ), mutable)
}
HirLValue::MemberAccess { object, field_name, .. } => {
let (lhs_type, object) = self.check_lvalue(*object, assign_span);
let (lhs_type, object, mut mutable) = self.check_lvalue(object, assign_span);
let mut object = Box::new(object);
let span = field_name.span();
let field_name = field_name.clone();

let object_ref = &mut object;
let mutable_ref = &mut mutable;

let (typ, field_index) = self
let (object_type, field_index) = self
.check_field_access(
&lhs_type,
&field_name.0.contents,
Expand All @@ -206,16 +224,19 @@ impl<'interner> TypeChecker<'interner> {

let lvalue = std::mem::replace(object_ref, Box::new(tmp_value));
*object_ref = Box::new(HirLValue::Dereference { lvalue, element_type });
*mutable_ref = true;
},
)
.unwrap_or((Type::Error, 0));

let field_index = Some(field_index);
(typ.clone(), HirLValue::MemberAccess { object, field_name, field_index, typ })
let typ = object_type.clone();
let lvalue = HirLValue::MemberAccess { object, field_name, field_index, typ };
(object_type, lvalue, mutable)
}
HirLValue::Index { array, index, .. } => {
let index_type = self.check_expression(&index);
let expr_span = self.interner.expr_span(&index);
let index_type = self.check_expression(index);
let expr_span = self.interner.expr_span(index);

index_type.unify(
&Type::polymorphic_integer(self.interner),
Expand All @@ -227,7 +248,7 @@ impl<'interner> TypeChecker<'interner> {
},
);

let (result, array) = self.check_lvalue(*array, assign_span);
let (result, array, mutable) = self.check_lvalue(array, assign_span);
let array = Box::new(array);

let typ = match result {
Expand All @@ -244,10 +265,10 @@ impl<'interner> TypeChecker<'interner> {
}
};

(typ.clone(), HirLValue::Index { array, index, typ })
(typ.clone(), HirLValue::Index { array, index: *index, typ }, mutable)
}
HirLValue::Dereference { lvalue, element_type: _ } => {
let (reference_type, lvalue) = self.check_lvalue(*lvalue, assign_span);
let (reference_type, lvalue, _) = self.check_lvalue(lvalue, assign_span);
let lvalue = Box::new(lvalue);

let element_type = Type::type_variable(self.interner.next_type_variable_id());
Expand All @@ -258,7 +279,8 @@ impl<'interner> TypeChecker<'interner> {
expr_span: assign_span,
});

(element_type.clone(), HirLValue::Dereference { lvalue, element_type })
// Dereferences are always mutable since we already type checked against a &mut T
(element_type.clone(), HirLValue::Dereference { lvalue, element_type }, true)
}
}
}
Expand Down

0 comments on commit 066d8d9

Please sign in to comment.