diff --git a/compiler/noirc_frontend/src/elaborator/mod.rs b/compiler/noirc_frontend/src/elaborator/mod.rs index 4a5947f15e..55144f8944 100644 --- a/compiler/noirc_frontend/src/elaborator/mod.rs +++ b/compiler/noirc_frontend/src/elaborator/mod.rs @@ -4,8 +4,8 @@ use std::{ }; use crate::{ - ast::ItemVisibility, hir_def::traits::ResolvedTraitBound, usage_tracker::UsageTracker, - StructField, StructType, TypeBindings, + ast::ItemVisibility, hir_def::traits::ResolvedTraitBound, node_interner::GlobalValue, + usage_tracker::UsageTracker, StructField, StructType, TypeBindings, }; use crate::{ ast::{ @@ -1689,7 +1689,7 @@ impl<'context> Elaborator<'context> { self.debug_comptime(location, |interner| value.display(interner).to_string()); - self.interner.get_global_mut(global_id).value = Some(value); + self.interner.get_global_mut(global_id).value = GlobalValue::Resolved(value); } } diff --git a/compiler/noirc_frontend/src/elaborator/types.rs b/compiler/noirc_frontend/src/elaborator/types.rs index 3551edf93f..9aafd690bb 100644 --- a/compiler/noirc_frontend/src/elaborator/types.rs +++ b/compiler/noirc_frontend/src/elaborator/types.rs @@ -28,8 +28,8 @@ use crate::{ traits::{NamedType, ResolvedTraitBound, Trait, TraitConstraint}, }, node_interner::{ - DependencyId, ExprId, ImplSearchErrorKind, NodeInterner, TraitId, TraitImplKind, - TraitMethodId, + DependencyId, ExprId, GlobalValue, ImplSearchErrorKind, NodeInterner, TraitId, + TraitImplKind, TraitMethodId, }, token::SecondaryAttribute, Generics, Kind, ResolvedGeneric, Type, TypeBinding, TypeBindings, UnificationError, @@ -426,7 +426,8 @@ impl<'context> Elaborator<'context> { let rhs = stmt.expression; let span = self.interner.expr_span(&rhs); - let Some(global_value) = &self.interner.get_global(id).value else { + let GlobalValue::Resolved(global_value) = &self.interner.get_global(id).value + else { self.push_err(ResolverError::UnevaluatedGlobalType { span }); return None; }; diff --git a/compiler/noirc_frontend/src/hir/comptime/errors.rs b/compiler/noirc_frontend/src/hir/comptime/errors.rs index 3df20b3920..6ff918328a 100644 --- a/compiler/noirc_frontend/src/hir/comptime/errors.rs +++ b/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) + } } } } diff --git a/compiler/noirc_frontend/src/hir/comptime/interpreter.rs b/compiler/noirc_frontend/src/hir/comptime/interpreter.rs index b6ae462976..e9e37243e0 100644 --- a/compiler/noirc_frontend/src/hir/comptime/interpreter.rs +++ b/compiler/noirc_frontend/src/hir/comptime/interpreter.rs @@ -20,6 +20,7 @@ use crate::monomorphization::{ perform_impl_bindings, perform_instantiation_bindings, resolve_trait_method, undo_instantiation_bindings, }; +use crate::node_interner::GlobalValue; use crate::token::{FmtStrFragment, Tokens}; use crate::TypeVariable; use crate::{ @@ -568,26 +569,39 @@ 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 { - Ok(value.clone()) - } 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( - || { + let global_id = *global_id; + let global_info = self.elaborator.interner.get_global(global_id); + let global_crate_id = global_info.crate_id; + match &global_info.value { + GlobalValue::Resolved(value) => Ok(value.clone()), + GlobalValue::Resolving => { + // Note that the error we issue here isn't very informative (it doesn't include the actual cycle) + // but the general dependency cycle detector will give a better error later on during compilation. + let location = self.elaborator.interner.expr_location(&id); + Err(InterpreterError::GlobalsDependencyCycle { location }) + } + GlobalValue::Unresolved => { + let let_ = self + .elaborator + .interner + .get_global_let_statement(global_id) + .ok_or_else(|| { let location = self.elaborator.interner.expr_location(&id); InterpreterError::VariableNotInScope { location } - }, - )?; + })?; - if let_.runs_comptime() || crate_of_global != self.crate_id { - self.evaluate_let(let_.clone())?; - } + self.elaborator.interner.get_global_mut(global_id).value = + GlobalValue::Resolving; + + if let_.runs_comptime() || global_crate_id != self.crate_id { + self.evaluate_let(let_.clone())?; + } - let value = self.lookup(&ident)?; - self.elaborator.interner.get_global_mut(global_id).value = Some(value.clone()); - Ok(value) + let value = self.lookup(&ident)?; + self.elaborator.interner.get_global_mut(global_id).value = + GlobalValue::Resolved(value.clone()); + Ok(value) + } } } DefinitionKind::NumericGeneric(type_variable, numeric_typ) => { diff --git a/compiler/noirc_frontend/src/monomorphization/mod.rs b/compiler/noirc_frontend/src/monomorphization/mod.rs index 5297a89cc9..8c07d71de2 100644 --- a/compiler/noirc_frontend/src/monomorphization/mod.rs +++ b/compiler/noirc_frontend/src/monomorphization/mod.rs @@ -11,7 +11,7 @@ use crate::ast::{FunctionKind, IntegerBitSize, Signedness, UnaryOp, Visibility}; use crate::hir::comptime::InterpreterError; use crate::hir::type_check::{NoMatchingImplFoundError, TypeCheckError}; -use crate::node_interner::{ExprId, ImplSearchErrorKind}; +use crate::node_interner::{ExprId, GlobalValue, ImplSearchErrorKind}; use crate::token::FmtStrFragment; use crate::{ debug::DebugInstrumenter, @@ -895,7 +895,7 @@ impl<'interner> Monomorphizer<'interner> { DefinitionKind::Global(global_id) => { let global = self.interner.get_global(*global_id); - let expr = if let Some(value) = global.value.clone() { + let expr = if let GlobalValue::Resolved(value) = global.value.clone() { value .into_hir_expression(self.interner, global.location) .map_err(MonomorphizationError::InterpreterError)? diff --git a/compiler/noirc_frontend/src/node_interner.rs b/compiler/noirc_frontend/src/node_interner.rs index ae30b68c09..bee703a4de 100644 --- a/compiler/noirc_frontend/src/node_interner.rs +++ b/compiler/noirc_frontend/src/node_interner.rs @@ -609,7 +609,14 @@ pub struct GlobalInfo { pub crate_id: CrateId, pub location: Location, pub let_statement: StmtId, - pub value: Option, + pub value: GlobalValue, +} + +#[derive(Debug, Clone)] +pub enum GlobalValue { + Unresolved, + Resolving, + Resolved(comptime::Value), } #[derive(Debug, Copy, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)] @@ -861,7 +868,7 @@ impl NodeInterner { crate_id, let_statement, location, - value: None, + value: GlobalValue::Unresolved, }); self.global_attributes.insert(id, attributes); id diff --git a/compiler/noirc_frontend/src/tests.rs b/compiler/noirc_frontend/src/tests.rs index 1aed3d62d3..2d7cf8acca 100644 --- a/compiler/noirc_frontend/src/tests.rs +++ b/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 { .. }) + ))); +}