From f2d1a7dacc57f89e2765f80f346c50e5e0d06f4e Mon Sep 17 00:00:00 2001 From: Jake Fecher Date: Mon, 1 Jul 2024 15:17:46 -0500 Subject: [PATCH 1/3] Fix panic when using undefined variables --- .../noirc_frontend/src/hir/comptime/errors.rs | 7 +++++++ .../src/hir/comptime/interpreter.rs | 20 +++++++++---------- .../comptime_var_not_defined/Nargo.toml | 7 +++++++ .../comptime_var_not_defined/src/main.nr | 5 +++++ 4 files changed, 29 insertions(+), 10 deletions(-) create mode 100644 test_programs/compile_failure/comptime_var_not_defined/Nargo.toml create mode 100644 test_programs/compile_failure/comptime_var_not_defined/src/main.nr diff --git a/compiler/noirc_frontend/src/hir/comptime/errors.rs b/compiler/noirc_frontend/src/hir/comptime/errors.rs index d2c7acee2a3..353682f71e0 100644 --- a/compiler/noirc_frontend/src/hir/comptime/errors.rs +++ b/compiler/noirc_frontend/src/hir/comptime/errors.rs @@ -16,6 +16,7 @@ pub enum InterpreterError { ArgumentCountMismatch { expected: usize, actual: usize, location: Location }, TypeMismatch { expected: Type, value: Value, location: Location }, NonComptimeVarReferenced { name: String, location: Location }, + VariableNotInScope { location: Location }, IntegerOutOfRangeForType { value: FieldElement, typ: Type, location: Location }, ErrorNodeEncountered { location: Location }, NonFunctionCalled { value: Value, location: Location }, @@ -78,6 +79,7 @@ impl InterpreterError { InterpreterError::ArgumentCountMismatch { location, .. } | InterpreterError::TypeMismatch { location, .. } | InterpreterError::NonComptimeVarReferenced { location, .. } + | InterpreterError::VariableNotInScope { location, .. } | InterpreterError::IntegerOutOfRangeForType { location, .. } | InterpreterError::ErrorNodeEncountered { location, .. } | InterpreterError::NonFunctionCalled { location, .. } @@ -143,6 +145,11 @@ impl<'a> From<&'a InterpreterError> for CustomDiagnostic { let secondary = "Non-comptime variables can't be used in comptime code".to_string(); CustomDiagnostic::simple_error(msg, secondary, location.span) } + InterpreterError::VariableNotInScope { location } => { + let msg = "Variable not in scope".to_string(); + let secondary = "Could not find variable".to_string(); + CustomDiagnostic::simple_error(msg, secondary, location.span) + } InterpreterError::IntegerOutOfRangeForType { value, typ, location } => { let int = match value.try_into_u128() { Some(int) => int.to_string(), diff --git a/compiler/noirc_frontend/src/hir/comptime/interpreter.rs b/compiler/noirc_frontend/src/hir/comptime/interpreter.rs index d2b98569bbb..dc1a66a55c3 100644 --- a/compiler/noirc_frontend/src/hir/comptime/interpreter.rs +++ b/compiler/noirc_frontend/src/hir/comptime/interpreter.rs @@ -289,8 +289,7 @@ impl<'a> Interpreter<'a> { return Ok(()); } } - let name = self.interner.definition(id).name.clone(); - Err(InterpreterError::NonComptimeVarReferenced { name, location }) + Err(InterpreterError::VariableNotInScope { location }) } pub(super) fn lookup(&self, ident: &HirIdent) -> IResult { @@ -304,12 +303,7 @@ impl<'a> Interpreter<'a> { } } - // Justification for `NonComptimeVarReferenced`: - // If we have an id to lookup at all that means name resolution successfully - // found another variable in scope for this name. If the name is in scope - // but unknown by the interpreter it must be because it was not a comptime variable. - let name = self.interner.definition(id).name.clone(); - Err(InterpreterError::NonComptimeVarReferenced { name, location }) + Err(InterpreterError::VariableNotInScope { location }) } /// Evaluate an expression and return the result @@ -345,7 +339,10 @@ impl<'a> Interpreter<'a> { } pub(super) fn evaluate_ident(&mut self, ident: HirIdent, id: ExprId) -> IResult { - let definition = self.interner.definition(ident.id); + let definition = self.interner.try_definition(ident.id).ok_or_else(|| { + let location = self.interner.expr_location(&id); + InterpreterError::VariableNotInScope { location } + })?; match &definition.kind { DefinitionKind::Function(function_id) => { @@ -359,7 +356,10 @@ impl<'a> Interpreter<'a> { if let Ok(value) = self.lookup(&ident) { Ok(value) } else { - let let_ = self.interner.get_global_let_statement(*global_id).unwrap(); + let let_ = self + .interner + .get_global_let_statement(*global_id) + .expect("No definition for global"); if let_.comptime { self.evaluate_let(let_.clone())?; } diff --git a/test_programs/compile_failure/comptime_var_not_defined/Nargo.toml b/test_programs/compile_failure/comptime_var_not_defined/Nargo.toml new file mode 100644 index 00000000000..f2604ee0692 --- /dev/null +++ b/test_programs/compile_failure/comptime_var_not_defined/Nargo.toml @@ -0,0 +1,7 @@ +[package] +name = "comptime_var_not_defined" +type = "bin" +authors = [""] +compiler_version = ">=0.31.0" + +[dependencies] diff --git a/test_programs/compile_failure/comptime_var_not_defined/src/main.nr b/test_programs/compile_failure/comptime_var_not_defined/src/main.nr new file mode 100644 index 00000000000..ab4ea69e96c --- /dev/null +++ b/test_programs/compile_failure/comptime_var_not_defined/src/main.nr @@ -0,0 +1,5 @@ +fn main() { + comptime { + foo(); + } +} From 031840cdf48cb52394ad0657ff4208eb24a45ede Mon Sep 17 00:00:00 2001 From: Jake Fecher Date: Mon, 1 Jul 2024 15:57:32 -0500 Subject: [PATCH 2/3] Remove one more unwrap --- .../noirc_frontend/src/hir/comptime/interpreter.rs | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/compiler/noirc_frontend/src/hir/comptime/interpreter.rs b/compiler/noirc_frontend/src/hir/comptime/interpreter.rs index dc1a66a55c3..eebf6f9a948 100644 --- a/compiler/noirc_frontend/src/hir/comptime/interpreter.rs +++ b/compiler/noirc_frontend/src/hir/comptime/interpreter.rs @@ -356,10 +356,12 @@ impl<'a> Interpreter<'a> { if let Ok(value) = self.lookup(&ident) { Ok(value) } else { - let let_ = self - .interner - .get_global_let_statement(*global_id) - .expect("No definition for global"); + let let_ = + self.interner.get_global_let_statement(*global_id).ok_or_else(|| { + let location = self.interner.expr_location(&id); + InterpreterError::VariableNotInScope { location } + })?; + if let_.comptime { self.evaluate_let(let_.clone())?; } From d190dd96b1780c3c2bf0fad28af6adf1ce18962a Mon Sep 17 00:00:00 2001 From: Jake Fecher Date: Tue, 2 Jul 2024 12:26:45 -0500 Subject: [PATCH 3/3] Re-add old error --- compiler/noirc_frontend/src/hir/comptime/interpreter.rs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/compiler/noirc_frontend/src/hir/comptime/interpreter.rs b/compiler/noirc_frontend/src/hir/comptime/interpreter.rs index 00ef7d401cd..959163d2d61 100644 --- a/compiler/noirc_frontend/src/hir/comptime/interpreter.rs +++ b/compiler/noirc_frontend/src/hir/comptime/interpreter.rs @@ -312,7 +312,12 @@ impl<'a> Interpreter<'a> { } } - Err(InterpreterError::VariableNotInScope { location }) + if id == DefinitionId::dummy_id() { + Err(InterpreterError::VariableNotInScope { location }) + } else { + let name = self.interner.definition_name(id).to_string(); + Err(InterpreterError::NonComptimeVarReferenced { name, location }) + } } /// Evaluate an expression and return the result