fix: Error when a type variable is unbound during monomorphization instead of defaulting to Field (#4674)

# Description

## Problem\*

Resolves <!-- Link to GitHub Issue -->

## Summary\*

Adds an error when encountering an unbound `TypeVariableKind::Normal`
instead of defaulting it to a Field.

## Additional Context



## Documentation\*

Check one:
- [x] No documentation needed.
- [ ] Documentation included in this PR.
- [ ] **[For Experimental Features]** Documentation to be submitted in a
separate PR.

# PR Checklist\*

- [x] I have tested the changes locally.
- [x] I have formatted the changes with [Prettier](https://prettier.io/)
and/or `cargo fmt` on default settings.

Author: jfecher

compiler/noirc_driver/src/lib.rs

@@ -16,7 +16,9 @@ use noirc_frontend::graph::{CrateId, CrateName};
 use noirc_frontend::hir::def_map::{Contract, CrateDefMap};
 use noirc_frontend::hir::Context;
 use noirc_frontend::macros_api::MacroProcessor;
-use noirc_frontend::monomorphization::{monomorphize, monomorphize_debug, MonomorphizationError};
+use noirc_frontend::monomorphization::{
+    errors::MonomorphizationError, monomorphize, monomorphize_debug,
+};
 use noirc_frontend::node_interner::FuncId;
 use noirc_frontend::token::SecondaryAttribute;
 use std::path::Path;

compiler/noirc_frontend/src/ast/statement.rs

@@ -120,7 +120,7 @@ impl StatementKind {
         // Desugar `a <op>= b` to `a = a <op> b`. This relies on the evaluation of `a` having no side effects,
         // which is currently enforced by the restricted syntax of LValues.
         if operator != Token::Assign {
-            let lvalue_expr = lvalue.as_expression(span);
+            let lvalue_expr = lvalue.as_expression();
             let error_msg = "Token passed to Statement::assign is not a binary operator";
 
             let infix = crate::InfixExpression {
@@ -425,9 +425,9 @@ pub struct AssignStatement {
 #[derive(Debug, PartialEq, Eq, Clone)]
 pub enum LValue {
     Ident(Ident),
-    MemberAccess { object: Box<LValue>, field_name: Ident },
-    Index { array: Box<LValue>, index: Expression },
-    Dereference(Box<LValue>),
+    MemberAccess { object: Box<LValue>, field_name: Ident, span: Span },
+    Index { array: Box<LValue>, index: Expression, span: Span },
+    Dereference(Box<LValue>, Span),
 }
 
 #[derive(Debug, PartialEq, Eq, Clone)]
@@ -484,28 +484,40 @@ impl Recoverable for Pattern {
 }
 
 impl LValue {
-    fn as_expression(&self, span: Span) -> Expression {
+    fn as_expression(&self) -> Expression {
         let kind = match self {
             LValue::Ident(ident) => ExpressionKind::Variable(Path::from_ident(ident.clone())),
-            LValue::MemberAccess { object, field_name } => {
+            LValue::MemberAccess { object, field_name, span: _ } => {
                 ExpressionKind::MemberAccess(Box::new(MemberAccessExpression {
-                    lhs: object.as_expression(span),
+                    lhs: object.as_expression(),
                     rhs: field_name.clone(),
                 }))
             }
-            LValue::Index { array, index } => ExpressionKind::Index(Box::new(IndexExpression {
-                collection: array.as_expression(span),
-                index: index.clone(),
-            })),
-            LValue::Dereference(lvalue) => {
+            LValue::Index { array, index, span: _ } => {
+                ExpressionKind::Index(Box::new(IndexExpression {
+                    collection: array.as_expression(),
+                    index: index.clone(),
+                }))
+            }
+            LValue::Dereference(lvalue, _span) => {
                 ExpressionKind::Prefix(Box::new(crate::PrefixExpression {
                     operator: crate::UnaryOp::Dereference { implicitly_added: false },
-                    rhs: lvalue.as_expression(span),
+                    rhs: lvalue.as_expression(),
                 }))
             }
         };
+        let span = self.span();
         Expression::new(kind, span)
     }
+
+    pub fn span(&self) -> Span {
+        match self {
+            LValue::Ident(ident) => ident.span(),
+            LValue::MemberAccess { span, .. }
+            | LValue::Index { span, .. }
+            | LValue::Dereference(_, span) => *span,
+        }
+    }
 }
 
 #[derive(Debug, PartialEq, Eq, Clone)]
@@ -675,9 +687,11 @@ impl Display for LValue {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
         match self {
             LValue::Ident(ident) => ident.fmt(f),
-            LValue::MemberAccess { object, field_name } => write!(f, "{object}.{field_name}"),
-            LValue::Index { array, index } => write!(f, "{array}[{index}]"),
-            LValue::Dereference(lvalue) => write!(f, "*{lvalue}"),
+            LValue::MemberAccess { object, field_name, span: _ } => {
+                write!(f, "{object}.{field_name}")
+            }
+            LValue::Index { array, index, span: _ } => write!(f, "{array}[{index}]"),
+            LValue::Dereference(lvalue, _span) => write!(f, "*{lvalue}"),
         }
     }
 }

compiler/noirc_frontend/src/debug/mod.rs

@@ -282,7 +282,7 @@ impl DebugInstrumenter {
                     .unwrap_or_else(|| panic!("var lookup failed for var_name={}", &id.0.contents));
                 build_assign_var_stmt(var_id, id_expr(&ident("__debug_expr", id.span())))
             }
-            ast::LValue::Dereference(_lv) => {
+            ast::LValue::Dereference(_lv, span) => {
                 // TODO: this is a dummy statement for now, but we should
                 // somehow track the derefence and update the pointed to
                 // variable
@@ -303,16 +303,16 @@ impl DebugInstrumenter {
                             });
                             break;
                         }
-                        ast::LValue::MemberAccess { object, field_name } => {
+                        ast::LValue::MemberAccess { object, field_name, span } => {
                             cursor = object;
                             let field_name_id = self.insert_field_name(&field_name.0.contents);
-                            indexes.push(sint_expr(-(field_name_id.0 as i128), expression_span));
+                            indexes.push(sint_expr(-(field_name_id.0 as i128), *span));
                         }
-                        ast::LValue::Index { index, array } => {
+                        ast::LValue::Index { index, array, span: _ } => {
                             cursor = array;
                             indexes.push(index.clone());
                         }
-                        ast::LValue::Dereference(_ref) => {
+                        ast::LValue::Dereference(_ref, _span) => {
                             unimplemented![]
                         }
                     }

compiler/noirc_frontend/src/hir/resolution/resolver.rs

@@ -1303,18 +1303,23 @@ impl<'a> Resolver<'a> {
 
                 HirLValue::Ident(ident.0, Type::Error)
             }
-            LValue::MemberAccess { object, field_name } => {
-                let object = Box::new(self.resolve_lvalue(*object));
-                HirLValue::MemberAccess { object, field_name, field_index: None, typ: Type::Error }
-            }
-            LValue::Index { array, index } => {
+            LValue::MemberAccess { object, field_name, span } => HirLValue::MemberAccess {
+                object: Box::new(self.resolve_lvalue(*object)),
+                field_name,
+                location: Location::new(span, self.file),
+                field_index: None,
+                typ: Type::Error,
+            },
+            LValue::Index { array, index, span } => {
                 let array = Box::new(self.resolve_lvalue(*array));
                 let index = self.resolve_expression(index);
-                HirLValue::Index { array, index, typ: Type::Error }
+                let location = Location::new(span, self.file);
+                HirLValue::Index { array, index, location, typ: Type::Error }
             }
-            LValue::Dereference(lvalue) => {
+            LValue::Dereference(lvalue, span) => {
                 let lvalue = Box::new(self.resolve_lvalue(*lvalue));
-                HirLValue::Dereference { lvalue, element_type: Type::Error }
+                let location = Location::new(span, self.file);
+                HirLValue::Dereference { lvalue, location, element_type: Type::Error }
             }
         }
     }

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

@@ -1,5 +1,5 @@
 use iter_extended::vecmap;
-use noirc_errors::{Location, Span};
+use noirc_errors::Span;
 
 use crate::hir_def::expr::{HirExpression, HirIdent, HirLiteral};
 use crate::hir_def::stmt::{
@@ -195,41 +195,43 @@ impl<'interner> TypeChecker<'interner> {
 
                 (typ.clone(), HirLValue::Ident(ident.clone(), typ), mutable)
             }
-            HirLValue::MemberAccess { object, field_name, .. } => {
+            HirLValue::MemberAccess { object, field_name, location, .. } => {
                 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 location = *location;
 
                 let dereference_lhs = move |_: &mut Self, _, element_type| {
                     // We must create a temporary value first to move out of object_ref before
                     // we eventually reassign to it.
                     let id = DefinitionId::dummy_id();
-                    let location = Location::new(span, fm::FileId::dummy());
                     let ident = HirIdent::non_trait_method(id, location);
                     let tmp_value = HirLValue::Ident(ident, Type::Error);
 
                     let lvalue = std::mem::replace(object_ref, Box::new(tmp_value));
-                    *object_ref = Box::new(HirLValue::Dereference { lvalue, element_type });
+                    *object_ref =
+                        Box::new(HirLValue::Dereference { lvalue, element_type, location });
                     *mutable_ref = true;
                 };
 
                 let name = &field_name.0.contents;
                 let (object_type, field_index) = self
-                    .check_field_access(&lhs_type, name, span, Some(dereference_lhs))
+                    .check_field_access(&lhs_type, name, field_name.span(), Some(dereference_lhs))
                     .unwrap_or((Type::Error, 0));
 
                 let field_index = Some(field_index);
                 let typ = object_type.clone();
-                let lvalue = HirLValue::MemberAccess { object, field_name, field_index, typ };
+                let lvalue =
+                    HirLValue::MemberAccess { object, field_name, field_index, typ, location };
                 (object_type, lvalue, mutable)
             }
-            HirLValue::Index { array, index, .. } => {
+            HirLValue::Index { array, index, location, .. } => {
                 let index_type = self.check_expression(index);
                 let expr_span = self.interner.expr_span(index);
+                let location = *location;
 
                 index_type.unify(
                     &Type::polymorphic_integer_or_field(self.interner),
@@ -248,7 +250,8 @@ impl<'interner> TypeChecker<'interner> {
                 // as needed to unwrap any &mut wrappers.
                 while let Type::MutableReference(element) = lvalue_type.follow_bindings() {
                     let element_type = element.as_ref().clone();
-                    lvalue = HirLValue::Dereference { lvalue: Box::new(lvalue), element_type };
+                    lvalue =
+                        HirLValue::Dereference { lvalue: Box::new(lvalue), element_type, location };
                     lvalue_type = *element;
                     // We know this value to be mutable now since we found an `&mut`
                     mutable = true;
@@ -275,11 +278,12 @@ impl<'interner> TypeChecker<'interner> {
                 };
 
                 let array = Box::new(lvalue);
-                (typ.clone(), HirLValue::Index { array, index: *index, typ }, mutable)
+                (typ.clone(), HirLValue::Index { array, index: *index, typ, location }, mutable)
             }
-            HirLValue::Dereference { lvalue, element_type: _ } => {
+            HirLValue::Dereference { lvalue, element_type: _, location } => {
                 let (reference_type, lvalue, _) = self.check_lvalue(lvalue, assign_span);
                 let lvalue = Box::new(lvalue);
+                let location = *location;
 
                 let element_type = Type::type_variable(self.interner.next_type_variable_id());
                 let expected_type = Type::MutableReference(Box::new(element_type.clone()));
@@ -291,7 +295,11 @@ impl<'interner> TypeChecker<'interner> {
                 });
 
                 // Dereferences are always mutable since we already type checked against a &mut T
-                (element_type.clone(), HirLValue::Dereference { lvalue, element_type }, true)
+                (
+                    element_type.clone(),
+                    HirLValue::Dereference { lvalue, element_type, location },
+                    true,
+                )
             }
         }
     }

compiler/noirc_frontend/src/hir_def/stmt.rs

@@ -99,6 +99,15 @@ impl HirPattern {
             | HirPattern::Struct(_, _, location) => location.span,
         }
     }
+
+    pub(crate) fn location(&self) -> Location {
+        match self {
+            HirPattern::Identifier(ident) => ident.location,
+            HirPattern::Mutable(_, location)
+            | HirPattern::Tuple(_, location)
+            | HirPattern::Struct(_, _, location) => *location,
+        }
+    }
 }
 
 /// Represents an Ast form that can be assigned to. These
@@ -111,14 +120,17 @@ pub enum HirLValue {
         field_name: Ident,
         field_index: Option<usize>,
         typ: Type,
+        location: Location,
     },
     Index {
         array: Box<HirLValue>,
         index: ExprId,
         typ: Type,
+        location: Location,
     },
     Dereference {
         lvalue: Box<HirLValue>,
         element_type: Type,
+        location: Location,
     },
 }

compiler/noirc_frontend/src/hir_def/types.rs

@@ -543,11 +543,11 @@ impl TypeBinding {
 pub struct TypeVariableId(pub usize);
 
 impl Type {
-    pub fn default_int_type() -> Type {
+    pub fn default_int_or_field_type() -> Type {
         Type::FieldElement
     }
 
-    pub fn default_range_loop_type() -> Type {
+    pub fn default_int_type() -> Type {
         Type::Integer(Signedness::Unsigned, IntegerBitSize::SixtyFour)
     }
 
@@ -787,7 +787,7 @@ impl std::fmt::Display for Type {
             Type::TypeVariable(var, TypeVariableKind::Normal) => write!(f, "{}", var.borrow()),
             Type::TypeVariable(binding, TypeVariableKind::Integer) => {
                 if let TypeBinding::Unbound(_) = &*binding.borrow() {
-                    write!(f, "{}", TypeVariableKind::Integer.default_type())
+                    write!(f, "{}", Type::default_int_type())
                 } else {
                     write!(f, "{}", binding.borrow())
                 }
@@ -1706,11 +1706,12 @@ impl BinaryTypeOperator {
 impl TypeVariableKind {
     /// Returns the default type this type variable should be bound to if it is still unbound
     /// during monomorphization.
-    pub(crate) fn default_type(&self) -> Type {
+    pub(crate) fn default_type(&self) -> Option<Type> {
         match self {
-            TypeVariableKind::IntegerOrField | TypeVariableKind::Normal => Type::default_int_type(),
-            TypeVariableKind::Integer => Type::default_range_loop_type(),
-            TypeVariableKind::Constant(length) => Type::Constant(*length),
+            TypeVariableKind::IntegerOrField => Some(Type::default_int_or_field_type()),
+            TypeVariableKind::Integer => Some(Type::default_int_type()),
+            TypeVariableKind::Constant(length) => Some(Type::Constant(*length)),
+            TypeVariableKind::Normal => None,
         }
     }
 }
@@ -1744,12 +1745,12 @@ impl From<&Type> for PrintableType {
             },
             Type::TypeVariable(binding, TypeVariableKind::Integer) => match &*binding.borrow() {
                 TypeBinding::Bound(typ) => typ.into(),
-                TypeBinding::Unbound(_) => Type::default_range_loop_type().into(),
+                TypeBinding::Unbound(_) => Type::default_int_type().into(),
             },
             Type::TypeVariable(binding, TypeVariableKind::IntegerOrField) => {
                 match &*binding.borrow() {
                     TypeBinding::Bound(typ) => typ.into(),
-                    TypeBinding::Unbound(_) => Type::default_int_type().into(),
+                    TypeBinding::Unbound(_) => Type::default_int_or_field_type().into(),
                 }
             }
             Type::Bool => PrintableType::Boolean,

compiler/noirc_frontend/src/monomorphization/errors.rs

@@ -0,0 +1,39 @@
+use thiserror::Error;
+
+use noirc_errors::{CustomDiagnostic, FileDiagnostic, Location};
+
+#[derive(Debug, Error)]
+pub enum MonomorphizationError {
+    #[error("Length of generic array could not be determined.")]
+    UnknownArrayLength { location: Location },
+
+    #[error("Type annotations needed")]
+    TypeAnnotationsNeeded { location: Location },
+}
+
+impl MonomorphizationError {
+    fn location(&self) -> Location {
+        match self {
+            MonomorphizationError::UnknownArrayLength { location }
+            | MonomorphizationError::TypeAnnotationsNeeded { location } => *location,
+        }
+    }
+}
+
+impl From<MonomorphizationError> for FileDiagnostic {
+    fn from(error: MonomorphizationError) -> FileDiagnostic {
+        let location = error.location();
+        let call_stack = vec![location];
+        let diagnostic = error.into_diagnostic();
+        diagnostic.in_file(location.file).with_call_stack(call_stack)
+    }
+}
+
+impl MonomorphizationError {
+    fn into_diagnostic(self) -> CustomDiagnostic {
+        let message = self.to_string();
+        let location = self.location();
+
+        CustomDiagnostic::simple_error(message, String::new(), location.span)
+    }
+}