Skip to content

Commit

Permalink
fix!: Infer globals to be u32 when used in a type (noir-lang/noir#6083)
Browse files Browse the repository at this point in the history
fix: Fix comptime type formatting (noir-lang/noir#6079)
fix: don't crash on untyped global used as array length (noir-lang/noir#6076)
feat(perf): Remove unused loads in mem2reg and last stores per function (noir-lang/noir#5925)
fix: Correct stack trace order in comptime assertion failures (noir-lang/noir#6066)
chore!: removing implicit numeric generics (noir-lang/noir#5837)
fix: Always parse all tokens from quoted token streams (noir-lang/noir#6064)
fix: Update databus in flattening (noir-lang/noir#6063)
fix: Be more lenient with semicolons on interned expressions (noir-lang/noir#6062)
feat: check unconstrained trait impl method matches (noir-lang/noir#6057)
chore: remove unused TypeVariableKind::Constant (noir-lang/noir#6053)
  • Loading branch information
AztecBot committed Sep 18, 2024
2 parents d2a3340 + e7e01cd commit 95454bc
Show file tree
Hide file tree
Showing 12 changed files with 185 additions and 52 deletions.
2 changes: 1 addition & 1 deletion .noir-sync-commit
Original file line number Diff line number Diff line change
@@ -1 +1 @@
19eef30cdbd8a3a4671aabbbe66b5481a5dec3f7
78262c96d5b116c77e50653f9059da60824db812
Original file line number Diff line number Diff line change
Expand Up @@ -215,7 +215,7 @@ impl<'context> Elaborator<'context> {
UnresolvedTypeExpression::Constant(0, span)
});

let length = self.convert_expression_type(length, span);
let length = self.convert_expression_type(length, &Kind::u32(), span);
let (repeated_element, elem_type) = self.elaborate_expression(*repeated_element);

let length_clone = length.clone();
Expand Down
80 changes: 44 additions & 36 deletions noir/noir-repo/compiler/noirc_frontend/src/elaborator/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -77,22 +77,22 @@ impl<'context> Elaborator<'context> {
FieldElement => Type::FieldElement,
Array(size, elem) => {
let elem = Box::new(self.resolve_type_inner(*elem, kind));
let size = self.convert_expression_type(size, span);
let size = self.convert_expression_type(size, &Kind::u32(), span);
Type::Array(Box::new(size), elem)
}
Slice(elem) => {
let elem = Box::new(self.resolve_type_inner(*elem, kind));
Type::Slice(elem)
}
Expression(expr) => self.convert_expression_type(expr, span),
Expression(expr) => self.convert_expression_type(expr, kind, span),
Integer(sign, bits) => Type::Integer(sign, bits),
Bool => Type::Bool,
String(size) => {
let resolved_size = self.convert_expression_type(size, span);
let resolved_size = self.convert_expression_type(size, &Kind::u32(), span);
Type::String(Box::new(resolved_size))
}
FormatString(size, fields) => {
let resolved_size = self.convert_expression_type(size, span);
let resolved_size = self.convert_expression_type(size, &Kind::u32(), span);
let fields = self.resolve_type_inner(*fields, kind);
Type::FmtString(Box::new(resolved_size), Box::new(fields))
}
Expand Down Expand Up @@ -426,37 +426,25 @@ impl<'context> Elaborator<'context> {
pub(super) fn convert_expression_type(
&mut self,
length: UnresolvedTypeExpression,
expected_kind: &Kind,
span: Span,
) -> Type {
match length {
UnresolvedTypeExpression::Variable(path) => {
let resolved_length =
self.lookup_generic_or_global_type(&path).unwrap_or_else(|| {
self.push_err(ResolverError::NoSuchNumericTypeVariable { path });
Type::Constant(0, Kind::u32())
});

if let Type::NamedGeneric(ref _type_var, ref _name, ref kind) = resolved_length {
if !kind.is_numeric() {
self.push_err(TypeCheckError::TypeKindMismatch {
expected_kind: Kind::u32().to_string(),
expr_kind: kind.to_string(),
expr_span: span,
});
return Type::Error;
}
}
resolved_length
let typ = self.resolve_named_type(path, GenericTypeArgs::default());
self.check_kind(typ, expected_kind, span)
}
UnresolvedTypeExpression::Constant(int, _span) => {
Type::Constant(int, expected_kind.clone())
}
UnresolvedTypeExpression::Constant(int, _span) => Type::Constant(int, Kind::u32()),
UnresolvedTypeExpression::BinaryOperation(lhs, op, rhs, span) => {
let (lhs_span, rhs_span) = (lhs.span(), rhs.span());
let lhs = self.convert_expression_type(*lhs, lhs_span);
let rhs = self.convert_expression_type(*rhs, rhs_span);
let lhs = self.convert_expression_type(*lhs, expected_kind, lhs_span);
let rhs = self.convert_expression_type(*rhs, expected_kind, rhs_span);

match (lhs, rhs) {
(Type::Constant(lhs, lhs_kind), Type::Constant(rhs, rhs_kind)) => {
if lhs_kind != rhs_kind {
if !lhs_kind.unifies(&rhs_kind) {
self.push_err(TypeCheckError::TypeKindMismatch {
expected_kind: lhs_kind.to_string(),
expr_kind: rhs_kind.to_string(),
Expand All @@ -474,10 +462,27 @@ impl<'context> Elaborator<'context> {
(lhs, rhs) => Type::InfixExpr(Box::new(lhs), op, Box::new(rhs)).canonicalize(),
}
}
UnresolvedTypeExpression::AsTraitPath(path) => self.resolve_as_trait_path(*path),
UnresolvedTypeExpression::AsTraitPath(path) => {
let typ = self.resolve_as_trait_path(*path);
self.check_kind(typ, expected_kind, span)
}
}
}

fn check_kind(&mut self, typ: Type, expected_kind: &Kind, span: Span) -> Type {
if let Some(kind) = typ.kind() {
if !kind.unifies(expected_kind) {
self.push_err(TypeCheckError::TypeKindMismatch {
expected_kind: expected_kind.to_string(),
expr_kind: kind.to_string(),
expr_span: span,
});
return Type::Error;
}
}
typ
}

fn resolve_as_trait_path(&mut self, path: AsTraitPath) -> Type {
let span = path.trait_path.span;
let Some(trait_id) = self.resolve_trait_by_path(path.trait_path.clone()) else {
Expand Down Expand Up @@ -655,18 +660,21 @@ impl<'context> Elaborator<'context> {
int.try_into_u128().ok_or(Some(ResolverError::IntegerTooLarge { span }))
}
HirExpression::Ident(ident, _) => {
let definition = self.interner.definition(ident.id);
match definition.kind {
DefinitionKind::Global(global_id) => {
let let_statement = self.interner.get_global_let_statement(global_id);
if let Some(let_statement) = let_statement {
let expression = let_statement.expression;
self.try_eval_array_length_id_with_fuel(expression, span, fuel - 1)
} else {
Err(Some(ResolverError::InvalidArrayLengthExpr { span }))
if let Some(definition) = self.interner.try_definition(ident.id) {
match definition.kind {
DefinitionKind::Global(global_id) => {
let let_statement = self.interner.get_global_let_statement(global_id);
if let Some(let_statement) = let_statement {
let expression = let_statement.expression;
self.try_eval_array_length_id_with_fuel(expression, span, fuel - 1)
} else {
Err(Some(ResolverError::InvalidArrayLengthExpr { span }))
}
}
_ => Err(Some(ResolverError::InvalidArrayLengthExpr { span })),
}
_ => Err(Some(ResolverError::InvalidArrayLengthExpr { span })),
} else {
Err(Some(ResolverError::InvalidArrayLengthExpr { span }))
}
}
HirExpression::Infix(infix) => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -682,7 +682,7 @@ impl<'value, 'interner> Display for ValuePrinter<'value, 'interner> {
}
}
Value::Zeroed(typ) => write!(f, "(zeroed {typ})"),
Value::Type(typ) => write!(f, "{:?}", typ),
Value::Type(typ) => write!(f, "{}", typ),
Value::Expr(ExprValue::Expression(expr)) => {
write!(f, "{}", remove_interned_in_expression_kind(self.interner, expr.clone()))
}
Expand Down
30 changes: 27 additions & 3 deletions noir/noir-repo/compiler/noirc_frontend/src/hir_def/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -151,12 +151,36 @@ impl Kind {
}

pub(crate) fn matches_opt(&self, other: Option<Self>) -> bool {
other.as_ref().map_or(true, |other_kind| self == other_kind)
other.as_ref().map_or(true, |other_kind| self.unifies(other_kind))
}

pub(crate) fn u32() -> Self {
Self::Numeric(Box::new(Type::Integer(Signedness::Unsigned, IntegerBitSize::ThirtyTwo)))
}

/// Unifies this kind with the other. Returns true on success
pub(crate) fn unifies(&self, other: &Kind) -> bool {
match (self, other) {
(Kind::Normal, Kind::Normal) => true,
(Kind::Numeric(lhs), Kind::Numeric(rhs)) => {
let mut bindings = TypeBindings::new();
let unifies = lhs.try_unify(rhs, &mut bindings).is_ok();
if unifies {
Type::apply_type_bindings(bindings);
}
unifies
}
_ => false,
}
}

pub(crate) fn unify(&self, other: &Kind) -> Result<(), UnificationError> {
if self.unifies(other) {
Ok(())
} else {
Err(UnificationError)
}
}
}

impl std::fmt::Display for Kind {
Expand Down Expand Up @@ -1465,13 +1489,13 @@ impl Type {
}
}

(NamedGeneric(binding_a, name_a, _), NamedGeneric(binding_b, name_b, _)) => {
(NamedGeneric(binding_a, name_a, kind_a), NamedGeneric(binding_b, name_b, kind_b)) => {
// Bound NamedGenerics are caught by the check above
assert!(binding_a.borrow().is_unbound());
assert!(binding_b.borrow().is_unbound());

if name_a == name_b {
Ok(())
kind_a.unify(kind_b)
} else {
Err(UnificationError)
}
Expand Down
74 changes: 65 additions & 9 deletions noir/noir-repo/compiler/noirc_frontend/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1616,25 +1616,30 @@ fn numeric_generic_binary_operation_type_mismatch() {
#[test]
fn bool_generic_as_loop_bound() {
let src = r#"
pub fn read<let N: bool>() {
let mut fields = [0; N];
for i in 0..N {
pub fn read<let N: bool>() { // error here
let mut fields = [0; N]; // error here
for i in 0..N { // error here
fields[i] = i + 1;
}
assert(fields[0] == 1);
}
"#;
let errors = get_program_errors(src);
assert_eq!(errors.len(), 2);
assert_eq!(errors.len(), 3);

assert!(matches!(
errors[0].0,
CompilationError::ResolverError(ResolverError::UnsupportedNumericGenericType { .. }),
));

assert!(matches!(
errors[1].0,
CompilationError::TypeError(TypeCheckError::TypeKindMismatch { .. }),
));

let CompilationError::TypeError(TypeCheckError::TypeMismatch {
expected_typ, expr_typ, ..
}) = &errors[1].0
}) = &errors[2].0
else {
panic!("Got an error other than a type mismatch");
};
Expand All @@ -1646,7 +1651,7 @@ fn bool_generic_as_loop_bound() {
#[test]
fn numeric_generic_in_function_signature() {
let src = r#"
pub fn foo<let N: u8>(arr: [Field; N]) -> [Field; N] { arr }
pub fn foo<let N: u32>(arr: [Field; N]) -> [Field; N] { arr }
"#;
assert_no_errors(src);
}
Expand Down Expand Up @@ -1718,7 +1723,7 @@ fn numeric_generic_used_in_nested_type_fails() {
a: Field,
b: Bar<N>,
}
pub struct Bar<let N: u32> {
struct Bar<let N: u32> {
inner: N
}
"#;
Expand Down Expand Up @@ -1943,7 +1948,7 @@ fn numeric_generics_type_kind_mismatch() {
}
global M: u16 = 3;
fn main() {
let _ = bar::<M>();
}
Expand All @@ -1967,7 +1972,7 @@ fn numeric_generics_value_kind_mismatch_u32_u64() {
}
impl<T, let MaxLen: u32> BoundedVec<T, MaxLen> {
pub fn extend_from_bounded_vec<let Len: u32>(&mut self, _vec: BoundedVec<T, Len>) {
pub fn extend_from_bounded_vec<let Len: u32>(&mut self, _vec: BoundedVec<T, Len>) {
// We do this to avoid an unused variable warning on `self`
let _ = self.len;
for _ in 0..Len { }
Expand Down Expand Up @@ -3644,3 +3649,54 @@ fn does_not_crash_when_passing_mutable_undefined_variable() {

assert_eq!(name, "undefined");
}

#[test]
fn infer_globals_to_u32_from_type_use() {
let src = r#"
global ARRAY_LEN = 3;
global STR_LEN = 2;
global FMT_STR_LEN = 2;
fn main() {
let _a: [u32; ARRAY_LEN] = [1, 2, 3];
let _b: str<STR_LEN> = "hi";
let _c: fmtstr<FMT_STR_LEN, _> = f"hi";
}
"#;

let errors = get_program_errors(src);
assert_eq!(errors.len(), 0);
}

#[test]
fn non_u32_in_array_length() {
let src = r#"
global ARRAY_LEN: u8 = 3;
fn main() {
let _a: [u32; ARRAY_LEN] = [1, 2, 3];
}
"#;

let errors = get_program_errors(src);
assert_eq!(errors.len(), 1);

assert!(matches!(
errors[0].0,
CompilationError::TypeError(TypeCheckError::TypeKindMismatch { .. })
));
}

#[test]
fn use_non_u32_generic_in_struct() {
let src = r#"
struct S<let N: u8> {}
fn main() {
let _: S<3> = S {};
}
"#;

let errors = get_program_errors(src);
assert_eq!(errors.len(), 0);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
[package]
name = "global_without_a_type_used_as_array_length"
type = "bin"
authors = [""]
compiler_version = ">=0.33.0"

[dependencies]
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
global BAR = OOPS;
global X: [Field; BAR] = [];
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ fn main() {
}

// Used in the signature of a function
fn id<let I: Field>(x: [Field; I]) -> [Field; I] {
fn id<let I: u32>(x: [Field; I]) -> [Field; I] {
x
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
[package]
name = "regression_6077"
type = "bin"
authors = [""]
compiler_version = ">=0.30.0"

[dependencies]
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
a = 0
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
struct WeirdStruct<T, U> {
a: T,
b: U,
}

#[mangle_fn]
pub fn my_fn() -> [u8; 3] {
[0; 3]
}

comptime fn mangle_fn(f: FunctionDefinition) {
let return_type = f.return_type();

// This relies on how types are displayed
let generics = f"Field,{return_type}".quoted_contents();
let new_return_type = quote { WeirdStruct<$generics>}.as_type();

let new_body = quote {
{
WeirdStruct { a: 1, b: [0;3] }
}
}.as_expr().unwrap();

f.set_return_type(new_return_type);
f.set_body(new_body);
}

fn main() {}

0 comments on commit 95454bc

Please sign in to comment.