From 4cc3e787f75196e9fce86600d4042d5ccb453324 Mon Sep 17 00:00:00 2001 From: "Michael J. Klein" Date: Wed, 31 Jan 2024 16:13:41 -0500 Subject: [PATCH 01/11] add test for mapping slice --- test_programs/compile_success_empty/map_slice/Nargo.toml | 7 +++++++ .../compile_success_empty/map_slice/src/main.nr | 9 +++++++++ 2 files changed, 16 insertions(+) create mode 100644 test_programs/compile_success_empty/map_slice/Nargo.toml create mode 100644 test_programs/compile_success_empty/map_slice/src/main.nr diff --git a/test_programs/compile_success_empty/map_slice/Nargo.toml b/test_programs/compile_success_empty/map_slice/Nargo.toml new file mode 100644 index 00000000000..6e5ea92fd16 --- /dev/null +++ b/test_programs/compile_success_empty/map_slice/Nargo.toml @@ -0,0 +1,7 @@ +[package] +name = "map_slice" +type = "bin" +authors = [""] +compiler_version = ">=0.23.0" + +[dependencies] \ No newline at end of file diff --git a/test_programs/compile_success_empty/map_slice/src/main.nr b/test_programs/compile_success_empty/map_slice/src/main.nr new file mode 100644 index 00000000000..bd8007a8642 --- /dev/null +++ b/test_programs/compile_success_empty/map_slice/src/main.nr @@ -0,0 +1,9 @@ +fn main() { + let x: [Field] = [3; 4]; + let _ = x.map(|y| y); +} + +#[test] +fn test_main() { + main(); +} From ac2af8bada92a1fd9329665691d44731db1bd4c1 Mon Sep 17 00:00:00 2001 From: "Michael J. Klein" Date: Mon, 5 Feb 2024 11:33:45 -0500 Subject: [PATCH 02/11] wip: generic ident for N?-style generics, add lexer case, update parser, add wip test case --- compiler/noirc_frontend/src/ast/expression.rs | 4 ++-- compiler/noirc_frontend/src/ast/statement.rs | 19 +++++++++++++++++++ compiler/noirc_frontend/src/ast/structure.rs | 4 ++-- compiler/noirc_frontend/src/ast/traits.rs | 6 +++--- .../src/hir/resolution/resolver.rs | 15 ++++++++------- .../src/hir/resolution/traits.rs | 3 ++- compiler/noirc_frontend/src/lexer/lexer.rs | 4 +++- compiler/noirc_frontend/src/lexer/token.rs | 3 +++ compiler/noirc_frontend/src/parser/parser.rs | 12 ++++++++---- .../array_slice_polymorphism/Nargo.toml | 7 +++++++ .../array_slice_polymorphism/src/main.nr | 18 ++++++++++++++++++ 11 files changed, 75 insertions(+), 20 deletions(-) create mode 100644 test_programs/compile_success_empty/array_slice_polymorphism/Nargo.toml create mode 100644 test_programs/compile_success_empty/array_slice_polymorphism/src/main.nr diff --git a/compiler/noirc_frontend/src/ast/expression.rs b/compiler/noirc_frontend/src/ast/expression.rs index c78deaf6dbb..7a64c675d97 100644 --- a/compiler/noirc_frontend/src/ast/expression.rs +++ b/compiler/noirc_frontend/src/ast/expression.rs @@ -3,7 +3,7 @@ use std::fmt::Display; use crate::token::{Attributes, Token}; use crate::{ - Distinctness, FunctionVisibility, Ident, Path, Pattern, Recoverable, Statement, StatementKind, + Distinctness, FunctionVisibility, Ident, GenericIdent, Path, Pattern, Recoverable, Statement, StatementKind, UnresolvedTraitConstraint, UnresolvedType, UnresolvedTypeData, Visibility, }; use acvm::FieldElement; @@ -32,7 +32,7 @@ pub enum ExpressionKind { /// A Vec of unresolved names for type variables. /// For `fn foo(...)` this corresponds to vec!["A", "B"]. -pub type UnresolvedGenerics = Vec; +pub type UnresolvedGenerics = Vec; impl ExpressionKind { pub fn into_path(self) -> Option { diff --git a/compiler/noirc_frontend/src/ast/statement.rs b/compiler/noirc_frontend/src/ast/statement.rs index a2c29a8c52e..7d59db15203 100644 --- a/compiler/noirc_frontend/src/ast/statement.rs +++ b/compiler/noirc_frontend/src/ast/statement.rs @@ -233,6 +233,25 @@ impl Recoverable for Vec { } } +#[derive(Eq, PartialEq, Debug, Clone)] +pub struct GenericIdent { + pub ident: Ident, + /// "N?" prevents "N" from being used as a numeric. + /// This allows "N" to be used for both array and slice lengths. + pub prevent_numeric: bool, +} + +impl Display for GenericIdent { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + let question_mark = if self.prevent_numeric { + "?" + } else { + "" + }; + write!(f, "{}{}", self.ident, question_mark) + } +} + /// Trait for recoverable nodes during parsing. /// This is similar to Default but is expected /// to return an Error node of the appropriate type. diff --git a/compiler/noirc_frontend/src/ast/structure.rs b/compiler/noirc_frontend/src/ast/structure.rs index 6a32fa717f3..b674d43a9e9 100644 --- a/compiler/noirc_frontend/src/ast/structure.rs +++ b/compiler/noirc_frontend/src/ast/structure.rs @@ -1,6 +1,6 @@ use std::fmt::Display; -use crate::{token::SecondaryAttribute, Ident, UnresolvedGenerics, UnresolvedType}; +use crate::{token::SecondaryAttribute, Ident, GenericIdent, UnresolvedGenerics, UnresolvedType}; use iter_extended::vecmap; use noirc_errors::Span; @@ -18,7 +18,7 @@ impl NoirStruct { pub fn new( name: Ident, attributes: Vec, - generics: Vec, + generics: Vec, fields: Vec<(Ident, UnresolvedType)>, span: Span, ) -> NoirStruct { diff --git a/compiler/noirc_frontend/src/ast/traits.rs b/compiler/noirc_frontend/src/ast/traits.rs index feb627df60b..56281206a6e 100644 --- a/compiler/noirc_frontend/src/ast/traits.rs +++ b/compiler/noirc_frontend/src/ast/traits.rs @@ -4,7 +4,7 @@ use iter_extended::vecmap; use noirc_errors::Span; use crate::{ - node_interner::TraitId, BlockExpression, Expression, FunctionReturnType, Ident, NoirFunction, + node_interner::TraitId, BlockExpression, Expression, FunctionReturnType, Ident, GenericIdent, NoirFunction, Path, UnresolvedGenerics, UnresolvedType, }; @@ -13,7 +13,7 @@ use crate::{ #[derive(Clone, Debug)] pub struct NoirTrait { pub name: Ident, - pub generics: Vec, + pub generics: Vec, pub where_clause: Vec, pub span: Span, pub items: Vec, @@ -25,7 +25,7 @@ pub struct NoirTrait { pub enum TraitItem { Function { name: Ident, - generics: Vec, + generics: Vec, parameters: Vec<(Ident, UnresolvedType)>, return_type: FunctionReturnType, where_clause: Vec, diff --git a/compiler/noirc_frontend/src/hir/resolution/resolver.rs b/compiler/noirc_frontend/src/hir/resolution/resolver.rs index 7b876244601..63c5986e08a 100644 --- a/compiler/noirc_frontend/src/hir/resolution/resolver.rs +++ b/compiler/noirc_frontend/src/hir/resolution/resolver.rs @@ -105,7 +105,8 @@ pub struct Resolver<'a> { /// unique type variables if we're resolving a struct. Empty otherwise. /// This is a Vec rather than a map to preserve the order a functions generics /// were declared in. - generics: Vec<(Rc, TypeVariable, Span)>, + /// The bool represents 'prevent_numeric', i.e. "N?" + generics: Vec<(Rc, TypeVariable, Span, bool)>, /// When resolving lambda expressions, we need to keep track of the variables /// that are captured. We do this in order to create the hidden environment @@ -782,14 +783,14 @@ impl<'a> Resolver<'a> { // Map the generic to a fresh type variable let id = self.interner.next_type_variable_id(); let typevar = TypeVariable::unbound(id); - let span = generic.0.span(); + let span = generic.ident.0.span(); // Check for name collisions of this generic - let name = Rc::new(generic.0.contents.clone()); + let name = Rc::new(generic.ident.0.contents.clone()); if let Some((_, _, first_span)) = self.find_generic(&name) { self.errors.push(ResolverError::DuplicateDefinition { - name: generic.0.contents.clone(), + name: generic.ident.0.contents.clone(), first_span: *first_span, second_span: span, }); @@ -808,11 +809,11 @@ impl<'a> Resolver<'a> { assert_eq!(names.len(), generics.len()); for (name, typevar) in names.iter().zip(generics) { - self.add_existing_generic(&name.0.contents, name.0.span(), typevar.clone()); + self.add_existing_generic(&name.ident.0.contents, name.ident.0.span(), typevar.clone(), name.prevent_numeric); } } - pub fn add_existing_generic(&mut self, name: &str, span: Span, typevar: TypeVariable) { + pub fn add_existing_generic(&mut self, name: &str, span: Span, typevar: TypeVariable, prevent_numeric: bool) { // Check for name collisions of this generic let rc_name = Rc::new(name.to_owned()); @@ -823,7 +824,7 @@ impl<'a> Resolver<'a> { second_span: span, }); } else { - self.generics.push((rc_name, typevar, span)); + self.generics.push((rc_name, typevar, span, prevent_numeric)); } } diff --git a/compiler/noirc_frontend/src/hir/resolution/traits.rs b/compiler/noirc_frontend/src/hir/resolution/traits.rs index 8f966be312b..109de8f343e 100644 --- a/compiler/noirc_frontend/src/hir/resolution/traits.rs +++ b/compiler/noirc_frontend/src/hir/resolution/traits.rs @@ -121,11 +121,12 @@ fn resolve_trait_methods( let self_typevar = the_trait.self_type_typevar.clone(); let self_type = Type::TypeVariable(self_typevar.clone(), TypeVariableKind::Normal); let name_span = the_trait.name.span(); + let self_prevent_numeric = false; let mut resolver = Resolver::new(interner, &path_resolver, def_maps, file); resolver.add_generics(generics); resolver.add_existing_generics(&unresolved_trait.trait_def.generics, trait_generics); - resolver.add_existing_generic("Self", name_span, self_typevar); + resolver.add_existing_generic("Self", name_span, self_typevar, self_prevent_numeric); resolver.set_self_type(Some(self_type.clone())); let func_id = unresolved_trait.method_ids[&name.0.contents]; diff --git a/compiler/noirc_frontend/src/lexer/lexer.rs b/compiler/noirc_frontend/src/lexer/lexer.rs index fd8168e36c6..0ee082dc417 100644 --- a/compiler/noirc_frontend/src/lexer/lexer.rs +++ b/compiler/noirc_frontend/src/lexer/lexer.rs @@ -109,6 +109,7 @@ impl<'a> Lexer<'a> { Some('.') => self.glue(Token::Dot), Some(':') => self.glue(Token::Colon), Some('!') => self.glue(Token::Bang), + Some('?') => self.glue(Token::Question), Some('-') => self.glue(Token::Minus), Some('&') => self.ampersand(), Some('|') => self.single_char_token(Token::Pipe), @@ -584,10 +585,11 @@ mod tests { use crate::token::{FunctionAttribute, SecondaryAttribute, TestScope}; #[test] fn test_single_double_char() { - let input = "! != + ( ) { } [ ] | , ; : :: < <= > >= & - -> . .. % / * = == << >>"; + let input = "! ? != + ( ) { } [ ] | , ; : :: < <= > >= & - -> . .. % / * = == << >>"; let expected = vec![ Token::Bang, + Token::Question, Token::NotEqual, Token::Plus, Token::LeftParen, diff --git a/compiler/noirc_frontend/src/lexer/token.rs b/compiler/noirc_frontend/src/lexer/token.rs index 835a0baae3f..d9a6deda151 100644 --- a/compiler/noirc_frontend/src/lexer/token.rs +++ b/compiler/noirc_frontend/src/lexer/token.rs @@ -84,6 +84,8 @@ pub enum Token { Semicolon, /// ! Bang, + /// ? + Question, /// = Assign, #[allow(clippy::upper_case_acronyms)] @@ -199,6 +201,7 @@ impl fmt::Display for Token { Token::Semicolon => write!(f, ";"), Token::Assign => write!(f, "="), Token::Bang => write!(f, "!"), + Token::Question => write!(f, "?"), Token::EOF => write!(f, "end of input"), Token::Invalid(c) => write!(f, "{c}"), Token::Whitespace(ref s) => write!(f, "{s}"), diff --git a/compiler/noirc_frontend/src/parser/parser.rs b/compiler/noirc_frontend/src/parser/parser.rs index 1bdb276d4fb..a527a7fd2e4 100644 --- a/compiler/noirc_frontend/src/parser/parser.rs +++ b/compiler/noirc_frontend/src/parser/parser.rs @@ -38,7 +38,7 @@ use crate::parser::{force, ignore_then_commit, statement_recovery}; use crate::token::{Attribute, Attributes, Keyword, SecondaryAttribute, Token, TokenKind}; use crate::{ BinaryOp, BinaryOpKind, BlockExpression, ConstrainKind, ConstrainStatement, Distinctness, - ForLoopStatement, ForRange, FunctionDefinition, FunctionReturnType, FunctionVisibility, Ident, + ForLoopStatement, ForRange, FunctionDefinition, FunctionReturnType, FunctionVisibility, Ident, GenericIdent, IfExpression, InfixExpression, LValue, Lambda, Literal, NoirFunction, NoirStruct, NoirTrait, NoirTraitImpl, NoirTypeAlias, Param, Path, PathKind, Pattern, Recoverable, Statement, TraitBound, TraitImplItem, TraitItem, TypeImpl, UnaryOp, UnresolvedTraitConstraint, @@ -233,13 +233,17 @@ fn is_pub_crate() -> impl NoirParser { .map(|a| a.is_some()) } -/// non_empty_ident_list: ident ',' non_empty_ident_list -/// | ident +/// opt_ident: ident | ident '?' +/// +/// non_empty_ident_list: opt_ident ',' non_empty_ident_list +/// | opt_ident /// /// generics: '<' non_empty_ident_list '>' /// | %empty -fn generics() -> impl NoirParser> { +fn generics() -> impl NoirParser> { ident() + .then(just(Token::Question).or_not()) + .map(|(id, opt_question)| GenericIdent { ident: id, prevent_numeric: opt_question.is_some() }) .separated_by(just(Token::Comma)) .allow_trailing() .at_least(1) diff --git a/test_programs/compile_success_empty/array_slice_polymorphism/Nargo.toml b/test_programs/compile_success_empty/array_slice_polymorphism/Nargo.toml new file mode 100644 index 00000000000..0b35bbf9c02 --- /dev/null +++ b/test_programs/compile_success_empty/array_slice_polymorphism/Nargo.toml @@ -0,0 +1,7 @@ +[package] +name = "array_slice_polymorphism" +type = "bin" +authors = [""] +compiler_version = ">=0.23.0" + +[dependencies] \ No newline at end of file diff --git a/test_programs/compile_success_empty/array_slice_polymorphism/src/main.nr b/test_programs/compile_success_empty/array_slice_polymorphism/src/main.nr new file mode 100644 index 00000000000..df704cf31bb --- /dev/null +++ b/test_programs/compile_success_empty/array_slice_polymorphism/src/main.nr @@ -0,0 +1,18 @@ + +fn array_zeros(_x: [u64; N]) -> [u64; N] { + [0; N] +} + +fn main(x: Field, y: pub Field) { + let xs: [u64] = [0; 0]; + assert(array_zeros(xs) == xs); + assert(x != y); +} + +#[test] +fn test_main() { + main(1, 2); + + // Uncomment to make test fail + // main(1, 1); +} From 35931000505832912f78c7acf7c7c9b28adaa533 Mon Sep 17 00:00:00 2001 From: "Michael J. Klein" Date: Wed, 7 Feb 2024 10:25:10 -0500 Subject: [PATCH 03/11] wip propagating prevent_numeric field --- .../src/hir/resolution/resolver.rs | 30 +++++++++++-------- 1 file changed, 18 insertions(+), 12 deletions(-) diff --git a/compiler/noirc_frontend/src/hir/resolution/resolver.rs b/compiler/noirc_frontend/src/hir/resolution/resolver.rs index 1eca9195fee..ccdbb0cb046 100644 --- a/compiler/noirc_frontend/src/hir/resolution/resolver.rs +++ b/compiler/noirc_frontend/src/hir/resolution/resolver.rs @@ -537,8 +537,8 @@ impl<'a> Resolver<'a> { resolved_type } - fn find_generic(&self, target_name: &str) -> Option<&(Rc, TypeVariable, Span)> { - self.generics.iter().find(|(name, _, _)| name.as_ref() == target_name) + fn find_generic(&self, target_name: &str) -> Option<&(Rc, TypeVariable, Span, bool)> { + self.generics.iter().find(|(name, _, _, _)| name.as_ref() == target_name) } fn resolve_named_type( @@ -653,8 +653,8 @@ impl<'a> Resolver<'a> { fn lookup_generic_or_global_type(&mut self, path: &Path) -> Option { if path.segments.len() == 1 { let name = &path.last_segment().0.contents; - if let Some((name, var, _)) = self.find_generic(name) { - return Some(Type::NamedGeneric(var.clone(), name.clone())); + if let Some((name, var, _, prevent_numeric)) = self.find_generic(name) { + return Some(Type::NamedGeneric(var.clone(), name.clone(), prevent_numeric)); } } @@ -768,14 +768,14 @@ impl<'a> Resolver<'a> { /// Return the current generics. /// Needed to keep referring to the same type variables across many /// methods in a single impl. - pub fn get_generics(&self) -> &[(Rc, TypeVariable, Span)] { + pub fn get_generics(&self) -> &[(Rc, TypeVariable, Span, bool)] { &self.generics } /// Set the current generics that are in scope. /// Unlike add_generics, this function will not create any new type variables, /// opting to reuse the existing ones it is directly given. - pub fn set_generics(&mut self, generics: Vec<(Rc, TypeVariable, Span)>) { + pub fn set_generics(&mut self, generics: Vec<(Rc, TypeVariable, Span, bool)>) { self.generics = generics; } @@ -796,18 +796,19 @@ impl<'a> Resolver<'a> { let id = self.interner.next_type_variable_id(); let typevar = TypeVariable::unbound(id); let span = generic.ident.0.span(); + let prevent_numeric = generic.prevent_numeric; // Check for name collisions of this generic let name = Rc::new(generic.ident.0.contents.clone()); - if let Some((_, _, first_span)) = self.find_generic(&name) { + if let Some((_, _, first_span, _)) = self.find_generic(&name) { self.errors.push(ResolverError::DuplicateDefinition { name: generic.ident.0.contents.clone(), first_span: *first_span, second_span: span, }); } else { - self.generics.push((name, typevar.clone(), span)); + self.generics.push((name, typevar.clone(), span, prevent_numeric)); } typevar @@ -829,7 +830,7 @@ impl<'a> Resolver<'a> { // Check for name collisions of this generic let rc_name = Rc::new(name.to_owned()); - if let Some((_, _, first_span)) = self.find_generic(&rc_name) { + if let Some((_, _, first_span, _)) = self.find_generic(&rc_name) { self.errors.push(ResolverError::DuplicateDefinition { name: name.to_owned(), first_span: *first_span, @@ -890,7 +891,8 @@ impl<'a> Resolver<'a> { let attributes = func.attributes().clone(); - let mut generics = vecmap(&self.generics, |(_, typevar, _)| typevar.clone()); + // TODO: prevent_numeric needed here? + let mut generics = vecmap(&self.generics, |(_, typevar, _, _)| typevar.clone()); let mut parameters = vec![]; let mut parameter_types = vec![]; @@ -1043,9 +1045,13 @@ impl<'a> Resolver<'a> { // We can fail to find the generic in self.generics if it is an implicit one created // by the compiler. This can happen when, e.g. eliding array lengths using the slice // syntax [T]. - if let Some((name, _, span)) = - self.generics.iter().find(|(name, _, _)| name.as_ref() == &name_to_find) + if let Some((name, _, span, prevent_numeric)) = + self.generics.iter().find(|(name, _, _, _)| name.as_ref() == &name_to_find) { + if *prevent_numeric { + panic!("declare_numeric_generics: TODO: likely do nothing here?") + } + let ident = Ident::new(name.to_string(), *span); let definition = DefinitionKind::GenericType(type_variable); self.add_variable_decl_inner(ident, false, false, false, definition); From a1204ab6a6589bb29a3c57a0f2f6087e16656135 Mon Sep 17 00:00:00 2001 From: "Michael J. Klein" Date: Mon, 12 Feb 2024 14:06:07 -0500 Subject: [PATCH 04/11] wip 'prevent_numeric', updating tuples, etc --- .../src/hir/resolution/functions.rs | 2 +- .../src/hir/resolution/resolver.rs | 31 ++++++++++++------- .../src/hir/resolution/traits.rs | 4 +-- .../noirc_frontend/src/hir/type_check/expr.rs | 2 +- compiler/noirc_frontend/src/hir_def/types.rs | 30 +++++++++++------- 5 files changed, 42 insertions(+), 27 deletions(-) diff --git a/compiler/noirc_frontend/src/hir/resolution/functions.rs b/compiler/noirc_frontend/src/hir/resolution/functions.rs index e63de9b9173..e66dd515645 100644 --- a/compiler/noirc_frontend/src/hir/resolution/functions.rs +++ b/compiler/noirc_frontend/src/hir/resolution/functions.rs @@ -24,7 +24,7 @@ pub(crate) fn resolve_function_set( mut unresolved_functions: UnresolvedFunctions, self_type: Option, trait_impl_id: Option, - impl_generics: Vec<(Rc, TypeVariable, Span)>, + impl_generics: Vec<(Rc, TypeVariable, Span, bool)>, errors: &mut Vec<(CompilationError, FileId)>, ) -> Vec<(FileId, FuncId)> { let file_id = unresolved_functions.file_id; diff --git a/compiler/noirc_frontend/src/hir/resolution/resolver.rs b/compiler/noirc_frontend/src/hir/resolution/resolver.rs index c8ba8c5e489..a065170042c 100644 --- a/compiler/noirc_frontend/src/hir/resolution/resolver.rs +++ b/compiler/noirc_frontend/src/hir/resolution/resolver.rs @@ -507,7 +507,7 @@ impl<'a> Resolver<'a> { let env = Box::new(self.resolve_type_inner(*env, new_variables)); match *env { - Type::Unit | Type::Tuple(_) | Type::NamedGeneric(_, _) => { + Type::Unit | Type::Tuple(_) | Type::NamedGeneric(_, _, _) => { Type::Function(args, ret, env) } _ => { @@ -654,7 +654,7 @@ impl<'a> Resolver<'a> { if path.segments.len() == 1 { let name = &path.last_segment().0.contents; if let Some((name, var, _, prevent_numeric)) = self.find_generic(name) { - return Some(Type::NamedGeneric(var.clone(), name.clone(), prevent_numeric)); + return Some(Type::NamedGeneric(var.clone(), name.clone(), *prevent_numeric)); } } @@ -684,7 +684,8 @@ impl<'a> Resolver<'a> { // 'Named'Generic is a bit of a misnomer here, we want a type variable that // wont be bound over but this one has no name since we do not currently // require users to explicitly be generic over array lengths. - Type::NamedGeneric(typevar, Rc::new("".into())) + let prevent_numeric = true; + Type::NamedGeneric(typevar, Rc::new("".into()), prevent_numeric) } Some(length) => self.convert_expression_type(length), } @@ -1080,14 +1081,16 @@ impl<'a> Resolver<'a> { | Type::Error | Type::TypeVariable(_, _) | Type::Constant(_) - | Type::NamedGeneric(_, _) + | Type::NamedGeneric(_, _, _) | Type::NotConstant | Type::TraitAsType(..) | Type::Forall(_, _) => (), Type::Array(length, element_type) => { - if let Type::NamedGeneric(type_variable, name) = length.as_ref() { - found.insert(name.to_string(), type_variable.clone()); + if let Type::NamedGeneric(type_variable, name, prevent_numeric) = length.as_ref() { + if !prevent_numeric { + found.insert(name.to_string(), type_variable.clone()); + } } Self::find_numeric_generics_in_type(element_type, found); } @@ -1107,8 +1110,8 @@ impl<'a> Resolver<'a> { Type::Struct(struct_type, generics) => { for (i, generic) in generics.iter().enumerate() { - if let Type::NamedGeneric(type_variable, name) = generic { - if struct_type.borrow().generic_is_numeric(i) { + if let Type::NamedGeneric(type_variable, name, prevent_numeric) = generic { + if struct_type.borrow().generic_is_numeric(i) && !prevent_numeric { found.insert(name.to_string(), type_variable.clone()); } } else { @@ -1118,13 +1121,17 @@ impl<'a> Resolver<'a> { } Type::MutableReference(element) => Self::find_numeric_generics_in_type(element, found), Type::String(length) => { - if let Type::NamedGeneric(type_variable, name) = length.as_ref() { - found.insert(name.to_string(), type_variable.clone()); + if let Type::NamedGeneric(type_variable, name, prevent_numeric) = length.as_ref() { + if !prevent_numeric { + found.insert(name.to_string(), type_variable.clone()); + } } } Type::FmtString(length, fields) => { - if let Type::NamedGeneric(type_variable, name) = length.as_ref() { - found.insert(name.to_string(), type_variable.clone()); + if let Type::NamedGeneric(type_variable, name, prevent_numeric) = length.as_ref() { + if !prevent_numeric { + found.insert(name.to_string(), type_variable.clone()); + } } Self::find_numeric_generics_in_type(fields, found); } diff --git a/compiler/noirc_frontend/src/hir/resolution/traits.rs b/compiler/noirc_frontend/src/hir/resolution/traits.rs index 109de8f343e..02160cad6a9 100644 --- a/compiler/noirc_frontend/src/hir/resolution/traits.rs +++ b/compiler/noirc_frontend/src/hir/resolution/traits.rs @@ -142,7 +142,7 @@ fn resolve_trait_methods( let arguments = vecmap(parameters, |param| resolver.resolve_type(param.1.clone())); let return_type = resolver.resolve_type(return_type.get_type().into_owned()); - let generics = vecmap(resolver.get_generics(), |(_, type_var, _)| type_var.clone()); + let generics = vecmap(resolver.get_generics(), |(_, type_var, _, _)| type_var.clone()); let default_impl_list: Vec<_> = unresolved_trait .fns_with_default_impl @@ -455,7 +455,7 @@ pub(crate) fn resolve_trait_impls( methods: vecmap(&impl_methods, |(_, func_id)| *func_id), }); - let impl_generics = vecmap(impl_generics, |(_, type_variable, _)| type_variable); + let impl_generics = vecmap(impl_generics, |(_, type_variable, _, _)| type_variable); if let Err((prev_span, prev_file)) = interner.add_trait_implementation( self_type.clone(), diff --git a/compiler/noirc_frontend/src/hir/type_check/expr.rs b/compiler/noirc_frontend/src/hir/type_check/expr.rs index 4bd7779587d..60d350e8644 100644 --- a/compiler/noirc_frontend/src/hir/type_check/expr.rs +++ b/compiler/noirc_frontend/src/hir/type_check/expr.rs @@ -920,7 +920,7 @@ impl<'interner> TypeChecker<'interner> { }); None } - Type::NamedGeneric(_, _) => { + Type::NamedGeneric(_, _, _) => { let func_meta = self.interner.function_meta( &self.current_function.expect("unexpected method outside a function"), ); diff --git a/compiler/noirc_frontend/src/hir_def/types.rs b/compiler/noirc_frontend/src/hir_def/types.rs index 0ba4cb2da65..385ebcf10cd 100644 --- a/compiler/noirc_frontend/src/hir_def/types.rs +++ b/compiler/noirc_frontend/src/hir_def/types.rs @@ -70,7 +70,9 @@ pub enum Type { /// NamedGenerics are the 'T' or 'U' in a user-defined generic function /// like `fn foo(...) {}`. Unlike TypeVariables, they cannot be bound over. - NamedGeneric(TypeVariable, Rc), + /// + /// The `bool` refers to whether to `prevent_numeric`, i.e. for `N?`. + NamedGeneric(TypeVariable, Rc, bool), /// A functions with arguments, a return type and environment. /// the environment should be `Unit` by default, @@ -86,6 +88,8 @@ pub enum Type { /// but it makes handling them both easier. The TypeVariableId should /// never be bound over during type checking, but during monomorphization it /// will be and thus needs the full TypeVariable link. + /// + /// TODO: before merge do we need N? here? Forall(Generics, Box), /// A type-level integer. Included to let an Array's size type variable @@ -134,7 +138,7 @@ impl Type { | Type::Unit | Type::TypeVariable(_, _) | Type::TraitAsType(..) - | Type::NamedGeneric(_, _) + | Type::NamedGeneric(_, _, _) | Type::Function(_, _, _) | Type::MutableReference(_) | Type::Forall(_, _) @@ -591,12 +595,16 @@ impl Type { fn contains_numeric_typevar(&self, target_id: TypeVariableId) -> bool { // True if the given type is a NamedGeneric with the target_id let named_generic_id_matches_target = |typ: &Type| { - if let Type::NamedGeneric(type_variable, _) = typ { - match &*type_variable.borrow() { - TypeBinding::Bound(_) => { - unreachable!("Named generics should not be bound until monomorphization") + if let Type::NamedGeneric(type_variable, _, prevent_numeric) = typ { + if prevent_numeric { + false + } else { + match &*type_variable.borrow() { + TypeBinding::Bound(_) => { + unreachable!("Named generics should not be bound until monomorphization") + } + TypeBinding::Unbound(id) => target_id == *id, } - TypeBinding::Unbound(id) => target_id == *id, } } else { false @@ -611,7 +619,7 @@ impl Type { | Type::Error | Type::TypeVariable(_, _) | Type::Constant(_) - | Type::NamedGeneric(_, _) + | Type::NamedGeneric(_, _, _) | Type::NotConstant | Type::Forall(_, _) | Type::TraitAsType(..) => false, @@ -666,7 +674,7 @@ impl Type { Type::FmtString(_, _) | Type::TypeVariable(_, _) - | Type::NamedGeneric(_, _) + | Type::NamedGeneric(_, _, _) | Type::Function(_, _, _) | Type::MutableReference(_) | Type::Forall(_, _) @@ -691,7 +699,7 @@ impl Type { pub fn generic_count(&self) -> usize { match self { Type::Forall(generics, _) => generics.len(), - Type::TypeVariable(type_variable, _) | Type::NamedGeneric(type_variable, _) => { + Type::TypeVariable(type_variable, _) | Type::NamedGeneric(type_variable, _, _) => { match &*type_variable.borrow() { TypeBinding::Bound(binding) => binding.generic_count(), TypeBinding::Unbound(_) => 0, @@ -1008,7 +1016,7 @@ impl Type { fn get_inner_type_variable(&self) -> Option> { match self { - Type::TypeVariable(var, _) | Type::NamedGeneric(var, _) => Some(var.1.clone()), + Type::TypeVariable(var, _) | Type::NamedGeneric(var, _, _) => Some(var.1.clone()), _ => None, } } From 010719aad0453ec361ed6813dd80f38999a79316 Mon Sep 17 00:00:00 2001 From: "Michael J. Klein" Date: Tue, 13 Feb 2024 23:12:54 -0500 Subject: [PATCH 05/11] wip: propagate named_generic, add formatting, add formatting and test in nargo_fmt, fix lexing, debugging --- compiler/noirc_frontend/src/ast/expression.rs | 4 +- compiler/noirc_frontend/src/ast/statement.rs | 6 +-- compiler/noirc_frontend/src/ast/structure.rs | 2 +- compiler/noirc_frontend/src/ast/traits.rs | 4 +- .../src/hir/resolution/resolver.rs | 30 +++++++++------ compiler/noirc_frontend/src/hir_def/types.rs | 38 ++++++++++++------- compiler/noirc_frontend/src/lexer/lexer.rs | 3 +- .../src/monomorphization/mod.rs | 2 +- compiler/noirc_frontend/src/node_interner.rs | 2 +- compiler/noirc_frontend/src/parser/parser.rs | 15 +++++--- tooling/nargo_fmt/src/utils.rs | 12 +++++- tooling/nargo_fmt/tests/expected/fn.nr | 14 +++++++ tooling/nargo_fmt/tests/input/fn.nr | 8 ++++ 13 files changed, 94 insertions(+), 46 deletions(-) diff --git a/compiler/noirc_frontend/src/ast/expression.rs b/compiler/noirc_frontend/src/ast/expression.rs index 7a64c675d97..ca7cd69494d 100644 --- a/compiler/noirc_frontend/src/ast/expression.rs +++ b/compiler/noirc_frontend/src/ast/expression.rs @@ -3,8 +3,8 @@ use std::fmt::Display; use crate::token::{Attributes, Token}; use crate::{ - Distinctness, FunctionVisibility, Ident, GenericIdent, Path, Pattern, Recoverable, Statement, StatementKind, - UnresolvedTraitConstraint, UnresolvedType, UnresolvedTypeData, Visibility, + Distinctness, FunctionVisibility, GenericIdent, Ident, Path, Pattern, Recoverable, Statement, + StatementKind, UnresolvedTraitConstraint, UnresolvedType, UnresolvedTypeData, Visibility, }; use acvm::FieldElement; use iter_extended::vecmap; diff --git a/compiler/noirc_frontend/src/ast/statement.rs b/compiler/noirc_frontend/src/ast/statement.rs index 1642cc7ed73..84026e6c8f3 100644 --- a/compiler/noirc_frontend/src/ast/statement.rs +++ b/compiler/noirc_frontend/src/ast/statement.rs @@ -243,11 +243,7 @@ pub struct GenericIdent { impl Display for GenericIdent { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - let question_mark = if self.prevent_numeric { - "?" - } else { - "" - }; + let question_mark = if self.prevent_numeric { "?" } else { "" }; write!(f, "{}{}", self.ident, question_mark) } } diff --git a/compiler/noirc_frontend/src/ast/structure.rs b/compiler/noirc_frontend/src/ast/structure.rs index b674d43a9e9..a3ef5d044d9 100644 --- a/compiler/noirc_frontend/src/ast/structure.rs +++ b/compiler/noirc_frontend/src/ast/structure.rs @@ -1,6 +1,6 @@ use std::fmt::Display; -use crate::{token::SecondaryAttribute, Ident, GenericIdent, UnresolvedGenerics, UnresolvedType}; +use crate::{token::SecondaryAttribute, GenericIdent, Ident, UnresolvedGenerics, UnresolvedType}; use iter_extended::vecmap; use noirc_errors::Span; diff --git a/compiler/noirc_frontend/src/ast/traits.rs b/compiler/noirc_frontend/src/ast/traits.rs index a55cd959613..beec5782f2f 100644 --- a/compiler/noirc_frontend/src/ast/traits.rs +++ b/compiler/noirc_frontend/src/ast/traits.rs @@ -4,8 +4,8 @@ use iter_extended::vecmap; use noirc_errors::Span; use crate::{ - node_interner::TraitId, BlockExpression, Expression, FunctionReturnType, Ident, GenericIdent, NoirFunction, - Path, UnresolvedGenerics, UnresolvedType, + node_interner::TraitId, BlockExpression, Expression, FunctionReturnType, GenericIdent, Ident, + NoirFunction, Path, UnresolvedGenerics, UnresolvedType, }; /// AST node for trait definitions: diff --git a/compiler/noirc_frontend/src/hir/resolution/resolver.rs b/compiler/noirc_frontend/src/hir/resolution/resolver.rs index bd73ff3b2d2..dfa4f7e9856 100644 --- a/compiler/noirc_frontend/src/hir/resolution/resolver.rs +++ b/compiler/noirc_frontend/src/hir/resolution/resolver.rs @@ -831,11 +831,22 @@ impl<'a> Resolver<'a> { assert_eq!(names.len(), generics.len()); for (name, typevar) in names.iter().zip(generics) { - self.add_existing_generic(&name.ident.0.contents, name.ident.0.span(), typevar.clone(), name.prevent_numeric); + self.add_existing_generic( + &name.ident.0.contents, + name.ident.0.span(), + typevar.clone(), + name.prevent_numeric, + ); } } - pub fn add_existing_generic(&mut self, name: &str, span: Span, typevar: TypeVariable, prevent_numeric: bool) { + pub fn add_existing_generic( + &mut self, + name: &str, + span: Span, + typevar: TypeVariable, + prevent_numeric: bool, + ) { // Check for name collisions of this generic let rc_name = Rc::new(name.to_owned()); @@ -902,7 +913,6 @@ impl<'a> Resolver<'a> { let attributes = func.attributes().clone(); - // TODO: prevent_numeric needed here? let mut generics = vecmap(&self.generics, |(_, typevar, _, _)| typevar.clone()); let mut parameters = vec![]; let mut parameter_types = vec![]; @@ -1065,13 +1075,11 @@ impl<'a> Resolver<'a> { if let Some((name, _, span, prevent_numeric)) = self.generics.iter().find(|(name, _, _, _)| name.as_ref() == &name_to_find) { - if *prevent_numeric { - panic!("declare_numeric_generics: TODO: likely do nothing here?") + if !*prevent_numeric { + let ident = Ident::new(name.to_string(), *span); + let definition = DefinitionKind::GenericType(type_variable); + self.add_variable_decl_inner(ident, false, false, false, definition); } - - let ident = Ident::new(name.to_string(), *span); - let definition = DefinitionKind::GenericType(type_variable); - self.add_variable_decl_inner(ident, false, false, false, definition); } } } @@ -1137,8 +1145,8 @@ impl<'a> Resolver<'a> { } Type::Alias(alias, generics) => { for (i, generic) in generics.iter().enumerate() { - if let Type::NamedGeneric(type_variable, name) = generic { - if alias.borrow().generic_is_numeric(i) { + if let Type::NamedGeneric(type_variable, name, prevent_numeric) = generic { + if !prevent_numeric && alias.borrow().generic_is_numeric(i) { found.insert(name.to_string(), type_variable.clone()); } } else { diff --git a/compiler/noirc_frontend/src/hir_def/types.rs b/compiler/noirc_frontend/src/hir_def/types.rs index 22d7aae6d2e..f626b5ef609 100644 --- a/compiler/noirc_frontend/src/hir_def/types.rs +++ b/compiler/noirc_frontend/src/hir_def/types.rs @@ -95,7 +95,7 @@ pub enum Type { /// never be bound over during type checking, but during monomorphization it /// will be and thus needs the full TypeVariable link. /// - /// TODO: before merge do we need N? here? + /// TODO: prevent_numeric before merge do we need N? here? Forall(Generics, Box), /// A type-level integer. Included to let an Array's size type variable @@ -612,12 +612,14 @@ impl Type { // True if the given type is a NamedGeneric with the target_id let named_generic_id_matches_target = |typ: &Type| { if let Type::NamedGeneric(type_variable, _, prevent_numeric) = typ { - if prevent_numeric { + if *prevent_numeric { false } else { match &*type_variable.borrow() { TypeBinding::Bound(_) => { - unreachable!("Named generics should not be bound until monomorphization") + unreachable!( + "Named generics should not be bound until monomorphization" + ) } TypeBinding::Unbound(id) => target_id == *id, } @@ -842,11 +844,16 @@ impl std::fmt::Display for Type { } Type::Unit => write!(f, "()"), Type::Error => write!(f, "error"), - Type::NamedGeneric(binding, name) => match &*binding.borrow() { - TypeBinding::Bound(binding) => binding.fmt(f), - TypeBinding::Unbound(_) if name.is_empty() => write!(f, "_"), - TypeBinding::Unbound(_) => write!(f, "{name}"), - }, + Type::NamedGeneric(binding, name, prevent_numeric) => { + let prevent_numeric_str = if *prevent_numeric { "?" } else { "" }; + match &*binding.borrow() { + TypeBinding::Bound(binding) => binding.fmt(f), + TypeBinding::Unbound(_) if name.is_empty() => { + write!(f, "_{prevent_numeric_str}") + } + TypeBinding::Unbound(_) => write!(f, "{name}{prevent_numeric_str}"), + } + } Type::Constant(x) => x.fmt(f), Type::Forall(typevars, typ) => { let typevars = vecmap(typevars, |var| var.id().to_string()); @@ -1197,7 +1204,7 @@ impl Type { } } - (NamedGeneric(binding, _), other) | (other, NamedGeneric(binding, _)) + (NamedGeneric(binding, _, _), other) | (other, NamedGeneric(binding, _, _)) if !binding.borrow().is_unbound() => { if let TypeBinding::Bound(link) = &*binding.borrow() { @@ -1207,7 +1214,7 @@ impl Type { } } - (NamedGeneric(binding_a, name_a), NamedGeneric(binding_b, name_b)) => { + (NamedGeneric(binding_a, name_a, _), NamedGeneric(binding_b, name_b, _)) => { // Bound NamedGenerics are caught by the check above assert!(binding_a.borrow().is_unbound()); assert!(binding_b.borrow().is_unbound()); @@ -1499,7 +1506,7 @@ impl Type { let fields = fields.substitute_helper(type_bindings, substitute_bound_typevars); Type::FmtString(Box::new(size), Box::new(fields)) } - Type::NamedGeneric(binding, _) | Type::TypeVariable(binding, _) => { + Type::NamedGeneric(binding, _, _) | Type::TypeVariable(binding, _) => { substitute_binding(binding) } // Do not substitute_helper fields, it can lead to infinite recursion @@ -1568,7 +1575,7 @@ impl Type { generic_args.iter().any(|arg| arg.occurs(target_id)) } Type::Tuple(fields) => fields.iter().any(|field| field.occurs(target_id)), - Type::NamedGeneric(binding, _) | Type::TypeVariable(binding, _) => { + Type::NamedGeneric(binding, _, _) | Type::TypeVariable(binding, _) => { match &*binding.borrow() { TypeBinding::Bound(binding) => binding.occurs(target_id), TypeBinding::Unbound(id) => *id == target_id, @@ -1623,7 +1630,7 @@ impl Type { def.borrow().get_type(args).follow_bindings() } Tuple(args) => Tuple(vecmap(args, |arg| arg.follow_bindings())), - TypeVariable(var, _) | NamedGeneric(var, _) => { + TypeVariable(var, _) | NamedGeneric(var, _, _) => { if let TypeBinding::Bound(typ) = &*var.borrow() { return typ.follow_bindings(); } @@ -1839,7 +1846,10 @@ impl std::fmt::Debug for Type { } Type::Unit => write!(f, "()"), Type::Error => write!(f, "error"), - Type::NamedGeneric(binding, name) => write!(f, "{}{:?}", name, binding), + Type::NamedGeneric(binding, name, prevent_numeric) => { + let prevent_numeric_str = if *prevent_numeric { "?" } else { "" }; + write!(f, "{}{:?}{}", name, binding, prevent_numeric_str) + } Type::Constant(x) => x.fmt(f), Type::Forall(typevars, typ) => { let typevars = vecmap(typevars, |var| format!("{:?}", var)); diff --git a/compiler/noirc_frontend/src/lexer/lexer.rs b/compiler/noirc_frontend/src/lexer/lexer.rs index 047b0d0612f..9addb175b80 100644 --- a/compiler/noirc_frontend/src/lexer/lexer.rs +++ b/compiler/noirc_frontend/src/lexer/lexer.rs @@ -109,7 +109,7 @@ impl<'a> Lexer<'a> { Some('.') => self.glue(Token::Dot), Some(':') => self.glue(Token::Colon), Some('!') => self.glue(Token::Bang), - Some('?') => self.glue(Token::Question), + Some('?') => self.single_char_token(Token::Question), Some('-') => self.glue(Token::Minus), Some('&') => self.ampersand(), Some('|') => self.single_char_token(Token::Pipe), @@ -589,7 +589,6 @@ mod tests { let expected = vec![ Token::Bang, - Token::Question, Token::NotEqual, Token::Plus, Token::LeftParen, diff --git a/compiler/noirc_frontend/src/monomorphization/mod.rs b/compiler/noirc_frontend/src/monomorphization/mod.rs index f691a0c9065..d8a56f35172 100644 --- a/compiler/noirc_frontend/src/monomorphization/mod.rs +++ b/compiler/noirc_frontend/src/monomorphization/mod.rs @@ -783,7 +783,7 @@ impl<'interner> Monomorphizer<'interner> { HirType::TraitAsType(..) => { unreachable!("All TraitAsType should be replaced before calling convert_type"); } - HirType::NamedGeneric(binding, _) => { + HirType::NamedGeneric(binding, _, _) => { if let TypeBinding::Bound(binding) = &*binding.borrow() { return self.convert_type(binding); } diff --git a/compiler/noirc_frontend/src/node_interner.rs b/compiler/noirc_frontend/src/node_interner.rs index 9a45268d111..56733d926a9 100644 --- a/compiler/noirc_frontend/src/node_interner.rs +++ b/compiler/noirc_frontend/src/node_interner.rs @@ -1718,7 +1718,7 @@ fn get_type_method_key(typ: &Type) -> Option { Type::Unit => Some(Unit), Type::Tuple(_) => Some(Tuple), Type::Function(_, _, _) => Some(Function), - Type::NamedGeneric(_, _) => Some(Generic), + Type::NamedGeneric(_, _, _) => Some(Generic), Type::MutableReference(element) => get_type_method_key(element), Type::Alias(alias, _) => get_type_method_key(&alias.borrow().typ), diff --git a/compiler/noirc_frontend/src/parser/parser.rs b/compiler/noirc_frontend/src/parser/parser.rs index 7c1ed8ec42a..7d67e6d571a 100644 --- a/compiler/noirc_frontend/src/parser/parser.rs +++ b/compiler/noirc_frontend/src/parser/parser.rs @@ -38,11 +38,11 @@ use crate::parser::{force, ignore_then_commit, statement_recovery}; use crate::token::{Attribute, Attributes, Keyword, SecondaryAttribute, Token, TokenKind}; use crate::{ BinaryOp, BinaryOpKind, BlockExpression, ConstrainKind, ConstrainStatement, Distinctness, - ForLoopStatement, ForRange, FunctionDefinition, FunctionReturnType, FunctionVisibility, Ident, GenericIdent, - IfExpression, InfixExpression, LValue, Lambda, Literal, NoirFunction, NoirStruct, NoirTrait, - NoirTraitImpl, NoirTypeAlias, Param, Path, PathKind, Pattern, Recoverable, Statement, - TraitBound, TraitImplItem, TraitItem, TypeImpl, UnaryOp, UnresolvedTraitConstraint, - UnresolvedTypeExpression, UseTree, UseTreeKind, Visibility, + ForLoopStatement, ForRange, FunctionDefinition, FunctionReturnType, FunctionVisibility, + GenericIdent, Ident, IfExpression, InfixExpression, LValue, Lambda, Literal, NoirFunction, + NoirStruct, NoirTrait, NoirTraitImpl, NoirTypeAlias, Param, Path, PathKind, Pattern, + Recoverable, Statement, TraitBound, TraitImplItem, TraitItem, TypeImpl, UnaryOp, + UnresolvedTraitConstraint, UnresolvedTypeExpression, UseTree, UseTreeKind, Visibility, }; use chumsky::prelude::*; @@ -243,7 +243,10 @@ fn is_pub_crate() -> impl NoirParser { fn generics() -> impl NoirParser> { ident() .then(just(Token::Question).or_not()) - .map(|(id, opt_question)| GenericIdent { ident: id, prevent_numeric: opt_question.is_some() }) + .map(|(id, opt_question)| GenericIdent { + ident: id, + prevent_numeric: opt_question.is_some(), + }) .separated_by(just(Token::Comma)) .allow_trailing() .at_least(1) diff --git a/tooling/nargo_fmt/src/utils.rs b/tooling/nargo_fmt/src/utils.rs index 5874ebdebbc..6090ce7a3b5 100644 --- a/tooling/nargo_fmt/src/utils.rs +++ b/tooling/nargo_fmt/src/utils.rs @@ -4,7 +4,7 @@ use crate::visitor::{FmtVisitor, Shape}; use noirc_frontend::hir::resolution::errors::Span; use noirc_frontend::lexer::Lexer; use noirc_frontend::token::Token; -use noirc_frontend::{Expression, Ident, Param, Visibility}; +use noirc_frontend::{Expression, GenericIdent, Ident, Param, Visibility}; pub(crate) fn changed_comment_content(original: &str, new: &str) -> bool { comments(original).ne(comments(new)) @@ -167,6 +167,16 @@ impl HasItem for Ident { } } +impl HasItem for GenericIdent { + fn span(&self) -> Span { + self.ident.span() + } + + fn format(self, visitor: &FmtVisitor, _shape: Shape) -> String { + visitor.slice(self.span()).into() + } +} + pub(crate) fn first_line_width(exprs: &str) -> usize { exprs.lines().next().map_or(0, |line: &str| line.chars().count()) } diff --git a/tooling/nargo_fmt/tests/expected/fn.nr b/tooling/nargo_fmt/tests/expected/fn.nr index 0088dba6a8f..fec6b2eb68e 100644 --- a/tooling/nargo_fmt/tests/expected/fn.nr +++ b/tooling/nargo_fmt/tests/expected/fn.nr @@ -4,6 +4,8 @@ fn main(x: pub u8, y: u8) -> pub Field {} fn main(x: A, y: B) -> pub Field where A: Eq, B: Eq {} +fn main(x: A, y: B) -> pub Field where A: Eq, B: Eq {} + fn main() // hello {} @@ -19,6 +21,8 @@ fn main( fn main() where T: Eq {} +fn main() where T: Eq {} + fn main( tape: [Field; TAPE_LEN], initial_registers: [Field; REGISTER_COUNT], @@ -36,6 +40,14 @@ fn apply_binary_field_op( registers: &mut Registers ) -> bool {} +fn apply_binary_field_op( + lhs: RegisterIndex, + rhs: RegisterIndex, + result: RegisterIndex, + op: u8, + registers: &mut Registers +) -> bool {} + fn main() -> distinct pub [Field; 2] {} fn ret_normal_lambda1() -> ((fn() -> Field)) {} @@ -52,6 +64,8 @@ fn make_counter() -> fn[(&mut Field,)]() -> Field {} fn get_some(generator: fn[Env]() -> Field) -> [Field; 5] {} +fn get_some(generator: fn[Env]() -> Field) -> [Field; 5] {} + fn main( message: [u8; 10], message_field: Field, diff --git a/tooling/nargo_fmt/tests/input/fn.nr b/tooling/nargo_fmt/tests/input/fn.nr index 26ff5933802..fd15a1c151d 100644 --- a/tooling/nargo_fmt/tests/input/fn.nr +++ b/tooling/nargo_fmt/tests/input/fn.nr @@ -4,6 +4,8 @@ fn main(x: pub u8, y: u8) -> pub Field {} fn main(x: A, y: B) -> pub Field where A: Eq, B: Eq {} +fn main(x: A, y: B) -> pub Field where A: Eq, B: Eq {} + fn main() // hello {} @@ -19,10 +21,14 @@ fn main( fn main() where T: Eq {} +fn main() where T: Eq {} + fn main(tape: [Field; TAPE_LEN], initial_registers: [Field; REGISTER_COUNT], initial_memory: [Field; MEM_COUNT], initial_program_counter: Field, initial_call_stack: [Field; MAX_CALL_STACK], initial_call_stack_pointer: u64) -> pub ExecutionResult {} fn apply_binary_field_op(lhs: RegisterIndex, rhs: RegisterIndex, result: RegisterIndex, op: u8, registers: &mut Registers) -> bool {} +fn apply_binary_field_op(lhs: RegisterIndex, rhs: RegisterIndex, result: RegisterIndex, op: u8, registers: &mut Registers) -> bool {} + fn main() -> distinct pub [Field;2] {} fn ret_normal_lambda1() -> ((fn() -> Field)) {} @@ -39,6 +45,8 @@ fn make_counter() -> fn[(&mut Field,)]() -> Field {} fn get_some(generator: fn[Env]() -> Field) -> [Field;5] {} +fn get_some(generator: fn[Env]() -> Field) -> [Field;5] {} + fn main( message: [u8; 10], message_field: Field, pub_key_x: Field, pub_key_y: Field, signature: [u8; 64] ) {} From c3b67aaf9e8a35f693464b3cb61457f608dac663 Mon Sep 17 00:00:00 2001 From: "Michael J. Klein" Date: Mon, 19 Feb 2024 12:11:13 -0500 Subject: [PATCH 06/11] wip debugging propagation of N? --- .../src/hir/resolution/resolver.rs | 6 +- .../noirc_frontend/src/hir/type_check/expr.rs | 2 + compiler/noirc_frontend/src/hir_def/types.rs | 56 ++++++++++++------- .../src/monomorphization/mod.rs | 4 +- compiler/noirc_frontend/src/node_interner.rs | 2 +- .../array_slice_polymorphism/src/main.nr | 2 +- tooling/noirc_abi/src/lib.rs | 2 +- 7 files changed, 47 insertions(+), 27 deletions(-) diff --git a/compiler/noirc_frontend/src/hir/resolution/resolver.rs b/compiler/noirc_frontend/src/hir/resolution/resolver.rs index cb8fdb2a327..4e9acbed000 100644 --- a/compiler/noirc_frontend/src/hir/resolution/resolver.rs +++ b/compiler/noirc_frontend/src/hir/resolution/resolver.rs @@ -470,7 +470,9 @@ impl<'a> Resolver<'a> { Array(size, elem) => { let elem = Box::new(self.resolve_type_inner(*elem, new_variables)); let size = if size.is_none() { - Type::NotConstant + println!("TODO: Array NotConstant Size"); + + Type::NotConstant(false) } else { self.resolve_array_size(size, new_variables) }; @@ -1106,7 +1108,7 @@ impl<'a> Resolver<'a> { | Type::TypeVariable(_, _) | Type::Constant(_) | Type::NamedGeneric(_, _, _) - | Type::NotConstant + | Type::NotConstant(_) | Type::TraitAsType(..) | Type::Forall(_, _) => (), diff --git a/compiler/noirc_frontend/src/hir/type_check/expr.rs b/compiler/noirc_frontend/src/hir/type_check/expr.rs index 88da5be8c8b..36599a4a754 100644 --- a/compiler/noirc_frontend/src/hir/type_check/expr.rs +++ b/compiler/noirc_frontend/src/hir/type_check/expr.rs @@ -97,6 +97,8 @@ impl<'interner> TypeChecker<'interner> { let elem_type = self.check_expression(&repeated_element); let length = match length { Type::Constant(length) => { + println!("Type::Constant({length}): {:?}", repeated_element); + Type::constant_variable(length, self.interner) } other => other, diff --git a/compiler/noirc_frontend/src/hir_def/types.rs b/compiler/noirc_frontend/src/hir_def/types.rs index 479180d4cf3..886fd4d4668 100644 --- a/compiler/noirc_frontend/src/hir_def/types.rs +++ b/compiler/noirc_frontend/src/hir_def/types.rs @@ -105,7 +105,9 @@ pub enum Type { /// The type of a slice is an array of size NotConstant. /// The size of an array literal is resolved to this if it ever uses operations /// involving slices. - NotConstant, + /// + /// prevent_numeric: bool (prevent binding to a Constant(_)) + NotConstant(bool), /// The result of some type error. Remembering type errors as their own type variant lets /// us avoid issuing repeat type errors for the same item. For example, a lambda with @@ -154,7 +156,7 @@ impl Type { | Type::MutableReference(_) | Type::Forall(_, _) | Type::Constant(_) - | Type::NotConstant + | Type::NotConstant(_) | Type::Error => unreachable!("This type cannot exist as a parameter to main"), } } @@ -162,7 +164,7 @@ impl Type { pub(crate) fn is_nested_slice(&self) -> bool { match self { Type::Array(size, elem) => { - if let Type::NotConstant = size.as_ref() { + if let Type::NotConstant(_) = size.as_ref() { elem.as_ref().contains_slice() } else { false @@ -174,7 +176,7 @@ impl Type { pub(crate) fn contains_slice(&self) -> bool { match self { - Type::Array(size, _) => matches!(size.as_ref(), Type::NotConstant), + Type::Array(size, _) => matches!(size.as_ref(), Type::NotConstant(_)), Type::Struct(struct_typ, generics) => { let fields = struct_typ.borrow().get_fields(generics); for field in fields.iter() { @@ -638,7 +640,7 @@ impl Type { | Type::TypeVariable(_, _) | Type::Constant(_) | Type::NamedGeneric(_, _, _) - | Type::NotConstant + | Type::NotConstant(_) | Type::Forall(_, _) | Type::TraitAsType(..) => false, @@ -704,7 +706,7 @@ impl Type { | Type::MutableReference(_) | Type::Forall(_, _) | Type::TraitAsType(..) - | Type::NotConstant => false, + | Type::NotConstant(_) => false, // This function is called during name resolution before we've verified aliases // are not cyclic. As a result, it wouldn't be safe to check this alias' definition @@ -773,7 +775,7 @@ impl std::fmt::Display for Type { write!(f, "Field") } Type::Array(len, typ) => { - if matches!(len.follow_bindings(), Type::NotConstant) { + if matches!(len.follow_bindings(), Type::NotConstant(_)) { write!(f, "[{typ}]") } else { write!(f, "[{typ}; {len}]") @@ -872,7 +874,7 @@ impl std::fmt::Display for Type { Type::MutableReference(element) => { write!(f, "&mut {element}") } - Type::NotConstant => write!(f, "_"), + Type::NotConstant(_) => write!(f, "_"), } } } @@ -921,6 +923,8 @@ impl Type { TypeBinding::Unbound(id) => *id, }; + println!("try_bind_to_maybe_constant: {:?} {:?} {:?} {:?}", self, var, target_length, bindings); + let this = self.substitute(bindings).follow_bindings(); match &this { @@ -928,9 +932,13 @@ impl Type { bindings.insert(target_id, (var.clone(), this)); Ok(()) } - Type::NotConstant => { - bindings.insert(target_id, (var.clone(), Type::NotConstant)); - Ok(()) + Type::NotConstant(prevent_numeric) => { + if *prevent_numeric { + Err(UnificationError) + } else { + bindings.insert(target_id, (var.clone(), Type::NotConstant(*prevent_numeric))); + Ok(()) + } } // A TypeVariable is less specific than a MaybeConstant, so we bind // to the other type variable instead. @@ -963,8 +971,8 @@ impl Type { // just set them both to NotConstant. See issue 2370 TypeVariableKind::Constant(_) => { // *length != target_length - bindings.insert(target_id, (var.clone(), Type::NotConstant)); - bindings.insert(*new_target_id, (new_var.clone(), Type::NotConstant)); + bindings.insert(target_id, (var.clone(), Type::NotConstant(false))); + bindings.insert(*new_target_id, (new_var.clone(), Type::NotConstant(false))); Ok(()) } TypeVariableKind::IntegerOrField => Err(UnificationError), @@ -1164,6 +1172,8 @@ impl Type { (TypeVariable(var, Kind::Constant(length)), other) | (other, TypeVariable(var, Kind::Constant(length))) => other .try_unify_to_type_variable(var, bindings, |bindings| { + println!("PRE try_bind_to_maybe_constant: {:?} {:?} {:?}", var, length, other); + other.try_bind_to_maybe_constant(var, *length, bindings) }), @@ -1208,6 +1218,10 @@ impl Type { if !binding.borrow().is_unbound() => { if let TypeBinding::Bound(link) = &*binding.borrow() { + // println!("try_unify NamedGeneric: {:?} {:?} {:?} {:?}", self, other, bindings, link); + // other.typ.contains_numeric_typevar(target_id) + + link.try_unify(other, bindings) } else { unreachable!("If guard ensures binding is bound") @@ -1265,6 +1279,8 @@ impl Type { // bind to the given type or not. bind_variable: impl FnOnce(&mut TypeBindings) -> Result<(), UnificationError>, ) -> Result<(), UnificationError> { + println!("try_unify_to_type_variable: {:?} {:?}", type_variable, bindings); + match &*type_variable.borrow() { // If it is already bound, unify against what it is bound to TypeBinding::Bound(link) => link.try_unify(self, bindings), @@ -1321,7 +1337,7 @@ impl Type { let size2 = size2.follow_bindings(); // If we have an array and our target is a slice - if matches!(size1, Type::Constant(_)) && matches!(size2, Type::NotConstant) { + if matches!(size1, Type::Constant(_)) && matches!(size2, Type::NotConstant(_)) { // Still have to ensure the element types match. // Don't need to issue an error here if not, it will be done in unify_with_coercions let mut bindings = TypeBindings::new(); @@ -1556,7 +1572,7 @@ impl Type { | Type::Constant(_) | Type::TraitAsType(..) | Type::Error - | Type::NotConstant + | Type::NotConstant(_) | Type::Unit => self.clone(), } } @@ -1597,7 +1613,7 @@ impl Type { | Type::Constant(_) | Type::TraitAsType(..) | Type::Error - | Type::NotConstant + | Type::NotConstant(_) | Type::Unit => false, } } @@ -1655,7 +1671,7 @@ impl Type { | Constant(_) | Unit | Error - | NotConstant => self.clone(), + | NotConstant(_) => self.clone(), } } @@ -1778,7 +1794,7 @@ impl From<&Type> for PrintableType { Type::MutableReference(typ) => { PrintableType::MutableReference { typ: Box::new(typ.as_ref().into()) } } - Type::NotConstant => unreachable!(), + Type::NotConstant(_) => unreachable!(), } } } @@ -1790,7 +1806,7 @@ impl std::fmt::Debug for Type { write!(f, "Field") } Type::Array(len, typ) => { - if matches!(len.follow_bindings(), Type::NotConstant) { + if matches!(len.follow_bindings(), Type::NotConstant(_)) { write!(f, "[{typ:?}]") } else { write!(f, "[{typ:?}; {len:?}]") @@ -1867,7 +1883,7 @@ impl std::fmt::Debug for Type { Type::MutableReference(element) => { write!(f, "&mut {element:?}") } - Type::NotConstant => write!(f, "NotConstant"), + Type::NotConstant(prevent_numeric) => write!(f, "NotConstant({})", prevent_numeric), } } } diff --git a/compiler/noirc_frontend/src/monomorphization/mod.rs b/compiler/noirc_frontend/src/monomorphization/mod.rs index c9fef6f6da3..8f501f99c53 100644 --- a/compiler/noirc_frontend/src/monomorphization/mod.rs +++ b/compiler/noirc_frontend/src/monomorphization/mod.rs @@ -857,7 +857,7 @@ impl<'interner> Monomorphizer<'interner> { HirType::Forall(_, _) | HirType::Constant(_) - | HirType::NotConstant + | HirType::NotConstant(_) | HirType::Error => { unreachable!("Unexpected type {} found", typ) } @@ -1074,7 +1074,7 @@ impl<'interner> Monomorphizer<'interner> { // Disallow printing slices and mutable references for consistency, // since they cannot be passed from ACIR into Brillig if let HirType::Array(size, _) = typ { - if let HirType::NotConstant = **size { + if let HirType::NotConstant(_) = **size { unreachable!("println and format strings do not support slices. Convert the slice to an array before passing it to println"); } } else if matches!(typ, HirType::MutableReference(_)) { diff --git a/compiler/noirc_frontend/src/node_interner.rs b/compiler/noirc_frontend/src/node_interner.rs index be10523d932..ef0b680edda 100644 --- a/compiler/noirc_frontend/src/node_interner.rs +++ b/compiler/noirc_frontend/src/node_interner.rs @@ -1714,7 +1714,7 @@ fn get_type_method_key(typ: &Type) -> Option { | Type::Forall(_, _) | Type::Constant(_) | Type::Error - | Type::NotConstant + | Type::NotConstant(_) | Type::Struct(_, _) | Type::TraitAsType(..) => None, } diff --git a/test_programs/compile_success_empty/array_slice_polymorphism/src/main.nr b/test_programs/compile_success_empty/array_slice_polymorphism/src/main.nr index df704cf31bb..5627ffd09e4 100644 --- a/test_programs/compile_success_empty/array_slice_polymorphism/src/main.nr +++ b/test_programs/compile_success_empty/array_slice_polymorphism/src/main.nr @@ -4,7 +4,7 @@ fn array_zeros(_x: [u64; N]) -> [u64; N] { } fn main(x: Field, y: pub Field) { - let xs: [u64] = [0; 0]; + let xs: [u64] = [11111; 1337]; assert(array_zeros(xs) == xs); assert(x != y); } diff --git a/tooling/noirc_abi/src/lib.rs b/tooling/noirc_abi/src/lib.rs index 26feab65d83..2c7dec289f0 100644 --- a/tooling/noirc_abi/src/lib.rs +++ b/tooling/noirc_abi/src/lib.rs @@ -178,7 +178,7 @@ impl AbiType { | Type::TypeVariable(_, _) | Type::NamedGeneric(..) | Type::Forall(..) - | Type::NotConstant + | Type::NotConstant(_) | Type::Function(_, _, _) => unreachable!("Type cannot be used in the abi"), Type::FmtString(_, _) => unreachable!("format strings cannot be used in the abi"), Type::MutableReference(_) => unreachable!("&mut cannot be used in the abi"), From 71b2ce1abd3aa447ecb53ca85b0bd6e86523ae72 Mon Sep 17 00:00:00 2001 From: "Michael J. Klein" Date: Mon, 19 Feb 2024 12:11:45 -0500 Subject: [PATCH 07/11] Revert "wip debugging propagation of N?": need to propagate from somewhere else This reverts commit c3b67aaf9e8a35f693464b3cb61457f608dac663. --- .../src/hir/resolution/resolver.rs | 6 +- .../noirc_frontend/src/hir/type_check/expr.rs | 2 - compiler/noirc_frontend/src/hir_def/types.rs | 56 +++++++------------ .../src/monomorphization/mod.rs | 4 +- compiler/noirc_frontend/src/node_interner.rs | 2 +- .../array_slice_polymorphism/src/main.nr | 2 +- tooling/noirc_abi/src/lib.rs | 2 +- 7 files changed, 27 insertions(+), 47 deletions(-) diff --git a/compiler/noirc_frontend/src/hir/resolution/resolver.rs b/compiler/noirc_frontend/src/hir/resolution/resolver.rs index 4e9acbed000..cb8fdb2a327 100644 --- a/compiler/noirc_frontend/src/hir/resolution/resolver.rs +++ b/compiler/noirc_frontend/src/hir/resolution/resolver.rs @@ -470,9 +470,7 @@ impl<'a> Resolver<'a> { Array(size, elem) => { let elem = Box::new(self.resolve_type_inner(*elem, new_variables)); let size = if size.is_none() { - println!("TODO: Array NotConstant Size"); - - Type::NotConstant(false) + Type::NotConstant } else { self.resolve_array_size(size, new_variables) }; @@ -1108,7 +1106,7 @@ impl<'a> Resolver<'a> { | Type::TypeVariable(_, _) | Type::Constant(_) | Type::NamedGeneric(_, _, _) - | Type::NotConstant(_) + | Type::NotConstant | Type::TraitAsType(..) | Type::Forall(_, _) => (), diff --git a/compiler/noirc_frontend/src/hir/type_check/expr.rs b/compiler/noirc_frontend/src/hir/type_check/expr.rs index 36599a4a754..88da5be8c8b 100644 --- a/compiler/noirc_frontend/src/hir/type_check/expr.rs +++ b/compiler/noirc_frontend/src/hir/type_check/expr.rs @@ -97,8 +97,6 @@ impl<'interner> TypeChecker<'interner> { let elem_type = self.check_expression(&repeated_element); let length = match length { Type::Constant(length) => { - println!("Type::Constant({length}): {:?}", repeated_element); - Type::constant_variable(length, self.interner) } other => other, diff --git a/compiler/noirc_frontend/src/hir_def/types.rs b/compiler/noirc_frontend/src/hir_def/types.rs index 886fd4d4668..479180d4cf3 100644 --- a/compiler/noirc_frontend/src/hir_def/types.rs +++ b/compiler/noirc_frontend/src/hir_def/types.rs @@ -105,9 +105,7 @@ pub enum Type { /// The type of a slice is an array of size NotConstant. /// The size of an array literal is resolved to this if it ever uses operations /// involving slices. - /// - /// prevent_numeric: bool (prevent binding to a Constant(_)) - NotConstant(bool), + NotConstant, /// The result of some type error. Remembering type errors as their own type variant lets /// us avoid issuing repeat type errors for the same item. For example, a lambda with @@ -156,7 +154,7 @@ impl Type { | Type::MutableReference(_) | Type::Forall(_, _) | Type::Constant(_) - | Type::NotConstant(_) + | Type::NotConstant | Type::Error => unreachable!("This type cannot exist as a parameter to main"), } } @@ -164,7 +162,7 @@ impl Type { pub(crate) fn is_nested_slice(&self) -> bool { match self { Type::Array(size, elem) => { - if let Type::NotConstant(_) = size.as_ref() { + if let Type::NotConstant = size.as_ref() { elem.as_ref().contains_slice() } else { false @@ -176,7 +174,7 @@ impl Type { pub(crate) fn contains_slice(&self) -> bool { match self { - Type::Array(size, _) => matches!(size.as_ref(), Type::NotConstant(_)), + Type::Array(size, _) => matches!(size.as_ref(), Type::NotConstant), Type::Struct(struct_typ, generics) => { let fields = struct_typ.borrow().get_fields(generics); for field in fields.iter() { @@ -640,7 +638,7 @@ impl Type { | Type::TypeVariable(_, _) | Type::Constant(_) | Type::NamedGeneric(_, _, _) - | Type::NotConstant(_) + | Type::NotConstant | Type::Forall(_, _) | Type::TraitAsType(..) => false, @@ -706,7 +704,7 @@ impl Type { | Type::MutableReference(_) | Type::Forall(_, _) | Type::TraitAsType(..) - | Type::NotConstant(_) => false, + | Type::NotConstant => false, // This function is called during name resolution before we've verified aliases // are not cyclic. As a result, it wouldn't be safe to check this alias' definition @@ -775,7 +773,7 @@ impl std::fmt::Display for Type { write!(f, "Field") } Type::Array(len, typ) => { - if matches!(len.follow_bindings(), Type::NotConstant(_)) { + if matches!(len.follow_bindings(), Type::NotConstant) { write!(f, "[{typ}]") } else { write!(f, "[{typ}; {len}]") @@ -874,7 +872,7 @@ impl std::fmt::Display for Type { Type::MutableReference(element) => { write!(f, "&mut {element}") } - Type::NotConstant(_) => write!(f, "_"), + Type::NotConstant => write!(f, "_"), } } } @@ -923,8 +921,6 @@ impl Type { TypeBinding::Unbound(id) => *id, }; - println!("try_bind_to_maybe_constant: {:?} {:?} {:?} {:?}", self, var, target_length, bindings); - let this = self.substitute(bindings).follow_bindings(); match &this { @@ -932,13 +928,9 @@ impl Type { bindings.insert(target_id, (var.clone(), this)); Ok(()) } - Type::NotConstant(prevent_numeric) => { - if *prevent_numeric { - Err(UnificationError) - } else { - bindings.insert(target_id, (var.clone(), Type::NotConstant(*prevent_numeric))); - Ok(()) - } + Type::NotConstant => { + bindings.insert(target_id, (var.clone(), Type::NotConstant)); + Ok(()) } // A TypeVariable is less specific than a MaybeConstant, so we bind // to the other type variable instead. @@ -971,8 +963,8 @@ impl Type { // just set them both to NotConstant. See issue 2370 TypeVariableKind::Constant(_) => { // *length != target_length - bindings.insert(target_id, (var.clone(), Type::NotConstant(false))); - bindings.insert(*new_target_id, (new_var.clone(), Type::NotConstant(false))); + bindings.insert(target_id, (var.clone(), Type::NotConstant)); + bindings.insert(*new_target_id, (new_var.clone(), Type::NotConstant)); Ok(()) } TypeVariableKind::IntegerOrField => Err(UnificationError), @@ -1172,8 +1164,6 @@ impl Type { (TypeVariable(var, Kind::Constant(length)), other) | (other, TypeVariable(var, Kind::Constant(length))) => other .try_unify_to_type_variable(var, bindings, |bindings| { - println!("PRE try_bind_to_maybe_constant: {:?} {:?} {:?}", var, length, other); - other.try_bind_to_maybe_constant(var, *length, bindings) }), @@ -1218,10 +1208,6 @@ impl Type { if !binding.borrow().is_unbound() => { if let TypeBinding::Bound(link) = &*binding.borrow() { - // println!("try_unify NamedGeneric: {:?} {:?} {:?} {:?}", self, other, bindings, link); - // other.typ.contains_numeric_typevar(target_id) - - link.try_unify(other, bindings) } else { unreachable!("If guard ensures binding is bound") @@ -1279,8 +1265,6 @@ impl Type { // bind to the given type or not. bind_variable: impl FnOnce(&mut TypeBindings) -> Result<(), UnificationError>, ) -> Result<(), UnificationError> { - println!("try_unify_to_type_variable: {:?} {:?}", type_variable, bindings); - match &*type_variable.borrow() { // If it is already bound, unify against what it is bound to TypeBinding::Bound(link) => link.try_unify(self, bindings), @@ -1337,7 +1321,7 @@ impl Type { let size2 = size2.follow_bindings(); // If we have an array and our target is a slice - if matches!(size1, Type::Constant(_)) && matches!(size2, Type::NotConstant(_)) { + if matches!(size1, Type::Constant(_)) && matches!(size2, Type::NotConstant) { // Still have to ensure the element types match. // Don't need to issue an error here if not, it will be done in unify_with_coercions let mut bindings = TypeBindings::new(); @@ -1572,7 +1556,7 @@ impl Type { | Type::Constant(_) | Type::TraitAsType(..) | Type::Error - | Type::NotConstant(_) + | Type::NotConstant | Type::Unit => self.clone(), } } @@ -1613,7 +1597,7 @@ impl Type { | Type::Constant(_) | Type::TraitAsType(..) | Type::Error - | Type::NotConstant(_) + | Type::NotConstant | Type::Unit => false, } } @@ -1671,7 +1655,7 @@ impl Type { | Constant(_) | Unit | Error - | NotConstant(_) => self.clone(), + | NotConstant => self.clone(), } } @@ -1794,7 +1778,7 @@ impl From<&Type> for PrintableType { Type::MutableReference(typ) => { PrintableType::MutableReference { typ: Box::new(typ.as_ref().into()) } } - Type::NotConstant(_) => unreachable!(), + Type::NotConstant => unreachable!(), } } } @@ -1806,7 +1790,7 @@ impl std::fmt::Debug for Type { write!(f, "Field") } Type::Array(len, typ) => { - if matches!(len.follow_bindings(), Type::NotConstant(_)) { + if matches!(len.follow_bindings(), Type::NotConstant) { write!(f, "[{typ:?}]") } else { write!(f, "[{typ:?}; {len:?}]") @@ -1883,7 +1867,7 @@ impl std::fmt::Debug for Type { Type::MutableReference(element) => { write!(f, "&mut {element:?}") } - Type::NotConstant(prevent_numeric) => write!(f, "NotConstant({})", prevent_numeric), + Type::NotConstant => write!(f, "NotConstant"), } } } diff --git a/compiler/noirc_frontend/src/monomorphization/mod.rs b/compiler/noirc_frontend/src/monomorphization/mod.rs index 8f501f99c53..c9fef6f6da3 100644 --- a/compiler/noirc_frontend/src/monomorphization/mod.rs +++ b/compiler/noirc_frontend/src/monomorphization/mod.rs @@ -857,7 +857,7 @@ impl<'interner> Monomorphizer<'interner> { HirType::Forall(_, _) | HirType::Constant(_) - | HirType::NotConstant(_) + | HirType::NotConstant | HirType::Error => { unreachable!("Unexpected type {} found", typ) } @@ -1074,7 +1074,7 @@ impl<'interner> Monomorphizer<'interner> { // Disallow printing slices and mutable references for consistency, // since they cannot be passed from ACIR into Brillig if let HirType::Array(size, _) = typ { - if let HirType::NotConstant(_) = **size { + if let HirType::NotConstant = **size { unreachable!("println and format strings do not support slices. Convert the slice to an array before passing it to println"); } } else if matches!(typ, HirType::MutableReference(_)) { diff --git a/compiler/noirc_frontend/src/node_interner.rs b/compiler/noirc_frontend/src/node_interner.rs index ef0b680edda..be10523d932 100644 --- a/compiler/noirc_frontend/src/node_interner.rs +++ b/compiler/noirc_frontend/src/node_interner.rs @@ -1714,7 +1714,7 @@ fn get_type_method_key(typ: &Type) -> Option { | Type::Forall(_, _) | Type::Constant(_) | Type::Error - | Type::NotConstant(_) + | Type::NotConstant | Type::Struct(_, _) | Type::TraitAsType(..) => None, } diff --git a/test_programs/compile_success_empty/array_slice_polymorphism/src/main.nr b/test_programs/compile_success_empty/array_slice_polymorphism/src/main.nr index 5627ffd09e4..df704cf31bb 100644 --- a/test_programs/compile_success_empty/array_slice_polymorphism/src/main.nr +++ b/test_programs/compile_success_empty/array_slice_polymorphism/src/main.nr @@ -4,7 +4,7 @@ fn array_zeros(_x: [u64; N]) -> [u64; N] { } fn main(x: Field, y: pub Field) { - let xs: [u64] = [11111; 1337]; + let xs: [u64] = [0; 0]; assert(array_zeros(xs) == xs); assert(x != y); } diff --git a/tooling/noirc_abi/src/lib.rs b/tooling/noirc_abi/src/lib.rs index 2c7dec289f0..26feab65d83 100644 --- a/tooling/noirc_abi/src/lib.rs +++ b/tooling/noirc_abi/src/lib.rs @@ -178,7 +178,7 @@ impl AbiType { | Type::TypeVariable(_, _) | Type::NamedGeneric(..) | Type::Forall(..) - | Type::NotConstant(_) + | Type::NotConstant | Type::Function(_, _, _) => unreachable!("Type cannot be used in the abi"), Type::FmtString(_, _) => unreachable!("format strings cannot be used in the abi"), Type::MutableReference(_) => unreachable!("&mut cannot be used in the abi"), From f4933b4865362c3751a34afc63fe2c939018f9d7 Mon Sep 17 00:00:00 2001 From: "Michael J. Klein" Date: Mon, 19 Feb 2024 16:27:28 -0500 Subject: [PATCH 08/11] wip: add bool:prevent_numeric to Generics type, debugging failing tests, add assertion that prevent_numeric is retained for different copies of generic with same name --- .../src/hir/def_collector/dc_crate.rs | 4 +- .../src/hir/resolution/resolver.rs | 11 +- .../src/hir/resolution/traits.rs | 8 +- .../noirc_frontend/src/hir/type_check/expr.rs | 2 +- compiler/noirc_frontend/src/hir_def/traits.rs | 2 +- compiler/noirc_frontend/src/hir_def/types.rs | 120 +++++++++++++++--- .../src/monomorphization/mod.rs | 17 ++- compiler/noirc_frontend/src/node_interner.rs | 14 +- .../array_slice_polymorphism/src/main.nr | 12 +- 9 files changed, 138 insertions(+), 52 deletions(-) diff --git a/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs b/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs index 0d1dd1b4337..c7aee5acce2 100644 --- a/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs +++ b/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs @@ -501,7 +501,7 @@ pub(crate) fn check_methods_signatures( } // We also need to bind the traits generics to the trait's generics on the impl - for (generic, binding) in the_trait.generics.iter().zip(trait_generics) { + for ((generic, _prevent_numeric), binding) in the_trait.generics.iter().zip(trait_generics) { generic.bind(binding); } @@ -609,7 +609,7 @@ pub(crate) fn check_methods_signatures( the_trait.set_methods(trait_methods); the_trait.self_type_typevar.unbind(the_trait.self_type_typevar_id); - for generic in &the_trait.generics { + for (generic, _prevent_numeric) in &the_trait.generics { generic.unbind(generic.id()); } } diff --git a/compiler/noirc_frontend/src/hir/resolution/resolver.rs b/compiler/noirc_frontend/src/hir/resolution/resolver.rs index cb8fdb2a327..e60c27a47ee 100644 --- a/compiler/noirc_frontend/src/hir/resolution/resolver.rs +++ b/compiler/noirc_frontend/src/hir/resolution/resolver.rs @@ -684,12 +684,12 @@ impl<'a> Resolver<'a> { None => { let id = self.interner.next_type_variable_id(); let typevar = TypeVariable::unbound(id); - new_variables.push(typevar.clone()); + let prevent_numeric = true; + new_variables.push((typevar.clone(), prevent_numeric)); // 'Named'Generic is a bit of a misnomer here, we want a type variable that // wont be bound over but this one has no name since we do not currently // require users to explicitly be generic over array lengths. - let prevent_numeric = true; Type::NamedGeneric(typevar, Rc::new("".into()), prevent_numeric) } Some(length) => self.convert_expression_type(length), @@ -820,7 +820,7 @@ impl<'a> Resolver<'a> { self.generics.push((name, typevar.clone(), span, prevent_numeric)); } - typevar + (typevar, prevent_numeric) }) } @@ -830,7 +830,8 @@ impl<'a> Resolver<'a> { pub fn add_existing_generics(&mut self, names: &UnresolvedGenerics, generics: &Generics) { assert_eq!(names.len(), generics.len()); - for (name, typevar) in names.iter().zip(generics) { + for (name, (typevar, prevent_numeric)) in names.iter().zip(generics) { + assert!(*prevent_numeric == name.prevent_numeric); self.add_existing_generic( &name.ident.0.contents, name.ident.0.span(), @@ -913,7 +914,7 @@ impl<'a> Resolver<'a> { let attributes = func.attributes().clone(); - let mut generics = vecmap(&self.generics, |(_, typevar, _, _)| typevar.clone()); + let mut generics = vecmap(&self.generics, |(_, typevar, _, prevent_numeric)| (typevar.clone(), *prevent_numeric)); let mut parameters = vec![]; let mut parameter_types = vec![]; diff --git a/compiler/noirc_frontend/src/hir/resolution/traits.rs b/compiler/noirc_frontend/src/hir/resolution/traits.rs index 02160cad6a9..a750601db88 100644 --- a/compiler/noirc_frontend/src/hir/resolution/traits.rs +++ b/compiler/noirc_frontend/src/hir/resolution/traits.rs @@ -41,8 +41,8 @@ pub(crate) fn resolve_traits( let mut all_errors = Vec::new(); for (trait_id, unresolved_trait) in traits { - let generics = vecmap(&unresolved_trait.trait_def.generics, |_| { - TypeVariable::unbound(context.def_interner.next_type_variable_id()) + let generics = vecmap(&unresolved_trait.trait_def.generics, |generic_ident| { + (TypeVariable::unbound(context.def_interner.next_type_variable_id()), generic_ident.prevent_numeric) }); // Resolve order @@ -142,7 +142,7 @@ fn resolve_trait_methods( let arguments = vecmap(parameters, |param| resolver.resolve_type(param.1.clone())); let return_type = resolver.resolve_type(return_type.get_type().into_owned()); - let generics = vecmap(resolver.get_generics(), |(_, type_var, _, _)| type_var.clone()); + let generics = vecmap(resolver.get_generics(), |(_, type_var, _, prevent_numeric)| (type_var.clone(), *prevent_numeric)); let default_impl_list: Vec<_> = unresolved_trait .fns_with_default_impl @@ -455,7 +455,7 @@ pub(crate) fn resolve_trait_impls( methods: vecmap(&impl_methods, |(_, func_id)| *func_id), }); - let impl_generics = vecmap(impl_generics, |(_, type_variable, _, _)| type_variable); + let impl_generics = vecmap(impl_generics, |(_, type_variable, _, prevent_numeric)| (type_variable, prevent_numeric)); if let Err((prev_span, prev_file)) = interner.add_trait_implementation( self_type.clone(), diff --git a/compiler/noirc_frontend/src/hir/type_check/expr.rs b/compiler/noirc_frontend/src/hir/type_check/expr.rs index 88da5be8c8b..9e2574301aa 100644 --- a/compiler/noirc_frontend/src/hir/type_check/expr.rs +++ b/compiler/noirc_frontend/src/hir/type_check/expr.rs @@ -325,7 +325,7 @@ impl<'interner> TypeChecker<'interner> { let the_trait = self.interner.get_trait(constraint.trait_id); assert_eq!(the_trait.generics.len(), constraint.trait_generics.len()); - for (param, arg) in the_trait.generics.iter().zip(&constraint.trait_generics) { + for ((param, _prevent_numeric), arg) in the_trait.generics.iter().zip(&constraint.trait_generics) { bindings.insert(param.id(), (param.clone(), arg.clone())); } } diff --git a/compiler/noirc_frontend/src/hir_def/traits.rs b/compiler/noirc_frontend/src/hir_def/traits.rs index 16b9899039f..3b501f4a025 100644 --- a/compiler/noirc_frontend/src/hir_def/traits.rs +++ b/compiler/noirc_frontend/src/hir_def/traits.rs @@ -147,7 +147,7 @@ impl TraitFunction { } } - pub fn generics(&self) -> &[TypeVariable] { + pub fn generics(&self) -> &[(TypeVariable, bool)] { match &self.typ { Type::Function(..) => &[], Type::Forall(generics, _) => generics, diff --git a/compiler/noirc_frontend/src/hir_def/types.rs b/compiler/noirc_frontend/src/hir_def/types.rs index 479180d4cf3..2654c0f410c 100644 --- a/compiler/noirc_frontend/src/hir_def/types.rs +++ b/compiler/noirc_frontend/src/hir_def/types.rs @@ -222,8 +222,9 @@ pub struct StructType { pub location: Location, } -/// Corresponds to generic lists such as `` in the source program. -pub type Generics = Vec; +/// Corresponds to generic lists such as `` in the source program. +/// The bool is used to to prevent_numeric. +pub type Generics = Vec<(TypeVariable, bool)>; impl std::hash::Hash for StructType { fn hash(&self, state: &mut H) { @@ -272,7 +273,7 @@ impl StructType { .generics .iter() .zip(generic_args) - .map(|(old, new)| (old.id(), (old.clone(), new.clone()))) + .map(|(old, new)| (old.0.id(), (old.0.clone(), new.clone()))) .collect(); (typ.substitute(&substitutions), i) @@ -288,7 +289,7 @@ impl StructType { .generics .iter() .zip(generic_args) - .map(|(old, new)| (old.id(), (old.clone(), new.clone()))) + .map(|(old, new)| (old.0.id(), (old.0.clone(), new.clone()))) .collect(); vecmap(&self.fields, |(name, typ)| { @@ -305,7 +306,7 @@ impl StructType { /// which is expected to be a numeric generic. /// This is needed because we infer type kinds in Noir and don't have extensive kind checking. pub fn generic_is_numeric(&self, index_of_generic: usize) -> bool { - let target_id = self.generics[index_of_generic].0; + let target_id = self.generics[index_of_generic].0.0; self.fields.iter().any(|(_, field)| field.contains_numeric_typevar(target_id)) } @@ -374,7 +375,7 @@ impl TypeAlias { .generics .iter() .zip(generic_args) - .map(|(old, new)| (old.id(), (old.clone(), new.clone()))) + .map(|(old, new)| (old.0.id(), (old.0.clone(), new.clone()))) .collect(); self.typ.substitute(&substitutions) @@ -384,7 +385,7 @@ impl TypeAlias { /// which is expected to be a numeric generic. /// This is needed because we infer type kinds in Noir and don't have extensive kind checking. pub fn generic_is_numeric(&self, index_of_generic: usize) -> bool { - let target_id = self.generics[index_of_generic].0; + let target_id = self.generics[index_of_generic].0.0; self.typ.contains_numeric_typevar(target_id) } } @@ -465,6 +466,9 @@ pub enum TypeVariableKind { /// Type::Constant(n) with a matching size. This defaults to Type::Constant(n) if still unbound /// during monomorphization. Constant(u64), + + /// The type of a slice is an array of size NotConstant. + NotConstant, } /// A TypeVariable is a mutable reference that is either @@ -728,7 +732,11 @@ impl Type { /// Returns 0 if this is not a Type::Forall pub fn generic_count(&self) -> usize { match self { - Type::Forall(generics, _) => generics.len(), + Type::Forall(generics, _) => { + // println!("TODO: remove: generic_count: {:?}, {:?}", self, generics); + + generics.len() + }, Type::TypeVariable(type_variable, _) | Type::NamedGeneric(type_variable, _, _) => { match &*type_variable.borrow() { TypeBinding::Bound(binding) => binding.generic_count(), @@ -742,7 +750,14 @@ impl Type { /// Takes a monomorphic type and generalizes it over each of the type variables in the /// given type bindings, ignoring what each type variable is bound to in the TypeBindings. pub(crate) fn generalize_from_substitutions(self, type_bindings: TypeBindings) -> Type { - let polymorphic_type_vars = vecmap(type_bindings, |(_, (type_var, _))| type_var); + let polymorphic_type_vars = vecmap(type_bindings, |(_, (type_var, binding))| { + // TODO: does this make sense? + let kind = match binding { + Type::TypeVariable(_, kind) => kind, + _ => panic!("Type of TypeVariable is non-variable"), + }; + (type_var, kind == TypeVariableKind::NotConstant) + }); Type::Forall(polymorphic_type_vars, Box::new(self)) } @@ -809,6 +824,13 @@ impl std::fmt::Display for Type { write!(f, "{}", binding.borrow()) } } + Type::TypeVariable(binding, TypeVariableKind::NotConstant) => { + if let TypeBinding::Unbound(_) = &*binding.borrow() { + write!(f, "_") + } else { + write!(f, "{}", binding.borrow()) + } + } Type::Struct(s, args) => { let args = vecmap(args, |arg| arg.to_string()); if args.is_empty() { @@ -856,7 +878,10 @@ impl std::fmt::Display for Type { } Type::Constant(x) => x.fmt(f), Type::Forall(typevars, typ) => { - let typevars = vecmap(typevars, |var| var.id().to_string()); + let typevars = vecmap(typevars, |(var, prevent_numeric)| { + let prevent_numeric_str = if *prevent_numeric { "?" } else { "" }; + format!("{}{}", var.id().to_string(), prevent_numeric_str) + }); write!(f, "forall {}. {}", typevars.join(" "), typ) } Type::Function(args, ret, env) => { @@ -922,6 +947,8 @@ impl Type { }; let this = self.substitute(bindings).follow_bindings(); + // println!("TODO remove try_bind_to_maybe_constant: {:?} {:?} {:?} {:?} ||| {:?}", self, var, target_length, bindings, this); + match &this { Type::Constant(length) if *length == target_length => { @@ -967,6 +994,7 @@ impl Type { bindings.insert(*new_target_id, (new_var.clone(), Type::NotConstant)); Ok(()) } + TypeVariableKind::NotConstant => Err(UnificationError), TypeVariableKind::IntegerOrField => Err(UnificationError), TypeVariableKind::Integer => Err(UnificationError), }, @@ -1131,6 +1159,7 @@ impl Type { use Type::*; use TypeVariableKind as Kind; + // println!("TODO try_unify: {:?} {:?} {:?}", self, other, bindings); match (self, other) { (Error, _) | (_, Error) => Ok(()), @@ -1164,6 +1193,11 @@ impl Type { (TypeVariable(var, Kind::Constant(length)), other) | (other, TypeVariable(var, Kind::Constant(length))) => other .try_unify_to_type_variable(var, bindings, |bindings| { + // let other_follow = other.follow_bindings(); + let other_follow = other.substitute(bindings).follow_bindings(); + + // println!("TODO remove: try_unify: {:?} {:?} {:?} {:?}", var, length, other, other_follow); + other.try_bind_to_maybe_constant(var, *length, bindings) }), @@ -1204,22 +1238,28 @@ impl Type { } } - (NamedGeneric(binding, _, _), other) | (other, NamedGeneric(binding, _, _)) + (NamedGeneric(binding, _, prevent_numeric), other) | (other, NamedGeneric(binding, _, prevent_numeric)) if !binding.borrow().is_unbound() => { if let TypeBinding::Bound(link) = &*binding.borrow() { + + // if *prevent_numeric { + // println!("TODO try_unify NamedGeneric: {:?} {:?} {:?} {:?} {:?}", prevent_numeric, self, other, bindings, link); + // } + link.try_unify(other, bindings) } else { unreachable!("If guard ensures binding is bound") } } - (NamedGeneric(binding_a, name_a, _), NamedGeneric(binding_b, name_b, _)) => { + (NamedGeneric(binding_a, name_a, prevent_numeric_a), NamedGeneric(binding_b, name_b, prevent_numeric_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 { + assert!(prevent_numeric_a == prevent_numeric_b); Ok(()) } else { Err(UnificationError) @@ -1326,6 +1366,8 @@ impl Type { // Don't need to issue an error here if not, it will be done in unify_with_coercions let mut bindings = TypeBindings::new(); if element1.try_unify(element2, &mut bindings).is_ok() { + // println!("TODO remove: convert_array_expression_to_slice {:?} {:?} {:?} {:?}", expression, this, target, interner); + convert_array_expression_to_slice(expression, this, target, interner); Self::apply_type_bindings(bindings); return true; @@ -1405,7 +1447,7 @@ impl Type { ) -> (Type, TypeBindings) { match self { Type::Forall(typevars, typ) => { - for var in typevars { + for (var, _prevent_numeric) in typevars { bindings .entry(var.id()) .or_insert_with(|| (var.clone(), interner.next_type_variable())); @@ -1426,7 +1468,7 @@ impl Type { Type::Forall(typevars, typ) => { let replacements = typevars .iter() - .map(|var| { + .map(|(var, _prevent_numeric)| { let new = interner.next_type_variable(); (var.id(), (var.clone(), new)) }) @@ -1495,6 +1537,11 @@ impl Type { Type::Array(size, element) => { let size = size.substitute_helper(type_bindings, substitute_bound_typevars); let element = element.substitute_helper(type_bindings, substitute_bound_typevars); + + // let size_follow = size.follow_bindings(); + // let size_follow = size.substitute(type_bindings).follow_bindings(); + // println!("TODO substitute_helper: {:?} {:?} {:?}", self, size, element); + Type::Array(Box::new(size), Box::new(element)) } Type::String(size) => { @@ -1506,9 +1553,34 @@ impl Type { let fields = fields.substitute_helper(type_bindings, substitute_bound_typevars); Type::FmtString(Box::new(size), Box::new(fields)) } - Type::NamedGeneric(binding, _, _) | Type::TypeVariable(binding, _) => { + + + // TODO: cleanup + Type::NamedGeneric(binding, _, prevent_numeric) => { + // Type::NamedGeneric(binding, _, prevent_numeric) | Type::TypeVariable(binding, _) => { + // if *prevent_numeric { + // + // for (_type_variable, binding) in type_bindings.values() { + // println!("TODO: {:?}", binding.follow_bindings()); + // } + // + // println!("TODO substitute_binding {:?} {:?} {:?}", binding, prevent_numeric, type_bindings); + // self.clone() + // } else { + // + // // TODO: run in both cases + // substitute_binding(binding) + // } + + substitute_binding(binding) + } + Type::TypeVariable(binding, _) => { substitute_binding(binding) } + // TODO: cleanup above + + + // 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) => { @@ -1532,7 +1604,7 @@ impl Type { Type::Forall(typevars, typ) => { // Trying to substitute_helper a variable de, substitute_bound_typevarsfined within a nested Forall // is usually impossible and indicative of an error in the type checker somewhere. - for var in typevars { + for (var, _prevent_numeric) in typevars { assert!(!type_bindings.contains_key(&var.id())); } let typ = Box::new(typ.substitute_helper(type_bindings, substitute_bound_typevars)); @@ -1582,7 +1654,7 @@ impl Type { } } Type::Forall(typevars, typ) => { - !typevars.iter().any(|var| var.id() == target_id) && typ.occurs(target_id) + !typevars.iter().any(|(var, _prevent_numeric)| var.id() == target_id) && typ.occurs(target_id) } Type::Function(args, ret, env) => { args.iter().any(|arg| arg.occurs(target_id)) @@ -1660,7 +1732,13 @@ impl Type { } pub fn from_generics(generics: &Generics) -> Vec { - vecmap(generics, |var| Type::TypeVariable(var.clone(), TypeVariableKind::Normal)) + vecmap(generics, |(var, prevent_numeric)| { + if *prevent_numeric { + Type::TypeVariable(var.clone(), TypeVariableKind::NotConstant) + } else { + Type::TypeVariable(var.clone(), TypeVariableKind::Normal) + } + }) } } @@ -1714,6 +1792,7 @@ impl TypeVariableKind { TypeVariableKind::IntegerOrField | TypeVariableKind::Normal => Type::default_int_type(), TypeVariableKind::Integer => Type::default_range_loop_type(), TypeVariableKind::Constant(length) => Type::Constant(*length), + TypeVariableKind::NotConstant => Type::NotConstant, } } } @@ -1745,7 +1824,7 @@ impl From<&Type> for PrintableType { TypeBinding::Bound(typ) => typ.into(), TypeBinding::Unbound(_) => Type::default_range_loop_type().into(), }, - Type::TypeVariable(binding, TypeVariableKind::IntegerOrField) => { + Type::TypeVariable(binding, TypeVariableKind::IntegerOrField) | Type::TypeVariable(binding, TypeVariableKind::NotConstant) => { match &*binding.borrow() { TypeBinding::Bound(typ) => typ.into(), TypeBinding::Unbound(_) => Type::default_int_type().into(), @@ -1810,6 +1889,9 @@ impl std::fmt::Debug for Type { Type::TypeVariable(binding, TypeVariableKind::Constant(n)) => { write!(f, "{}{:?}", n, binding) } + Type::TypeVariable(binding, TypeVariableKind::NotConstant) => { + write!(f, "{:?}?", binding) + } Type::Struct(s, args) => { let args = vecmap(args, |arg| format!("{:?}", arg)); if args.is_empty() { diff --git a/compiler/noirc_frontend/src/monomorphization/mod.rs b/compiler/noirc_frontend/src/monomorphization/mod.rs index c9fef6f6da3..11dc50be6ee 100644 --- a/compiler/noirc_frontend/src/monomorphization/mod.rs +++ b/compiler/noirc_frontend/src/monomorphization/mod.rs @@ -785,7 +785,11 @@ impl<'interner> Monomorphizer<'interner> { HirType::TraitAsType(..) => { unreachable!("All TraitAsType should be replaced before calling convert_type"); } - HirType::NamedGeneric(binding, _, _) => { + HirType::NamedGeneric(binding, _, prevent_numeric) => { + if *prevent_numeric { + panic!("TODO: convert_type NamedGeneric: {:?} {:?} {:?}", typ, binding, prevent_numeric); + } + if let TypeBinding::Bound(binding) = &*binding.borrow() { return self.convert_type(binding); } @@ -1580,12 +1584,17 @@ impl<'interner> Monomorphizer<'interner> { let (generics, impl_method_type) = self.interner.function_meta(&impl_method).typ.unwrap_forall(); - let replace_type_variable = |var: &TypeVariable| { - (var.id(), (var.clone(), Type::TypeVariable(var.clone(), TypeVariableKind::Normal))) + let replace_type_variable = |var: &TypeVariable, prevent_numeric: bool| { + let kind = if prevent_numeric { + TypeVariableKind::NotConstant + } else { + TypeVariableKind::Normal + }; + (var.id(), (var.clone(), Type::TypeVariable(var.clone(), kind))) }; // Replace each NamedGeneric with a TypeVariable containing the same internal type variable - let type_bindings = generics.iter().map(replace_type_variable).collect(); + let type_bindings = generics.iter().map(|(var, prevent_numeric)| replace_type_variable(var, *prevent_numeric)).collect(); let impl_method_type = impl_method_type.force_substitute(&type_bindings); trait_method_type.try_unify(&impl_method_type, &mut bindings).unwrap_or_else(|_| { diff --git a/compiler/noirc_frontend/src/node_interner.rs b/compiler/noirc_frontend/src/node_interner.rs index be10523d932..6d26590b1ed 100644 --- a/compiler/noirc_frontend/src/node_interner.rs +++ b/compiler/noirc_frontend/src/node_interner.rs @@ -544,12 +544,12 @@ impl NodeInterner { name: unresolved_trait.trait_def.name.clone(), crate_id: unresolved_trait.crate_id, location: Location::new(unresolved_trait.trait_def.span, unresolved_trait.file_id), - generics: vecmap(&unresolved_trait.trait_def.generics, |_| { + generics: vecmap(&unresolved_trait.trait_def.generics, |generic_ident| { // Temporary type variable ids before the trait is resolved to its actual ids. // This lets us record how many arguments the type expects so that other types // can refer to it with generic arguments before the generic parameters themselves // are resolved. - TypeVariable::unbound(TypeVariableId(0)) + (TypeVariable::unbound(TypeVariableId(0)), generic_ident.prevent_numeric) }), self_type_typevar_id, self_type_typevar: TypeVariable::unbound(self_type_typevar_id), @@ -574,12 +574,12 @@ impl NodeInterner { // Fields will be filled in later let no_fields = Vec::new(); - let generics = vecmap(&typ.struct_def.generics, |_| { + let generics = vecmap(&typ.struct_def.generics, |generic_ident| { // Temporary type variable ids before the struct is resolved to its actual ids. // This lets us record how many arguments the type expects so that other types // can refer to it with generic arguments before the generic parameters themselves // are resolved. - TypeVariable::unbound(TypeVariableId(0)) + (TypeVariable::unbound(TypeVariableId(0)), generic_ident.prevent_numeric) }); let location = Location::new(typ.struct_def.span, file_id); @@ -597,7 +597,9 @@ impl NodeInterner { typ.type_alias_def.name.clone(), Location::new(typ.type_alias_def.span, typ.file_id), Type::Error, - vecmap(&typ.type_alias_def.generics, |_| TypeVariable::unbound(TypeVariableId(0))), + vecmap(&typ.type_alias_def.generics, |generic_ident| { + (TypeVariable::unbound(TypeVariableId(0)), generic_ident.prevent_numeric) + }), ))); type_id @@ -1306,7 +1308,7 @@ impl NodeInterner { // Replace each generic with a fresh type variable let substitutions = impl_generics .into_iter() - .map(|typevar| (typevar.id(), (typevar, self.next_type_variable()))) + .map(|(typevar, _prevent_numeric)| (typevar.id(), (typevar, self.next_type_variable()))) .collect(); let instantiated_object_type = object_type.substitute(&substitutions); diff --git a/test_programs/compile_success_empty/array_slice_polymorphism/src/main.nr b/test_programs/compile_success_empty/array_slice_polymorphism/src/main.nr index df704cf31bb..a930dc37d65 100644 --- a/test_programs/compile_success_empty/array_slice_polymorphism/src/main.nr +++ b/test_programs/compile_success_empty/array_slice_polymorphism/src/main.nr @@ -3,16 +3,8 @@ fn array_zeros(_x: [u64; N]) -> [u64; N] { [0; N] } -fn main(x: Field, y: pub Field) { - let xs: [u64] = [0; 0]; +fn main() { + let xs: [u64] = [11111; 1234567]; assert(array_zeros(xs) == xs); - assert(x != y); } -#[test] -fn test_main() { - main(1, 2); - - // Uncomment to make test fail - // main(1, 1); -} From ce415a2a4e75a90940e4cf9fc0213b4aa863e723 Mon Sep 17 00:00:00 2001 From: "Michael J. Klein" Date: Mon, 19 Feb 2024 21:51:42 -0500 Subject: [PATCH 09/11] wip debugging: seem to have found unrelated issue --- compiler/noirc_frontend/src/hir_def/types.rs | 13 +++++-------- compiler/noirc_frontend/src/monomorphization/mod.rs | 6 +----- .../compile_success_empty/map_slice/src/main.nr | 12 ++++++++++-- 3 files changed, 16 insertions(+), 15 deletions(-) diff --git a/compiler/noirc_frontend/src/hir_def/types.rs b/compiler/noirc_frontend/src/hir_def/types.rs index 2654c0f410c..24f572eac5d 100644 --- a/compiler/noirc_frontend/src/hir_def/types.rs +++ b/compiler/noirc_frontend/src/hir_def/types.rs @@ -94,8 +94,6 @@ pub enum Type { /// but it makes handling them both easier. The TypeVariableId should /// never be bound over during type checking, but during monomorphization it /// will be and thus needs the full TypeVariable link. - /// - /// TODO: prevent_numeric before merge do we need N? here? Forall(Generics, Box), /// A type-level integer. Included to let an Array's size type variable @@ -751,7 +749,6 @@ impl Type { /// given type bindings, ignoring what each type variable is bound to in the TypeBindings. pub(crate) fn generalize_from_substitutions(self, type_bindings: TypeBindings) -> Type { let polymorphic_type_vars = vecmap(type_bindings, |(_, (type_var, binding))| { - // TODO: does this make sense? let kind = match binding { Type::TypeVariable(_, kind) => kind, _ => panic!("Type of TypeVariable is non-variable"), @@ -947,9 +944,6 @@ impl Type { }; let this = self.substitute(bindings).follow_bindings(); - // println!("TODO remove try_bind_to_maybe_constant: {:?} {:?} {:?} {:?} ||| {:?}", self, var, target_length, bindings, this); - - match &this { Type::Constant(length) if *length == target_length => { bindings.insert(target_id, (var.clone(), this)); @@ -962,6 +956,9 @@ impl Type { // A TypeVariable is less specific than a MaybeConstant, so we bind // to the other type variable instead. Type::TypeVariable(new_var, kind) => { + if *kind == TypeVariableKind::NotConstant { + return Err(UnificationError) + } let borrow = new_var.borrow(); match &*borrow { TypeBinding::Bound(typ) => { @@ -1196,7 +1193,7 @@ impl Type { // let other_follow = other.follow_bindings(); let other_follow = other.substitute(bindings).follow_bindings(); - // println!("TODO remove: try_unify: {:?} {:?} {:?} {:?}", var, length, other, other_follow); + // println!("TODO remove: try_unify constant: {:?} {:?} {:?} {:?}", var, length, other, other_follow); other.try_bind_to_maybe_constant(var, *length, bindings) }), @@ -1366,7 +1363,7 @@ impl Type { // Don't need to issue an error here if not, it will be done in unify_with_coercions let mut bindings = TypeBindings::new(); if element1.try_unify(element2, &mut bindings).is_ok() { - // println!("TODO remove: convert_array_expression_to_slice {:?} {:?} {:?} {:?}", expression, this, target, interner); + println!("TODO remove: convert_array_expression_to_slice {:?} {:?} {:?} {:?}", expression, this, target, bindings); convert_array_expression_to_slice(expression, this, target, interner); Self::apply_type_bindings(bindings); diff --git a/compiler/noirc_frontend/src/monomorphization/mod.rs b/compiler/noirc_frontend/src/monomorphization/mod.rs index 11dc50be6ee..a80e1ceea30 100644 --- a/compiler/noirc_frontend/src/monomorphization/mod.rs +++ b/compiler/noirc_frontend/src/monomorphization/mod.rs @@ -785,11 +785,7 @@ impl<'interner> Monomorphizer<'interner> { HirType::TraitAsType(..) => { unreachable!("All TraitAsType should be replaced before calling convert_type"); } - HirType::NamedGeneric(binding, _, prevent_numeric) => { - if *prevent_numeric { - panic!("TODO: convert_type NamedGeneric: {:?} {:?} {:?}", typ, binding, prevent_numeric); - } - + HirType::NamedGeneric(binding, _, _prevent_numeric) => { if let TypeBinding::Bound(binding) = &*binding.borrow() { return self.convert_type(binding); } diff --git a/test_programs/compile_success_empty/map_slice/src/main.nr b/test_programs/compile_success_empty/map_slice/src/main.nr index bd8007a8642..cbb327714bb 100644 --- a/test_programs/compile_success_empty/map_slice/src/main.nr +++ b/test_programs/compile_success_empty/map_slice/src/main.nr @@ -1,6 +1,14 @@ +fn foo(x: [Field]) -> [Field] { + x +} + fn main() { - let x: [Field] = [3; 4]; - let _ = x.map(|y| y); + let x0: [Field] = [3333333; 444444]; + let _ = foo(x0); + let x: [Field; 444444] = [3333333; 444444]; + let y: [Field] = x; + let z = foo(y); + let _ = z.map(|w| w); } #[test] From 793a52d510e6d19ffea9498402d8f6956af73db3 Mon Sep 17 00:00:00 2001 From: "Michael J. Klein" Date: Wed, 21 Feb 2024 13:18:53 -0500 Subject: [PATCH 10/11] wip debugging, brought back annotation for array in node interner, fix lexing test, add noirc_frontend test (passing locally), add mock stdlib option for noirc_frontend unit tests --- .../noirc_frontend/src/hir/type_check/stmt.rs | 4 +- compiler/noirc_frontend/src/hir_def/types.rs | 8 +-- compiler/noirc_frontend/src/lexer/lexer.rs | 1 + compiler/noirc_frontend/src/tests.rs | 65 +++++++++++++++++-- 4 files changed, 69 insertions(+), 9 deletions(-) diff --git a/compiler/noirc_frontend/src/hir/type_check/stmt.rs b/compiler/noirc_frontend/src/hir/type_check/stmt.rs index 370b4ee7b17..5bcfa657520 100644 --- a/compiler/noirc_frontend/src/hir/type_check/stmt.rs +++ b/compiler/noirc_frontend/src/hir/type_check/stmt.rs @@ -338,8 +338,10 @@ impl<'interner> TypeChecker<'interner> { if annotated_type.is_unsigned() { self.lint_overflowing_uint(&rhs_expr, &annotated_type); } + annotated_type + } else { + expr_type } - expr_type } /// Check if an assignment is overflowing with respect to `annotated_type` diff --git a/compiler/noirc_frontend/src/hir_def/types.rs b/compiler/noirc_frontend/src/hir_def/types.rs index 24f572eac5d..c4899a67c17 100644 --- a/compiler/noirc_frontend/src/hir_def/types.rs +++ b/compiler/noirc_frontend/src/hir_def/types.rs @@ -950,6 +950,9 @@ impl Type { Ok(()) } Type::NotConstant => { + // let other = var.clone().substitute(bindings).follow_bindings(); + // println!("try_bind_to_maybe_constant: {:?} {:?} {:?}", target_id, (var.clone(), Type::NotConstant), ()); + bindings.insert(target_id, (var.clone(), Type::NotConstant)); Ok(()) } @@ -1190,11 +1193,8 @@ impl Type { (TypeVariable(var, Kind::Constant(length)), other) | (other, TypeVariable(var, Kind::Constant(length))) => other .try_unify_to_type_variable(var, bindings, |bindings| { - // let other_follow = other.follow_bindings(); - let other_follow = other.substitute(bindings).follow_bindings(); - + // let other_follow = other.substitute(bindings).follow_bindings(); // println!("TODO remove: try_unify constant: {:?} {:?} {:?} {:?}", var, length, other, other_follow); - other.try_bind_to_maybe_constant(var, *length, bindings) }), diff --git a/compiler/noirc_frontend/src/lexer/lexer.rs b/compiler/noirc_frontend/src/lexer/lexer.rs index 9addb175b80..db381d74d87 100644 --- a/compiler/noirc_frontend/src/lexer/lexer.rs +++ b/compiler/noirc_frontend/src/lexer/lexer.rs @@ -589,6 +589,7 @@ mod tests { let expected = vec![ Token::Bang, + Token::Question, Token::NotEqual, Token::Plus, Token::LeftParen, diff --git a/compiler/noirc_frontend/src/tests.rs b/compiler/noirc_frontend/src/tests.rs index 8a56b337398..dd21e9e4694 100644 --- a/compiler/noirc_frontend/src/tests.rs +++ b/compiler/noirc_frontend/src/tests.rs @@ -48,6 +48,7 @@ mod test { } pub(crate) fn get_program( + mock_stdlib: bool, src: &str, ) -> (ParsedModule, Context, Vec<(CompilationError, FileId)>) { let root = std::path::Path::new("/"); @@ -56,7 +57,11 @@ mod test { let mut context = Context::new(fm, Default::default()); context.def_interner.populate_dummy_operator_traits(); let root_file_id = FileId::dummy(); - let root_crate_id = context.crate_graph.add_crate_root(root_file_id); + let root_crate_id = if mock_stdlib { + context.crate_graph.add_stdlib(root_file_id) + } else { + context.crate_graph.add_crate_root(root_file_id) + }; let (program, parser_errors) = parse_program(src); let mut errors = vecmap(parser_errors, |e| (e.into(), root_file_id)); @@ -87,8 +92,12 @@ mod test { (program, context, errors) } + pub(crate) fn get_program_mock_stdlib_errors(src: &str) -> Vec<(CompilationError, FileId)> { + get_program(true, src).2 + } + pub(crate) fn get_program_errors(src: &str) -> Vec<(CompilationError, FileId)> { - get_program(src).2 + get_program(false, src).2 } #[test] @@ -749,7 +758,7 @@ mod test { } fn get_program_captures(src: &str) -> Vec> { - let (program, context, _errors) = get_program(src); + let (program, context, _errors) = get_program(false, src); let interner = context.def_interner; let mut all_captures: Vec> = Vec::new(); for func in program.into_sorted().functions { @@ -1128,7 +1137,7 @@ mod test { } fn check_rewrite(src: &str, expected: &str) { - let (_program, mut context, _errors) = get_program(src); + let (_program, mut context, _errors) = get_program(false, src); let main_func_id = context.def_interner.find_function("main").unwrap(); let program = monomorphize(main_func_id, &mut context.def_interner); assert!(format!("{}", program) == expected); @@ -1206,4 +1215,52 @@ fn lambda$f1(mut env$l1: (Field)) -> Field { "#; assert_eq!(get_program_errors(src).len(), 1); } + + #[test] + fn ensure_map_slice_type_checks() { + let src = r#" + impl [T] { + #[builtin(slice_push_back)] + pub fn push_back(self, elem: T) -> Self { } + } + + impl [T; N] { + #[builtin(array_len)] + pub fn len(self) -> Field {} + + pub fn as_slice(self) -> [T] { + let mut slice = []; + for elem in self { + slice = slice.push_back(elem); + } + slice + } + + pub fn map(self, f: fn[Env](T) -> U) -> [U; N] { + let first_elem = f(self[0]); + let mut ret = [first_elem; N]; + + for i in 1 .. self.len() { + ret[i] = f(self[i]); + } + + ret + } + } + + fn foo(x: [Field]) -> [Field] { + x + } + + fn main() { + let x0: [Field] = [3333333; 444444]; + let _ = foo(x0); + let x: [Field; 444444] = [3333333; 444444]; + let y: [Field] = x; + let z = foo(y); + let _ = z.map(|w| w); + } + "#; + assert_eq!(get_program_mock_stdlib_errors(src).len(), 0); + } } From 0f53ad37c2184826fd03ed988d0657747a62e32c Mon Sep 17 00:00:00 2001 From: "Michael J. Klein" Date: Thu, 22 Feb 2024 11:48:28 -0500 Subject: [PATCH 11/11] clippy / fmt --- .../src/hir/resolution/resolver.rs | 4 +- .../src/hir/resolution/traits.rs | 13 +++++-- .../noirc_frontend/src/hir/type_check/expr.rs | 4 +- compiler/noirc_frontend/src/hir_def/types.rs | 39 ++++++++++--------- .../src/monomorphization/mod.rs | 5 ++- 5 files changed, 41 insertions(+), 24 deletions(-) diff --git a/compiler/noirc_frontend/src/hir/resolution/resolver.rs b/compiler/noirc_frontend/src/hir/resolution/resolver.rs index 906406abd1e..ce68bb50de7 100644 --- a/compiler/noirc_frontend/src/hir/resolution/resolver.rs +++ b/compiler/noirc_frontend/src/hir/resolution/resolver.rs @@ -914,7 +914,9 @@ impl<'a> Resolver<'a> { let attributes = func.attributes().clone(); - let mut generics = vecmap(&self.generics, |(_, typevar, _, prevent_numeric)| (typevar.clone(), *prevent_numeric)); + let mut generics = vecmap(&self.generics, |(_, typevar, _, prevent_numeric)| { + (typevar.clone(), *prevent_numeric) + }); let mut parameters = vec![]; let mut parameter_types = vec![]; diff --git a/compiler/noirc_frontend/src/hir/resolution/traits.rs b/compiler/noirc_frontend/src/hir/resolution/traits.rs index a750601db88..e264f50f300 100644 --- a/compiler/noirc_frontend/src/hir/resolution/traits.rs +++ b/compiler/noirc_frontend/src/hir/resolution/traits.rs @@ -42,7 +42,10 @@ pub(crate) fn resolve_traits( for (trait_id, unresolved_trait) in traits { let generics = vecmap(&unresolved_trait.trait_def.generics, |generic_ident| { - (TypeVariable::unbound(context.def_interner.next_type_variable_id()), generic_ident.prevent_numeric) + ( + TypeVariable::unbound(context.def_interner.next_type_variable_id()), + generic_ident.prevent_numeric, + ) }); // Resolve order @@ -142,7 +145,9 @@ fn resolve_trait_methods( let arguments = vecmap(parameters, |param| resolver.resolve_type(param.1.clone())); let return_type = resolver.resolve_type(return_type.get_type().into_owned()); - let generics = vecmap(resolver.get_generics(), |(_, type_var, _, prevent_numeric)| (type_var.clone(), *prevent_numeric)); + let generics = vecmap(resolver.get_generics(), |(_, type_var, _, prevent_numeric)| { + (type_var.clone(), *prevent_numeric) + }); let default_impl_list: Vec<_> = unresolved_trait .fns_with_default_impl @@ -455,7 +460,9 @@ pub(crate) fn resolve_trait_impls( methods: vecmap(&impl_methods, |(_, func_id)| *func_id), }); - let impl_generics = vecmap(impl_generics, |(_, type_variable, _, prevent_numeric)| (type_variable, prevent_numeric)); + let impl_generics = vecmap(impl_generics, |(_, type_variable, _, prevent_numeric)| { + (type_variable, prevent_numeric) + }); if let Err((prev_span, prev_file)) = interner.add_trait_implementation( self_type.clone(), diff --git a/compiler/noirc_frontend/src/hir/type_check/expr.rs b/compiler/noirc_frontend/src/hir/type_check/expr.rs index a046ba51cdf..f7278b53a49 100644 --- a/compiler/noirc_frontend/src/hir/type_check/expr.rs +++ b/compiler/noirc_frontend/src/hir/type_check/expr.rs @@ -325,7 +325,9 @@ impl<'interner> TypeChecker<'interner> { let the_trait = self.interner.get_trait(constraint.trait_id); assert_eq!(the_trait.generics.len(), constraint.trait_generics.len()); - for ((param, _prevent_numeric), arg) in the_trait.generics.iter().zip(&constraint.trait_generics) { + for ((param, _prevent_numeric), arg) in + the_trait.generics.iter().zip(&constraint.trait_generics) + { bindings.insert(param.id(), (param.clone(), arg.clone())); } } diff --git a/compiler/noirc_frontend/src/hir_def/types.rs b/compiler/noirc_frontend/src/hir_def/types.rs index 182d4139b54..b5483ca3600 100644 --- a/compiler/noirc_frontend/src/hir_def/types.rs +++ b/compiler/noirc_frontend/src/hir_def/types.rs @@ -304,7 +304,7 @@ impl StructType { /// which is expected to be a numeric generic. /// This is needed because we infer type kinds in Noir and don't have extensive kind checking. pub fn generic_is_numeric(&self, index_of_generic: usize) -> bool { - let target_id = self.generics[index_of_generic].0.0; + let target_id = self.generics[index_of_generic].0 .0; self.fields.iter().any(|(_, field)| field.contains_numeric_typevar(target_id)) } @@ -383,7 +383,7 @@ impl TypeAlias { /// which is expected to be a numeric generic. /// This is needed because we infer type kinds in Noir and don't have extensive kind checking. pub fn generic_is_numeric(&self, index_of_generic: usize) -> bool { - let target_id = self.generics[index_of_generic].0.0; + let target_id = self.generics[index_of_generic].0 .0; self.typ.contains_numeric_typevar(target_id) } } @@ -741,7 +741,7 @@ impl Type { // println!("TODO: remove: generic_count: {:?}, {:?}", self, generics); generics.len() - }, + } Type::TypeVariable(type_variable, _) | Type::NamedGeneric(type_variable, _, _) => { match &*type_variable.borrow() { TypeBinding::Bound(binding) => binding.generic_count(), @@ -967,7 +967,7 @@ impl Type { // to the other type variable instead. Type::TypeVariable(new_var, kind) => { if *kind == TypeVariableKind::NotConstant { - return Err(UnificationError) + return Err(UnificationError); } let borrow = new_var.borrow(); match &*borrow { @@ -1246,11 +1246,11 @@ impl Type { } } - (NamedGeneric(binding, _, prevent_numeric), other) | (other, NamedGeneric(binding, _, prevent_numeric)) + (NamedGeneric(binding, _, prevent_numeric), other) + | (other, NamedGeneric(binding, _, prevent_numeric)) if !binding.borrow().is_unbound() => { if let TypeBinding::Bound(link) = &*binding.borrow() { - // if *prevent_numeric { // println!("TODO try_unify NamedGeneric: {:?} {:?} {:?} {:?} {:?}", prevent_numeric, self, other, bindings, link); // } @@ -1261,7 +1261,10 @@ impl Type { } } - (NamedGeneric(binding_a, name_a, prevent_numeric_a), NamedGeneric(binding_b, name_b, prevent_numeric_b)) => { + ( + NamedGeneric(binding_a, name_a, prevent_numeric_a), + NamedGeneric(binding_b, name_b, prevent_numeric_b), + ) => { // Bound NamedGenerics are caught by the check above assert!(binding_a.borrow().is_unbound()); assert!(binding_b.borrow().is_unbound()); @@ -1374,7 +1377,10 @@ impl Type { // Don't need to issue an error here if not, it will be done in unify_with_coercions let mut bindings = TypeBindings::new(); if element1.try_unify(element2, &mut bindings).is_ok() { - println!("TODO remove: convert_array_expression_to_slice {:?} {:?} {:?} {:?}", expression, this, target, bindings); + println!( + "TODO remove: convert_array_expression_to_slice {:?} {:?} {:?} {:?}", + expression, this, target, bindings + ); convert_array_expression_to_slice(expression, this, target, interner); Self::apply_type_bindings(bindings); @@ -1545,7 +1551,7 @@ impl Type { Type::Array(size, element) => { let size = size.substitute_helper(type_bindings, substitute_bound_typevars); let element = element.substitute_helper(type_bindings, substitute_bound_typevars); - + // let size_follow = size.follow_bindings(); // let size_follow = size.substitute(type_bindings).follow_bindings(); // println!("TODO substitute_helper: {:?} {:?} {:?}", self, size, element); @@ -1562,10 +1568,9 @@ impl Type { Type::FmtString(Box::new(size), Box::new(fields)) } - // TODO: cleanup Type::NamedGeneric(binding, _, prevent_numeric) => { - // Type::NamedGeneric(binding, _, prevent_numeric) | Type::TypeVariable(binding, _) => { + // Type::NamedGeneric(binding, _, prevent_numeric) | Type::TypeVariable(binding, _) => { // if *prevent_numeric { // // for (_type_variable, binding) in type_bindings.values() { @@ -1582,13 +1587,9 @@ impl Type { substitute_binding(binding) } - Type::TypeVariable(binding, _) => { - substitute_binding(binding) - } + Type::TypeVariable(binding, _) => substitute_binding(binding), // TODO: cleanup above - - // 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) => { @@ -1662,7 +1663,8 @@ impl Type { } } Type::Forall(typevars, typ) => { - !typevars.iter().any(|(var, _prevent_numeric)| var.id() == target_id) && typ.occurs(target_id) + !typevars.iter().any(|(var, _prevent_numeric)| var.id() == target_id) + && typ.occurs(target_id) } Type::Function(args, ret, env) => { args.iter().any(|arg| arg.occurs(target_id)) @@ -1832,7 +1834,8 @@ impl From<&Type> for PrintableType { TypeBinding::Bound(typ) => typ.into(), TypeBinding::Unbound(_) => Type::default_range_loop_type().into(), }, - Type::TypeVariable(binding, TypeVariableKind::IntegerOrField) | Type::TypeVariable(binding, TypeVariableKind::NotConstant) => { + Type::TypeVariable(binding, TypeVariableKind::IntegerOrField) + | Type::TypeVariable(binding, TypeVariableKind::NotConstant) => { match &*binding.borrow() { TypeBinding::Bound(typ) => typ.into(), TypeBinding::Unbound(_) => Type::default_int_type().into(), diff --git a/compiler/noirc_frontend/src/monomorphization/mod.rs b/compiler/noirc_frontend/src/monomorphization/mod.rs index 541c2eb56b0..485fd2efd04 100644 --- a/compiler/noirc_frontend/src/monomorphization/mod.rs +++ b/compiler/noirc_frontend/src/monomorphization/mod.rs @@ -1591,7 +1591,10 @@ impl<'interner> Monomorphizer<'interner> { }; // Replace each NamedGeneric with a TypeVariable containing the same internal type variable - let type_bindings = generics.iter().map(|(var, prevent_numeric)| replace_type_variable(var, *prevent_numeric)).collect(); + let type_bindings = generics + .iter() + .map(|(var, prevent_numeric)| replace_type_variable(var, *prevent_numeric)) + .collect(); let impl_method_type = impl_method_type.force_substitute(&type_bindings); trait_method_type.try_unify(&impl_method_type, &mut bindings).unwrap_or_else(|_| {