Skip to content
Merged
2 changes: 1 addition & 1 deletion compiler/noirc_frontend/src/elaborator/patterns.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
})
}

Expand Down
34 changes: 24 additions & 10 deletions compiler/noirc_frontend/src/elaborator/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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())
}
Expand Down Expand Up @@ -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,
Expand All @@ -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 {
Expand Down
44 changes: 30 additions & 14 deletions compiler/noirc_frontend/src/hir/comptime/value.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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) => {
Expand Down
21 changes: 20 additions & 1 deletion compiler/noirc_frontend/src/lexer/token.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand All @@ -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)]
Expand Down
6 changes: 4 additions & 2 deletions compiler/noirc_frontend/src/parser/parser/expression.rs
Original file line number Diff line number Diff line change
Expand Up @@ -248,8 +248,10 @@ impl Parser<'_> {
fn parse_member_access_field_name(&mut self) -> Option<Ident> {
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(
Expand Down
76 changes: 56 additions & 20 deletions compiler/noirc_frontend/src/tests/meta_quote_roundtrip.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::*;
Expand All @@ -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::<u128>()) {
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::<u8>()) {
fn roundtrip_u8_values(n in any::<u8>()) {
let src = make_roundtrip_test("u8", n.to_string());
assert_no_errors(&src);
}

#[test]
fn roundtrip_u16_values(n in any::<u16>()) {
let src = make_roundtrip_test("u16", n.to_string());
assert_no_errors(&src);
}

#[test]
fn roundtrip_u32_values(n in any::<u32>()) {
let src = make_roundtrip_test("u32", n.to_string());
assert_no_errors(&src);
}

#[test]
fn roundtrip_u64_values(n in any::<u64>()) {
let src = make_roundtrip_test("u64", n.to_string());
assert_no_errors(&src);
}

#[test]
fn roundtrip_u128_values(n in any::<u128>()) {
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::<i8>()) {
// let src = make_roundtrip_test("i8", n.to_string());
// assert_no_errors(&src);
// }
// #[test]
// fn roundtrip_i16_values(n in any::<i16>()) {
// let src = make_roundtrip_test("i16", n.to_string());
// assert_no_errors(&src);
// }
// #[test]
// fn roundtrip_i32_values(n in any::<i32>()) {
// let src = make_roundtrip_test("i32", n.to_string());
// assert_no_errors(&src);
// }
// #[test]
// fn roundtrip_i64_values(n in any::<i64>()) {
// let src = make_roundtrip_test("i64", n.to_string());
// assert_no_errors(&src);
// }
}

#[test]
Expand All @@ -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() {
Expand Down
38 changes: 38 additions & 0 deletions compiler/noirc_frontend/src/tests/numeric_generics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Loading