fix: Don't panic when checking if an undeclared variable is mutable (#…

…1987) Fix name resolution panic
noir-lang · Jul 20, 2023 · 0449518 · 0449518
1 parent ac865b1
commit 0449518
Show file tree

Hide file tree

Showing 3 changed files with 34 additions and 21 deletions.
diff --git a/crates/noirc_frontend/src/hir/resolution/resolver.rs b/crates/noirc_frontend/src/hir/resolution/resolver.rs
@@ -160,11 +160,12 @@ impl<'a> Resolver<'a> {
         }
 
         for unused_var in unused_vars.iter() {
-            let definition_info = self.interner.definition(unused_var.id);
-            let name = &definition_info.name;
-            if name != ERROR_IDENT && !definition_info.is_global() {
-                let ident = Ident(Spanned::from(unused_var.location.span, name.to_owned()));
-                self.push_err(ResolverError::UnusedVariable { ident });
+            if let Some(definition_info) = self.interner.try_definition(unused_var.id) {
+                let name = &definition_info.name;
+                if name != ERROR_IDENT && !definition_info.is_global() {
+                    let ident = Ident(Spanned::from(unused_var.location.span, name.to_owned()));
+                    self.push_err(ResolverError::UnusedVariable { ident });
+                }
             }
         }
     }
@@ -1283,14 +1284,15 @@ pub fn verify_mutable_reference(interner: &NodeInterner, rhs: ExprId) -> Result<
             Err(ResolverError::MutableReferenceToArrayElement { span })
         }
         HirExpression::Ident(ident) => {
-            let definition = interner.definition(ident.id);
-            if !definition.mutable {
-                let span = interner.expr_span(&rhs);
-                let variable = definition.name.clone();
-                Err(ResolverError::MutableReferenceToImmutableVariable { span, variable })
-            } else {
-                Ok(())
+            if let Some(definition) = interner.try_definition(ident.id) {
+                if !definition.mutable {
+                    return Err(ResolverError::MutableReferenceToImmutableVariable {
+                        span: interner.expr_span(&rhs),
+                        variable: definition.name.clone(),
+                    });
+                }
             }
+            Ok(())
         }
         _ => Ok(()),
     }

diff --git a/crates/noirc_frontend/src/hir/type_check/stmt.rs b/crates/noirc_frontend/src/hir/type_check/stmt.rs
@@ -128,15 +128,16 @@ impl<'interner> TypeChecker<'interner> {
                     let typ = self.interner.id_type(ident.id).instantiate(self.interner).0;
                     let typ = typ.follow_bindings();
 
-                    let definition = self.interner.definition(ident.id);
-                    if !definition.mutable && !matches!(typ, Type::MutableReference(_)) {
-                        self.errors.push(TypeCheckError::Unstructured {
-                            msg: format!(
-                                "Variable {} must be mutable to be assigned to",
-                                definition.name
-                            ),
-                            span: ident.location.span,
-                        });
+                    if let Some(definition) = self.interner.try_definition(ident.id) {
+                        if !definition.mutable && !matches!(typ, Type::MutableReference(_)) {
+                            self.errors.push(TypeCheckError::Unstructured {
+                                msg: format!(
+                                    "Variable {} must be mutable to be assigned to",
+                                    definition.name
+                                ),
+                                span: ident.location.span,
+                            });
+                        }
                     }
 
                     typ

diff --git a/crates/noirc_frontend/src/node_interner.rs b/crates/noirc_frontend/src/node_interner.rs
@@ -478,10 +478,20 @@ impl NodeInterner {
         }
     }
 
+    /// Retrieves the definition where the given id was defined.
+    /// This will panic if given DefinitionId::dummy_id. Use try_definition for
+    /// any call with a possibly undefined variable.
     pub fn definition(&self, id: DefinitionId) -> &DefinitionInfo {
         &self.definitions[id.0]
     }
 
+    /// Tries to retrieve the given id's definition.
+    /// This function should be used during name resolution or type checking when we cannot be sure
+    /// all variables have corresponding definitions (in case of an error in the user's code).
+    pub fn try_definition(&self, id: DefinitionId) -> Option<&DefinitionInfo> {
+        self.definitions.get(id.0)
+    }
+
     /// Returns the name of the definition
     ///
     /// This is needed as the Environment needs to map variable names to witness indices