diff --git a/compiler/noirc_frontend/src/elaborator/patterns.rs b/compiler/noirc_frontend/src/elaborator/patterns.rs index 10fecbffbdb..0e8b16ba3f7 100644 --- a/compiler/noirc_frontend/src/elaborator/patterns.rs +++ b/compiler/noirc_frontend/src/elaborator/patterns.rs @@ -573,7 +573,7 @@ impl Elaborator<'_> { let location = located_type.location(); let typ = located_type.contents; let typ = typ.substitute_kind_any_with_kind(&kind); - self.check_kind(typ, &kind, location) + self.check_type_kind(typ, &kind, location) }) } diff --git a/compiler/noirc_frontend/src/elaborator/types.rs b/compiler/noirc_frontend/src/elaborator/types.rs index c01575deb53..375c9f51876 100644 --- a/compiler/noirc_frontend/src/elaborator/types.rs +++ b/compiler/noirc_frontend/src/elaborator/types.rs @@ -252,7 +252,7 @@ impl Elaborator<'_> { _ => (), } - self.check_kind(resolved_type, kind, location) + self.check_type_kind(resolved_type, kind, location) } /// Resolve Self::Foo to an associated type on the current trait or trait impl @@ -705,11 +705,11 @@ impl Elaborator<'_> { if let Type::Alias(alias, vec) = typ { typ = alias.borrow().get_type(&vec); } - self.check_kind(typ, expected_kind, location) + self.check_type_kind(typ, expected_kind, location) } UnresolvedTypeExpression::Constant(int, suffix, _span) => { if let Some(suffix) = suffix { - self.check_kind(suffix.as_type(), expected_kind, location); + self.check_kind(suffix.as_kind(), expected_kind, location); } Type::Constant(int, expected_kind.clone()) } @@ -758,13 +758,14 @@ impl Elaborator<'_> { } UnresolvedTypeExpression::AsTraitPath(path) => { let typ = self.resolve_as_trait_path(*path, wildcard_allowed); - self.check_kind(typ, expected_kind, location) + self.check_type_kind(typ, expected_kind, location) } } } - /// Check that a [Type] unifies with an expected [Kind] and returned the unified result. - pub(super) fn check_kind( + /// Checks that the type's [Kind] matches the expected kind, issuing an error if it does not. + /// Returns `typ` unless an error occurs - in which case [Type::Error] is returned. + pub(super) fn check_type_kind( &mut self, typ: Type, expected_kind: &Kind, @@ -774,15 +775,28 @@ impl Elaborator<'_> { self.push_err(TypeCheckError::CyclicType { typ, location }); return Type::Error; } - if !typ.kind().unifies(expected_kind) { + + if self.check_kind(typ.kind(), expected_kind, location) { typ } else { Type::Error } + } + + /// Checks that `expr_kind` matches `expected_kind`, issuing an error if it does not. + /// Returns `true` if the kinds unify. + pub(super) fn check_kind( + &mut self, + expr_kind: Kind, + expected_kind: &Kind, + location: Location, + ) -> bool { + if !expr_kind.unifies(expected_kind) { self.push_err(TypeCheckError::TypeKindMismatch { expected_kind: expected_kind.clone(), - expr_kind: typ.kind(), + expr_kind, expr_location: location, }); - return Type::Error; + false + } else { + true } - typ } fn resolve_as_trait_path(&mut self, path: AsTraitPath, wildcard_allowed: bool) -> Type { diff --git a/compiler/noirc_frontend/src/hir/comptime/value.rs b/compiler/noirc_frontend/src/hir/comptime/value.rs index 60891fe959f..6ee7993c71c 100644 --- a/compiler/noirc_frontend/src/hir/comptime/value.rs +++ b/compiler/noirc_frontend/src/hir/comptime/value.rs @@ -556,46 +556,62 @@ impl Value { } Value::TypedExpr(TypedExpr::ExprId(expr_id)) => vec![Token::UnquoteMarker(expr_id)], Value::Bool(bool) => vec![Token::Bool(bool)], - Value::U1(bool) => vec![Token::Int(u128::from(bool).into(), None)], + Value::U1(bool) => { + vec![Token::Int(u128::from(bool).into(), Some(IntegerTypeSuffix::U1))] + } Value::U8(value) => { - vec![Token::Int(u128::from(value).into(), None)] + vec![Token::Int(u128::from(value).into(), Some(IntegerTypeSuffix::U8))] } Value::U16(value) => { - vec![Token::Int(u128::from(value).into(), None)] + vec![Token::Int(u128::from(value).into(), Some(IntegerTypeSuffix::U16))] } Value::U32(value) => { - vec![Token::Int(u128::from(value).into(), None)] + vec![Token::Int(u128::from(value).into(), Some(IntegerTypeSuffix::U32))] } Value::U64(value) => { - vec![Token::Int(u128::from(value).into(), None)] + vec![Token::Int(u128::from(value).into(), Some(IntegerTypeSuffix::U64))] + } + Value::U128(value) => { + vec![Token::Int(value.into(), Some(IntegerTypeSuffix::U128))] } - Value::U128(value) => vec![Token::Int(value.into(), None)], Value::I8(value) => { if value < 0 { - vec![Token::Minus, Token::Int((-value as u128).into(), None)] + vec![ + Token::Minus, + Token::Int((-value as u128).into(), Some(IntegerTypeSuffix::I8)), + ] } else { - vec![Token::Int((value as u128).into(), None)] + vec![Token::Int((value as u128).into(), Some(IntegerTypeSuffix::I8))] } } Value::I16(value) => { if value < 0 { - vec![Token::Minus, Token::Int((-value as u128).into(), None)] + vec![ + Token::Minus, + Token::Int((-value as u128).into(), Some(IntegerTypeSuffix::I16)), + ] } else { - vec![Token::Int((value as u128).into(), None)] + vec![Token::Int((value as u128).into(), Some(IntegerTypeSuffix::I16))] } } Value::I32(value) => { if value < 0 { - vec![Token::Minus, Token::Int((-value as u128).into(), None)] + vec![ + Token::Minus, + Token::Int((-value as u128).into(), Some(IntegerTypeSuffix::I32)), + ] } else { - vec![Token::Int((value as u128).into(), None)] + vec![Token::Int((value as u128).into(), Some(IntegerTypeSuffix::I32))] } } Value::I64(value) => { if value < 0 { - vec![Token::Minus, Token::Int((-value as u128).into(), None)] + vec![ + Token::Minus, + Token::Int((-value as u128).into(), Some(IntegerTypeSuffix::I64)), + ] } else { - vec![Token::Int((value as u128).into(), None)] + vec![Token::Int((value as u128).into(), Some(IntegerTypeSuffix::I64))] } } Value::Field(value) => { diff --git a/compiler/noirc_frontend/src/lexer/token.rs b/compiler/noirc_frontend/src/lexer/token.rs index 498c4a8c723..f67ef2267ce 100644 --- a/compiler/noirc_frontend/src/lexer/token.rs +++ b/compiler/noirc_frontend/src/lexer/token.rs @@ -147,7 +147,14 @@ pub enum IntegerTypeSuffix { } impl IntegerTypeSuffix { - pub(crate) fn as_type(&self) -> crate::Type { + /// Returns the type of this integer suffix when used in a value position. + /// Note that this is _not_ the type of the integer when the integer is in a type position! + /// + /// An integer value like `3u32` has type `u32` but when used in a type `[Field; 3u32]`, + /// `3u32` will have the type `Type::Constant(3, Kind::Numeric(u32))`. As a result, using + /// this method for any kind checks on integer types will result in a kind error! For those + /// cases, use [IntegerTypeSuffix::as_kind] instead. + pub(crate) fn as_type(self) -> crate::Type { use crate::{Type::Integer, ast::IntegerBitSize::*, shared::Signedness::*}; match self { IntegerTypeSuffix::I8 => Integer(Signed, Eight), @@ -163,6 +170,18 @@ impl IntegerTypeSuffix { IntegerTypeSuffix::Field => crate::Type::FieldElement, } } + + /// Returns the kind of this integer constant when used in a type position. + /// For example, when used as `[Field; 3u32]`, this [IntegerTypeSuffix::U32] + /// will return `Kind::Numeric(Type::U32)`. + /// + /// This method should generally be used whenever an integer is used in a type position. + /// [IntegerTypeSuffix::as_type] would return a raw `u32` type which is not the actual + /// type of an integer in a type position - that'd be `Type::Constant(3, Kind::Numeric(u32))` + /// for `3u32`. + pub(crate) fn as_kind(self) -> crate::Kind { + crate::Kind::Numeric(Box::new(self.as_type())) + } } #[derive(PartialEq, Eq, Hash, Debug, Clone, PartialOrd, Ord)] diff --git a/compiler/noirc_frontend/src/parser/parser/expression.rs b/compiler/noirc_frontend/src/parser/parser/expression.rs index fbe288de7c0..38aee9f8a99 100644 --- a/compiler/noirc_frontend/src/parser/parser/expression.rs +++ b/compiler/noirc_frontend/src/parser/parser/expression.rs @@ -248,8 +248,10 @@ impl Parser<'_> { fn parse_member_access_field_name(&mut self) -> Option { if let Some(ident) = self.eat_ident() { Some(ident) - // Using `None` because we don't want to allow integer type suffixes on tuple field names - } else if let Some((int, None)) = self.eat_int() { + // We allow integer type suffixes on tuple field names since this lets + // users unquote typed integers in macros to use as a tuple access expression. + // See https://github.com/noir-lang/noir/pull/10330#issuecomment-3499399843 + } else if let Some((int, _)) = self.eat_int() { Some(Ident::new(int.to_string(), self.previous_token_location)) } else { self.push_error( diff --git a/compiler/noirc_frontend/src/tests/meta_quote_roundtrip.rs b/compiler/noirc_frontend/src/tests/meta_quote_roundtrip.rs index 9fc069ba823..e1c30f1e75d 100644 --- a/compiler/noirc_frontend/src/tests/meta_quote_roundtrip.rs +++ b/compiler/noirc_frontend/src/tests/meta_quote_roundtrip.rs @@ -8,15 +8,6 @@ //! 4. Assert resulting value equals original //! //! This tests the full metaprogramming pipeline that users rely on. -//! -//! ## Known Limitation -//! -//! TODO(https://github.com/noir-lang/noir/issues/10326): -//! Integer types (u8, u16, u32, u64, u128, i8, i16, i32, i64) do NOT preserve their type -//! through quote/unquote. The `Value::into_tokens()` implementation in value.rs strips -//! type suffixes, converting all integers to `Token::Int(value, None)`. -//! When parsed, these become Field by default. Only Field, Bool, and Unit types -//! correctly roundtrip. use crate::tests::assert_no_errors; use proptest::prelude::*; @@ -41,31 +32,68 @@ fn make_roundtrip_test(type_annotation: &str, value_expr: String) -> String { ) } -// Primitive types +// Primitive Types proptest! { #![proptest_config(ProptestConfig::with_cases(100))] - /// Field values preserve correctly through quote/unquote - /// Using u128 for simplicity. #[test] fn roundtrip_field_values(n in any::()) { let src = make_roundtrip_test("Field", n.to_string()); assert_no_errors(&src); } -} -// TODO(https://github.com/noir-lang/noir/issues/10326): Integer types are not preserved. Extend the property tests for primitive types once that issue is resolved. -proptest! { - #![proptest_config(ProptestConfig::with_cases(1))] - - /// into_tokens() strips type suffixes, so integers become a `Field` when unquoted. #[test] - #[ignore = "integers don't preserve type through quote/unquote"] - fn roundtrip_u8_values_fails(n in any::()) { + fn roundtrip_u8_values(n in any::()) { let src = make_roundtrip_test("u8", n.to_string()); assert_no_errors(&src); } + + #[test] + fn roundtrip_u16_values(n in any::()) { + let src = make_roundtrip_test("u16", n.to_string()); + assert_no_errors(&src); + } + + #[test] + fn roundtrip_u32_values(n in any::()) { + let src = make_roundtrip_test("u32", n.to_string()); + assert_no_errors(&src); + } + + #[test] + fn roundtrip_u64_values(n in any::()) { + let src = make_roundtrip_test("u64", n.to_string()); + assert_no_errors(&src); + } + + #[test] + fn roundtrip_u128_values(n in any::()) { + let src = make_roundtrip_test("u128", n.to_string()); + assert_no_errors(&src); + } + + // TODO(https://github.com/noir-lang/noir/issues/10328): Although it is a very low chance, all these tests have the possibility to be flakey + // #[test] + // fn roundtrip_i8_values(n in any::()) { + // let src = make_roundtrip_test("i8", n.to_string()); + // assert_no_errors(&src); + // } + // #[test] + // fn roundtrip_i16_values(n in any::()) { + // let src = make_roundtrip_test("i16", n.to_string()); + // assert_no_errors(&src); + // } + // #[test] + // fn roundtrip_i32_values(n in any::()) { + // let src = make_roundtrip_test("i32", n.to_string()); + // assert_no_errors(&src); + // } + // #[test] + // fn roundtrip_i64_values(n in any::()) { + // let src = make_roundtrip_test("i64", n.to_string()); + // assert_no_errors(&src); + // } } #[test] @@ -74,6 +102,14 @@ fn roundtrip_zero_field() { assert_no_errors(&src); } +// TODO(https://github.com/noir-lang/noir/issues/10328) +#[test] +#[ignore] +fn roundtrip_i64_min() { + let src = make_roundtrip_test("i64", i64::MIN.to_string()); + assert_no_errors(&src); +} + /// Boolean values preserve correctly through quote/unquote #[test] fn roundtrip_false() { diff --git a/compiler/noirc_frontend/src/tests/numeric_generics.rs b/compiler/noirc_frontend/src/tests/numeric_generics.rs index 5e5013c6aee..676e1ac16e7 100644 --- a/compiler/noirc_frontend/src/tests/numeric_generics.rs +++ b/compiler/noirc_frontend/src/tests/numeric_generics.rs @@ -572,3 +572,41 @@ fn bool_generic_as_loop_bound() { "#; check_errors(src); } + +/// Regression for CI issue in https://github.com/noir-lang/noir/pull/10330 +#[test] +fn integer_with_suffix_used_as_type_in_quote() { + let src = " + #[make_bar] + fn main() { + bar([1, 2]); + } + + comptime fn make_bar(_f: FunctionDefinition) -> Quoted { + let n = 2u32; + quote { + fn bar(_array: [Field; $n]) {} + } + } + "; + assert_no_errors(src); +} + +/// Regression for https://github.com/noir-lang/noir/pull/10330#issuecomment-3499399843 +#[test] +fn integer_with_suffix_used_as_tuple_index() { + let src = " + fn main() { + macro!(); + } + + comptime fn macro() -> Quoted { + let n = 0u32; + quote { + let tuple = (0u8, 1u16, 2i8); + assert_eq(tuple.$n, 0); + } + } + "; + assert_no_errors(src); +}