chore: proptest for canonicalize on infix type expressions (#6269)

Co-authored-by: Tom French <[email protected]>
noir-lang · Nov 6, 2024 · 9ef8369 · 9ef8369
1 parent fb1a8ca
commit 9ef8369
Show file tree

Hide file tree

Showing 12 changed files with 412 additions and 130 deletions.
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/compiler/noirc_frontend/Cargo.toml b/compiler/noirc_frontend/Cargo.toml
@@ -36,6 +36,8 @@ strum_macros = "0.24"
 
 [dev-dependencies]
 base64.workspace = true
+proptest.workspace = true
+proptest-derive = "0.5.0"
 
 [features]
 experimental_parser = []

diff --git a/compiler/noirc_frontend/proptest-regressions/tests/arithmetic_generics.txt b/compiler/noirc_frontend/proptest-regressions/tests/arithmetic_generics.txt
@@ -0,0 +1,7 @@
+# Seeds for failure cases proptest has generated in the past. It is
+# automatically read and these particular cases re-run before any
+# novel cases are generated.
+#
+# It is recommended to check this file in to source control so that
+# everyone who runs the test benefits from these saved cases.
+cc fc27f4091dfa5f938973048209b5fcf22aefa1cfaffaaa3e349f30e9b1f93f49 # shrinks to infix_and_bindings = (((0: numeric bool) % (Numeric(Shared(RefCell { value: Unbound('2, Numeric(bool)) }): bool) + Numeric(Shared(RefCell { value: Unbound('0, Numeric(bool)) }): bool))), [('0, (0: numeric bool)), ('1, (0: numeric bool)), ('2, (0: numeric bool))])
diff --git a/compiler/noirc_frontend/src/ast/expression.rs b/compiler/noirc_frontend/src/ast/expression.rs
@@ -98,7 +98,7 @@ impl UnresolvedGeneric {
             UnresolvedGeneric::Variable(_) => Ok(Kind::Normal),
             UnresolvedGeneric::Numeric { typ, .. } => {
                 let typ = self.resolve_numeric_kind_type(typ)?;
-                Ok(Kind::Numeric(Box::new(typ)))
+                Ok(Kind::numeric(typ))
             }
             UnresolvedGeneric::Resolved(..) => {
                 panic!("Don't know the kind of a resolved generic here")

diff --git a/compiler/noirc_frontend/src/ast/mod.rs b/compiler/noirc_frontend/src/ast/mod.rs
@@ -19,6 +19,9 @@ pub use visitor::Visitor;
 pub use expression::*;
 pub use function::*;
 
+#[cfg(test)]
+use proptest_derive::Arbitrary;
+
 use acvm::FieldElement;
 pub use docs::*;
 use noirc_errors::Span;
@@ -37,6 +40,7 @@ use crate::{
 use acvm::acir::AcirField;
 use iter_extended::vecmap;
 
+#[cfg_attr(test, derive(Arbitrary))]
 #[derive(Debug, PartialEq, Eq, Clone, Copy, Hash, Ord, PartialOrd)]
 pub enum IntegerBitSize {
     One,

diff --git a/compiler/noirc_frontend/src/elaborator/mod.rs b/compiler/noirc_frontend/src/elaborator/mod.rs
@@ -641,7 +641,7 @@ impl<'context> Elaborator<'context> {
             let typ = if unresolved_typ.is_type_expression() {
                 self.resolve_type_inner(
                     unresolved_typ.clone(),
-                    &Kind::Numeric(Box::new(Type::default_int_type())),
+                    &Kind::numeric(Type::default_int_type()),
                 )
             } else {
                 self.resolve_type(unresolved_typ.clone())
@@ -654,7 +654,7 @@ impl<'context> Elaborator<'context> {
                     });
                 self.push_err(unsupported_typ_err);
             }
-            Kind::Numeric(Box::new(typ))
+            Kind::numeric(typ)
         } else {
             Kind::Normal
         }

diff --git a/compiler/noirc_frontend/src/elaborator/types.rs b/compiler/noirc_frontend/src/elaborator/types.rs
@@ -414,7 +414,7 @@ impl<'context> Elaborator<'context> {
                 let kind = self
                     .interner
                     .get_global_let_statement(id)
-                    .map(|let_statement| Kind::Numeric(Box::new(let_statement.r#type)))
+                    .map(|let_statement| Kind::numeric(let_statement.r#type))
                     .unwrap_or(Kind::u32());
 
                 // TODO(https://github.com/noir-lang/noir/issues/6238):

diff --git a/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs b/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs
@@ -541,7 +541,7 @@ impl<'a> ModCollector<'a> {
                                 name: Rc::new(name.to_string()),
                                 type_var: TypeVariable::unbound(
                                     type_variable_id,
-                                    Kind::Numeric(Box::new(typ)),
+                                    Kind::numeric(typ),
                                 ),
                                 span: name.span(),
                             });

diff --git a/compiler/noirc_frontend/src/hir_def/types.rs b/compiler/noirc_frontend/src/hir_def/types.rs
@@ -5,6 +5,9 @@ use std::{
     rc::Rc,
 };
 
+#[cfg(test)]
+use proptest_derive::Arbitrary;
+
 use acvm::{AcirField, FieldElement};
 
 use crate::{
@@ -169,6 +172,11 @@ pub enum Kind {
 }
 
 impl Kind {
+    // Kind::Numeric constructor helper
+    pub fn numeric(typ: Type) -> Kind {
+        Kind::Numeric(Box::new(typ))
+    }
+
     pub(crate) fn is_error(&self) -> bool {
         match self.follow_bindings() {
             Self::Numeric(typ) => *typ == Type::Error,
@@ -196,7 +204,7 @@ impl Kind {
     }
 
     pub(crate) fn u32() -> Self {
-        Self::Numeric(Box::new(Type::Integer(Signedness::Unsigned, IntegerBitSize::ThirtyTwo)))
+        Self::numeric(Type::Integer(Signedness::Unsigned, IntegerBitSize::ThirtyTwo))
     }
 
     pub(crate) fn follow_bindings(&self) -> Self {
@@ -205,7 +213,7 @@ impl Kind {
             Self::Normal => Self::Normal,
             Self::Integer => Self::Integer,
             Self::IntegerOrField => Self::IntegerOrField,
-            Self::Numeric(typ) => Self::Numeric(Box::new(typ.follow_bindings())),
+            Self::Numeric(typ) => Self::numeric(typ.follow_bindings()),
         }
     }
 
@@ -671,6 +679,7 @@ impl<T> Shared<T> {
 
 /// A restricted subset of binary operators useable on
 /// type level integers for use in the array length positions of types.
+#[cfg_attr(test, derive(Arbitrary))]
 #[derive(Debug, Copy, Clone, Hash, PartialEq, Eq, PartialOrd, Ord)]
 pub enum BinaryTypeOperator {
     Addition,
@@ -1422,7 +1431,7 @@ impl Type {
         if self_kind.unifies(&other_kind) {
             self_kind
         } else {
-            Kind::Numeric(Box::new(Type::Error))
+            Kind::numeric(Type::Error)
         }
     }
 

diff --git a/compiler/noirc_frontend/src/monomorphization/mod.rs b/compiler/noirc_frontend/src/monomorphization/mod.rs
@@ -944,9 +944,7 @@ impl<'interner> Monomorphizer<'interner> {
                 };
                 let location = self.interner.id_location(expr_id);
 
-                if !Kind::Numeric(numeric_typ.clone())
-                    .unifies(&Kind::Numeric(Box::new(typ.clone())))
-                {
+                if !Kind::Numeric(numeric_typ.clone()).unifies(&Kind::numeric(typ.clone())) {
                     let message = "ICE: Generic's kind does not match expected type";
                     return Err(MonomorphizationError::InternalError { location, message });
                 }

diff --git a/compiler/noirc_frontend/src/tests.rs b/compiler/noirc_frontend/src/tests.rs
@@ -1,6 +1,7 @@
 #![cfg(test)]
 
 mod aliases;
+mod arithmetic_generics;
 mod bound_checks;
 mod imports;
 mod metaprogramming;
@@ -16,7 +17,6 @@ mod visibility;
 // A test harness will allow for more expressive and readable tests
 use std::collections::BTreeMap;
 
-use acvm::{AcirField, FieldElement};
 use fm::FileId;
 
 use iter_extended::vecmap;
@@ -37,7 +37,6 @@ use crate::hir::def_collector::dc_crate::DefCollector;
 use crate::hir::def_map::{CrateDefMap, LocalModuleId};
 use crate::hir_def::expr::HirExpression;
 use crate::hir_def::stmt::HirStatement;
-use crate::hir_def::types::{BinaryTypeOperator, Type};
 use crate::monomorphization::ast::Program;
 use crate::monomorphization::errors::MonomorphizationError;
 use crate::monomorphization::monomorphize;
@@ -3210,122 +3209,6 @@ fn as_trait_path_syntax_no_impl() {
     assert!(matches!(&errors[0].0, TypeError(TypeCheckError::NoMatchingImplFound { .. })));
 }
 
-#[test]
-fn arithmetic_generics_canonicalization_deduplication_regression() {
-    let source = r#"
-        struct ArrData<let N: u32> {
-            a: [Field; N],
-            b: [Field; N + N - 1],
-        }
-
-        fn main() {
-            let _f: ArrData<5> = ArrData {
-                a: [0; 5],
-                b: [0; 9],
-            };
-        }
-    "#;
-    let errors = get_program_errors(source);
-    assert_eq!(errors.len(), 0);
-}
-
-#[test]
-fn arithmetic_generics_checked_cast_zeros() {
-    let source = r#"
-        struct W<let N: u1> {}
-        
-        fn foo<let N: u1>(_x: W<N>) -> W<(0 * N) / (N % N)> {
-            W {}
-        }
-        
-        fn bar<let N: u1>(_x: W<N>) -> u1 {
-            N
-        }
-        
-        fn main() -> pub u1 {
-            let w_0: W<0> = W {};
-            let w: W<_> = foo(w_0);
-            bar(w)
-        }
-    "#;
-
-    let errors = get_program_errors(source);
-    assert_eq!(errors.len(), 0);
-
-    let monomorphization_error = get_monomorphization_error(source);
-    assert!(monomorphization_error.is_some());
-
-    // Expect a CheckedCast (0 % 0) failure
-    let monomorphization_error = monomorphization_error.unwrap();
-    if let MonomorphizationError::UnknownArrayLength { ref length, ref err, location: _ } =
-        monomorphization_error
-    {
-        match length {
-            Type::CheckedCast { from, to } => {
-                assert!(matches!(*from.clone(), Type::InfixExpr { .. }));
-                assert!(matches!(*to.clone(), Type::InfixExpr { .. }));
-            }
-            _ => panic!("unexpected length: {:?}", length),
-        }
-        assert!(matches!(
-            err,
-            TypeCheckError::FailingBinaryOp { op: BinaryTypeOperator::Modulo, lhs: 0, rhs: 0, .. }
-        ));
-    } else {
-        panic!("unexpected error: {:?}", monomorphization_error);
-    }
-}
-
-#[test]
-fn arithmetic_generics_checked_cast_indirect_zeros() {
-    let source = r#"
-        struct W<let N: Field> {}
-        
-        fn foo<let N: Field>(_x: W<N>) -> W<(N - N) % (N - N)> {
-            W {}
-        }
-        
-        fn bar<let N: Field>(_x: W<N>) -> Field {
-            N
-        }
-        
-        fn main() {
-            let w_0: W<0> = W {};
-            let w = foo(w_0);
-            let _ = bar(w);
-        }
-    "#;
-
-    let errors = get_program_errors(source);
-    assert_eq!(errors.len(), 0);
-
-    let monomorphization_error = get_monomorphization_error(source);
-    assert!(monomorphization_error.is_some());
-
-    // Expect a CheckedCast (0 % 0) failure
-    let monomorphization_error = monomorphization_error.unwrap();
-    if let MonomorphizationError::UnknownArrayLength { ref length, ref err, location: _ } =
-        monomorphization_error
-    {
-        match length {
-            Type::CheckedCast { from, to } => {
-                assert!(matches!(*from.clone(), Type::InfixExpr { .. }));
-                assert!(matches!(*to.clone(), Type::InfixExpr { .. }));
-            }
-            _ => panic!("unexpected length: {:?}", length),
-        }
-        match err {
-            TypeCheckError::ModuloOnFields { lhs, rhs, .. } => {
-                assert_eq!(lhs.clone(), FieldElement::zero());
-                assert_eq!(rhs.clone(), FieldElement::zero());
-            }
-            _ => panic!("expected ModuloOnFields, but found: {:?}", err),
-        }
-    } else {
-        panic!("unexpected error: {:?}", monomorphization_error);
-    }
-}
-
 #[test]
 fn infer_globals_to_u32_from_type_use() {
     let src = r#"