From ddb46733fcf596b5c8508a208b2690df52aa16e3 Mon Sep 17 00:00:00 2001 From: Michael J Klein Date: Mon, 16 Dec 2024 15:05:52 -0500 Subject: [PATCH] feat: replace `eval_global_as_array_length` with type/interpreter evaluation (#6469) Co-authored-by: Tom French <15848336+TomAFrench@users.noreply.github.com> Co-authored-by: jfecher Co-authored-by: Ary Borenszweig --- compiler/noirc_frontend/src/ast/statement.rs | 2 + compiler/noirc_frontend/src/elaborator/mod.rs | 23 +-- .../noirc_frontend/src/elaborator/patterns.rs | 4 +- .../src/elaborator/statements.rs | 5 +- .../noirc_frontend/src/elaborator/types.rs | 158 ++++-------------- .../src/hir/comptime/interpreter.rs | 2 +- .../noirc_frontend/src/hir/comptime/value.rs | 39 +++-- .../src/hir/resolution/errors.rs | 43 ++++- .../src/hir/type_check/errors.rs | 12 -- compiler/noirc_frontend/src/hir_def/stmt.rs | 16 ++ compiler/noirc_frontend/src/hir_def/types.rs | 10 +- .../src/monomorphization/mod.rs | 3 +- compiler/noirc_frontend/src/node_interner.rs | 2 + .../src/parser/parser/global.rs | 5 +- .../src/parser/parser/statement.rs | 9 +- compiler/noirc_frontend/src/tests.rs | 93 +++-------- .../src/tests/arithmetic_generics.rs | 46 +++++ tooling/lsp/src/requests/hover.rs | 28 ++-- 18 files changed, 248 insertions(+), 252 deletions(-) diff --git a/compiler/noirc_frontend/src/ast/statement.rs b/compiler/noirc_frontend/src/ast/statement.rs index c77fe7513a1..0ce21f9f798 100644 --- a/compiler/noirc_frontend/src/ast/statement.rs +++ b/compiler/noirc_frontend/src/ast/statement.rs @@ -154,6 +154,7 @@ impl StatementKind { r#type, expression, comptime: false, + is_global_let: false, attributes, }) } @@ -562,6 +563,7 @@ pub struct LetStatement { // True if this should only be run during compile-time pub comptime: bool, + pub is_global_let: bool, } #[derive(Debug, PartialEq, Eq, Clone)] diff --git a/compiler/noirc_frontend/src/elaborator/mod.rs b/compiler/noirc_frontend/src/elaborator/mod.rs index fe1d1e38e1a..4a5947f15e3 100644 --- a/compiler/noirc_frontend/src/elaborator/mod.rs +++ b/compiler/noirc_frontend/src/elaborator/mod.rs @@ -1652,20 +1652,13 @@ impl<'context> Elaborator<'context> { self.push_err(ResolverError::MutableGlobal { span }); } - let comptime = let_stmt.comptime; - - let (let_statement, _typ) = if comptime { - self.elaborate_in_comptime_context(|this| this.elaborate_let(let_stmt, Some(global_id))) - } else { - self.elaborate_let(let_stmt, Some(global_id)) - }; + let (let_statement, _typ) = self + .elaborate_in_comptime_context(|this| this.elaborate_let(let_stmt, Some(global_id))); let statement_id = self.interner.get_global(global_id).let_statement; self.interner.replace_statement(statement_id, let_statement); - if comptime { - self.elaborate_comptime_global(global_id); - } + self.elaborate_comptime_global(global_id); if let Some(name) = name { self.interner.register_global(global_id, name, global.visibility, self.module_id()); @@ -1700,6 +1693,16 @@ impl<'context> Elaborator<'context> { } } + /// If the given global is unresolved, elaborate it and return true + fn elaborate_global_if_unresolved(&mut self, global_id: &GlobalId) -> bool { + if let Some(global) = self.unresolved_globals.remove(global_id) { + self.elaborate_global(global); + true + } else { + false + } + } + fn define_function_metas( &mut self, functions: &mut [UnresolvedFunctions], diff --git a/compiler/noirc_frontend/src/elaborator/patterns.rs b/compiler/noirc_frontend/src/elaborator/patterns.rs index 3fbdadbbee8..0406321de42 100644 --- a/compiler/noirc_frontend/src/elaborator/patterns.rs +++ b/compiler/noirc_frontend/src/elaborator/patterns.rs @@ -666,9 +666,7 @@ impl<'context> Elaborator<'context> { self.interner.add_function_reference(func_id, hir_ident.location); } DefinitionKind::Global(global_id) => { - if let Some(global) = self.unresolved_globals.remove(&global_id) { - self.elaborate_global(global); - } + self.elaborate_global_if_unresolved(&global_id); if let Some(current_item) = self.current_item { self.interner.add_global_dependency(current_item, global_id); } diff --git a/compiler/noirc_frontend/src/elaborator/statements.rs b/compiler/noirc_frontend/src/elaborator/statements.rs index 93009f49071..8a46d85d563 100644 --- a/compiler/noirc_frontend/src/elaborator/statements.rs +++ b/compiler/noirc_frontend/src/elaborator/statements.rs @@ -76,6 +76,7 @@ impl<'context> Elaborator<'context> { ) -> (HirStatement, Type) { let expr_span = let_stmt.expression.span; let (expression, expr_type) = self.elaborate_expression(let_stmt.expression); + let type_contains_unspecified = let_stmt.r#type.contains_unspecified(); let annotated_type = self.resolve_inferred_type(let_stmt.r#type); @@ -123,7 +124,9 @@ impl<'context> Elaborator<'context> { let attributes = let_stmt.attributes; let comptime = let_stmt.comptime; - let let_ = HirLetStatement { pattern, r#type, expression, attributes, comptime }; + let is_global_let = let_stmt.is_global_let; + let let_ = + HirLetStatement::new(pattern, r#type, expression, attributes, comptime, is_global_let); (HirStatement::Let(let_), Type::Unit) } diff --git a/compiler/noirc_frontend/src/elaborator/types.rs b/compiler/noirc_frontend/src/elaborator/types.rs index 2e4809f3511..3551edf93f7 100644 --- a/compiler/noirc_frontend/src/elaborator/types.rs +++ b/compiler/noirc_frontend/src/elaborator/types.rs @@ -1,6 +1,5 @@ use std::{borrow::Cow, rc::Rc}; -use acvm::acir::AcirField; use iter_extended::vecmap; use noirc_errors::{Location, Span}; use rustc_hash::FxHashMap as HashMap; @@ -12,7 +11,6 @@ use crate::{ UnresolvedTypeData, UnresolvedTypeExpression, WILDCARD_TYPE, }, hir::{ - comptime::{Interpreter, Value}, def_collector::dc_crate::CompilationError, resolution::{errors::ResolverError, import::PathResolutionError}, type_check::{ @@ -30,8 +28,8 @@ use crate::{ traits::{NamedType, ResolvedTraitBound, Trait, TraitConstraint}, }, node_interner::{ - DefinitionKind, DependencyId, ExprId, GlobalId, ImplSearchErrorKind, NodeInterner, TraitId, - TraitImplKind, TraitMethodId, + DependencyId, ExprId, ImplSearchErrorKind, NodeInterner, TraitId, TraitImplKind, + TraitMethodId, }, token::SecondaryAttribute, Generics, Kind, ResolvedGeneric, Type, TypeBinding, TypeBindings, UnificationError, @@ -415,17 +413,40 @@ impl<'context> Elaborator<'context> { .map(|let_statement| Kind::numeric(let_statement.r#type)) .unwrap_or(Kind::u32()); - // TODO(https://github.com/noir-lang/noir/issues/6238): - // support non-u32 generics here - if !kind.unifies(&Kind::u32()) { - let error = TypeCheckError::EvaluatedGlobalIsntU32 { - expected_kind: Kind::u32().to_string(), - expr_kind: kind.to_string(), - expr_span: path.span(), - }; - self.push_err(error); - } - Some(Type::Constant(self.eval_global_as_array_length(id, path).into(), kind)) + let Some(stmt) = self.interner.get_global_let_statement(id) else { + if self.elaborate_global_if_unresolved(&id) { + return self.lookup_generic_or_global_type(path); + } else { + let path = path.clone(); + self.push_err(ResolverError::NoSuchNumericTypeVariable { path }); + return None; + } + }; + + let rhs = stmt.expression; + let span = self.interner.expr_span(&rhs); + + let Some(global_value) = &self.interner.get_global(id).value else { + self.push_err(ResolverError::UnevaluatedGlobalType { span }); + return None; + }; + + let Some(global_value) = global_value.to_field_element() else { + let global_value = global_value.clone(); + if global_value.is_integral() { + self.push_err(ResolverError::NegativeGlobalType { span, global_value }); + } else { + self.push_err(ResolverError::NonIntegralGlobalType { span, global_value }); + } + return None; + }; + + let Ok(global_value) = kind.ensure_value_fits(global_value, span) else { + self.push_err(ResolverError::GlobalLargerThanKind { span, global_value, kind }); + return None; + }; + + Some(Type::Constant(global_value, kind)) } _ => None, } @@ -633,31 +654,6 @@ impl<'context> Elaborator<'context> { .or_else(|| self.resolve_trait_method_by_named_generic(path)) } - fn eval_global_as_array_length(&mut self, global_id: GlobalId, path: &Path) -> u32 { - let Some(stmt) = self.interner.get_global_let_statement(global_id) else { - if let Some(global) = self.unresolved_globals.remove(&global_id) { - self.elaborate_global(global); - return self.eval_global_as_array_length(global_id, path); - } else { - let path = path.clone(); - self.push_err(ResolverError::NoSuchNumericTypeVariable { path }); - return 0; - } - }; - - let length = stmt.expression; - let span = self.interner.expr_span(&length); - let result = try_eval_array_length_id(self.interner, length, span); - - match result.map(|length| length.try_into()) { - Ok(Ok(length_value)) => return length_value, - Ok(Err(_cast_err)) => self.push_err(ResolverError::IntegerTooLarge { span }), - Err(Some(error)) => self.push_err(error), - Err(None) => (), - } - 0 - } - pub(super) fn unify( &mut self, actual: &Type, @@ -1834,88 +1830,6 @@ fn bind_generic(param: &ResolvedGeneric, arg: &Type, bindings: &mut TypeBindings } } -pub fn try_eval_array_length_id( - interner: &NodeInterner, - rhs: ExprId, - span: Span, -) -> Result> { - // Arbitrary amount of recursive calls to try before giving up - let fuel = 100; - try_eval_array_length_id_with_fuel(interner, rhs, span, fuel) -} - -fn try_eval_array_length_id_with_fuel( - interner: &NodeInterner, - rhs: ExprId, - span: Span, - fuel: u32, -) -> Result> { - if fuel == 0 { - // If we reach here, it is likely from evaluating cyclic globals. We expect an error to - // be issued for them after name resolution so issue no error now. - return Err(None); - } - - match interner.expression(&rhs) { - HirExpression::Literal(HirLiteral::Integer(int, false)) => { - int.try_into_u128().ok_or(Some(ResolverError::IntegerTooLarge { span })) - } - HirExpression::Ident(ident, _) => { - if let Some(definition) = interner.try_definition(ident.id) { - match definition.kind { - DefinitionKind::Global(global_id) => { - let let_statement = interner.get_global_let_statement(global_id); - if let Some(let_statement) = let_statement { - let expression = let_statement.expression; - try_eval_array_length_id_with_fuel(interner, expression, span, fuel - 1) - } else { - Err(Some(ResolverError::InvalidArrayLengthExpr { span })) - } - } - _ => Err(Some(ResolverError::InvalidArrayLengthExpr { span })), - } - } else { - Err(Some(ResolverError::InvalidArrayLengthExpr { span })) - } - } - HirExpression::Infix(infix) => { - let lhs = try_eval_array_length_id_with_fuel(interner, infix.lhs, span, fuel - 1)?; - let rhs = try_eval_array_length_id_with_fuel(interner, infix.rhs, span, fuel - 1)?; - - match infix.operator.kind { - BinaryOpKind::Add => Ok(lhs + rhs), - BinaryOpKind::Subtract => Ok(lhs - rhs), - BinaryOpKind::Multiply => Ok(lhs * rhs), - BinaryOpKind::Divide => Ok(lhs / rhs), - BinaryOpKind::Equal => Ok((lhs == rhs) as u128), - BinaryOpKind::NotEqual => Ok((lhs != rhs) as u128), - BinaryOpKind::Less => Ok((lhs < rhs) as u128), - BinaryOpKind::LessEqual => Ok((lhs <= rhs) as u128), - BinaryOpKind::Greater => Ok((lhs > rhs) as u128), - BinaryOpKind::GreaterEqual => Ok((lhs >= rhs) as u128), - BinaryOpKind::And => Ok(lhs & rhs), - BinaryOpKind::Or => Ok(lhs | rhs), - BinaryOpKind::Xor => Ok(lhs ^ rhs), - BinaryOpKind::ShiftRight => Ok(lhs >> rhs), - BinaryOpKind::ShiftLeft => Ok(lhs << rhs), - BinaryOpKind::Modulo => Ok(lhs % rhs), - } - } - HirExpression::Cast(cast) => { - let lhs = try_eval_array_length_id_with_fuel(interner, cast.lhs, span, fuel - 1)?; - let lhs_value = Value::Field(lhs.into()); - let evaluated_value = - Interpreter::evaluate_cast_one_step(&cast, rhs, lhs_value, interner) - .map_err(|error| Some(ResolverError::ArrayLengthInterpreter { error }))?; - - evaluated_value - .to_u128() - .ok_or_else(|| Some(ResolverError::InvalidArrayLengthExpr { span })) - } - _other => Err(Some(ResolverError::InvalidArrayLengthExpr { span })), - } -} - /// Gives an error if a user tries to create a mutable reference /// to an immutable variable. fn verify_mutable_reference(interner: &NodeInterner, rhs: ExprId) -> Result<(), ResolverError> { diff --git a/compiler/noirc_frontend/src/hir/comptime/interpreter.rs b/compiler/noirc_frontend/src/hir/comptime/interpreter.rs index dfa55a9d79b..d717ec84d1f 100644 --- a/compiler/noirc_frontend/src/hir/comptime/interpreter.rs +++ b/compiler/noirc_frontend/src/hir/comptime/interpreter.rs @@ -581,7 +581,7 @@ impl<'local, 'interner> Interpreter<'local, 'interner> { }, )?; - if let_.comptime || crate_of_global != self.crate_id { + if let_.runs_comptime() || crate_of_global != self.crate_id { self.evaluate_let(let_.clone())?; } diff --git a/compiler/noirc_frontend/src/hir/comptime/value.rs b/compiler/noirc_frontend/src/hir/comptime/value.rs index 945fb45026d..e5c55e2b0be 100644 --- a/compiler/noirc_frontend/src/hir/comptime/value.rs +++ b/compiler/noirc_frontend/src/hir/comptime/value.rs @@ -1,6 +1,6 @@ use std::{borrow::Cow, rc::Rc, vec}; -use acvm::{AcirField, FieldElement}; +use acvm::FieldElement; use im::Vector; use iter_extended::{try_vecmap, vecmap}; use noirc_errors::{Location, Span}; @@ -409,6 +409,9 @@ impl Value { Value::Pointer(element, true) => { return element.unwrap_or_clone().into_hir_expression(interner, location); } + Value::Closure(hir_lambda, _args, _typ, _opt_func_id, _module_id) => { + HirExpression::Lambda(hir_lambda) + } Value::TypedExpr(TypedExpr::StmtId(..)) | Value::Expr(..) | Value::Pointer(..) @@ -420,7 +423,6 @@ impl Value { | Value::Zeroed(_) | Value::Type(_) | Value::UnresolvedType(_) - | Value::Closure(..) | Value::ModuleDefinition(_) => { let typ = self.get_type().into_owned(); let value = self.display(interner).to_string(); @@ -502,19 +504,28 @@ impl Value { Ok(vec![token]) } - /// Converts any unsigned `Value` into a `u128`. - /// Returns `None` for negative integers. - pub(crate) fn to_u128(&self) -> Option { + /// Returns false for non-integral `Value`s. + pub(crate) fn is_integral(&self) -> bool { + use Value::*; + matches!( + self, + Field(_) | I8(_) | I16(_) | I32(_) | I64(_) | U8(_) | U16(_) | U32(_) | U64(_) + ) + } + + /// Converts any non-negative `Value` into a `FieldElement`. + /// Returns `None` for negative integers and non-integral `Value`s. + pub(crate) fn to_field_element(&self) -> Option { match self { - Self::Field(value) => Some(value.to_u128()), - Self::I8(value) => (*value >= 0).then_some(*value as u128), - Self::I16(value) => (*value >= 0).then_some(*value as u128), - Self::I32(value) => (*value >= 0).then_some(*value as u128), - Self::I64(value) => (*value >= 0).then_some(*value as u128), - Self::U8(value) => Some(*value as u128), - Self::U16(value) => Some(*value as u128), - Self::U32(value) => Some(*value as u128), - Self::U64(value) => Some(*value as u128), + Self::Field(value) => Some(*value), + Self::I8(value) => (*value >= 0).then_some((*value as u128).into()), + Self::I16(value) => (*value >= 0).then_some((*value as u128).into()), + Self::I32(value) => (*value >= 0).then_some((*value as u128).into()), + Self::I64(value) => (*value >= 0).then_some((*value as u128).into()), + Self::U8(value) => Some((*value as u128).into()), + Self::U16(value) => Some((*value as u128).into()), + Self::U32(value) => Some((*value as u128).into()), + Self::U64(value) => Some((*value as u128).into()), _ => None, } } diff --git a/compiler/noirc_frontend/src/hir/resolution/errors.rs b/compiler/noirc_frontend/src/hir/resolution/errors.rs index 774836f8992..8817904c6f5 100644 --- a/compiler/noirc_frontend/src/hir/resolution/errors.rs +++ b/compiler/noirc_frontend/src/hir/resolution/errors.rs @@ -5,10 +5,13 @@ use thiserror::Error; use crate::{ ast::{Ident, UnsupportedNumericGenericType}, - hir::{comptime::InterpreterError, type_check::TypeCheckError}, + hir::{ + comptime::{InterpreterError, Value}, + type_check::TypeCheckError, + }, parser::ParserError, usage_tracker::UnusedItem, - Type, + Kind, Type, }; use super::import::PathResolutionError; @@ -101,6 +104,14 @@ pub enum ResolverError { MutableGlobal { span: Span }, #[error("Globals must have a specified type")] UnspecifiedGlobalType { span: Span, expected_type: Type }, + #[error("Global failed to evaluate")] + UnevaluatedGlobalType { span: Span }, + #[error("Globals used in a type position must be non-negative")] + NegativeGlobalType { span: Span, global_value: Value }, + #[error("Globals used in a type position must be integers")] + NonIntegralGlobalType { span: Span, global_value: Value }, + #[error("Global value `{global_value}` is larger than its kind's maximum value")] + GlobalLargerThanKind { span: Span, global_value: FieldElement, kind: Kind }, #[error("Self-referential structs are not supported")] SelfReferentialStruct { span: Span }, #[error("#[no_predicates] attribute is only allowed on constrained functions")] @@ -443,6 +454,34 @@ impl<'a> From<&'a ResolverError> for Diagnostic { *span, ) }, + ResolverError::UnevaluatedGlobalType { span } => { + Diagnostic::simple_error( + "Global failed to evaluate".to_string(), + String::new(), + *span, + ) + } + ResolverError::NegativeGlobalType { span, global_value } => { + Diagnostic::simple_error( + "Globals used in a type position must be non-negative".to_string(), + format!("But found value `{global_value:?}`"), + *span, + ) + } + ResolverError::NonIntegralGlobalType { span, global_value } => { + Diagnostic::simple_error( + "Globals used in a type position must be integers".to_string(), + format!("But found value `{global_value:?}`"), + *span, + ) + } + ResolverError::GlobalLargerThanKind { span, global_value, kind } => { + Diagnostic::simple_error( + format!("Global value `{global_value}` is larger than its kind's maximum value"), + format!("Global's kind inferred to be `{kind}`"), + *span, + ) + } ResolverError::SelfReferentialStruct { span } => { Diagnostic::simple_error( "Self-referential structs are not supported".into(), diff --git a/compiler/noirc_frontend/src/hir/type_check/errors.rs b/compiler/noirc_frontend/src/hir/type_check/errors.rs index 15b8d50c78b..08864b919e3 100644 --- a/compiler/noirc_frontend/src/hir/type_check/errors.rs +++ b/compiler/noirc_frontend/src/hir/type_check/errors.rs @@ -66,9 +66,6 @@ pub enum TypeCheckError { from_value: FieldElement, span: Span, }, - // TODO(https://github.com/noir-lang/noir/issues/6238): implement handling for larger types - #[error("Expected type {expected_kind} when evaluating globals, but found {expr_kind} (this warning may become an error in the future)")] - EvaluatedGlobalIsntU32 { expected_kind: String, expr_kind: String, expr_span: Span }, #[error("Expected {expected:?} found {found:?}")] ArityMisMatch { expected: usize, found: usize, span: Span }, #[error("Return type in a function cannot be public")] @@ -275,15 +272,6 @@ impl<'a> From<&'a TypeCheckError> for Diagnostic { *span, ) } - // TODO(https://github.com/noir-lang/noir/issues/6238): implement - // handling for larger types - TypeCheckError::EvaluatedGlobalIsntU32 { expected_kind, expr_kind, expr_span } => { - Diagnostic::simple_warning( - format!("Expected type {expected_kind} when evaluating globals, but found {expr_kind} (this warning may become an error in the future)"), - String::new(), - *expr_span, - ) - } TypeCheckError::TraitMethodParameterTypeMismatch { method_name, expected_typ, actual_typ, parameter_index, parameter_span } => { Diagnostic::simple_error( format!("Parameter #{parameter_index} of method `{method_name}` must be of type {expected_typ}, not {actual_typ}"), diff --git a/compiler/noirc_frontend/src/hir_def/stmt.rs b/compiler/noirc_frontend/src/hir_def/stmt.rs index 0258cfd8ddb..c42b8230290 100644 --- a/compiler/noirc_frontend/src/hir_def/stmt.rs +++ b/compiler/noirc_frontend/src/hir_def/stmt.rs @@ -31,15 +31,31 @@ pub struct HirLetStatement { pub expression: ExprId, pub attributes: Vec, pub comptime: bool, + pub is_global_let: bool, } impl HirLetStatement { + pub fn new( + pattern: HirPattern, + r#type: Type, + expression: ExprId, + attributes: Vec, + comptime: bool, + is_global_let: bool, + ) -> HirLetStatement { + Self { pattern, r#type, expression, attributes, comptime, is_global_let } + } + pub fn ident(&self) -> HirIdent { match &self.pattern { HirPattern::Identifier(ident) => ident.clone(), _ => panic!("can only fetch hir ident from HirPattern::Identifier"), } } + + pub fn runs_comptime(&self) -> bool { + self.comptime || self.is_global_let + } } #[derive(Debug, Clone)] diff --git a/compiler/noirc_frontend/src/hir_def/types.rs b/compiler/noirc_frontend/src/hir_def/types.rs index 2c9a44c079d..0ed6399296b 100644 --- a/compiler/noirc_frontend/src/hir_def/types.rs +++ b/compiler/noirc_frontend/src/hir_def/types.rs @@ -226,6 +226,10 @@ impl Kind { // Kind::Integer unifies with Kind::IntegerOrField (Kind::Integer | Kind::IntegerOrField, Kind::Integer | Kind::IntegerOrField) => true, + // Kind::IntegerOrField unifies with Kind::Numeric(_) + (Kind::IntegerOrField, Kind::Numeric(_typ)) + | (Kind::Numeric(_typ), Kind::IntegerOrField) => true, + // Kind::Numeric unifies along its Type argument (Kind::Numeric(lhs), Kind::Numeric(rhs)) => { let mut bindings = TypeBindings::new(); @@ -255,7 +259,8 @@ impl Kind { match self { Kind::IntegerOrField => Some(Type::default_int_or_field_type()), Kind::Integer => Some(Type::default_int_type()), - Kind::Any | Kind::Normal | Kind::Numeric(..) => None, + Kind::Numeric(typ) => Some(*typ.clone()), + Kind::Any | Kind::Normal => None, } } @@ -267,7 +272,7 @@ impl Kind { } /// Ensure the given value fits in self.integral_maximum_size() - fn ensure_value_fits( + pub(crate) fn ensure_value_fits( &self, value: FieldElement, span: Span, @@ -2210,6 +2215,7 @@ impl Type { Type::NamedGeneric(binding, _) | Type::TypeVariable(binding) => { substitute_binding(binding) } + // Do not substitute_helper fields, it can lead to infinite recursion // and we should not match fields when type checking anyway. Type::Struct(fields, args) => { diff --git a/compiler/noirc_frontend/src/monomorphization/mod.rs b/compiler/noirc_frontend/src/monomorphization/mod.rs index b31a5744d09..5297a89cc95 100644 --- a/compiler/noirc_frontend/src/monomorphization/mod.rs +++ b/compiler/noirc_frontend/src/monomorphization/mod.rs @@ -1133,7 +1133,7 @@ impl<'interner> Monomorphizer<'interner> { | HirType::Error | HirType::Quoted(_) => Ok(()), HirType::Constant(_value, kind) => { - if kind.is_error() { + if kind.is_error() || kind.default_type().is_none() { Err(MonomorphizationError::UnknownConstant { location }) } else { Ok(()) @@ -1154,7 +1154,6 @@ impl<'interner> Monomorphizer<'interner> { Ok(()) } - HirType::TypeVariable(ref binding) => { let type_var_kind = match &*binding.borrow() { TypeBinding::Bound(binding) => { diff --git a/compiler/noirc_frontend/src/node_interner.rs b/compiler/noirc_frontend/src/node_interner.rs index 6d70ea2fd6d..ae30b68c095 100644 --- a/compiler/noirc_frontend/src/node_interner.rs +++ b/compiler/noirc_frontend/src/node_interner.rs @@ -849,6 +849,7 @@ impl NodeInterner { let id = GlobalId(self.globals.len()); let location = Location::new(ident.span(), file); let name = ident.to_string(); + let definition_id = self.push_definition(name, mutable, comptime, DefinitionKind::Global(id), location); @@ -884,6 +885,7 @@ impl NodeInterner { ) -> GlobalId { let statement = self.push_stmt(HirStatement::Error); let span = name.span(); + let id = self .push_global(name, local_id, crate_id, statement, file, attributes, mutable, comptime); self.push_stmt_location(statement, span, file); diff --git a/compiler/noirc_frontend/src/parser/parser/global.rs b/compiler/noirc_frontend/src/parser/parser/global.rs index 5ece510022a..3a4f6d922a4 100644 --- a/compiler/noirc_frontend/src/parser/parser/global.rs +++ b/compiler/noirc_frontend/src/parser/parser/global.rs @@ -23,6 +23,7 @@ impl<'a> Parser<'a> { // and throw the error in name resolution. let attributes = self.validate_secondary_attributes(attributes); + let is_global_let = true; let Some(ident) = self.eat_ident() else { self.eat_semicolons(); @@ -35,6 +36,7 @@ impl<'a> Parser<'a> { expression: Expression { kind: ExpressionKind::Error, span: Span::default() }, attributes, comptime, + is_global_let, }; }; @@ -53,7 +55,7 @@ impl<'a> Parser<'a> { self.expected_token(Token::Semicolon); } - LetStatement { pattern, r#type: typ, expression, attributes, comptime } + LetStatement { pattern, r#type: typ, expression, attributes, comptime, is_global_let } } } @@ -105,6 +107,7 @@ mod tests { assert_eq!("foo", name.to_string()); assert!(matches!(let_statement.r#type.typ, UnresolvedTypeData::Unspecified)); assert!(!let_statement.comptime); + assert!(let_statement.is_global_let); assert_eq!(visibility, ItemVisibility::Private); } diff --git a/compiler/noirc_frontend/src/parser/parser/statement.rs b/compiler/noirc_frontend/src/parser/parser/statement.rs index ff6117c9348..ab86f7d6649 100644 --- a/compiler/noirc_frontend/src/parser/parser/statement.rs +++ b/compiler/noirc_frontend/src/parser/parser/statement.rs @@ -364,7 +364,14 @@ impl<'a> Parser<'a> { Expression { kind: ExpressionKind::Error, span: self.current_token_span } }; - Some(LetStatement { pattern, r#type, expression, attributes, comptime: false }) + Some(LetStatement { + pattern, + r#type, + expression, + attributes, + comptime: false, + is_global_let: false, + }) } /// ConstrainStatement diff --git a/compiler/noirc_frontend/src/tests.rs b/compiler/noirc_frontend/src/tests.rs index 8ddf1b571e6..1aed3d62d30 100644 --- a/compiler/noirc_frontend/src/tests.rs +++ b/compiler/noirc_frontend/src/tests.rs @@ -1287,11 +1287,15 @@ fn lambda$f1(mut env$l1: (Field)) -> Field { check_rewrite(src, expected_rewrite); } +// TODO(https://github.com/noir-lang/noir/issues/6780): currently failing +// with a stack overflow #[test] +#[ignore] fn deny_cyclic_globals() { let src = r#" global A: u32 = B; global B: u32 = A; + fn main() {} "#; @@ -1991,9 +1995,6 @@ fn numeric_generic_u16_array_size() { )); } -// TODO(https://github.com/noir-lang/noir/issues/6238): -// The EvaluatedGlobalIsntU32 warning is a stopgap -// (originally from https://github.com/noir-lang/noir/issues/6125) #[test] fn numeric_generic_field_larger_than_u32() { let src = r#" @@ -2006,29 +2007,16 @@ fn numeric_generic_field_larger_than_u32() { } "#; let errors = get_program_errors(src); - assert_eq!(errors.len(), 2); - assert!(matches!( - errors[0].0, - CompilationError::TypeError(TypeCheckError::EvaluatedGlobalIsntU32 { .. }), - )); - assert!(matches!( - errors[1].0, - CompilationError::ResolverError(ResolverError::IntegerTooLarge { .. }) - )); + assert_eq!(errors.len(), 0); } -// TODO(https://github.com/noir-lang/noir/issues/6238): -// The EvaluatedGlobalIsntU32 warning is a stopgap -// (originally from https://github.com/noir-lang/noir/issues/6126) #[test] fn numeric_generic_field_arithmetic_larger_than_u32() { let src = r#" struct Foo {} - impl Foo { - fn size(self) -> Field { - F - } + fn size(_x: Foo) -> Field { + F } // 2^32 - 1 @@ -2039,21 +2027,11 @@ fn numeric_generic_field_arithmetic_larger_than_u32() { } fn main() { - let _ = foo::().size(); + let _ = size(foo::()); } "#; let errors = get_program_errors(src); - assert_eq!(errors.len(), 2); - - assert!(matches!( - errors[0].0, - CompilationError::TypeError(TypeCheckError::EvaluatedGlobalIsntU32 { .. }), - )); - - assert!(matches!( - errors[1].0, - CompilationError::ResolverError(ResolverError::UnusedVariable { .. }) - )); + assert_eq!(errors.len(), 0); } #[test] @@ -2180,25 +2158,11 @@ fn numeric_generics_type_kind_mismatch() { } "#; let errors = get_program_errors(src); - assert_eq!(errors.len(), 3); - - // TODO(https://github.com/noir-lang/noir/issues/6238): - // The EvaluatedGlobalIsntU32 warning is a stopgap + assert_eq!(errors.len(), 1); assert!(matches!( errors[0].0, - CompilationError::TypeError(TypeCheckError::EvaluatedGlobalIsntU32 { .. }), - )); - - assert!(matches!( - errors[1].0, CompilationError::TypeError(TypeCheckError::TypeKindMismatch { .. }), )); - - // TODO(https://github.com/noir-lang/noir/issues/6238): see above - assert!(matches!( - errors[2].0, - CompilationError::TypeError(TypeCheckError::EvaluatedGlobalIsntU32 { .. }), - )); } #[test] @@ -3219,20 +3183,20 @@ fn dont_infer_globals_to_u32_from_type_use() { } "#; - let errors = get_program_errors(src); - assert_eq!(errors.len(), 3); - assert!(matches!( - errors[0].0, - CompilationError::ResolverError(ResolverError::UnspecifiedGlobalType { .. }) - )); - assert!(matches!( - errors[1].0, - CompilationError::ResolverError(ResolverError::UnspecifiedGlobalType { .. }) - )); - assert!(matches!( - errors[2].0, - CompilationError::ResolverError(ResolverError::UnspecifiedGlobalType { .. }) - )); + let mut errors = get_program_errors(src); + assert_eq!(errors.len(), 6); + for (error, _file_id) in errors.drain(0..3) { + assert!(matches!( + error, + CompilationError::ResolverError(ResolverError::UnspecifiedGlobalType { .. }) + )); + } + for (error, _file_id) in errors { + assert!(matches!( + error, + CompilationError::TypeError(TypeCheckError::TypeKindMismatch { .. }) + )); + } } #[test] @@ -3242,7 +3206,8 @@ fn dont_infer_partial_global_types() { pub global NESTED_ARRAY: [[Field; _]; 3] = [[]; 3]; pub global STR: str<_> = "hi"; pub global NESTED_STR: [str<_>] = &["hi"]; - pub global FMT_STR: fmtstr<_, _> = f"hi {ARRAY}"; + pub global FORMATTED_VALUE: str<5> = "there"; + pub global FMT_STR: fmtstr<_, _> = f"hi {FORMATTED_VALUE}"; pub global TUPLE_WITH_MULTIPLE: ([str<_>], [[Field; _]; 3]) = (&["hi"], [[]; 3]); fn main() { } @@ -3318,13 +3283,9 @@ fn non_u32_as_array_length() { "#; let errors = get_program_errors(src); - assert_eq!(errors.len(), 2); + assert_eq!(errors.len(), 1); assert!(matches!( errors[0].0, - CompilationError::TypeError(TypeCheckError::EvaluatedGlobalIsntU32 { .. }) - )); - assert!(matches!( - errors[1].0, CompilationError::TypeError(TypeCheckError::TypeKindMismatch { .. }) )); } diff --git a/compiler/noirc_frontend/src/tests/arithmetic_generics.rs b/compiler/noirc_frontend/src/tests/arithmetic_generics.rs index 3328fe439ae..83de9c077ab 100644 --- a/compiler/noirc_frontend/src/tests/arithmetic_generics.rs +++ b/compiler/noirc_frontend/src/tests/arithmetic_generics.rs @@ -153,3 +153,49 @@ fn arithmetic_generics_checked_cast_indirect_zeros() { panic!("unexpected error: {:?}", monomorphization_error); } } + +#[test] +fn global_numeric_generic_larger_than_u32() { + // Regression test for https://github.com/noir-lang/noir/issues/6125 + let source = r#" + global A: Field = 4294967297; + + fn foo() { } + + fn main() { + let _ = foo::(); + } + "#; + let errors = get_program_errors(source); + assert_eq!(errors.len(), 0); +} + +#[test] +fn global_arithmetic_generic_larger_than_u32() { + // Regression test for https://github.com/noir-lang/noir/issues/6126 + let source = r#" + struct Foo {} + + impl Foo { + fn size(self) -> Field { + let _ = self; + F + } + } + + // 2^32 - 1 + global A: Field = 4294967295; + + // Avoiding overflow succeeds: + // fn foo() -> Foo { + fn foo() -> Foo { + Foo {} + } + + fn main() { + let _ = foo::().size(); + } + "#; + let errors = get_program_errors(source); + assert_eq!(errors.len(), 0); +} diff --git a/tooling/lsp/src/requests/hover.rs b/tooling/lsp/src/requests/hover.rs index bb1ea661719..42b9705f3b1 100644 --- a/tooling/lsp/src/requests/hover.rs +++ b/tooling/lsp/src/requests/hover.rs @@ -5,7 +5,6 @@ use fm::{FileMap, PathString}; use lsp_types::{Hover, HoverContents, HoverParams, MarkupContent, MarkupKind}; use noirc_frontend::{ ast::{ItemVisibility, Visibility}, - elaborator::types::try_eval_array_length_id, hir::def_map::ModuleId, hir_def::{ expr::{HirArrayLiteral, HirExpression, HirLiteral}, @@ -205,8 +204,17 @@ fn format_global(id: GlobalId, args: &ProcessRequestCallbackArgs) -> String { string.push('\n'); } + let mut print_comptime = definition.comptime; + let mut opt_value = None; + + // See if we can figure out what's the global's value + if let Some(stmt) = args.interner.get_global_let_statement(id) { + print_comptime = stmt.comptime; + opt_value = get_global_value(args.interner, stmt.expression); + } + string.push_str(" "); - if definition.comptime { + if print_comptime { string.push_str("comptime "); } if definition.mutable { @@ -217,12 +225,9 @@ fn format_global(id: GlobalId, args: &ProcessRequestCallbackArgs) -> String { string.push_str(": "); string.push_str(&format!("{}", typ)); - // See if we can figure out what's the global's value - if let Some(stmt) = args.interner.get_global_let_statement(id) { - if let Some(value) = get_global_value(args.interner, stmt.expression) { - string.push_str(" = "); - string.push_str(&value); - } + if let Some(value) = opt_value { + string.push_str(" = "); + string.push_str(&value); } string.push_str(&go_to_type_links(&typ, args.interner, args.files)); @@ -233,13 +238,6 @@ fn format_global(id: GlobalId, args: &ProcessRequestCallbackArgs) -> String { } fn get_global_value(interner: &NodeInterner, expr: ExprId) -> Option { - let span = interner.expr_span(&expr); - - // Globals as array lengths are extremely common, so we try that first. - if let Ok(result) = try_eval_array_length_id(interner, expr, span) { - return Some(result.to_string()); - } - match interner.expression(&expr) { HirExpression::Literal(literal) => match literal { HirLiteral::Array(hir_array_literal) => {