Skip to content

Commit

Permalink
fix: Improve type error when indexing a variable of unknown type (#6744)
Browse files Browse the repository at this point in the history
  • Loading branch information
jfecher authored Dec 9, 2024
1 parent 36bca82 commit 909b22b
Show file tree
Hide file tree
Showing 3 changed files with 28 additions and 13 deletions.
4 changes: 4 additions & 0 deletions compiler/noirc_frontend/src/elaborator/expressions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -346,6 +346,10 @@ impl<'context> Elaborator<'context> {
Type::Array(_, base_type) => *base_type,
Type::Slice(base_type) => *base_type,
Type::Error => Type::Error,
Type::TypeVariable(_) => {
self.push_err(TypeCheckError::TypeAnnotationsNeededForIndex { span: lhs_span });
Type::Error
}
typ => {
self.push_err(TypeCheckError::TypeMismatch {
expected_typ: "Array".to_owned(),
Expand Down
28 changes: 15 additions & 13 deletions compiler/noirc_frontend/src/elaborator/statements.rs
Original file line number Diff line number Diff line change
Expand Up @@ -183,20 +183,20 @@ impl<'context> Elaborator<'context> {
}

pub(super) fn elaborate_assign(&mut self, assign: AssignStatement) -> (HirStatement, Type) {
let span = assign.expression.span;
let expr_span = assign.expression.span;
let (expression, expr_type) = self.elaborate_expression(assign.expression);
let (lvalue, lvalue_type, mutable) = self.elaborate_lvalue(assign.lvalue, span);
let (lvalue, lvalue_type, mutable) = self.elaborate_lvalue(assign.lvalue);

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

self.unify_with_coercions(&expr_type, &lvalue_type, expression, span, || {
self.unify_with_coercions(&expr_type, &lvalue_type, expression, expr_span, || {
TypeCheckError::TypeMismatchWithSource {
actual: expr_type.clone(),
expected: lvalue_type.clone(),
span,
span: expr_span,
source: Source::Assignment,
}
});
Expand Down Expand Up @@ -296,7 +296,7 @@ impl<'context> Elaborator<'context> {
}
}

fn elaborate_lvalue(&mut self, lvalue: LValue, assign_span: Span) -> (HirLValue, Type, bool) {
fn elaborate_lvalue(&mut self, lvalue: LValue) -> (HirLValue, Type, bool) {
match lvalue {
LValue::Ident(ident) => {
let mut mutable = true;
Expand Down Expand Up @@ -330,7 +330,7 @@ impl<'context> Elaborator<'context> {
(HirLValue::Ident(ident.clone(), typ.clone()), typ, mutable)
}
LValue::MemberAccess { object, field_name, span } => {
let (object, lhs_type, mut mutable) = self.elaborate_lvalue(*object, assign_span);
let (object, lhs_type, mut mutable) = self.elaborate_lvalue(*object);
let mut object = Box::new(object);
let field_name = field_name.clone();

Expand Down Expand Up @@ -374,8 +374,7 @@ impl<'context> Elaborator<'context> {
expr_span,
});

let (mut lvalue, mut lvalue_type, mut mutable) =
self.elaborate_lvalue(*array, assign_span);
let (mut lvalue, mut lvalue_type, mut mutable) = self.elaborate_lvalue(*array);

// Before we check that the lvalue is an array, try to dereference it as many times
// as needed to unwrap any &mut wrappers.
Expand All @@ -397,12 +396,15 @@ impl<'context> Elaborator<'context> {
self.push_err(TypeCheckError::StringIndexAssign { span: lvalue_span });
Type::Error
}
Type::TypeVariable(_) => {
self.push_err(TypeCheckError::TypeAnnotationsNeededForIndex { span });
Type::Error
}
other => {
// TODO: Need a better span here
self.push_err(TypeCheckError::TypeMismatch {
expected_typ: "array".to_string(),
expr_typ: other.to_string(),
expr_span: assign_span,
expr_span: span,
});
Type::Error
}
Expand All @@ -413,7 +415,7 @@ impl<'context> Elaborator<'context> {
(HirLValue::Index { array, index, typ, location }, array_type, mutable)
}
LValue::Dereference(lvalue, span) => {
let (lvalue, reference_type, _) = self.elaborate_lvalue(*lvalue, assign_span);
let (lvalue, reference_type, _) = self.elaborate_lvalue(*lvalue);
let lvalue = Box::new(lvalue);
let location = Location::new(span, self.file);

Expand All @@ -423,7 +425,7 @@ impl<'context> Elaborator<'context> {
self.unify(&reference_type, &expected_type, || TypeCheckError::TypeMismatch {
expected_typ: expected_type.to_string(),
expr_typ: reference_type.to_string(),
expr_span: assign_span,
expr_span: span,
});

// Dereferences are always mutable since we already type checked against a &mut T
Expand All @@ -433,7 +435,7 @@ impl<'context> Elaborator<'context> {
}
LValue::Interned(id, span) => {
let lvalue = self.interner.get_lvalue(id, span).clone();
self.elaborate_lvalue(lvalue, assign_span)
self.elaborate_lvalue(lvalue)
}
}
}
Expand Down
9 changes: 9 additions & 0 deletions compiler/noirc_frontend/src/hir/type_check/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -206,6 +206,8 @@ pub enum TypeCheckError {
UnspecifiedType { span: Span },
#[error("Binding `{typ}` here to the `_` inside would create a cyclic type")]
CyclicType { typ: Type, span: Span },
#[error("Type annotations required before indexing this array or slice")]
TypeAnnotationsNeededForIndex { span: Span },
}

#[derive(Debug, Clone, PartialEq, Eq)]
Expand Down Expand Up @@ -520,6 +522,13 @@ impl<'a> From<&'a TypeCheckError> for Diagnostic {
*span,
)
},
TypeCheckError::TypeAnnotationsNeededForIndex { span } => {
Diagnostic::simple_error(
"Type annotations required before indexing this array or slice".into(),
"Type annotations needed before this point, can't decide if this is an array or slice".into(),
*span,
)
},
}
}
}
Expand Down

0 comments on commit 909b22b

Please sign in to comment.