fix: detect cycles in globals

compiler/noirc_frontend/src/hir/comptime/errors.rs

@@ -243,6 +243,9 @@ pub enum InterpreterError {
     CannotInterpretFormatStringWithErrors {
         location: Location,
     },
+    GlobalsDependencyCycle {
+        location: Location,
+    },
 
     // These cases are not errors, they are just used to prevent us from running more code
     // until the loop can be resumed properly. These cases will never be displayed to users.
@@ -319,7 +322,8 @@ impl InterpreterError {
             | InterpreterError::CannotResolveExpression { location, .. }
             | InterpreterError::CannotSetFunctionBody { location, .. }
             | InterpreterError::UnknownArrayLength { location, .. }
-            | InterpreterError::CannotInterpretFormatStringWithErrors { location } => *location,
+            | InterpreterError::CannotInterpretFormatStringWithErrors { location }
+            | InterpreterError::GlobalsDependencyCycle { location } => *location,
 
             InterpreterError::FailedToParseMacro { error, file, .. } => {
                 Location::new(error.span(), *file)
@@ -674,6 +678,11 @@ impl<'a> From<&'a InterpreterError> for CustomDiagnostic {
                     "Some of the variables to interpolate could not be evaluated".to_string();
                 CustomDiagnostic::simple_error(msg, secondary, location.span)
             }
+            InterpreterError::GlobalsDependencyCycle { location } => {
+                let msg = "This global recursively depends on itself".to_string();
+                let secondary = String::new();
+                CustomDiagnostic::simple_error(msg, secondary, location.span)
+            }
         }
     }
 }

compiler/noirc_frontend/src/hir/comptime/interpreter.rs

@@ -1,4 +1,4 @@
-use std::collections::VecDeque;
+use std::collections::{HashSet, VecDeque};
 use std::{collections::hash_map::Entry, rc::Rc};
 
 use acvm::blackbox_solver::BigIntSolverWithId;
@@ -20,6 +20,7 @@ use crate::monomorphization::{
     perform_impl_bindings, perform_instantiation_bindings, resolve_trait_method,
     undo_instantiation_bindings,
 };
+use crate::node_interner::GlobalId;
 use crate::token::{FmtStrFragment, Tokens};
 use crate::TypeVariable;
 use crate::{
@@ -66,6 +67,12 @@ pub struct Interpreter<'local, 'interner> {
 
     /// Stateful bigint calculator.
     bigint_solver: BigIntSolverWithId,
+
+    /// Globals currently being interpreted, to detect recursive cycles.
+    /// Note that recursive cycles are also detected by NodeInterner,
+    /// it's just that the error message there happens after we evaluate globals:
+    /// if we don't detect cycles here too the program will stack overflow.
+    globals_being_interpreted: HashSet<GlobalId>,
 }
 
 #[allow(unused)]
@@ -82,6 +89,7 @@ impl<'local, 'interner> Interpreter<'local, 'interner> {
             bound_generics: Vec::new(),
             in_loop: false,
             bigint_solver: BigIntSolverWithId::default(),
+            globals_being_interpreted: HashSet::new(),
         }
     }
 
@@ -568,11 +576,14 @@ impl<'local, 'interner> Interpreter<'local, 'interner> {
             DefinitionKind::Local(_) => self.lookup(&ident),
             DefinitionKind::Global(global_id) => {
                 // Avoid resetting the value if it is already known
-                if let Some(value) = &self.elaborator.interner.get_global(*global_id).value {
+                let global_id = *global_id;
+                let global_info = self.elaborator.interner.get_global(global_id);
+                if let Some(value) = &global_info.value {
                     Ok(value.clone())
+                } else if self.globals_being_interpreted.contains(&global_id) {
+                    let location = self.elaborator.interner.expr_location(&id);
+                    Err(InterpreterError::GlobalsDependencyCycle { location })
                 } else {
-                    let global_id = *global_id;
-                    let crate_of_global = self.elaborator.interner.get_global(global_id).crate_id;
                     let let_ =
                         self.elaborator.interner.get_global_let_statement(global_id).ok_or_else(
                             || {
@@ -581,8 +592,10 @@ impl<'local, 'interner> Interpreter<'local, 'interner> {
                             },
                         )?;
 
-                    if let_.runs_comptime() || crate_of_global != self.crate_id {
+                    if let_.runs_comptime() || global_info.crate_id != self.crate_id {
+                        self.globals_being_interpreted.insert(global_id);
                         self.evaluate_let(let_.clone())?;
+                        self.globals_being_interpreted.remove(&global_id);
                     }
 
                     let value = self.lookup(&ident)?;

compiler/noirc_frontend/src/tests.rs

@@ -3855,3 +3855,23 @@ fn disallows_underscore_on_right_hand_side() {
 
     assert_eq!(name, "_");
 }
+
+#[test]
+fn errors_on_cyclic_globals() {
+    let src = r#"
+    pub comptime global A: u32 = B;
+    pub comptime global B: u32 = A;
+
+    fn main() { }
+    "#;
+    let errors = get_program_errors(src);
+
+    assert!(errors.iter().any(|(error, _)| matches!(
+        error,
+        CompilationError::InterpreterError(InterpreterError::GlobalsDependencyCycle { .. })
+    )));
+    assert!(errors.iter().any(|(error, _)| matches!(
+        error,
+        CompilationError::ResolverError(ResolverError::DependencyCycle { .. })
+    )));
+}