From 04ea5d0b6da220b7ed9d9e92100bff67fcacc944 Mon Sep 17 00:00:00 2001 From: Jake Fecher Date: Tue, 21 Nov 2023 15:26:18 -0600 Subject: [PATCH 01/55] Add turbofish operator; add stack overflow crash --- compiler/noirc_frontend/src/ast/expression.rs | 26 +++++-- compiler/noirc_frontend/src/ast/mod.rs | 2 +- compiler/noirc_frontend/src/ast/statement.rs | 8 +- .../src/hir/resolution/resolver.rs | 10 ++- .../noirc_frontend/src/hir/type_check/expr.rs | 10 ++- compiler/noirc_frontend/src/hir_def/expr.rs | 6 +- compiler/noirc_frontend/src/hir_def/types.rs | 25 +++++- .../src/monomorphization/mod.rs | 14 ++-- compiler/noirc_frontend/src/parser/parser.rs | 78 +++++++++++-------- tooling/nargo_fmt/src/visitor/expr.rs | 2 +- 10 files changed, 121 insertions(+), 60 deletions(-) diff --git a/compiler/noirc_frontend/src/ast/expression.rs b/compiler/noirc_frontend/src/ast/expression.rs index d29e1670944..4b29ab4f4ce 100644 --- a/compiler/noirc_frontend/src/ast/expression.rs +++ b/compiler/noirc_frontend/src/ast/expression.rs @@ -23,7 +23,9 @@ pub enum ExpressionKind { Cast(Box), Infix(Box), If(Box), - Variable(Path), + // The optional vec here is the optional list of generics + // provided by the turbofish operator, if used + Variable(Path, Option>), Tuple(Vec), Lambda(Box), Parenthesized(Box), @@ -37,7 +39,7 @@ pub type UnresolvedGenerics = Vec; impl ExpressionKind { pub fn into_path(self) -> Option { match self { - ExpressionKind::Variable(path) => Some(path), + ExpressionKind::Variable(path, _) => Some(path), _ => None, } } @@ -165,7 +167,11 @@ impl Expression { Expression::new(kind, span) } - pub fn call(lhs: Expression, arguments: Vec, span: Span) -> Expression { + pub fn call( + lhs: Expression, + arguments: Vec, + span: Span, + ) -> Expression { // Need to check if lhs is an if expression since users can sequence if expressions // with tuples without calling them. E.g. `if c { t } else { e }(a, b)` is interpreted // as a sequence of { if, tuple } rather than a function call. This behavior matches rust. @@ -181,7 +187,10 @@ impl Expression { }, ])) } else { - ExpressionKind::Call(Box::new(CallExpression { func: Box::new(lhs), arguments })) + ExpressionKind::Call(Box::new(CallExpression { + func: Box::new(lhs), + arguments, + })) }; Expression::new(kind, span) } @@ -480,7 +489,14 @@ impl Display for ExpressionKind { Cast(cast) => cast.fmt(f), Infix(infix) => infix.fmt(f), If(if_expr) => if_expr.fmt(f), - Variable(path) => path.fmt(f), + Variable(path, generics) => { + if let Some(generics) = generics { + let generics = vecmap(generics, ToString::to_string); + write!(f, "{path}::<{}>", generics.join(", ")) + } else { + path.fmt(f) + } + } Constructor(constructor) => constructor.fmt(f), MemberAccess(access) => access.fmt(f), Tuple(elements) => { diff --git a/compiler/noirc_frontend/src/ast/mod.rs b/compiler/noirc_frontend/src/ast/mod.rs index 7ce01f461ed..a533d31eeac 100644 --- a/compiler/noirc_frontend/src/ast/mod.rs +++ b/compiler/noirc_frontend/src/ast/mod.rs @@ -234,7 +234,7 @@ impl UnresolvedTypeExpression { Some(int) => Ok(UnresolvedTypeExpression::Constant(int, expr.span)), None => Err(expr), }, - ExpressionKind::Variable(path) => Ok(UnresolvedTypeExpression::Variable(path)), + ExpressionKind::Variable(path, _) => Ok(UnresolvedTypeExpression::Variable(path)), ExpressionKind::Prefix(prefix) if prefix.operator == UnaryOp::Minus => { let lhs = Box::new(UnresolvedTypeExpression::Constant(0, expr.span)); let rhs = Box::new(UnresolvedTypeExpression::from_expr_helper(prefix.rhs)?); diff --git a/compiler/noirc_frontend/src/ast/statement.rs b/compiler/noirc_frontend/src/ast/statement.rs index 1ca4d3101a9..273259ff069 100644 --- a/compiler/noirc_frontend/src/ast/statement.rs +++ b/compiler/noirc_frontend/src/ast/statement.rs @@ -198,7 +198,7 @@ impl From for Expression { fn from(i: Ident) -> Expression { Expression { span: i.0.span(), - kind: ExpressionKind::Variable(Path { segments: vec![i], kind: PathKind::Plain }), + kind: ExpressionKind::Variable(Path { segments: vec![i], kind: PathKind::Plain }, None), } } } @@ -469,7 +469,7 @@ impl Recoverable for Pattern { impl LValue { fn as_expression(&self, span: Span) -> Expression { let kind = match self { - LValue::Ident(ident) => ExpressionKind::Variable(Path::from_ident(ident.clone())), + LValue::Ident(ident) => ExpressionKind::Variable(Path::from_ident(ident.clone()), None), LValue::MemberAccess { object, field_name } => { ExpressionKind::MemberAccess(Box::new(MemberAccessExpression { lhs: object.as_expression(span), @@ -546,7 +546,7 @@ impl ForRange { // array.len() let segments = vec![array_ident]; let array_ident = - ExpressionKind::Variable(Path { segments, kind: PathKind::Plain }); + ExpressionKind::Variable(Path { segments, kind: PathKind::Plain }, None); let end_range = ExpressionKind::MethodCall(Box::new(MethodCallExpression { object: Expression::new(array_ident.clone(), array_span), @@ -562,7 +562,7 @@ impl ForRange { // array[i] let segments = vec![Ident::new(index_name, array_span)]; let index_ident = - ExpressionKind::Variable(Path { segments, kind: PathKind::Plain }); + ExpressionKind::Variable(Path { segments, kind: PathKind::Plain }, None); let loop_element = ExpressionKind::Index(Box::new(IndexExpression { collection: Expression::new(array_ident, array_span), diff --git a/compiler/noirc_frontend/src/hir/resolution/resolver.rs b/compiler/noirc_frontend/src/hir/resolution/resolver.rs index 4b829932b76..792a7fea380 100644 --- a/compiler/noirc_frontend/src/hir/resolution/resolver.rs +++ b/compiler/noirc_frontend/src/hir/resolution/resolver.rs @@ -1206,7 +1206,7 @@ impl<'a> Resolver<'a> { Literal::FmtStr(str) => self.resolve_fmt_str_literal(str, expr.span), Literal::Unit => HirLiteral::Unit, }), - ExpressionKind::Variable(path) => { + ExpressionKind::Variable(path, generics) => { if let Some((hir_expr, object_type)) = self.resolve_trait_generic_path(&path) { let expr_id = self.interner.push_expr(hir_expr); self.interner.push_expr_location(expr_id, expr.span, self.file); @@ -1252,7 +1252,9 @@ impl<'a> Resolver<'a> { } } - HirExpression::Ident(hir_ident) + let generics = generics.map(|generics| vecmap(generics, |typ| self.resolve_type(typ))); + + HirExpression::Ident(hir_ident, generics) } } ExpressionKind::Prefix(prefix) => { @@ -1708,7 +1710,7 @@ impl<'a> Resolver<'a> { let variable = scope_tree.find(ident_name); if let Some((old_value, _)) = variable { old_value.num_times_used += 1; - let expr_id = self.interner.push_expr(HirExpression::Ident(old_value.ident)); + let expr_id = self.interner.push_expr(HirExpression::Ident(old_value.ident, None)); self.interner.push_expr_location(expr_id, call_expr_span, self.file); fmt_str_idents.push(expr_id); } else if ident_name.parse::().is_ok() { @@ -1820,7 +1822,7 @@ pub fn verify_mutable_reference(interner: &NodeInterner, rhs: ExprId) -> Result< let span = interner.expr_span(&rhs); Err(ResolverError::MutableReferenceToArrayElement { span }) } - HirExpression::Ident(ident) => { + HirExpression::Ident(ident, _) => { if let Some(definition) = interner.try_definition(ident.id) { if !definition.mutable { return Err(ResolverError::MutableReferenceToImmutableVariable { diff --git a/compiler/noirc_frontend/src/hir/type_check/expr.rs b/compiler/noirc_frontend/src/hir/type_check/expr.rs index 74f076212fa..3b5a8a38f71 100644 --- a/compiler/noirc_frontend/src/hir/type_check/expr.rs +++ b/compiler/noirc_frontend/src/hir/type_check/expr.rs @@ -18,7 +18,7 @@ use super::{errors::TypeCheckError, TypeChecker}; impl<'interner> TypeChecker<'interner> { fn check_if_deprecated(&mut self, expr: &ExprId) { - if let HirExpression::Ident(expr::HirIdent { location, id }) = + if let HirExpression::Ident(expr::HirIdent { location, id }, _) = self.interner.expression(expr) { if let Some(DefinitionKind::Function(func_id)) = @@ -46,13 +46,17 @@ impl<'interner> TypeChecker<'interner> { /// function `foo` to refer to. pub(crate) fn check_expression(&mut self, expr_id: &ExprId) -> Type { let typ = match self.interner.expression(expr_id) { - HirExpression::Ident(ident) => { + HirExpression::Ident(ident, generics) => { // An identifiers type may be forall-quantified in the case of generic functions. // E.g. `fn foo(t: T, field: Field) -> T` has type `forall T. fn(T, Field) -> T`. // We must instantiate identifiers at every call site to replace this T with a new type // variable to handle generic functions. let t = self.interner.id_type_substitute_trait_as_type(ident.id); - let (typ, bindings) = t.instantiate(self.interner); + + let (typ, bindings) = match generics { + Some(generics) => t.instantiate_with(generics), + None => t.instantiate(self.interner), + }; // Push any trait constraints required by this definition to the context // to be checked later when the type of this variable is further constrained. diff --git a/compiler/noirc_frontend/src/hir_def/expr.rs b/compiler/noirc_frontend/src/hir_def/expr.rs index 755817ba1e6..2de3358701d 100644 --- a/compiler/noirc_frontend/src/hir_def/expr.rs +++ b/compiler/noirc_frontend/src/hir_def/expr.rs @@ -15,7 +15,9 @@ use super::types::{StructType, Type}; /// from the definition that refers to them so there is no ambiguity with names. #[derive(Debug, Clone)] pub enum HirExpression { - Ident(HirIdent), + // The optional vec here is the optional list of generics + // provided by the turbofish operator, if it was used + Ident(HirIdent, Option>), Literal(HirLiteral), Block(HirBlockExpression), Prefix(HirPrefixExpression), @@ -172,7 +174,7 @@ impl HirMethodCallExpression { let expr = match method { HirMethodReference::FuncId(func_id) => { let id = interner.function_definition_id(func_id); - HirExpression::Ident(HirIdent { location, id }) + HirExpression::Ident(HirIdent { location, id }, None) } HirMethodReference::TraitMethodId(method_id) => { HirExpression::TraitMethodReference(method_id) diff --git a/compiler/noirc_frontend/src/hir_def/types.rs b/compiler/noirc_frontend/src/hir_def/types.rs index 8ad38b526de..e698d52a227 100644 --- a/compiler/noirc_frontend/src/hir_def/types.rs +++ b/compiler/noirc_frontend/src/hir_def/types.rs @@ -1126,6 +1126,29 @@ impl Type { } } + /// Instantiates a type with the given types. + /// This differs from substitute in that only the quantified type variables + /// are matched against the type list and are eligible for substitution - similar + /// to normal instantiation. This function is used when the turbofish operator + /// is used and generic substitutions are provided manually by users. + /// + /// Expects the given type vector to be the same length as the Forall type variables. + pub fn instantiate_with(&self, types: Vec) -> (Type, TypeBindings) { + match self { + Type::Forall(typevars, typ) => { + let replacements = typevars + .iter() + .zip(types) + .map(|((id, var), binding)| (*id, (var.clone(), binding))) + .collect(); + + let instantiated = typ.substitute(&replacements); + (instantiated, replacements) + } + other => (other.clone(), HashMap::new()), + } + } + /// Replace each NamedGeneric (and TypeVariable) in this type with a fresh type variable pub(crate) fn instantiate_type_variables( &self, @@ -1381,7 +1404,7 @@ fn convert_array_expression_to_slice( let as_slice_id = interner.function_definition_id(as_slice_method); let location = interner.expr_location(&expression); - let as_slice = HirExpression::Ident(HirIdent { location, id: as_slice_id }); + let as_slice = HirExpression::Ident(HirIdent { location, id: as_slice_id }, None); let func = interner.push_expr(as_slice); let arguments = vec![expression]; diff --git a/compiler/noirc_frontend/src/monomorphization/mod.rs b/compiler/noirc_frontend/src/monomorphization/mod.rs index 57e4e6cdeb0..c72f2adc911 100644 --- a/compiler/noirc_frontend/src/monomorphization/mod.rs +++ b/compiler/noirc_frontend/src/monomorphization/mod.rs @@ -299,7 +299,7 @@ impl<'interner> Monomorphizer<'interner> { use ast::Literal::*; match self.interner.expression(&expr) { - HirExpression::Ident(ident) => self.ident(ident, expr), + HirExpression::Ident(ident, _) => self.ident(ident, expr), HirExpression::Literal(HirLiteral::Str(contents)) => Literal(Str(contents)), HirExpression::Literal(HirLiteral::FmtStr(contents, idents)) => { let fields = vecmap(idents, |ident| self.expr(ident)); @@ -955,17 +955,15 @@ impl<'interner> Monomorphizer<'interner> { arguments: &mut Vec, ) { match hir_argument { - HirExpression::Ident(ident) => { - let typ = self.interner.id_type(ident.id); - let typ: Type = typ.follow_bindings(); - let is_fmt_str = match typ { + HirExpression::Ident(ident, _) => { + let is_fmt_str = match self.interner.id_type(ident.id).follow_bindings() { // A format string has many different possible types that need to be handled. // Loop over each element in the format string to fetch each type's relevant metadata Type::FmtString(_, elements) => { match *elements { Type::Tuple(element_types) => { - for typ in element_types { - Self::append_printable_type_info_inner(&typ, arguments); + for element in element_types { + Self::append_printable_type_info_inner(&element, arguments); } } _ => unreachable!( @@ -974,7 +972,7 @@ impl<'interner> Monomorphizer<'interner> { } true } - _ => { + typ => { Self::append_printable_type_info_inner(&typ, arguments); false } diff --git a/compiler/noirc_frontend/src/parser/parser.rs b/compiler/noirc_frontend/src/parser/parser.rs index 6b8589cc6e5..fd385eef565 100644 --- a/compiler/noirc_frontend/src/parser/parser.rs +++ b/compiler/noirc_frontend/src/parser/parser.rs @@ -1003,14 +1003,14 @@ fn parse_type<'a>() -> impl NoirParser + 'a { recursive(parse_type_inner) } -fn parse_type_inner( - recursive_type_parser: impl NoirParser, -) -> impl NoirParser { +fn parse_type_inner<'a>( + recursive_type_parser: impl NoirParser + 'a, +) -> impl NoirParser + 'a { choice(( field_type(), int_type(), bool_type(), - string_type(), + string_type(recursive_type_parser.clone()), format_string_type(recursive_type_parser.clone()), named_type(recursive_type_parser.clone()), named_trait(recursive_type_parser.clone()), @@ -1056,20 +1056,22 @@ fn bool_type() -> impl NoirParser { .map_with_span(|_, span| UnresolvedTypeData::Bool.with_span(span)) } -fn string_type() -> impl NoirParser { +fn string_type<'a>( + type_parser: impl NoirParser + 'a, + ) -> impl NoirParser + 'a { keyword(Keyword::String) .ignore_then( - type_expression().delimited_by(just(Token::Less), just(Token::Greater)).or_not(), + type_expression(type_parser).delimited_by(just(Token::Less), just(Token::Greater)).or_not(), ) .map_with_span(|expr, span| UnresolvedTypeData::String(expr).with_span(span)) } -fn format_string_type( - type_parser: impl NoirParser, -) -> impl NoirParser { +fn format_string_type<'a>( + type_parser: impl NoirParser + 'a, +) -> impl NoirParser + 'a { keyword(Keyword::FormatString) .ignore_then( - type_expression() + type_expression(type_parser.clone()) .then_ignore(just(Token::Comma)) .then(type_parser) .delimited_by(just(Token::Less), just(Token::Greater)), @@ -1097,28 +1099,28 @@ fn int_type() -> impl NoirParser { }) } -fn named_type(type_parser: impl NoirParser) -> impl NoirParser { +fn named_type<'a>(type_parser: impl NoirParser + 'a) -> impl NoirParser + 'a { path() .then(generic_type_args(type_parser)) .map_with_span(|(path, args), span| UnresolvedTypeData::Named(path, args).with_span(span)) } -fn named_trait(type_parser: impl NoirParser) -> impl NoirParser { +fn named_trait<'a>(type_parser: impl NoirParser + 'a) -> impl NoirParser + 'a { keyword(Keyword::Impl).ignore_then(path()).then(generic_type_args(type_parser)).map_with_span( |(path, args), span| UnresolvedTypeData::TraitAsType(path, args).with_span(span), ) } -fn generic_type_args( - type_parser: impl NoirParser, -) -> impl NoirParser> { - type_parser +fn generic_type_args<'a>( + type_parser: impl NoirParser + 'a, +) -> impl NoirParser> + 'a { + type_parser.clone() // Without checking for a terminating ',' or '>' here we may incorrectly // parse a generic `N * 2` as just the type `N` then fail when there is no // separator afterward. Failing early here ensures we try the `type_expression` // parser afterward. .then_ignore(one_of([Token::Comma, Token::Greater]).rewind()) - .or(type_expression() + .or(type_expression(type_parser) .map_with_span(|expr, span| UnresolvedTypeData::Expression(expr).with_span(span))) .separated_by(just(Token::Comma)) .allow_trailing() @@ -1128,23 +1130,26 @@ fn generic_type_args( .map(Option::unwrap_or_default) } -fn array_type(type_parser: impl NoirParser) -> impl NoirParser { +fn array_type<'a>(type_parser: impl NoirParser + 'a) -> impl NoirParser + 'a { just(Token::LeftBracket) - .ignore_then(type_parser) - .then(just(Token::Semicolon).ignore_then(type_expression()).or_not()) + .ignore_then(type_parser.clone()) + .then(just(Token::Semicolon).ignore_then(type_expression(type_parser)).or_not()) .then_ignore(just(Token::RightBracket)) .map_with_span(|(element_type, size), span| { UnresolvedTypeData::Array(size, Box::new(element_type)).with_span(span) }) } -fn type_expression() -> impl NoirParser { +fn type_expression<'a, T>(type_parser: T) -> impl NoirParser + 'a + where T: NoirParser + 'a +{ recursive(|expr| { expression_with_precedence( Precedence::lowest_type_precedence(), expr, nothing(), nothing(), + type_parser, true, false, ) @@ -1201,6 +1206,10 @@ where }) } +fn turbofish<'a>(type_parser: impl NoirParser + 'a) -> impl NoirParser>> + 'a { + just(Token::DoubleColon).ignore_then(generic_type_args(type_parser)).or_not() +} + fn expression() -> impl ExprParser { recursive(|expr| { expression_with_precedence( @@ -1208,6 +1217,7 @@ fn expression() -> impl ExprParser { expr.clone(), expression_no_constructors(expr.clone()), statement(expr.clone(), expression_no_constructors(expr)), + parse_type(), false, true, ) @@ -1225,6 +1235,7 @@ where expr_parser.clone(), expr_no_constructors.clone(), statement(expr_parser, expr_no_constructors), + parse_type(), false, false, ) @@ -1247,11 +1258,12 @@ where // An expression is a single term followed by 0 or more (OP subexpression)* // where OP is an operator at the given precedence level and subexpression // is an expression at the current precedence level plus one. -fn expression_with_precedence<'a, P, P2, S>( +fn expression_with_precedence<'a, P, P2, S, T>( precedence: Precedence, expr_parser: P, expr_no_constructors: P2, statement: S, + type_parser: T, // True if we should only parse the restricted subset of operators valid within type expressions is_type_expression: bool, // True if we should also parse constructors `Foo { field1: value1, ... }` as an expression. @@ -1263,10 +1275,11 @@ where P: ExprParser + 'a, P2: ExprParser + 'a, S: NoirParser + 'a, + T: NoirParser + 'a, { if precedence == Precedence::Highest { if is_type_expression { - type_expression_term(expr_parser).boxed().labelled(ParsingRuleLabel::Term) + type_expression_term(expr_parser, type_parser).boxed().labelled(ParsingRuleLabel::Term) } else { term(expr_parser, expr_no_constructors, statement, allow_constructors) .boxed() @@ -1281,6 +1294,7 @@ where expr_parser, expr_no_constructors, statement, + type_parser, is_type_expression, allow_constructors, ); @@ -1353,12 +1367,13 @@ where /// The equivalent of a 'term' for use in type expressions. Unlike regular terms, the grammar here /// is restricted to no longer include right-unary expressions, unary not, and most atoms. -fn type_expression_term<'a, P>(expr_parser: P) -> impl NoirParser + 'a +fn type_expression_term<'a, P, P2>(expr_parser: P, type_parser: P2) -> impl NoirParser + 'a where P: ExprParser + 'a, + P2: NoirParser + 'a, { recursive(move |term_parser| { - negation(term_parser).map_with_span(Expression::new).or(type_expression_atom(expr_parser)) + negation(term_parser).map_with_span(Expression::new).or(type_expression_atom(expr_parser, type_parser)) }) } @@ -1582,7 +1597,7 @@ where }, lambda(expr_parser.clone()), block(statement).map(ExpressionKind::Block), - variable(), + variable(parse_type()), literal(), )) .map_with_span(Expression::new) @@ -1595,11 +1610,12 @@ where /// Atoms within type expressions are limited to only variables, literals, and parenthesized /// type expressions. -fn type_expression_atom<'a, P>(expr_parser: P) -> impl NoirParser + 'a +fn type_expression_atom<'a, P, P2>(expr_parser: P, type_parser: P2) -> impl NoirParser + 'a where P: ExprParser + 'a, + P2: NoirParser + 'a, { - variable() + variable(type_parser) .or(literal()) .map_with_span(Expression::new) .or(parenthesized(expr_parser)) @@ -1648,8 +1664,8 @@ where long_form.or(short_form) } -fn variable() -> impl NoirParser { - path().map(ExpressionKind::Variable) +fn variable<'a>(type_parser: impl NoirParser + 'a) -> impl NoirParser + 'a { + path().then(turbofish(type_parser)).map(|(path, generics)| ExpressionKind::Variable(path, generics)) } fn literal() -> impl NoirParser { @@ -1913,7 +1929,7 @@ mod test { #[test] fn parse_type_expression() { - parse_all(type_expression(), vec!["(123)", "123", "(1 + 1)", "(1 + (1))"]); + parse_all(type_expression(parse_type()), vec!["(123)", "123", "(1 + 1)", "(1 + (1))"]); } #[test] diff --git a/tooling/nargo_fmt/src/visitor/expr.rs b/tooling/nargo_fmt/src/visitor/expr.rs index 8492fd5c05d..206d0ed6349 100644 --- a/tooling/nargo_fmt/src/visitor/expr.rs +++ b/tooling/nargo_fmt/src/visitor/expr.rs @@ -199,7 +199,7 @@ impl FmtVisitor<'_> { self.format_if(*if_expr) } - ExpressionKind::Lambda(_) | ExpressionKind::Variable(_) => self.slice(span).to_string(), + ExpressionKind::Lambda(_) | ExpressionKind::Variable(..) => self.slice(span).to_string(), ExpressionKind::Error => unreachable!(), } } From 3a952291b6bcc645626ce8c2834bd43f7ef7e81e Mon Sep 17 00:00:00 2001 From: Jake Fecher Date: Wed, 22 Nov 2023 12:35:33 -0600 Subject: [PATCH 02/55] Fix parsing error --- compiler/noirc_frontend/src/ast/expression.rs | 28 +++-- compiler/noirc_frontend/src/ast/statement.rs | 1 + .../src/hir/resolution/resolver.rs | 9 +- .../noirc_frontend/src/hir/type_check/expr.rs | 17 +-- compiler/noirc_frontend/src/hir_def/expr.rs | 6 +- compiler/noirc_frontend/src/parser/parser.rs | 108 ++++++++---------- tooling/nargo_fmt/src/visitor/expr.rs | 4 +- 7 files changed, 88 insertions(+), 85 deletions(-) diff --git a/compiler/noirc_frontend/src/ast/expression.rs b/compiler/noirc_frontend/src/ast/expression.rs index 4b29ab4f4ce..3618428c33a 100644 --- a/compiler/noirc_frontend/src/ast/expression.rs +++ b/compiler/noirc_frontend/src/ast/expression.rs @@ -143,16 +143,19 @@ impl Expression { pub fn member_access_or_method_call( lhs: Expression, - (rhs, args): (Ident, Option>), + (rhs, args): (Ident, Option<(Option>, Vec)>), span: Span, ) -> Expression { let kind = match args { None => ExpressionKind::MemberAccess(Box::new(MemberAccessExpression { lhs, rhs })), - Some(arguments) => ExpressionKind::MethodCall(Box::new(MethodCallExpression { - object: lhs, - method_name: rhs, - arguments, - })), + Some((generics, arguments)) => { + ExpressionKind::MethodCall(Box::new(MethodCallExpression { + object: lhs, + method_name: rhs, + generics, + arguments, + })) + } }; Expression::new(kind, span) } @@ -167,11 +170,7 @@ impl Expression { Expression::new(kind, span) } - pub fn call( - lhs: Expression, - arguments: Vec, - span: Span, - ) -> Expression { + pub fn call(lhs: Expression, arguments: Vec, span: Span) -> Expression { // Need to check if lhs is an if expression since users can sequence if expressions // with tuples without calling them. E.g. `if c { t } else { e }(a, b)` is interpreted // as a sequence of { if, tuple } rather than a function call. This behavior matches rust. @@ -187,10 +186,7 @@ impl Expression { }, ])) } else { - ExpressionKind::Call(Box::new(CallExpression { - func: Box::new(lhs), - arguments, - })) + ExpressionKind::Call(Box::new(CallExpression { func: Box::new(lhs), arguments })) }; Expression::new(kind, span) } @@ -432,6 +428,8 @@ pub struct CallExpression { pub struct MethodCallExpression { pub object: Expression, pub method_name: Ident, + /// Method calls have an optional list of generics if the turbofish operator was used + pub generics: Option>, pub arguments: Vec, } diff --git a/compiler/noirc_frontend/src/ast/statement.rs b/compiler/noirc_frontend/src/ast/statement.rs index 273259ff069..e756dce7ac6 100644 --- a/compiler/noirc_frontend/src/ast/statement.rs +++ b/compiler/noirc_frontend/src/ast/statement.rs @@ -551,6 +551,7 @@ impl ForRange { let end_range = ExpressionKind::MethodCall(Box::new(MethodCallExpression { object: Expression::new(array_ident.clone(), array_span), method_name: Ident::new("len".to_string(), array_span), + generics: None, arguments: vec![], })); let end_range = Expression::new(end_range, array_span); diff --git a/compiler/noirc_frontend/src/hir/resolution/resolver.rs b/compiler/noirc_frontend/src/hir/resolution/resolver.rs index 792a7fea380..347ad1cac29 100644 --- a/compiler/noirc_frontend/src/hir/resolution/resolver.rs +++ b/compiler/noirc_frontend/src/hir/resolution/resolver.rs @@ -1252,7 +1252,8 @@ impl<'a> Resolver<'a> { } } - let generics = generics.map(|generics| vecmap(generics, |typ| self.resolve_type(typ))); + let generics = + generics.map(|generics| vecmap(generics, |typ| self.resolve_type(typ))); HirExpression::Ident(hir_ident, generics) } @@ -1290,12 +1291,16 @@ impl<'a> Resolver<'a> { ExpressionKind::MethodCall(call_expr) => { let method = call_expr.method_name; let object = self.resolve_expression(call_expr.object); + let generics = call_expr + .generics + .map(|generics| vecmap(generics, |typ| self.resolve_type(typ))); let arguments = vecmap(call_expr.arguments, |arg| self.resolve_expression(arg)); let location = Location::new(expr.span, self.file); HirExpression::MethodCall(HirMethodCallExpression { - arguments, method, object, + generics, + arguments, location, }) } diff --git a/compiler/noirc_frontend/src/hir/type_check/expr.rs b/compiler/noirc_frontend/src/hir/type_check/expr.rs index 3b5a8a38f71..8b2face32c7 100644 --- a/compiler/noirc_frontend/src/hir/type_check/expr.rs +++ b/compiler/noirc_frontend/src/hir/type_check/expr.rs @@ -203,14 +203,12 @@ impl<'interner> TypeChecker<'interner> { HirMethodReference::TraitMethodId(method) => Some(method.trait_id), }; - let (function_id, function_call) = method_call.into_function_call( - method_ref.clone(), - location, - self.interner, - ); + let (function_id, function_call, generics) = method_call + .into_function_call(method_ref.clone(), location, self.interner); let span = self.interner.expr_span(expr_id); - let ret = self.check_method_call(&function_id, method_ref, args, span); + let ret = + self.check_method_call(&function_id, method_ref, generics, args, span); if let Some(trait_id) = trait_id { self.verify_trait_constraint(&object_type, trait_id, function_id, span); @@ -534,6 +532,8 @@ impl<'interner> TypeChecker<'interner> { &mut self, function_ident_id: &ExprId, method_ref: HirMethodReference, + // The optional list of generics if the turbofish operator was used + generics: Option>, arguments: Vec<(Type, ExprId, Span)>, span: Span, ) -> Type { @@ -564,7 +564,10 @@ impl<'interner> TypeChecker<'interner> { }); } - let (function_type, instantiation_bindings) = fn_typ.instantiate(self.interner); + let (function_type, instantiation_bindings) = match generics { + Some(generics) => fn_typ.instantiate_with(generics), + None => fn_typ.instantiate(self.interner), + }; self.interner.store_instantiation_bindings(*function_ident_id, instantiation_bindings); self.interner.push_expr_type(function_ident_id, function_type.clone()); diff --git a/compiler/noirc_frontend/src/hir_def/expr.rs b/compiler/noirc_frontend/src/hir_def/expr.rs index 2de3358701d..93e3350b2ca 100644 --- a/compiler/noirc_frontend/src/hir_def/expr.rs +++ b/compiler/noirc_frontend/src/hir_def/expr.rs @@ -144,6 +144,8 @@ pub struct HirCallExpression { pub struct HirMethodCallExpression { pub method: Ident, pub object: ExprId, + /// Method calls have an optional list of generics provided by the turbofish operator + pub generics: Option>, pub arguments: Vec, pub location: Location, } @@ -167,7 +169,7 @@ impl HirMethodCallExpression { method: HirMethodReference, location: Location, interner: &mut NodeInterner, - ) -> (ExprId, HirExpression) { + ) -> (ExprId, HirExpression, Option>) { let mut arguments = vec![self.object]; arguments.append(&mut self.arguments); @@ -181,7 +183,7 @@ impl HirMethodCallExpression { } }; let func = interner.push_expr(expr); - (func, HirExpression::Call(HirCallExpression { func, arguments, location })) + (func, HirExpression::Call(HirCallExpression { func, arguments, location }), self.generics) } } diff --git a/compiler/noirc_frontend/src/parser/parser.rs b/compiler/noirc_frontend/src/parser/parser.rs index fd385eef565..fc0f9daebd3 100644 --- a/compiler/noirc_frontend/src/parser/parser.rs +++ b/compiler/noirc_frontend/src/parser/parser.rs @@ -1010,7 +1010,7 @@ fn parse_type_inner<'a>( field_type(), int_type(), bool_type(), - string_type(recursive_type_parser.clone()), + string_type(), format_string_type(recursive_type_parser.clone()), named_type(recursive_type_parser.clone()), named_trait(recursive_type_parser.clone()), @@ -1056,12 +1056,10 @@ fn bool_type() -> impl NoirParser { .map_with_span(|_, span| UnresolvedTypeData::Bool.with_span(span)) } -fn string_type<'a>( - type_parser: impl NoirParser + 'a, - ) -> impl NoirParser + 'a { +fn string_type() -> impl NoirParser { keyword(Keyword::String) .ignore_then( - type_expression(type_parser).delimited_by(just(Token::Less), just(Token::Greater)).or_not(), + type_expression().delimited_by(just(Token::Less), just(Token::Greater)).or_not(), ) .map_with_span(|expr, span| UnresolvedTypeData::String(expr).with_span(span)) } @@ -1071,7 +1069,7 @@ fn format_string_type<'a>( ) -> impl NoirParser + 'a { keyword(Keyword::FormatString) .ignore_then( - type_expression(type_parser.clone()) + type_expression() .then_ignore(just(Token::Comma)) .then(type_parser) .delimited_by(just(Token::Less), just(Token::Greater)), @@ -1099,13 +1097,17 @@ fn int_type() -> impl NoirParser { }) } -fn named_type<'a>(type_parser: impl NoirParser + 'a) -> impl NoirParser + 'a { +fn named_type<'a>( + type_parser: impl NoirParser + 'a, +) -> impl NoirParser + 'a { path() .then(generic_type_args(type_parser)) .map_with_span(|(path, args), span| UnresolvedTypeData::Named(path, args).with_span(span)) } -fn named_trait<'a>(type_parser: impl NoirParser + 'a) -> impl NoirParser + 'a { +fn named_trait<'a>( + type_parser: impl NoirParser + 'a, +) -> impl NoirParser + 'a { keyword(Keyword::Impl).ignore_then(path()).then(generic_type_args(type_parser)).map_with_span( |(path, args), span| UnresolvedTypeData::TraitAsType(path, args).with_span(span), ) @@ -1114,13 +1116,14 @@ fn named_trait<'a>(type_parser: impl NoirParser + 'a) -> impl No fn generic_type_args<'a>( type_parser: impl NoirParser + 'a, ) -> impl NoirParser> + 'a { - type_parser.clone() + type_parser + .clone() // Without checking for a terminating ',' or '>' here we may incorrectly // parse a generic `N * 2` as just the type `N` then fail when there is no // separator afterward. Failing early here ensures we try the `type_expression` // parser afterward. .then_ignore(one_of([Token::Comma, Token::Greater]).rewind()) - .or(type_expression(type_parser) + .or(type_expression() .map_with_span(|expr, span| UnresolvedTypeData::Expression(expr).with_span(span))) .separated_by(just(Token::Comma)) .allow_trailing() @@ -1130,26 +1133,25 @@ fn generic_type_args<'a>( .map(Option::unwrap_or_default) } -fn array_type<'a>(type_parser: impl NoirParser + 'a) -> impl NoirParser + 'a { +fn array_type<'a>( + type_parser: impl NoirParser + 'a, +) -> impl NoirParser + 'a { just(Token::LeftBracket) .ignore_then(type_parser.clone()) - .then(just(Token::Semicolon).ignore_then(type_expression(type_parser)).or_not()) + .then(just(Token::Semicolon).ignore_then(type_expression()).or_not()) .then_ignore(just(Token::RightBracket)) .map_with_span(|(element_type, size), span| { UnresolvedTypeData::Array(size, Box::new(element_type)).with_span(span) }) } -fn type_expression<'a, T>(type_parser: T) -> impl NoirParser + 'a - where T: NoirParser + 'a -{ +fn type_expression() -> impl NoirParser { recursive(|expr| { expression_with_precedence( Precedence::lowest_type_precedence(), expr, nothing(), nothing(), - type_parser, true, false, ) @@ -1206,7 +1208,9 @@ where }) } -fn turbofish<'a>(type_parser: impl NoirParser + 'a) -> impl NoirParser>> + 'a { +fn turbofish<'a>( + type_parser: impl NoirParser + 'a, +) -> impl NoirParser>> + 'a { just(Token::DoubleColon).ignore_then(generic_type_args(type_parser)).or_not() } @@ -1217,7 +1221,6 @@ fn expression() -> impl ExprParser { expr.clone(), expression_no_constructors(expr.clone()), statement(expr.clone(), expression_no_constructors(expr)), - parse_type(), false, true, ) @@ -1235,7 +1238,6 @@ where expr_parser.clone(), expr_no_constructors.clone(), statement(expr_parser, expr_no_constructors), - parse_type(), false, false, ) @@ -1258,12 +1260,11 @@ where // An expression is a single term followed by 0 or more (OP subexpression)* // where OP is an operator at the given precedence level and subexpression // is an expression at the current precedence level plus one. -fn expression_with_precedence<'a, P, P2, S, T>( +fn expression_with_precedence<'a, P, P2, S>( precedence: Precedence, expr_parser: P, expr_no_constructors: P2, statement: S, - type_parser: T, // True if we should only parse the restricted subset of operators valid within type expressions is_type_expression: bool, // True if we should also parse constructors `Foo { field1: value1, ... }` as an expression. @@ -1275,11 +1276,10 @@ where P: ExprParser + 'a, P2: ExprParser + 'a, S: NoirParser + 'a, - T: NoirParser + 'a, { if precedence == Precedence::Highest { if is_type_expression { - type_expression_term(expr_parser, type_parser).boxed().labelled(ParsingRuleLabel::Term) + type_expression_term(expr_parser).boxed().labelled(ParsingRuleLabel::Term) } else { term(expr_parser, expr_no_constructors, statement, allow_constructors) .boxed() @@ -1294,7 +1294,6 @@ where expr_parser, expr_no_constructors, statement, - type_parser, is_type_expression, allow_constructors, ); @@ -1367,13 +1366,12 @@ where /// The equivalent of a 'term' for use in type expressions. Unlike regular terms, the grammar here /// is restricted to no longer include right-unary expressions, unary not, and most atoms. -fn type_expression_term<'a, P, P2>(expr_parser: P, type_parser: P2) -> impl NoirParser + 'a +fn type_expression_term<'a, P>(expr_parser: P) -> impl NoirParser + 'a where P: ExprParser + 'a, - P2: NoirParser + 'a, { recursive(move |term_parser| { - negation(term_parser).map_with_span(Expression::new).or(type_expression_atom(expr_parser, type_parser)) + negation(term_parser).map_with_span(Expression::new).or(type_expression_atom(expr_parser)) }) } @@ -1392,7 +1390,7 @@ where Call(Vec), ArrayIndex(Expression), Cast(UnresolvedType), - MemberAccess((Ident, Option>)), + MemberAccess((Ident, Option<(Option>, Vec)>)), } // `(arg1, ..., argN)` in `my_func(arg1, ..., argN)` @@ -1410,10 +1408,13 @@ where .map(UnaryRhs::Cast) .labelled(ParsingRuleLabel::Cast); + // A turbofish operator is optional in a method call to specify generic types + let turbofish = turbofish(parse_type()); + // `.foo` or `.foo(args)` in `atom.foo` or `atom.foo(args)` let member_rhs = just(Token::Dot) .ignore_then(field_name()) - .then(parenthesized(expression_list(expr_parser.clone())).or_not()) + .then(turbofish.then(parenthesized(expression_list(expr_parser.clone()))).or_not()) .map(UnaryRhs::MemberAccess) .labelled(ParsingRuleLabel::FieldAccess); @@ -1597,7 +1598,7 @@ where }, lambda(expr_parser.clone()), block(statement).map(ExpressionKind::Block), - variable(parse_type()), + variable(), literal(), )) .map_with_span(Expression::new) @@ -1610,12 +1611,11 @@ where /// Atoms within type expressions are limited to only variables, literals, and parenthesized /// type expressions. -fn type_expression_atom<'a, P, P2>(expr_parser: P, type_parser: P2) -> impl NoirParser + 'a +fn type_expression_atom<'a, P>(expr_parser: P) -> impl NoirParser + 'a where P: ExprParser + 'a, - P2: NoirParser + 'a, { - variable(type_parser) + variable_no_turbofish() .or(literal()) .map_with_span(Expression::new) .or(parenthesized(expr_parser)) @@ -1664,8 +1664,14 @@ where long_form.or(short_form) } -fn variable<'a>(type_parser: impl NoirParser + 'a) -> impl NoirParser + 'a { - path().then(turbofish(type_parser)).map(|(path, generics)| ExpressionKind::Variable(path, generics)) +fn variable() -> impl NoirParser { + path() + .then(turbofish(parse_type())) + .map(|(path, generics)| ExpressionKind::Variable(path, generics)) +} + +fn variable_no_turbofish() -> impl NoirParser { + path().map(|path| ExpressionKind::Variable(path, None)) } fn literal() -> impl NoirParser { @@ -1852,22 +1858,13 @@ mod test { #[test] fn parse_cast() { + let expression_nc = expression_no_constructors(expression()); parse_all( - atom_or_right_unary( - expression(), - expression_no_constructors(expression()), - fresh_statement(), - true, - ), + atom_or_right_unary(expression(), expression_nc.clone(), fresh_statement(), true), vec!["x as u8", "0 as Field", "(x + 3) as [Field; 8]"], ); parse_all_failing( - atom_or_right_unary( - expression(), - expression_no_constructors(expression()), - fresh_statement(), - true, - ), + atom_or_right_unary(expression(), expression_nc, fresh_statement(), true), vec!["x as pub u8"], ); } @@ -1929,7 +1926,7 @@ mod test { #[test] fn parse_type_expression() { - parse_all(type_expression(parse_type()), vec!["(123)", "123", "(1 + 1)", "(1 + (1))"]); + parse_all(type_expression(), vec!["(123)", "123", "(1 + 1)", "(1 + (1))"]); } #[test] @@ -2217,14 +2214,8 @@ mod test { #[test] fn parse_parenthesized_expression() { - parse_all( - atom(expression(), expression_no_constructors(expression()), fresh_statement(), true), - vec!["(0)", "(x+a)", "({(({{({(nested)})}}))})"], - ); - parse_all_failing( - atom(expression(), expression_no_constructors(expression()), fresh_statement(), true), - vec!["(x+a", "((x+a)", "(,)"], - ); + parse_all(expression(), vec!["(0)", "(x+a)", "({(({{({(nested)})}}))})"]); + parse_all_failing(expression(), vec!["(x+a", "((x+a)", "(,)"]); } #[test] @@ -2333,12 +2324,13 @@ mod test { #[test] fn parse_unary() { + let expression_nc = expression_no_constructors(expression()); parse_all( - term(expression(), expression_no_constructors(expression()), fresh_statement(), true), + term(expression(), expression_nc.clone(), fresh_statement(), true), vec!["!hello", "-hello", "--hello", "-!hello", "!-hello"], ); parse_all_failing( - term(expression(), expression_no_constructors(expression()), fresh_statement(), true), + term(expression(), expression_nc, fresh_statement(), true), vec!["+hello", "/hello"], ); } diff --git a/tooling/nargo_fmt/src/visitor/expr.rs b/tooling/nargo_fmt/src/visitor/expr.rs index 206d0ed6349..04989b4b0de 100644 --- a/tooling/nargo_fmt/src/visitor/expr.rs +++ b/tooling/nargo_fmt/src/visitor/expr.rs @@ -199,7 +199,9 @@ impl FmtVisitor<'_> { self.format_if(*if_expr) } - ExpressionKind::Lambda(_) | ExpressionKind::Variable(..) => self.slice(span).to_string(), + ExpressionKind::Lambda(_) | ExpressionKind::Variable(..) => { + self.slice(span).to_string() + } ExpressionKind::Error => unreachable!(), } } From 427cb9539461e84e6ac520d6ce680630aa3c930e Mon Sep 17 00:00:00 2001 From: Jake Fecher Date: Wed, 22 Nov 2023 12:40:27 -0600 Subject: [PATCH 03/55] Add test --- .../tests/execution_success/generics/src/main.nr | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/tooling/nargo_cli/tests/execution_success/generics/src/main.nr b/tooling/nargo_cli/tests/execution_success/generics/src/main.nr index 3edce1ed8e7..277395499e2 100644 --- a/tooling/nargo_cli/tests/execution_success/generics/src/main.nr +++ b/tooling/nargo_cli/tests/execution_success/generics/src/main.nr @@ -29,6 +29,11 @@ impl Bar { fn get_other(self) -> Field { self.other } + + // This is to test that we can use turbofish on methods as well + fn zeroed(_self: Self) -> A { + dep::std::unsafe::zeroed() + } } fn main(x: Field, y: Field) { @@ -51,6 +56,10 @@ fn main(x: Field, y: Field) { let nested_generics: Bar> = Bar { one, two, other: Bar { one, two, other: 0 } }; assert(nested_generics.other.other == bar1.get_other()); + // Test turbofish operator + foo::(bar1); + assert(bar1.zeroed::() == 0); + let _ = regression_2055([1, 2, 3]); } From 3ab58b811595cda55773b12e88599ba57512061f Mon Sep 17 00:00:00 2001 From: Jake Fecher Date: Wed, 22 Nov 2023 13:52:28 -0600 Subject: [PATCH 04/55] Add compiler error for incorrect generic count --- .../src/hir/def_collector/dc_mod.rs | 1 + .../src/hir/resolution/resolver.rs | 5 +- .../src/hir/type_check/errors.rs | 10 ++++ .../noirc_frontend/src/hir/type_check/expr.rs | 54 ++++++++++++++----- compiler/noirc_frontend/src/hir_def/types.rs | 2 + compiler/noirc_frontend/src/node_interner.rs | 4 ++ 6 files changed, 63 insertions(+), 13 deletions(-) diff --git a/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs b/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs index 17aa5e9951f..a51977afbaa 100644 --- a/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs +++ b/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs @@ -389,6 +389,7 @@ impl<'a> ModCollector<'a> { is_unconstrained: false, contract_function_type: None, is_internal: None, + generic_count: generics.len(), }; context.def_interner.push_function_definition(func_id, modifiers, id.0); diff --git a/compiler/noirc_frontend/src/hir/resolution/resolver.rs b/compiler/noirc_frontend/src/hir/resolution/resolver.rs index 347ad1cac29..c9bb4178a26 100644 --- a/compiler/noirc_frontend/src/hir/resolution/resolver.rs +++ b/compiler/noirc_frontend/src/hir/resolution/resolver.rs @@ -1254,7 +1254,6 @@ impl<'a> Resolver<'a> { let generics = generics.map(|generics| vecmap(generics, |typ| self.resolve_type(typ))); - HirExpression::Ident(hir_ident, generics) } } @@ -1291,9 +1290,13 @@ impl<'a> Resolver<'a> { ExpressionKind::MethodCall(call_expr) => { let method = call_expr.method_name; let object = self.resolve_expression(call_expr.object); + + // Cannot verify the generic count here equals the expected count since we don't + // know which definition `method` refers to until it is resolved during type checking. let generics = call_expr .generics .map(|generics| vecmap(generics, |typ| self.resolve_type(typ))); + let arguments = vecmap(call_expr.arguments, |arg| self.resolve_expression(arg)); let location = Location::new(expr.span, self.file); HirExpression::MethodCall(HirMethodCallExpression { diff --git a/compiler/noirc_frontend/src/hir/type_check/errors.rs b/compiler/noirc_frontend/src/hir/type_check/errors.rs index 267dbd6b5be..42a220133a6 100644 --- a/compiler/noirc_frontend/src/hir/type_check/errors.rs +++ b/compiler/noirc_frontend/src/hir/type_check/errors.rs @@ -115,6 +115,10 @@ pub enum TypeCheckError { NoMatchingImplFound { constraints: Vec<(Type, String)>, span: Span }, #[error("Constraint for `{typ}: {trait_name}` is not needed, another matching impl is already in scope")] UnneededTraitConstraint { trait_name: String, typ: Type, span: Span }, + #[error( + "Expected {expected_count} generic(s) from this function, but {actual_count} were provided" + )] + IncorrectTurbofishGenericCount { expected_count: usize, actual_count: usize, span: Span }, } impl TypeCheckError { @@ -275,6 +279,12 @@ impl From for Diagnostic { let msg = format!("Constraint for `{typ}: {trait_name}` is not needed, another matching impl is already in scope"); Diagnostic::simple_warning(msg, "Unnecessary trait constraint in where clause".into(), span) } + TypeCheckError::IncorrectTurbofishGenericCount { expected_count, actual_count, span } => { + let expected_plural = if expected_count == 1 { "" } else { "s" }; + let actual_plural = if actual_count == 1 { "was" } else { "were" }; + let msg = format!("Expected {expected_count} generic{expected_plural} from this function, but {actual_count} {actual_plural} provided"); + Diagnostic::simple_error(msg, "".into(), span) + }, } } } diff --git a/compiler/noirc_frontend/src/hir/type_check/expr.rs b/compiler/noirc_frontend/src/hir/type_check/expr.rs index 8b2face32c7..eed8f7cb198 100644 --- a/compiler/noirc_frontend/src/hir/type_check/expr.rs +++ b/compiler/noirc_frontend/src/hir/type_check/expr.rs @@ -11,7 +11,7 @@ use crate::{ types::Type, }, node_interner::{DefinitionKind, ExprId, FuncId, TraitId, TraitMethodId}, - BinaryOpKind, Signedness, TypeBinding, TypeVariableKind, UnaryOp, + BinaryOpKind, Signedness, TypeBinding, TypeBindings, TypeVariableKind, UnaryOp, }; use super::{errors::TypeCheckError, TypeChecker}; @@ -52,11 +52,18 @@ impl<'interner> TypeChecker<'interner> { // We must instantiate identifiers at every call site to replace this T with a new type // variable to handle generic functions. let t = self.interner.id_type_substitute_trait_as_type(ident.id); + let span = self.interner.expr_span(expr_id); + + let definition = self.interner.try_definition(ident.id); + let expected_generic_count = + definition.map_or(0, |definition| match &definition.kind { + DefinitionKind::Function(function) => { + self.interner.function_modifiers(function).generic_count + } + _ => 0, + }); - let (typ, bindings) = match generics { - Some(generics) => t.instantiate_with(generics), - None => t.instantiate(self.interner), - }; + let (typ, bindings) = self.instantiate(t, generics, expected_generic_count, span); // Push any trait constraints required by this definition to the context // to be checked later when the type of this variable is further constrained. @@ -537,7 +544,7 @@ impl<'interner> TypeChecker<'interner> { arguments: Vec<(Type, ExprId, Span)>, span: Span, ) -> Type { - let (fn_typ, param_len) = match method_ref { + let (fn_typ, param_len, expected_generic_count) = match method_ref { HirMethodReference::FuncId(func_id) => { if func_id == FuncId::dummy_id() { return Type::Error; @@ -545,12 +552,13 @@ impl<'interner> TypeChecker<'interner> { let func_meta = self.interner.function_meta(&func_id); let param_len = func_meta.parameters.len(); - (func_meta.typ, param_len) + let generic_count = self.interner.function_modifiers(&func_id).generic_count; + (func_meta.typ, param_len, generic_count) } HirMethodReference::TraitMethodId(method) => { let the_trait = self.interner.get_trait(method.trait_id); let method = &the_trait.methods[method.method_index]; - (method.get_type(), method.arguments.len()) + (method.get_type(), method.arguments.len(), method.generics.len()) } }; @@ -564,16 +572,38 @@ impl<'interner> TypeChecker<'interner> { }); } - let (function_type, instantiation_bindings) = match generics { - Some(generics) => fn_typ.instantiate_with(generics), - None => fn_typ.instantiate(self.interner), - }; + let (function_type, instantiation_bindings) = + self.instantiate(fn_typ, generics, expected_generic_count, span); self.interner.store_instantiation_bindings(*function_ident_id, instantiation_bindings); self.interner.push_expr_type(function_ident_id, function_type.clone()); self.bind_function_type(function_type, arguments, span) } + fn instantiate( + &mut self, + typ: Type, + generics: Option>, + expected_generic_count: usize, + span: Span, + ) -> (Type, TypeBindings) { + match generics { + Some(generics) => { + if generics.len() != expected_generic_count { + self.errors.push(TypeCheckError::IncorrectTurbofishGenericCount { + expected_count: expected_generic_count, + actual_count: generics.len(), + span, + }); + typ.instantiate(self.interner) + } else { + typ.instantiate_with(generics) + } + } + None => typ.instantiate(self.interner), + } + } + fn check_if_expr(&mut self, if_expr: &expr::HirIfExpression, expr_id: &ExprId) -> Type { let cond_type = self.check_expression(&if_expr.condition); let then_type = self.check_expression(&if_expr.consequence); diff --git a/compiler/noirc_frontend/src/hir_def/types.rs b/compiler/noirc_frontend/src/hir_def/types.rs index e698d52a227..fa85ca01c9b 100644 --- a/compiler/noirc_frontend/src/hir_def/types.rs +++ b/compiler/noirc_frontend/src/hir_def/types.rs @@ -1136,6 +1136,8 @@ impl Type { pub fn instantiate_with(&self, types: Vec) -> (Type, TypeBindings) { match self { Type::Forall(typevars, typ) => { + assert_eq!(types.len(), typevars.len(), "Turbofish operator used with incorrect generic count which was not caught by name resolution"); + let replacements = typevars .iter() .zip(types) diff --git a/compiler/noirc_frontend/src/node_interner.rs b/compiler/noirc_frontend/src/node_interner.rs index 300a95f819c..a6882618abb 100644 --- a/compiler/noirc_frontend/src/node_interner.rs +++ b/compiler/noirc_frontend/src/node_interner.rs @@ -176,6 +176,8 @@ pub struct FunctionModifiers { pub is_unconstrained: bool, + pub generic_count: usize, + /// This function's type in its contract. /// If this function is not in a contract, this is always 'Secret'. pub contract_function_type: Option, @@ -198,6 +200,7 @@ impl FunctionModifiers { is_unconstrained: false, is_internal: None, contract_function_type: None, + generic_count: 0, } } } @@ -684,6 +687,7 @@ impl NodeInterner { is_unconstrained: function.is_unconstrained, contract_function_type: Some(if function.is_open { Open } else { Secret }), is_internal: Some(function.is_internal), + generic_count: function.generics.len(), }; self.push_function_definition(id, modifiers, module) } From 8fc453ab5c0bc31bcc75093a111bd317c6339e82 Mon Sep 17 00:00:00 2001 From: Jake Fecher Date: Wed, 22 Nov 2023 13:54:42 -0600 Subject: [PATCH 05/55] Edit example to have a more problematic case --- .../nargo_cli/tests/execution_success/generics/src/main.nr | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/tooling/nargo_cli/tests/execution_success/generics/src/main.nr b/tooling/nargo_cli/tests/execution_success/generics/src/main.nr index 277395499e2..5998b4cbaae 100644 --- a/tooling/nargo_cli/tests/execution_success/generics/src/main.nr +++ b/tooling/nargo_cli/tests/execution_success/generics/src/main.nr @@ -29,7 +29,9 @@ impl Bar { fn get_other(self) -> Field { self.other } +} +impl Bar { // This is to test that we can use turbofish on methods as well fn zeroed(_self: Self) -> A { dep::std::unsafe::zeroed() @@ -58,6 +60,10 @@ fn main(x: Field, y: Field) { // Test turbofish operator foo::(bar1); + + // Test that turbofish works on methods and that it uses the generics on the methods + // While still handilng the generic on the impl (T in this case) that is implicitly added + // to the method. assert(bar1.zeroed::() == 0); let _ = regression_2055([1, 2, 3]); From b289b875708d9affe571eea1c5f6ca3ce7147271 Mon Sep 17 00:00:00 2001 From: Maxim Vezenov Date: Tue, 14 May 2024 04:10:50 +0000 Subject: [PATCH 06/55] fixup remaining fmt stuff --- aztec_macros/src/transforms/functions.rs | 2 +- aztec_macros/src/utils/ast_utils.rs | 7 ++++--- tooling/nargo_fmt/src/rewrite/expr.rs | 2 +- tooling/nargo_fmt/src/visitor/expr.rs | 20 +++++++++++++++----- 4 files changed, 21 insertions(+), 10 deletions(-) diff --git a/aztec_macros/src/transforms/functions.rs b/aztec_macros/src/transforms/functions.rs index 90563c6085c..39d709ef520 100644 --- a/aztec_macros/src/transforms/functions.rs +++ b/aztec_macros/src/transforms/functions.rs @@ -680,7 +680,7 @@ fn add_struct_to_hasher(identifier: &Ident, hasher_name: &str) -> Statement { fn str_to_bytes(identifier: &Ident) -> (Statement, Ident) { // let identifier_as_bytes = identifier.as_bytes(); let var = variable_ident(identifier.clone()); - let contents = if let ExpressionKind::Variable(p) = &var.kind { + let contents = if let ExpressionKind::Variable(p, _) = &var.kind { p.segments.first().cloned().unwrap_or_else(|| panic!("No segments")).0.contents } else { panic!("Unexpected identifier type") diff --git a/aztec_macros/src/utils/ast_utils.rs b/aztec_macros/src/utils/ast_utils.rs index ebb4854f86e..ba51090c2be 100644 --- a/aztec_macros/src/utils/ast_utils.rs +++ b/aztec_macros/src/utils/ast_utils.rs @@ -27,15 +27,15 @@ pub fn expression(kind: ExpressionKind) -> Expression { } pub fn variable(name: &str) -> Expression { - expression(ExpressionKind::Variable(ident_path(name))) + expression(ExpressionKind::Variable(ident_path(name), None)) } pub fn variable_ident(identifier: Ident) -> Expression { - expression(ExpressionKind::Variable(path(identifier))) + expression(ExpressionKind::Variable(path(identifier), None)) } pub fn variable_path(path: Path) -> Expression { - expression(ExpressionKind::Variable(path)) + expression(ExpressionKind::Variable(path, None)) } pub fn method_call( @@ -47,6 +47,7 @@ pub fn method_call( object, method_name: ident(method_name), arguments, + generics: None, }))) } diff --git a/tooling/nargo_fmt/src/rewrite/expr.rs b/tooling/nargo_fmt/src/rewrite/expr.rs index 6b7dca6c5c7..5a6edc9b4b0 100644 --- a/tooling/nargo_fmt/src/rewrite/expr.rs +++ b/tooling/nargo_fmt/src/rewrite/expr.rs @@ -157,7 +157,7 @@ pub(crate) fn rewrite( visitor.format_if(*if_expr) } - ExpressionKind::Lambda(_) | ExpressionKind::Variable(_) => visitor.slice(span).to_string(), + ExpressionKind::Lambda(_) | ExpressionKind::Variable(_, _) => visitor.slice(span).to_string(), ExpressionKind::Quote(block) => format!("quote {}", rewrite_block(visitor, block, span)), ExpressionKind::Comptime(block) => { format!("comptime {}", rewrite_block(visitor, block, span)) diff --git a/tooling/nargo_fmt/src/visitor/expr.rs b/tooling/nargo_fmt/src/visitor/expr.rs index 27502af99af..6be2d4d4bc4 100644 --- a/tooling/nargo_fmt/src/visitor/expr.rs +++ b/tooling/nargo_fmt/src/visitor/expr.rs @@ -1,10 +1,10 @@ -use noirc_frontend::ast::Expression; +use noirc_frontend::ast::{ArrayLiteral, Expression, Literal, UnaryOp}; use noirc_frontend::ast::{ BlockExpression, ConstructorExpression, ExpressionKind, IfExpression, Statement, StatementKind, }; use noirc_frontend::{hir::resolution::errors::Span, lexer::Lexer, token::Token}; -use super::{ExpressionType, FmtVisitor, Shape}; +use super::{ExpressionType, FmtVisitor, Indent, Shape}; use crate::{ items::{HasItem, Item, Items}, rewrite, @@ -66,9 +66,12 @@ impl FmtVisitor<'_> { let args = format_parens( self.config.fn_call_width.into(), self.fork(), + self.shape(), false, call_expr.arguments, args_span, + false, + NewlineMode::Normal, ); format!("{callee}{args}") @@ -84,9 +87,12 @@ impl FmtVisitor<'_> { let args = format_parens( self.config.fn_call_width.into(), self.fork(), + self.shape(), false, method_call_expr.arguments, args_span, + false, + NewlineMode::Normal, ); format!("{object}.{method}{args}") @@ -105,10 +111,10 @@ impl FmtVisitor<'_> { format!("{collection}{index}") } ExpressionKind::Tuple(exprs) => { - format_parens(None, self.fork(), exprs.len() == 1, exprs, span) + format_parens(None, self.fork(), self.shape(), exprs.len() == 1, exprs, span, false, NewlineMode::Normal) } ExpressionKind::Literal(literal) => match literal { - Literal::Integer(_) | Literal::Bool(_) | Literal::Str(_) | Literal::FmtStr(_) => { + Literal::Integer(_, _) | Literal::Bool(_) | Literal::Str(_) | Literal::FmtStr(_) => { self.slice(span).to_string() } Literal::Array(ArrayLiteral::Repeated { repeated_element, length }) => { @@ -121,6 +127,8 @@ impl FmtVisitor<'_> { rewrite::array(self.fork(), exprs, span, false) } Literal::Unit => "()".to_string(), + Literal::Slice(_) => todo!(), + Literal::RawStr(_, _) => todo!(), }, ExpressionKind::Parenthesized(mut sub_expr) => { let remove_nested_parens = self.config.remove_nested_parens; @@ -204,10 +212,12 @@ impl FmtVisitor<'_> { self.slice(span).to_string() } ExpressionKind::Error => unreachable!(), + ExpressionKind::Quote(_) => todo!(), + ExpressionKind::Comptime(_) => todo!(), } } - fn format_if(&self, if_expr: IfExpression) -> String { + pub(crate) fn format_if(&self, if_expr: IfExpression) -> String { let condition_str = rewrite::sub_expr(self, self.shape(), if_expr.condition); let consequence_str = rewrite::sub_expr(self, self.shape(), if_expr.consequence); From 6127bafa345bff4e86d831273fcc9c33f450a816 Mon Sep 17 00:00:00 2001 From: Maxim Vezenov Date: Tue, 14 May 2024 04:59:48 +0000 Subject: [PATCH 07/55] cleanup --- compiler/noirc_frontend/src/parser/parser.rs | 32 -------------------- 1 file changed, 32 deletions(-) diff --git a/compiler/noirc_frontend/src/parser/parser.rs b/compiler/noirc_frontend/src/parser/parser.rs index 1223a6dc6a2..584c4b0f418 100644 --- a/compiler/noirc_frontend/src/parser/parser.rs +++ b/compiler/noirc_frontend/src/parser/parser.rs @@ -1633,38 +1633,6 @@ mod test { parse_with(module_declaration(), "mod 1").unwrap_err(); } - #[test] - fn parse_path() { - let cases = vec![ - ("std", vec!["std"]), - ("std::hash", vec!["std", "hash"]), - ("std::hash::collections", vec!["std", "hash", "collections"]), - ("dep::foo::bar", vec!["foo", "bar"]), - ("crate::std::hash", vec!["std", "hash"]), - ]; - - for (src, expected_segments) in cases { - let path: Path = parse_with(path(), src).unwrap(); - for (segment, expected) in path.segments.into_iter().zip(expected_segments) { - assert_eq!(segment.0.contents, expected); - } - } - - parse_all_failing(path(), vec!["std::", "::std", "std::hash::", "foo::1"]); - } - - #[test] - fn parse_unary() { - parse_all( - term(expression(), expression_no_constructors(expression()), fresh_statement(), true), - vec!["!hello", "-hello", "--hello", "-!hello", "!-hello"], - ); - parse_all_failing( - term(expression(), expression_no_constructors(expression()), fresh_statement(), true), - vec!["+hello", "/hello"], - ); - } - #[test] fn parse_use() { let valid_use_statements = [ From 9ebb3b59affd179cad949dc023693b2fe1b35bde Mon Sep 17 00:00:00 2001 From: Maxim Vezenov Date: Tue, 14 May 2024 04:59:55 +0000 Subject: [PATCH 08/55] cargo fmt --- compiler/noirc_frontend/src/debug/mod.rs | 74 +++++++++++-------- .../src/elaborator/expressions.rs | 13 +++- .../noirc_frontend/src/elaborator/patterns.rs | 6 +- .../src/hir/comptime/hir_to_ast.rs | 9 ++- .../src/hir/resolution/resolver.rs | 15 ++-- .../noirc_frontend/src/hir/type_check/expr.rs | 20 +++-- compiler/noirc_frontend/src/hir_def/types.rs | 10 ++- compiler/noirc_frontend/src/node_interner.rs | 2 +- compiler/noirc_frontend/src/parser/parser.rs | 10 ++- tooling/nargo_fmt/src/rewrite/expr.rs | 4 +- tooling/nargo_fmt/src/visitor/expr.rs | 20 +++-- 11 files changed, 118 insertions(+), 65 deletions(-) diff --git a/compiler/noirc_frontend/src/debug/mod.rs b/compiler/noirc_frontend/src/debug/mod.rs index 7a6cbe00c16..c222e08e77a 100644 --- a/compiler/noirc_frontend/src/debug/mod.rs +++ b/compiler/noirc_frontend/src/debug/mod.rs @@ -171,11 +171,14 @@ impl DebugInstrumenter { let last_stmt = if has_ret_expr { ast::Statement { kind: ast::StatementKind::Expression(ast::Expression { - kind: ast::ExpressionKind::Variable(ast::Path { - segments: vec![ident("__debug_expr", span)], - kind: PathKind::Plain, - span, - }, None), + kind: ast::ExpressionKind::Variable( + ast::Path { + segments: vec![ident("__debug_expr", span)], + kind: PathKind::Plain, + span, + }, + None, + ), span, }), span, @@ -568,11 +571,14 @@ fn build_assign_var_stmt(var_id: SourceVarId, expr: ast::Expression) -> ast::Sta let span = expr.span; let kind = ast::ExpressionKind::Call(Box::new(ast::CallExpression { func: Box::new(ast::Expression { - kind: ast::ExpressionKind::Variable(ast::Path { - segments: vec![ident("__debug_var_assign", span)], - kind: PathKind::Plain, - span, - }, None), + kind: ast::ExpressionKind::Variable( + ast::Path { + segments: vec![ident("__debug_var_assign", span)], + kind: PathKind::Plain, + span, + }, + None, + ), span, }), arguments: vec![uint_expr(var_id.0 as u128, span), expr], @@ -583,11 +589,14 @@ fn build_assign_var_stmt(var_id: SourceVarId, expr: ast::Expression) -> ast::Sta fn build_drop_var_stmt(var_id: SourceVarId, span: Span) -> ast::Statement { let kind = ast::ExpressionKind::Call(Box::new(ast::CallExpression { func: Box::new(ast::Expression { - kind: ast::ExpressionKind::Variable(ast::Path { - segments: vec![ident("__debug_var_drop", span)], - kind: PathKind::Plain, - span, - }, None), + kind: ast::ExpressionKind::Variable( + ast::Path { + segments: vec![ident("__debug_var_drop", span)], + kind: PathKind::Plain, + span, + }, + None, + ), span, }), arguments: vec![uint_expr(var_id.0 as u128, span)], @@ -607,11 +616,14 @@ fn build_assign_member_stmt( let span = expr.span; let kind = ast::ExpressionKind::Call(Box::new(ast::CallExpression { func: Box::new(ast::Expression { - kind: ast::ExpressionKind::Variable(ast::Path { - segments: vec![ident(&format!["__debug_member_assign_{arity}"], span)], - kind: PathKind::Plain, - span, - }, None), + kind: ast::ExpressionKind::Variable( + ast::Path { + segments: vec![ident(&format!["__debug_member_assign_{arity}"], span)], + kind: PathKind::Plain, + span, + }, + None, + ), span, }), arguments: [ @@ -627,11 +639,14 @@ fn build_assign_member_stmt( fn build_debug_call_stmt(fname: &str, fn_id: DebugFnId, span: Span) -> ast::Statement { let kind = ast::ExpressionKind::Call(Box::new(ast::CallExpression { func: Box::new(ast::Expression { - kind: ast::ExpressionKind::Variable(ast::Path { - segments: vec![ident(&format!["__debug_fn_{fname}"], span)], - kind: PathKind::Plain, - span, - }, None), + kind: ast::ExpressionKind::Variable( + ast::Path { + segments: vec![ident(&format!["__debug_fn_{fname}"], span)], + kind: PathKind::Plain, + span, + }, + None, + ), span, }), arguments: vec![uint_expr(fn_id.0 as u128, span)], @@ -693,11 +708,10 @@ fn ident(s: &str, span: Span) -> ast::Ident { fn id_expr(id: &ast::Ident) -> ast::Expression { ast::Expression { - kind: ast::ExpressionKind::Variable(Path { - segments: vec![id.clone()], - kind: PathKind::Plain, - span: id.span(), - }, None), + kind: ast::ExpressionKind::Variable( + Path { segments: vec![id.clone()], kind: PathKind::Plain, span: id.span() }, + None, + ), span: id.span(), } } diff --git a/compiler/noirc_frontend/src/elaborator/expressions.rs b/compiler/noirc_frontend/src/elaborator/expressions.rs index 2f74985ffbb..75c95c06d09 100644 --- a/compiler/noirc_frontend/src/elaborator/expressions.rs +++ b/compiler/noirc_frontend/src/elaborator/expressions.rs @@ -49,8 +49,10 @@ impl<'context> Elaborator<'context> { ExpressionKind::Infix(infix) => return self.elaborate_infix(*infix, expr.span), ExpressionKind::If(if_) => self.elaborate_if(*if_), ExpressionKind::Variable(variable, generics) => { - let generics = generics.map(|option_inner| option_inner.into_iter().map(|generic| self.resolve_type(generic)).collect()); - return self.elaborate_variable(variable, generics) + let generics = generics.map(|option_inner| { + option_inner.into_iter().map(|generic| self.resolve_type(generic)).collect() + }); + return self.elaborate_variable(variable, generics); } ExpressionKind::Tuple(tuple) => self.elaborate_tuple(tuple), ExpressionKind::Lambda(lambda) => self.elaborate_lambda(*lambda), @@ -317,8 +319,11 @@ impl<'context> Elaborator<'context> { let location = Location::new(span, self.file); let method = method_call.method_name; - let generics = method_call.generics.map(|option_inner| option_inner.into_iter().map(|generic| self.resolve_type(generic)).collect()); - let method_call = HirMethodCallExpression { method, object, arguments, location, generics }; + let generics = method_call.generics.map(|option_inner| { + option_inner.into_iter().map(|generic| self.resolve_type(generic)).collect() + }); + let method_call = + HirMethodCallExpression { method, object, arguments, location, generics }; // Desugar the method call into a normal, resolved function call // so that the backend doesn't need to worry about methods diff --git a/compiler/noirc_frontend/src/elaborator/patterns.rs b/compiler/noirc_frontend/src/elaborator/patterns.rs index abfd07ea580..810e1b90743 100644 --- a/compiler/noirc_frontend/src/elaborator/patterns.rs +++ b/compiler/noirc_frontend/src/elaborator/patterns.rs @@ -323,7 +323,11 @@ impl<'context> Elaborator<'context> { } } - pub(super) fn elaborate_variable(&mut self, variable: Path, generics: Option>) -> (ExprId, Type) { + pub(super) fn elaborate_variable( + &mut self, + variable: Path, + generics: Option>, + ) -> (ExprId, Type) { let span = variable.span; let expr = self.resolve_variable(variable); let id = self.interner.push_expr(HirExpression::Ident(expr.clone(), generics)); diff --git a/compiler/noirc_frontend/src/hir/comptime/hir_to_ast.rs b/compiler/noirc_frontend/src/hir/comptime/hir_to_ast.rs index d04774f89ee..e0fdd91adb4 100644 --- a/compiler/noirc_frontend/src/hir/comptime/hir_to_ast.rs +++ b/compiler/noirc_frontend/src/hir/comptime/hir_to_ast.rs @@ -84,7 +84,10 @@ impl ExprId { let kind = match expression { HirExpression::Ident(ident, generics) => { let path = Path::from_ident(ident.to_ast(interner)); - ExpressionKind::Variable(path, generics.map(|option| option.iter().map(|generic| generic.to_ast()).collect())) + ExpressionKind::Variable( + path, + generics.map(|option| option.iter().map(|generic| generic.to_ast()).collect()), + ) } HirExpression::Literal(HirLiteral::Array(array)) => { let array = array.into_ast(interner, span); @@ -146,7 +149,9 @@ impl ExprId { object: method_call.object.to_ast(interner), method_name: method_call.method, arguments: vecmap(method_call.arguments, |arg| arg.to_ast(interner)), - generics: method_call.generics.map(|option| option.iter().map(|generic| generic.to_ast()).collect()), + generics: method_call + .generics + .map(|option| option.iter().map(|generic| generic.to_ast()).collect()), })) } HirExpression::Cast(cast) => { diff --git a/compiler/noirc_frontend/src/hir/resolution/resolver.rs b/compiler/noirc_frontend/src/hir/resolution/resolver.rs index 537579272fd..50937e1eee5 100644 --- a/compiler/noirc_frontend/src/hir/resolution/resolver.rs +++ b/compiler/noirc_frontend/src/hir/resolution/resolver.rs @@ -1480,11 +1480,14 @@ impl<'a> Resolver<'a> { generics.map(|generics| vecmap(generics, |typ| self.resolve_type(typ))); if let Some((method, constraint, assumed)) = self.resolve_trait_generic_path(&path) { - HirExpression::Ident(HirIdent { - location: Location::new(expr.span, self.file), - id: self.interner.trait_method_id(method), - impl_kind: ImplKind::TraitMethod(method, constraint, assumed), - }, generics) + HirExpression::Ident( + HirIdent { + location: Location::new(expr.span, self.file), + id: self.interner.trait_method_id(method), + impl_kind: ImplKind::TraitMethod(method, constraint, assumed), + }, + generics, + ) } else { // If the Path is being used as an Expression, then it is referring to a global from a separate module // Otherwise, then it is referring to an Identifier @@ -1522,7 +1525,7 @@ impl<'a> Resolver<'a> { } // let generics = - // generics.map(|generics| vecmap(generics, |typ| self.resolve_type(typ))); + // generics.map(|generics| vecmap(generics, |typ| self.resolve_type(typ))); HirExpression::Ident(hir_ident, generics) } } diff --git a/compiler/noirc_frontend/src/hir/type_check/expr.rs b/compiler/noirc_frontend/src/hir/type_check/expr.rs index 7309accd73f..a151ecfdcff 100644 --- a/compiler/noirc_frontend/src/hir/type_check/expr.rs +++ b/compiler/noirc_frontend/src/hir/type_check/expr.rs @@ -349,7 +349,12 @@ impl<'interner> TypeChecker<'interner> { } /// Returns the type of the given identifier - fn check_ident(&mut self, ident: HirIdent, expr_id: &ExprId, generics: Option>) -> Type { + fn check_ident( + &mut self, + ident: HirIdent, + expr_id: &ExprId, + generics: Option>, + ) -> Type { let mut bindings = TypeBindings::new(); // Add type bindings from any constraints that were used. @@ -376,13 +381,12 @@ impl<'interner> TypeChecker<'interner> { let span = self.interner.expr_span(expr_id); let definition = self.interner.try_definition(ident.id); - let expected_generic_count = - definition.map_or(0, |definition| match &definition.kind { - DefinitionKind::Function(function) => { - self.interner.function_modifiers(function).generic_count - } - _ => 0, - }); + let expected_generic_count = definition.map_or(0, |definition| match &definition.kind { + DefinitionKind::Function(function) => { + self.interner.function_modifiers(function).generic_count + } + _ => 0, + }); // This instantiates a trait's generics as well which need to be set // when the constraint below is later solved for when the function is // finished. How to link the two? diff --git a/compiler/noirc_frontend/src/hir_def/types.rs b/compiler/noirc_frontend/src/hir_def/types.rs index 12fb10bf47f..7687bcf711e 100644 --- a/compiler/noirc_frontend/src/hir_def/types.rs +++ b/compiler/noirc_frontend/src/hir_def/types.rs @@ -1485,7 +1485,11 @@ impl Type { /// is used and generic substitutions are provided manually by users. /// /// Expects the given type vector to be the same length as the Forall type variables. - pub fn instantiate_with(&self, types: Vec, interner: &NodeInterner) -> (Type, TypeBindings) { + pub fn instantiate_with( + &self, + types: Vec, + interner: &NodeInterner, + ) -> (Type, TypeBindings) { match self { Type::Forall(typevars, typ) => { assert_eq!(types.len(), typevars.len(), "Turbofish operator used with incorrect generic count which was not caught by name resolution"); @@ -1493,7 +1497,9 @@ impl Type { let replacements = typevars .iter() .zip(types) - .map(|(var, binding)| (interner.next_type_variable_id(), (var.clone(), binding))) + .map(|(var, binding)| { + (interner.next_type_variable_id(), (var.clone(), binding)) + }) .collect(); let instantiated = typ.substitute(&replacements); diff --git a/compiler/noirc_frontend/src/node_interner.rs b/compiler/noirc_frontend/src/node_interner.rs index b40c92df048..7f1b67abfbd 100644 --- a/compiler/noirc_frontend/src/node_interner.rs +++ b/compiler/noirc_frontend/src/node_interner.rs @@ -245,7 +245,7 @@ pub struct FunctionModifiers { pub is_unconstrained: bool, pub generic_count: usize, - + pub is_comptime: bool, } diff --git a/compiler/noirc_frontend/src/parser/parser.rs b/compiler/noirc_frontend/src/parser/parser.rs index 584c4b0f418..779836c29cf 100644 --- a/compiler/noirc_frontend/src/parser/parser.rs +++ b/compiler/noirc_frontend/src/parser/parser.rs @@ -783,10 +783,12 @@ fn int_type() -> impl NoirParser { }) } -fn named_type<'a>(type_parser: impl NoirParser + 'a ) -> impl NoirParser + 'a { - path() - .then(generic_type_args(type_parser)) - .map_with_span(|(path, args), span| UnresolvedTypeData::Named(path, args, false).with_span(span)) +fn named_type<'a>( + type_parser: impl NoirParser + 'a, +) -> impl NoirParser + 'a { + path().then(generic_type_args(type_parser)).map_with_span(|(path, args), span| { + UnresolvedTypeData::Named(path, args, false).with_span(span) + }) } fn named_trait<'a>( diff --git a/tooling/nargo_fmt/src/rewrite/expr.rs b/tooling/nargo_fmt/src/rewrite/expr.rs index 5a6edc9b4b0..a51151b7909 100644 --- a/tooling/nargo_fmt/src/rewrite/expr.rs +++ b/tooling/nargo_fmt/src/rewrite/expr.rs @@ -157,7 +157,9 @@ pub(crate) fn rewrite( visitor.format_if(*if_expr) } - ExpressionKind::Lambda(_) | ExpressionKind::Variable(_, _) => visitor.slice(span).to_string(), + ExpressionKind::Lambda(_) | ExpressionKind::Variable(_, _) => { + visitor.slice(span).to_string() + } ExpressionKind::Quote(block) => format!("quote {}", rewrite_block(visitor, block, span)), ExpressionKind::Comptime(block) => { format!("comptime {}", rewrite_block(visitor, block, span)) diff --git a/tooling/nargo_fmt/src/visitor/expr.rs b/tooling/nargo_fmt/src/visitor/expr.rs index 6be2d4d4bc4..cf0de83285a 100644 --- a/tooling/nargo_fmt/src/visitor/expr.rs +++ b/tooling/nargo_fmt/src/visitor/expr.rs @@ -110,13 +110,21 @@ impl FmtVisitor<'_> { format!("{collection}{index}") } - ExpressionKind::Tuple(exprs) => { - format_parens(None, self.fork(), self.shape(), exprs.len() == 1, exprs, span, false, NewlineMode::Normal) - } + ExpressionKind::Tuple(exprs) => format_parens( + None, + self.fork(), + self.shape(), + exprs.len() == 1, + exprs, + span, + false, + NewlineMode::Normal, + ), ExpressionKind::Literal(literal) => match literal { - Literal::Integer(_, _) | Literal::Bool(_) | Literal::Str(_) | Literal::FmtStr(_) => { - self.slice(span).to_string() - } + Literal::Integer(_, _) + | Literal::Bool(_) + | Literal::Str(_) + | Literal::FmtStr(_) => self.slice(span).to_string(), Literal::Array(ArrayLiteral::Repeated { repeated_element, length }) => { let repeated = self.format_sub_expr(*repeated_element); let length = self.format_sub_expr(*length); From c6525412b2589e331ac380e63e7587b268d15481 Mon Sep 17 00:00:00 2001 From: Maxim Vezenov Date: Tue, 14 May 2024 22:47:33 +0000 Subject: [PATCH 09/55] working initial turbofish tests --- compiler/noirc_frontend/src/elaborator/mod.rs | 2 +- .../noirc_frontend/src/hir/type_check/expr.rs | 43 +++++++++++++------ .../noirc_frontend/src/hir/type_check/mod.rs | 23 +++++++++- compiler/noirc_frontend/src/hir_def/types.rs | 2 +- .../execution_success/generics/src/main.nr | 2 +- 5 files changed, 55 insertions(+), 17 deletions(-) diff --git a/compiler/noirc_frontend/src/elaborator/mod.rs b/compiler/noirc_frontend/src/elaborator/mod.rs index 446e5b62ead..83b22148e9f 100644 --- a/compiler/noirc_frontend/src/elaborator/mod.rs +++ b/compiler/noirc_frontend/src/elaborator/mod.rs @@ -230,7 +230,7 @@ impl<'context> Elaborator<'context> { let cycle_errors = this.interner.check_for_dependency_cycles(); this.errors.extend(cycle_errors); - + dbg!(this.errors.len()); this.errors } diff --git a/compiler/noirc_frontend/src/hir/type_check/expr.rs b/compiler/noirc_frontend/src/hir/type_check/expr.rs index a151ecfdcff..a7c350d09ed 100644 --- a/compiler/noirc_frontend/src/hir/type_check/expr.rs +++ b/compiler/noirc_frontend/src/hir/type_check/expr.rs @@ -355,6 +355,25 @@ impl<'interner> TypeChecker<'interner> { expr_id: &ExprId, generics: Option>, ) -> Type { + // An identifiers type may be forall-quantified in the case of generic functions. + // E.g. `fn foo(t: T, field: Field) -> T` has type `forall T. fn(T, Field) -> T`. + // We must instantiate identifiers at every call site to replace this T with a new type + // variable to handle generic functions. + // let t = self.interner.id_type_substitute_trait_as_type(ident.id); + + let span = self.interner.expr_span(expr_id); + + let definition = self.interner.try_definition(ident.id); + let expected_generic_count = definition.map_or(0, |definition| match &definition.kind { + DefinitionKind::Function(function) => { + self.interner.function_modifiers(function).generic_count + } + _ => 0, + }); + + // dbg!(expected_generic_count); + // let (typ, mut bindings) = self.instantiate(t, generics, expected_generic_count, span); + let mut bindings = TypeBindings::new(); // Add type bindings from any constraints that were used. @@ -372,25 +391,20 @@ impl<'interner> TypeChecker<'interner> { } } } - + if generics.is_some() { + dbg!(generics.clone()); + dbg!(bindings.clone()); + } // An identifiers type may be forall-quantified in the case of generic functions. // E.g. `fn foo(t: T, field: Field) -> T` has type `forall T. fn(T, Field) -> T`. // We must instantiate identifiers at every call site to replace this T with a new type // variable to handle generic functions. let t = self.interner.id_type_substitute_trait_as_type(ident.id); - let span = self.interner.expr_span(expr_id); - let definition = self.interner.try_definition(ident.id); - let expected_generic_count = definition.map_or(0, |definition| match &definition.kind { - DefinitionKind::Function(function) => { - self.interner.function_modifiers(function).generic_count - } - _ => 0, - }); // This instantiates a trait's generics as well which need to be set // when the constraint below is later solved for when the function is // finished. How to link the two? - let (typ, bindings) = self.instantiate(t, generics, expected_generic_count, span); + let (typ, bindings) = self.instantiate(t, bindings, generics, expected_generic_count, span); // let (typ, bindings) = t.instantiate_with_bindings(bindings, self.interner); // Push any trait constraints required by this definition to the context @@ -429,6 +443,7 @@ impl<'interner> TypeChecker<'interner> { fn instantiate( &mut self, typ: Type, + bindings: TypeBindings, generics: Option>, expected_generic_count: usize, span: Span, @@ -436,17 +451,18 @@ impl<'interner> TypeChecker<'interner> { match generics { Some(generics) => { if generics.len() != expected_generic_count { + dbg!("got this"); self.errors.push(TypeCheckError::IncorrectTurbofishGenericCount { expected_count: expected_generic_count, actual_count: generics.len(), span, }); - typ.instantiate(self.interner) + typ.instantiate_with_bindings(bindings, self.interner) } else { typ.instantiate_with(generics, &self.interner) } } - None => typ.instantiate(self.interner), + None => typ.instantiate_with_bindings(bindings, self.interner), } } @@ -460,9 +476,12 @@ impl<'interner> TypeChecker<'interner> { ) { match self.interner.lookup_trait_implementation(object_type, trait_id, trait_generics) { Ok(impl_kind) => { + // dbg!(impl_kind.clone()); + // dbg!(self.errors.len()); self.interner.select_impl_for_expression(function_ident_id, impl_kind); } Err(erroring_constraints) => { + dbg!(erroring_constraints.len()); if erroring_constraints.is_empty() { self.errors.push(TypeCheckError::TypeAnnotationsNeeded { span }); } else { diff --git a/compiler/noirc_frontend/src/hir/type_check/mod.rs b/compiler/noirc_frontend/src/hir/type_check/mod.rs index 2e448858d9e..5518bcf2dec 100644 --- a/compiler/noirc_frontend/src/hir/type_check/mod.rs +++ b/compiler/noirc_frontend/src/hir/type_check/mod.rs @@ -79,6 +79,9 @@ pub fn type_check_func(interner: &mut NodeInterner, func_id: FuncId) -> Vec Vec Vec Vec(bar1); // Test that turbofish works on methods and that it uses the generics on the methods - // While still handilng the generic on the impl (T in this case) that is implicitly added + // While still handling the generic on the impl (T in this case) that is implicitly added // to the method. assert(bar1.zeroed::() == 0); From 9660360bc00f7b123d7cd06096226969cac2e512 Mon Sep 17 00:00:00 2001 From: Maxim Vezenov Date: Tue, 14 May 2024 22:47:43 +0000 Subject: [PATCH 10/55] cargo fmt --- .../noirc_frontend/src/hir/type_check/expr.rs | 33 ++++++------------- compiler/noirc_frontend/src/hir_def/types.rs | 4 +-- 2 files changed, 11 insertions(+), 26 deletions(-) diff --git a/compiler/noirc_frontend/src/hir/type_check/expr.rs b/compiler/noirc_frontend/src/hir/type_check/expr.rs index a7c350d09ed..79f033e8be8 100644 --- a/compiler/noirc_frontend/src/hir/type_check/expr.rs +++ b/compiler/noirc_frontend/src/hir/type_check/expr.rs @@ -355,25 +355,6 @@ impl<'interner> TypeChecker<'interner> { expr_id: &ExprId, generics: Option>, ) -> Type { - // An identifiers type may be forall-quantified in the case of generic functions. - // E.g. `fn foo(t: T, field: Field) -> T` has type `forall T. fn(T, Field) -> T`. - // We must instantiate identifiers at every call site to replace this T with a new type - // variable to handle generic functions. - // let t = self.interner.id_type_substitute_trait_as_type(ident.id); - - let span = self.interner.expr_span(expr_id); - - let definition = self.interner.try_definition(ident.id); - let expected_generic_count = definition.map_or(0, |definition| match &definition.kind { - DefinitionKind::Function(function) => { - self.interner.function_modifiers(function).generic_count - } - _ => 0, - }); - - // dbg!(expected_generic_count); - // let (typ, mut bindings) = self.instantiate(t, generics, expected_generic_count, span); - let mut bindings = TypeBindings::new(); // Add type bindings from any constraints that were used. @@ -391,16 +372,22 @@ impl<'interner> TypeChecker<'interner> { } } } - if generics.is_some() { - dbg!(generics.clone()); - dbg!(bindings.clone()); - } + // An identifiers type may be forall-quantified in the case of generic functions. // E.g. `fn foo(t: T, field: Field) -> T` has type `forall T. fn(T, Field) -> T`. // We must instantiate identifiers at every call site to replace this T with a new type // variable to handle generic functions. let t = self.interner.id_type_substitute_trait_as_type(ident.id); + let span = self.interner.expr_span(expr_id); + + let definition = self.interner.try_definition(ident.id); + let expected_generic_count = definition.map_or(0, |definition| match &definition.kind { + DefinitionKind::Function(function) => { + self.interner.function_modifiers(function).generic_count + } + _ => 0, + }); // This instantiates a trait's generics as well which need to be set // when the constraint below is later solved for when the function is // finished. How to link the two? diff --git a/compiler/noirc_frontend/src/hir_def/types.rs b/compiler/noirc_frontend/src/hir_def/types.rs index cf4f1d191a5..74365d0e9aa 100644 --- a/compiler/noirc_frontend/src/hir_def/types.rs +++ b/compiler/noirc_frontend/src/hir_def/types.rs @@ -1497,9 +1497,7 @@ impl Type { let replacements = typevars .iter() .zip(types) - .map(|(var, binding)| { - (var.id(), (var.clone(), binding)) - }) + .map(|(var, binding)| (var.id(), (var.clone(), binding))) .collect(); let instantiated = typ.substitute(&replacements); From 4cee817cd58555946108e31b924713b747e5240d Mon Sep 17 00:00:00 2001 From: Maxim Vezenov Date: Wed, 15 May 2024 02:11:53 +0000 Subject: [PATCH 11/55] nargo fmt --- test_programs/execution_success/generics/src/main.nr | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test_programs/execution_success/generics/src/main.nr b/test_programs/execution_success/generics/src/main.nr index c8616960559..6c87786166b 100644 --- a/test_programs/execution_success/generics/src/main.nr +++ b/test_programs/execution_success/generics/src/main.nr @@ -64,7 +64,7 @@ fn main(x: Field, y: Field) { // Test that turbofish works on methods and that it uses the generics on the methods // While still handling the generic on the impl (T in this case) that is implicitly added // to the method. - assert(bar1.zeroed::() == 0); + assert(bar1.zeroed() == 0); let _ = regression_2055([1, 2, 3]); } From e89cde2149ded885d86cd124e49fcf95c2ab6784 Mon Sep 17 00:00:00 2001 From: Maxim Vezenov Date: Wed, 15 May 2024 02:19:02 +0000 Subject: [PATCH 12/55] clippy --- compiler/noirc_frontend/src/ast/expression.rs | 4 +- compiler/noirc_frontend/src/ast/mod.rs | 4 + .../src/hir/resolution/resolver.rs | 2 +- .../noirc_frontend/src/hir/type_check/expr.rs | 2 +- compiler/noirc_frontend/src/hir_def/types.rs | 6 +- compiler/noirc_frontend/src/parser/parser.rs | 22 +- tooling/nargo_fmt/src/visitor/expr.rs | 209 +----------------- 7 files changed, 14 insertions(+), 235 deletions(-) diff --git a/compiler/noirc_frontend/src/ast/expression.rs b/compiler/noirc_frontend/src/ast/expression.rs index edd933a3a08..0173b17d28f 100644 --- a/compiler/noirc_frontend/src/ast/expression.rs +++ b/compiler/noirc_frontend/src/ast/expression.rs @@ -10,6 +10,8 @@ use acvm::FieldElement; use iter_extended::vecmap; use noirc_errors::{Span, Spanned}; +use super::UnaryRhsMemberAccess; + #[derive(Debug, PartialEq, Eq, Clone)] pub enum ExpressionKind { Literal(Literal), @@ -166,7 +168,7 @@ impl Expression { pub fn member_access_or_method_call( lhs: Expression, - (rhs, args): (Ident, Option<(Option>, Vec)>), + (rhs, args): UnaryRhsMemberAccess, span: Span, ) -> Expression { let kind = match args { diff --git a/compiler/noirc_frontend/src/ast/mod.rs b/compiler/noirc_frontend/src/ast/mod.rs index aa2f90dee4f..c3556dac6af 100644 --- a/compiler/noirc_frontend/src/ast/mod.rs +++ b/compiler/noirc_frontend/src/ast/mod.rs @@ -132,6 +132,10 @@ pub struct UnresolvedType { pub span: Option, } +/// Type wrapper for a member access +pub(crate) type UnaryRhsMemberAccess = + (Ident, Option<(Option>, Vec)>); + /// The precursor to TypeExpression, this is the type that the parser allows /// to be used in the length position of an array type. Only constants, variables, /// and numeric binary operators are allowed here. diff --git a/compiler/noirc_frontend/src/hir/resolution/resolver.rs b/compiler/noirc_frontend/src/hir/resolution/resolver.rs index 50937e1eee5..a23924e5c7b 100644 --- a/compiler/noirc_frontend/src/hir/resolution/resolver.rs +++ b/compiler/noirc_frontend/src/hir/resolution/resolver.rs @@ -2053,7 +2053,7 @@ impl<'a> Resolver<'a> { HirExpression::Literal(HirLiteral::Integer(int, false)) => { int.try_into_u128().ok_or(Some(ResolverError::IntegerTooLarge { span })) } - HirExpression::Ident(ident, generics) => { + HirExpression::Ident(ident, _) => { let definition = self.interner.definition(ident.id); match definition.kind { DefinitionKind::Global(global_id) => { diff --git a/compiler/noirc_frontend/src/hir/type_check/expr.rs b/compiler/noirc_frontend/src/hir/type_check/expr.rs index 79f033e8be8..b993bdcfdf6 100644 --- a/compiler/noirc_frontend/src/hir/type_check/expr.rs +++ b/compiler/noirc_frontend/src/hir/type_check/expr.rs @@ -446,7 +446,7 @@ impl<'interner> TypeChecker<'interner> { }); typ.instantiate_with_bindings(bindings, self.interner) } else { - typ.instantiate_with(generics, &self.interner) + typ.instantiate_with(generics) } } None => typ.instantiate_with_bindings(bindings, self.interner), diff --git a/compiler/noirc_frontend/src/hir_def/types.rs b/compiler/noirc_frontend/src/hir_def/types.rs index 74365d0e9aa..8d72c3b8d8b 100644 --- a/compiler/noirc_frontend/src/hir_def/types.rs +++ b/compiler/noirc_frontend/src/hir_def/types.rs @@ -1485,11 +1485,7 @@ impl Type { /// is used and generic substitutions are provided manually by users. /// /// Expects the given type vector to be the same length as the Forall type variables. - pub fn instantiate_with( - &self, - types: Vec, - interner: &NodeInterner, - ) -> (Type, TypeBindings) { + pub fn instantiate_with(&self, types: Vec) -> (Type, TypeBindings) { match self { Type::Forall(typevars, typ) => { assert_eq!(types.len(), typevars.len(), "Turbofish operator used with incorrect generic count which was not caught by name resolution"); diff --git a/compiler/noirc_frontend/src/parser/parser.rs b/compiler/noirc_frontend/src/parser/parser.rs index 779836c29cf..7eac73dc44f 100644 --- a/compiler/noirc_frontend/src/parser/parser.rs +++ b/compiler/noirc_frontend/src/parser/parser.rs @@ -35,7 +35,7 @@ use super::{spanned, Item, ItemKind}; use crate::ast::{ BinaryOp, BinaryOpKind, BlockExpression, ForLoopStatement, ForRange, Ident, IfExpression, InfixExpression, LValue, Literal, ModuleDeclaration, NoirTypeAlias, Param, Path, Pattern, - Recoverable, Statement, TraitBound, TypeImpl, UnresolvedTraitConstraint, + Recoverable, Statement, TraitBound, TypeImpl, UnaryRhsMemberAccess, UnresolvedTraitConstraint, UnresolvedTypeExpression, UseTree, UseTreeKind, Visibility, }; use crate::ast::{ @@ -1080,7 +1080,7 @@ where Call(Vec), ArrayIndex(Expression), Cast(UnresolvedType), - MemberAccess((Ident, Option<(Option>, Vec)>)), + MemberAccess(UnaryRhsMemberAccess), } // `(arg1, ..., argN)` in `my_func(arg1, ..., argN)` @@ -1355,24 +1355,6 @@ where long_form.or(short_form) } -fn literal_with_sign() -> impl NoirParser { - choice(( - literal(), - just(Token::Minus).then(literal()).map(|(_, exp)| match exp { - ExpressionKind::Literal(Literal::Integer(value, sign)) => { - ExpressionKind::Literal(Literal::Integer(value, !sign)) - } - _ => unreachable!(), - }), - )) -} - -fn literal_or_collection<'a>( - expr_parser: impl ExprParser + 'a, -) -> impl NoirParser + 'a { - choice((literal_with_sign(), constructor(expr_parser.clone()), array_expr(expr_parser))) -} - #[cfg(test)] mod test { use super::test_helpers::*; diff --git a/tooling/nargo_fmt/src/visitor/expr.rs b/tooling/nargo_fmt/src/visitor/expr.rs index cf0de83285a..18b962a7f85 100644 --- a/tooling/nargo_fmt/src/visitor/expr.rs +++ b/tooling/nargo_fmt/src/visitor/expr.rs @@ -1,10 +1,10 @@ -use noirc_frontend::ast::{ArrayLiteral, Expression, Literal, UnaryOp}; +use noirc_frontend::ast::Expression; use noirc_frontend::ast::{ BlockExpression, ConstructorExpression, ExpressionKind, IfExpression, Statement, StatementKind, }; use noirc_frontend::{hir::resolution::errors::Span, lexer::Lexer, token::Token}; -use super::{ExpressionType, FmtVisitor, Indent, Shape}; +use super::{ExpressionType, FmtVisitor, Shape}; use crate::{ items::{HasItem, Item, Items}, rewrite, @@ -20,211 +20,6 @@ impl FmtVisitor<'_> { self.last_position = span.end(); } - pub(crate) fn format_sub_expr(&self, expression: Expression) -> String { - self.format_expr(expression, ExpressionType::SubExpression) - } - - pub(crate) fn format_expr( - &self, - Expression { kind, mut span }: Expression, - expr_type: ExpressionType, - ) -> String { - match kind { - ExpressionKind::Block(block) => { - let mut visitor = self.fork(); - visitor.visit_block(block, span); - visitor.buffer - } - ExpressionKind::Prefix(prefix) => { - let op = match prefix.operator { - UnaryOp::Minus => "-", - UnaryOp::Not => "!", - UnaryOp::MutableReference => "&mut ", - UnaryOp::Dereference { implicitly_added } => { - if implicitly_added { - "" - } else { - "*" - } - } - }; - - format!("{op}{}", self.format_sub_expr(prefix.rhs)) - } - ExpressionKind::Cast(cast) => { - format!("{} as {}", self.format_sub_expr(cast.lhs), cast.r#type) - } - kind @ ExpressionKind::Infix(_) => { - let shape = self.shape(); - rewrite::infix(self.fork(), Expression { kind, span }, shape) - } - ExpressionKind::Call(call_expr) => { - let args_span = - self.span_before(call_expr.func.span.end()..span.end(), Token::LeftParen); - - let callee = self.format_sub_expr(*call_expr.func); - let args = format_parens( - self.config.fn_call_width.into(), - self.fork(), - self.shape(), - false, - call_expr.arguments, - args_span, - false, - NewlineMode::Normal, - ); - - format!("{callee}{args}") - } - ExpressionKind::MethodCall(method_call_expr) => { - let args_span = self.span_before( - method_call_expr.method_name.span().end()..span.end(), - Token::LeftParen, - ); - - let object = self.format_sub_expr(method_call_expr.object); - let method = method_call_expr.method_name.to_string(); - let args = format_parens( - self.config.fn_call_width.into(), - self.fork(), - self.shape(), - false, - method_call_expr.arguments, - args_span, - false, - NewlineMode::Normal, - ); - - format!("{object}.{method}{args}") - } - ExpressionKind::MemberAccess(member_access_expr) => { - let lhs_str = self.format_sub_expr(member_access_expr.lhs); - format!("{}.{}", lhs_str, member_access_expr.rhs) - } - ExpressionKind::Index(index_expr) => { - let index_span = self - .span_before(index_expr.collection.span.end()..span.end(), Token::LeftBracket); - - let collection = self.format_sub_expr(index_expr.collection); - let index = format_brackets(self.fork(), false, vec![index_expr.index], index_span); - - format!("{collection}{index}") - } - ExpressionKind::Tuple(exprs) => format_parens( - None, - self.fork(), - self.shape(), - exprs.len() == 1, - exprs, - span, - false, - NewlineMode::Normal, - ), - ExpressionKind::Literal(literal) => match literal { - Literal::Integer(_, _) - | Literal::Bool(_) - | Literal::Str(_) - | Literal::FmtStr(_) => self.slice(span).to_string(), - Literal::Array(ArrayLiteral::Repeated { repeated_element, length }) => { - let repeated = self.format_sub_expr(*repeated_element); - let length = self.format_sub_expr(*length); - - format!("[{repeated}; {length}]") - } - Literal::Array(ArrayLiteral::Standard(exprs)) => { - rewrite::array(self.fork(), exprs, span, false) - } - Literal::Unit => "()".to_string(), - Literal::Slice(_) => todo!(), - Literal::RawStr(_, _) => todo!(), - }, - ExpressionKind::Parenthesized(mut sub_expr) => { - let remove_nested_parens = self.config.remove_nested_parens; - - let mut leading; - let mut trailing; - - loop { - let leading_span = span.start() + 1..sub_expr.span.start(); - let trailing_span = sub_expr.span.end()..span.end() - 1; - - leading = self.format_comment(leading_span.into()); - trailing = self.format_comment(trailing_span.into()); - - if let ExpressionKind::Parenthesized(ref sub_sub_expr) = sub_expr.kind { - if remove_nested_parens && leading.is_empty() && trailing.is_empty() { - span = sub_expr.span; - sub_expr = sub_sub_expr.clone(); - continue; - } - } - - break; - } - - if !leading.contains("//") && !trailing.contains("//") { - let sub_expr = self.format_sub_expr(*sub_expr); - format!("({leading}{sub_expr}{trailing})") - } else { - let mut visitor = self.fork(); - - let indent = visitor.indent.to_string_with_newline(); - visitor.indent.block_indent(self.config); - let nested_indent = visitor.indent.to_string_with_newline(); - - let sub_expr = visitor.format_sub_expr(*sub_expr); - - let mut result = String::new(); - result.push('('); - - if !leading.is_empty() { - result.push_str(&nested_indent); - result.push_str(&leading); - } - - result.push_str(&nested_indent); - result.push_str(&sub_expr); - - if !trailing.is_empty() { - result.push_str(&nested_indent); - result.push_str(&trailing); - } - - result.push_str(&indent); - result.push(')'); - - result - } - } - ExpressionKind::Constructor(constructor) => { - let type_name = self.slice(span.start()..constructor.type_name.span().end()); - let fields_span = self - .span_before(constructor.type_name.span().end()..span.end(), Token::LeftBrace); - - self.format_struct_lit(type_name, fields_span, *constructor) - } - ExpressionKind::If(if_expr) => { - let allow_single_line = expr_type == ExpressionType::SubExpression; - - if allow_single_line { - let mut visitor = self.fork(); - visitor.indent = Indent::default(); - if let Some(line) = visitor.format_if_single_line(*if_expr.clone()) { - return line; - } - } - - self.format_if(*if_expr) - } - ExpressionKind::Lambda(_) | ExpressionKind::Variable(..) => { - self.slice(span).to_string() - } - ExpressionKind::Error => unreachable!(), - ExpressionKind::Quote(_) => todo!(), - ExpressionKind::Comptime(_) => todo!(), - } - } - pub(crate) fn format_if(&self, if_expr: IfExpression) -> String { let condition_str = rewrite::sub_expr(self, self.shape(), if_expr.condition); let consequence_str = rewrite::sub_expr(self, self.shape(), if_expr.consequence); From a2da705e0100f23db1c25840547606f6c6aa191c Mon Sep 17 00:00:00 2001 From: Maxim Vezenov Date: Wed, 15 May 2024 02:24:59 +0000 Subject: [PATCH 13/55] fmt --- compiler/noirc_frontend/src/hir/type_check/expr.rs | 5 ----- test_programs/execution_success/generics/src/main.nr | 2 +- 2 files changed, 1 insertion(+), 6 deletions(-) diff --git a/compiler/noirc_frontend/src/hir/type_check/expr.rs b/compiler/noirc_frontend/src/hir/type_check/expr.rs index b993bdcfdf6..25caa73a62d 100644 --- a/compiler/noirc_frontend/src/hir/type_check/expr.rs +++ b/compiler/noirc_frontend/src/hir/type_check/expr.rs @@ -392,7 +392,6 @@ impl<'interner> TypeChecker<'interner> { // when the constraint below is later solved for when the function is // finished. How to link the two? let (typ, bindings) = self.instantiate(t, bindings, generics, expected_generic_count, span); - // let (typ, bindings) = t.instantiate_with_bindings(bindings, self.interner); // Push any trait constraints required by this definition to the context // to be checked later when the type of this variable is further constrained. @@ -438,7 +437,6 @@ impl<'interner> TypeChecker<'interner> { match generics { Some(generics) => { if generics.len() != expected_generic_count { - dbg!("got this"); self.errors.push(TypeCheckError::IncorrectTurbofishGenericCount { expected_count: expected_generic_count, actual_count: generics.len(), @@ -463,12 +461,9 @@ impl<'interner> TypeChecker<'interner> { ) { match self.interner.lookup_trait_implementation(object_type, trait_id, trait_generics) { Ok(impl_kind) => { - // dbg!(impl_kind.clone()); - // dbg!(self.errors.len()); self.interner.select_impl_for_expression(function_ident_id, impl_kind); } Err(erroring_constraints) => { - dbg!(erroring_constraints.len()); if erroring_constraints.is_empty() { self.errors.push(TypeCheckError::TypeAnnotationsNeeded { span }); } else { diff --git a/test_programs/execution_success/generics/src/main.nr b/test_programs/execution_success/generics/src/main.nr index 6c87786166b..c8616960559 100644 --- a/test_programs/execution_success/generics/src/main.nr +++ b/test_programs/execution_success/generics/src/main.nr @@ -64,7 +64,7 @@ fn main(x: Field, y: Field) { // Test that turbofish works on methods and that it uses the generics on the methods // While still handling the generic on the impl (T in this case) that is implicitly added // to the method. - assert(bar1.zeroed() == 0); + assert(bar1.zeroed::() == 0); let _ = regression_2055([1, 2, 3]); } From da2302937fbf8596a655e883ff6f384ce870711c Mon Sep 17 00:00:00 2001 From: Maxim Vezenov Date: Wed, 15 May 2024 02:54:52 +0000 Subject: [PATCH 14/55] nargo fmt --- test_programs/execution_success/generics/src/main.nr | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test_programs/execution_success/generics/src/main.nr b/test_programs/execution_success/generics/src/main.nr index c8616960559..6c87786166b 100644 --- a/test_programs/execution_success/generics/src/main.nr +++ b/test_programs/execution_success/generics/src/main.nr @@ -64,7 +64,7 @@ fn main(x: Field, y: Field) { // Test that turbofish works on methods and that it uses the generics on the methods // While still handling the generic on the impl (T in this case) that is implicitly added // to the method. - assert(bar1.zeroed::() == 0); + assert(bar1.zeroed() == 0); let _ = regression_2055([1, 2, 3]); } From da26bc38744b5e2772b6b31998da214cd9a3891b Mon Sep 17 00:00:00 2001 From: Maxim Vezenov Date: Wed, 15 May 2024 17:58:15 +0000 Subject: [PATCH 15/55] fix nargo fmt for turbofish on method calls --- .../execution_success/generics/src/main.nr | 2 +- tooling/nargo_fmt/src/items.rs | 2 ++ tooling/nargo_fmt/src/rewrite/expr.rs | 19 ++++++++++++++++-- tooling/nargo_fmt/src/utils.rs | 18 ++++++++++++++++- tooling/nargo_fmt/src/visitor/expr.rs | 20 +++++++++++++++++-- 5 files changed, 55 insertions(+), 6 deletions(-) diff --git a/test_programs/execution_success/generics/src/main.nr b/test_programs/execution_success/generics/src/main.nr index 6c87786166b..c8616960559 100644 --- a/test_programs/execution_success/generics/src/main.nr +++ b/test_programs/execution_success/generics/src/main.nr @@ -64,7 +64,7 @@ fn main(x: Field, y: Field) { // Test that turbofish works on methods and that it uses the generics on the methods // While still handling the generic on the impl (T in this case) that is implicitly added // to the method. - assert(bar1.zeroed() == 0); + assert(bar1.zeroed::() == 0); let _ = regression_2055([1, 2, 3]); } diff --git a/tooling/nargo_fmt/src/items.rs b/tooling/nargo_fmt/src/items.rs index 7f998f45b59..8f1b9ac3fdc 100644 --- a/tooling/nargo_fmt/src/items.rs +++ b/tooling/nargo_fmt/src/items.rs @@ -38,6 +38,8 @@ impl<'me, T: HasItem> Items<'me, T> { span: Span, elements: Vec, ) -> Self { + dbg!(span.start()); + dbg!(span.end()); Self { visitor, shape, diff --git a/tooling/nargo_fmt/src/rewrite/expr.rs b/tooling/nargo_fmt/src/rewrite/expr.rs index a51151b7909..d427ddfbd28 100644 --- a/tooling/nargo_fmt/src/rewrite/expr.rs +++ b/tooling/nargo_fmt/src/rewrite/expr.rs @@ -3,8 +3,9 @@ use noirc_frontend::ast::{ }; use noirc_frontend::{macros_api::Span, token::Token}; +use crate::rewrite; use crate::visitor::{ - expr::{format_brackets, format_parens, NewlineMode}, + expr::{format_brackets, format_parens, format_turbofish, NewlineMode}, ExpressionType, FmtVisitor, Indent, Shape, }; @@ -83,7 +84,21 @@ pub(crate) fn rewrite( NewlineMode::IfContainsNewLineAndWidth, ); - format!("{object}.{method}{args}") + if let Some(generics) = method_call_expr.generics { + let mut turbofish = "".to_owned(); + for (i, generic) in generics.into_iter().enumerate() { + let generic = rewrite::typ(&visitor, shape, generic); + turbofish = if i == 0 { + format!("<{}", generic) + } else { + format!("{turbofish}, {}", generic) + }; + } + turbofish = format!("{turbofish}>"); + format!("{object}.{method}::{turbofish}{args}") + } else { + format!("{object}.{method}{args}") + } } ExpressionKind::MemberAccess(member_access_expr) => { let lhs_str = rewrite_sub_expr(visitor, shape, member_access_expr.lhs); diff --git a/tooling/nargo_fmt/src/utils.rs b/tooling/nargo_fmt/src/utils.rs index 2c5c3085e66..92040940c47 100644 --- a/tooling/nargo_fmt/src/utils.rs +++ b/tooling/nargo_fmt/src/utils.rs @@ -3,7 +3,7 @@ use std::borrow::Cow; use crate::items::HasItem; use crate::rewrite; use crate::visitor::{FmtVisitor, Shape}; -use noirc_frontend::ast::{Expression, Ident, Param, Visibility}; +use noirc_frontend::ast::{Expression, Ident, Param, UnresolvedType, Visibility}; use noirc_frontend::hir::resolution::errors::Span; use noirc_frontend::lexer::Lexer; use noirc_frontend::token::Token; @@ -160,6 +160,22 @@ impl HasItem for Param { } } +impl HasItem for UnresolvedType { + fn span(&self) -> Span { + self.span.unwrap_or(Span::default()) + } + + fn format(self, visitor: &FmtVisitor, shape: Shape) -> String { + dbg!("got here"); + if self.is_synthesized() { + self.to_string() + } else { + let typ = rewrite::typ(visitor, shape, self); + format!("{typ}") + } + } +} + impl HasItem for Ident { fn span(&self) -> Span { self.span() diff --git a/tooling/nargo_fmt/src/visitor/expr.rs b/tooling/nargo_fmt/src/visitor/expr.rs index 18b962a7f85..bd1674e892c 100644 --- a/tooling/nargo_fmt/src/visitor/expr.rs +++ b/tooling/nargo_fmt/src/visitor/expr.rs @@ -1,7 +1,7 @@ -use noirc_frontend::ast::Expression; use noirc_frontend::ast::{ BlockExpression, ConstructorExpression, ExpressionKind, IfExpression, Statement, StatementKind, }; +use noirc_frontend::ast::{Expression, UnresolvedType}; use noirc_frontend::{hir::resolution::errors::Span, lexer::Lexer, token::Token}; use super::{ExpressionType, FmtVisitor, Shape}; @@ -207,8 +207,10 @@ pub(crate) fn format_seq( nested_indent.indent.block_indent(visitor.config); let exprs: Vec<_> = Items::new(&visitor, nested_indent, span, exprs).collect(); + // dbg!(exprs.clone()); + // dbg!("abotu format exprs"); let exprs = format_exprs(visitor.config, tactic, trailing_comma, exprs, nested_indent, reduce); - + // dbg!("got here"); wrap_exprs(prefix, suffix, exprs, nested_indent, shape, mode) } @@ -249,6 +251,20 @@ pub(crate) fn format_parens( format_seq(shape, "(", ")", visitor, trailing_comma, exprs, span, tactic, mode, reduce) } +pub(crate) fn format_turbofish( + max_width: Option, + visitor: FmtVisitor, + shape: Shape, + trailing_comma: bool, + generics: Vec, + span: Span, + reduce: bool, + mode: NewlineMode, +) -> String { + let tactic = max_width.map(Tactic::LimitedHorizontalVertical).unwrap_or(Tactic::Horizontal); + format_seq(shape, "<", ">", visitor, trailing_comma, generics, span, tactic, mode, reduce) +} + pub(crate) fn format_exprs( config: &Config, tactic: Tactic, From 77dfc19dc539e1890b1341b9aba3ed7687fdceb3 Mon Sep 17 00:00:00 2001 From: Maxim Vezenov Date: Wed, 15 May 2024 18:37:47 +0000 Subject: [PATCH 16/55] clippy --- tooling/nargo_fmt/src/rewrite/expr.rs | 4 ++-- tooling/nargo_fmt/src/utils.rs | 18 +----------------- tooling/nargo_fmt/src/visitor/expr.rs | 16 +--------------- 3 files changed, 4 insertions(+), 34 deletions(-) diff --git a/tooling/nargo_fmt/src/rewrite/expr.rs b/tooling/nargo_fmt/src/rewrite/expr.rs index d427ddfbd28..0c339e57fb8 100644 --- a/tooling/nargo_fmt/src/rewrite/expr.rs +++ b/tooling/nargo_fmt/src/rewrite/expr.rs @@ -5,7 +5,7 @@ use noirc_frontend::{macros_api::Span, token::Token}; use crate::rewrite; use crate::visitor::{ - expr::{format_brackets, format_parens, format_turbofish, NewlineMode}, + expr::{format_brackets, format_parens, NewlineMode}, ExpressionType, FmtVisitor, Indent, Shape, }; @@ -87,7 +87,7 @@ pub(crate) fn rewrite( if let Some(generics) = method_call_expr.generics { let mut turbofish = "".to_owned(); for (i, generic) in generics.into_iter().enumerate() { - let generic = rewrite::typ(&visitor, shape, generic); + let generic = rewrite::typ(visitor, shape, generic); turbofish = if i == 0 { format!("<{}", generic) } else { diff --git a/tooling/nargo_fmt/src/utils.rs b/tooling/nargo_fmt/src/utils.rs index 92040940c47..2c5c3085e66 100644 --- a/tooling/nargo_fmt/src/utils.rs +++ b/tooling/nargo_fmt/src/utils.rs @@ -3,7 +3,7 @@ use std::borrow::Cow; use crate::items::HasItem; use crate::rewrite; use crate::visitor::{FmtVisitor, Shape}; -use noirc_frontend::ast::{Expression, Ident, Param, UnresolvedType, Visibility}; +use noirc_frontend::ast::{Expression, Ident, Param, Visibility}; use noirc_frontend::hir::resolution::errors::Span; use noirc_frontend::lexer::Lexer; use noirc_frontend::token::Token; @@ -160,22 +160,6 @@ impl HasItem for Param { } } -impl HasItem for UnresolvedType { - fn span(&self) -> Span { - self.span.unwrap_or(Span::default()) - } - - fn format(self, visitor: &FmtVisitor, shape: Shape) -> String { - dbg!("got here"); - if self.is_synthesized() { - self.to_string() - } else { - let typ = rewrite::typ(visitor, shape, self); - format!("{typ}") - } - } -} - impl HasItem for Ident { fn span(&self) -> Span { self.span() diff --git a/tooling/nargo_fmt/src/visitor/expr.rs b/tooling/nargo_fmt/src/visitor/expr.rs index bd1674e892c..f9fdc8b9882 100644 --- a/tooling/nargo_fmt/src/visitor/expr.rs +++ b/tooling/nargo_fmt/src/visitor/expr.rs @@ -1,7 +1,7 @@ use noirc_frontend::ast::{ BlockExpression, ConstructorExpression, ExpressionKind, IfExpression, Statement, StatementKind, }; -use noirc_frontend::ast::{Expression, UnresolvedType}; +use noirc_frontend::ast::Expression; use noirc_frontend::{hir::resolution::errors::Span, lexer::Lexer, token::Token}; use super::{ExpressionType, FmtVisitor, Shape}; @@ -251,20 +251,6 @@ pub(crate) fn format_parens( format_seq(shape, "(", ")", visitor, trailing_comma, exprs, span, tactic, mode, reduce) } -pub(crate) fn format_turbofish( - max_width: Option, - visitor: FmtVisitor, - shape: Shape, - trailing_comma: bool, - generics: Vec, - span: Span, - reduce: bool, - mode: NewlineMode, -) -> String { - let tactic = max_width.map(Tactic::LimitedHorizontalVertical).unwrap_or(Tactic::Horizontal); - format_seq(shape, "<", ">", visitor, trailing_comma, generics, span, tactic, mode, reduce) -} - pub(crate) fn format_exprs( config: &Config, tactic: Tactic, From 0e5a57958a36354a158bcba0b4c94f79b9a30f2c Mon Sep 17 00:00:00 2001 From: Maxim Vezenov Date: Wed, 15 May 2024 20:08:28 +0000 Subject: [PATCH 17/55] initial work to get calling trait methods working, decided this would be better as a separate PR --- .../noirc_frontend/src/hir/type_check/expr.rs | 73 +++++++++++++++---- .../noirc_frontend/src/hir/type_check/mod.rs | 5 +- compiler/noirc_frontend/src/hir_def/expr.rs | 2 +- compiler/noirc_frontend/src/hir_def/types.rs | 44 +++++------ .../execution_success/generics/src/main.nr | 54 +++++++++----- 5 files changed, 123 insertions(+), 55 deletions(-) diff --git a/compiler/noirc_frontend/src/hir/type_check/expr.rs b/compiler/noirc_frontend/src/hir/type_check/expr.rs index 25caa73a62d..0367efe6030 100644 --- a/compiler/noirc_frontend/src/hir/type_check/expr.rs +++ b/compiler/noirc_frontend/src/hir/type_check/expr.rs @@ -234,18 +234,50 @@ impl<'interner> TypeChecker<'interner> { // so that the backend doesn't need to worry about methods let location = method_call.location; + if method_call.generics.is_none() { + dbg!(method_ref.clone()); + } // Automatically add `&mut` if the method expects a mutable reference and // the object is not already one. - if let HirMethodReference::FuncId(func_id) = &method_ref { - if *func_id != FuncId::dummy_id() { - let function_type = - self.interner.function_meta(func_id).typ.clone(); - - self.try_add_mutable_reference_to_object( - &mut method_call, - &function_type, - &mut object_type, - ); + // if let HirMethodReference::FuncId(func_id) = &method_ref { + // if *func_id != FuncId::dummy_id() { + // let function_type = + // self.interner.function_meta(func_id).typ.clone(); + // self.try_add_mutable_reference_to_object( + // &mut method_call, + // &function_type, + // &mut object_type, + // ); + // } + // } + match &method_ref { + HirMethodReference::FuncId(func_id) => { + if *func_id != FuncId::dummy_id() { + let function_type = + self.interner.function_meta(func_id).typ.clone(); + self.try_add_mutable_reference_to_object( + &mut method_call, + &function_type, + &mut object_type, + ); + } + } + HirMethodReference::TraitMethodId(method_id, _) => { + let id = self.interner.trait_method_id(*method_id); + let definition = self.interner.definition(id); + match definition.kind { + DefinitionKind::Function(func_id) => { + let function_type = + self.interner.function_meta(&func_id).typ.clone(); + dbg!(function_type.clone()); + self.try_add_mutable_reference_to_object( + &mut method_call, + &function_type, + &mut object_type, + ); + } + _ => {}, + } } } @@ -357,6 +389,9 @@ impl<'interner> TypeChecker<'interner> { ) -> Type { let mut bindings = TypeBindings::new(); + if generics.is_some() { + dbg!(ident.impl_kind.clone()); + } // Add type bindings from any constraints that were used. // We need to do this first since otherwise instantiating the type below // will replace each trait generic with a fresh type variable, rather than @@ -378,7 +413,6 @@ impl<'interner> TypeChecker<'interner> { // We must instantiate identifiers at every call site to replace this T with a new type // variable to handle generic functions. let t = self.interner.id_type_substitute_trait_as_type(ident.id); - let span = self.interner.expr_span(expr_id); let definition = self.interner.try_definition(ident.id); @@ -388,23 +422,32 @@ impl<'interner> TypeChecker<'interner> { } _ => 0, }); + if generics.is_some() { + dbg!(t.clone()); + dbg!(expected_generic_count); + } // This instantiates a trait's generics as well which need to be set // when the constraint below is later solved for when the function is // finished. How to link the two? - let (typ, bindings) = self.instantiate(t, bindings, generics, expected_generic_count, span); + let (typ, bindings) = self.instantiate(t, bindings, generics.clone(), expected_generic_count, span); // Push any trait constraints required by this definition to the context // to be checked later when the type of this variable is further constrained. if let Some(definition) = self.interner.try_definition(ident.id) { if let DefinitionKind::Function(function) = definition.kind { let function = self.interner.function_meta(&function); - + if generics.is_some() { + dbg!(function.trait_constraints.clone()); + } for mut constraint in function.trait_constraints.clone() { constraint.apply_bindings(&bindings); self.trait_constraints.push((constraint, *expr_id)); } } } + if generics.is_some() { + dbg!(self.trait_constraints.len()); + } if let ImplKind::TraitMethod(_, mut constraint, assumed) = ident.impl_kind { constraint.apply_bindings(&bindings); @@ -444,6 +487,7 @@ impl<'interner> TypeChecker<'interner> { }); typ.instantiate_with_bindings(bindings, self.interner) } else { + dbg!(generics.clone()); typ.instantiate_with(generics) } } @@ -515,7 +559,8 @@ impl<'interner> TypeChecker<'interner> { }, typ => unreachable!("Unexpected type for function: {typ}"), }; - + dbg!(function_type.clone()); + dbg!(expected_object_type.clone()); if let Some(expected_object_type) = expected_object_type { let actual_type = object_type.follow_bindings(); diff --git a/compiler/noirc_frontend/src/hir/type_check/mod.rs b/compiler/noirc_frontend/src/hir/type_check/mod.rs index 5518bcf2dec..1abfc22b026 100644 --- a/compiler/noirc_frontend/src/hir/type_check/mod.rs +++ b/compiler/noirc_frontend/src/hir/type_check/mod.rs @@ -91,6 +91,7 @@ pub fn type_check_func(interner: &mut NodeInterner, func_id: FuncId) -> Vec Vec (Type, TypeBindings) { - match self { - Type::Forall(typevars, typ) => { - let replacements = typevars - .iter() - .map(|var| { - let new = interner.next_type_variable(); - (var.id(), (var.clone(), new)) - }) - .collect(); - - let instantiated = typ.force_substitute(&replacements); - (instantiated, replacements) - } - other => (other.clone(), HashMap::new()), - } - } - /// Instantiates a type with the given types. /// This differs from substitute in that only the quantified type variables /// are matched against the type list and are eligible for substitution - similar @@ -1486,6 +1465,7 @@ impl Type { /// /// Expects the given type vector to be the same length as the Forall type variables. pub fn instantiate_with(&self, types: Vec) -> (Type, TypeBindings) { + dbg!(self.clone()); match self { Type::Forall(typevars, typ) => { assert_eq!(types.len(), typevars.len(), "Turbofish operator used with incorrect generic count which was not caught by name resolution"); @@ -1503,6 +1483,28 @@ impl Type { } } + + /// Instantiate this type, replacing any type variables it is quantified + /// over with fresh type variables. If this type is not a Type::Forall, + /// it is unchanged. + pub fn instantiate(&self, interner: &NodeInterner) -> (Type, TypeBindings) { + match self { + Type::Forall(typevars, typ) => { + let replacements = typevars + .iter() + .map(|var| { + let new = interner.next_type_variable(); + (var.id(), (var.clone(), new)) + }) + .collect(); + + let instantiated = typ.force_substitute(&replacements); + (instantiated, replacements) + } + other => (other.clone(), HashMap::new()), + } + } + // /// Replace each NamedGeneric (and TypeVariable) in this type with a fresh type variable // pub(crate) fn instantiate_type_variables( // &self, diff --git a/test_programs/execution_success/generics/src/main.nr b/test_programs/execution_success/generics/src/main.nr index c8616960559..c204feb2c49 100644 --- a/test_programs/execution_success/generics/src/main.nr +++ b/test_programs/execution_success/generics/src/main.nr @@ -1,3 +1,6 @@ +use dep::std::hash::Hasher; +use dep::std::hash::poseidon2::Poseidon2Hasher; + struct Bar { one: Field, two: Field, @@ -40,23 +43,23 @@ impl Bar { fn main(x: Field, y: Field) { let bar1: Bar = Bar { one: x, two: y, other: 0 }; - let bar2 = Bar { one: x, two: y, other: [0] }; + // let bar2 = Bar { one: x, two: y, other: [0] }; - foo(bar1); - foo(bar2); - // Test generic impls - let int1 = BigInt { limbs: [1] }; - let int2 = BigInt { limbs: [2] }; - let BigInt { limbs } = int1.second(int2).first(int1); - assert(limbs == int2.limbs); - // Test impl exclusively for Bar - assert(bar1.get_other() == bar1.other); - // Expected type error - // assert(bar2.get_other() == bar2.other); - let one = x; - let two = y; - let nested_generics: Bar> = Bar { one, two, other: Bar { one, two, other: 0 } }; - assert(nested_generics.other.other == bar1.get_other()); + // foo(bar1); + // foo(bar2); + // // Test generic impls + // let int1 = BigInt { limbs: [1] }; + // let int2 = BigInt { limbs: [2] }; + // let BigInt { limbs } = int1.second(int2).first(int1); + // assert(limbs == int2.limbs); + // // Test impl exclusively for Bar + // assert(bar1.get_other() == bar1.other); + // // Expected type error + // // assert(bar2.get_other() == bar2.other); + // let one = x; + // let two = y; + // let nested_generics: Bar> = Bar { one, two, other: Bar { one, two, other: 0 } }; + // assert(nested_generics.other.other == bar1.get_other()); // Test turbofish operator foo::(bar1); @@ -64,9 +67,11 @@ fn main(x: Field, y: Field) { // Test that turbofish works on methods and that it uses the generics on the methods // While still handling the generic on the impl (T in this case) that is implicitly added // to the method. - assert(bar1.zeroed::() == 0); + // assert(bar1.zeroed::() == 0); + + println(method_with_hasher::([x, y])); - let _ = regression_2055([1, 2, 3]); + // let _ = regression_2055([1, 2, 3]); } fn regression_2055(bytes: [u8; LEN]) -> Field { @@ -80,3 +85,16 @@ fn regression_2055(bytes: [u8; LEN]) -> Field { } f } + +fn method_with_hasher(input: [Field; 2]) -> Field where H: Hasher + Default { + let mut hasher: H = H::default(); + H::write(&mut hasher, input[0]); + H::write(&mut hasher, input[1]); + hasher.write(input[0]); + hasher.finish() + // let mut hasher = &mut hasher; + // hasher.write(input[0]); + // hasher.write(input[1]); + // hasher.finish() + // input[0] +} From c17e834ffea48906ffc2b5c16703cda133c74d33 Mon Sep 17 00:00:00 2001 From: Maxim Vezenov Date: Thu, 16 May 2024 16:41:56 +0000 Subject: [PATCH 18/55] working turbofish with implicit generics now --- Cargo.lock | 1 + compiler/noirc_frontend/Cargo.toml | 1 + .../src/elaborator/expressions.rs | 2 +- .../src/hir/resolution/resolver.rs | 6 +- .../noirc_frontend/src/hir/type_check/expr.rs | 63 ++++++++++++++++--- .../noirc_frontend/src/hir/type_check/mod.rs | 7 +++ compiler/noirc_frontend/src/hir_def/expr.rs | 28 ++++++++- .../turbofish_generic_count/Nargo.toml | 7 +++ .../turbofish_generic_count/src/main.nr | 22 +++++++ .../execution_success/generics/src/main.nr | 36 +++++------ .../noir_test_success/turbofish/Nargo.toml | 7 +++ .../noir_test_success/turbofish/src/main.nr | 34 ++++++++++ 12 files changed, 181 insertions(+), 33 deletions(-) create mode 100644 test_programs/compile_failure/turbofish_generic_count/Nargo.toml create mode 100644 test_programs/compile_failure/turbofish_generic_count/src/main.nr create mode 100644 test_programs/noir_test_success/turbofish/Nargo.toml create mode 100644 test_programs/noir_test_success/turbofish/src/main.nr diff --git a/Cargo.lock b/Cargo.lock index a8c63c032aa..1befb7a6e86 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3170,6 +3170,7 @@ dependencies = [ "base64 0.21.2", "chumsky", "fm", + "fxhash", "im", "iter-extended", "lalrpop", diff --git a/compiler/noirc_frontend/Cargo.toml b/compiler/noirc_frontend/Cargo.toml index 0430d214d53..e1b67393064 100644 --- a/compiler/noirc_frontend/Cargo.toml +++ b/compiler/noirc_frontend/Cargo.toml @@ -14,6 +14,7 @@ noirc_arena.workspace = true noirc_errors.workspace = true noirc_printable_type.workspace = true fm.workspace = true +fxhash.workspace = true iter-extended.workspace = true chumsky.workspace = true thiserror.workspace = true diff --git a/compiler/noirc_frontend/src/elaborator/expressions.rs b/compiler/noirc_frontend/src/elaborator/expressions.rs index 75c95c06d09..cf453fc60e8 100644 --- a/compiler/noirc_frontend/src/elaborator/expressions.rs +++ b/compiler/noirc_frontend/src/elaborator/expressions.rs @@ -328,7 +328,7 @@ impl<'context> Elaborator<'context> { // Desugar the method call into a normal, resolved function call // so that the backend doesn't need to worry about methods // TODO: update object_type here? - let ((function_id, function_name), function_call) = method_call.into_function_call( + let ((function_id, function_name), function_call, _) = method_call.into_function_call( &method_ref, object_type, location, diff --git a/compiler/noirc_frontend/src/hir/resolution/resolver.rs b/compiler/noirc_frontend/src/hir/resolution/resolver.rs index a23924e5c7b..483ab3580fb 100644 --- a/compiler/noirc_frontend/src/hir/resolution/resolver.rs +++ b/compiler/noirc_frontend/src/hir/resolution/resolver.rs @@ -1478,8 +1478,12 @@ impl<'a> Resolver<'a> { ExpressionKind::Variable(path, generics) => { let generics = generics.map(|generics| vecmap(generics, |typ| self.resolve_type(typ))); + if let Some((method, constraint, assumed)) = self.resolve_trait_generic_path(&path) { + if generics.is_some() { + + } HirExpression::Ident( HirIdent { location: Location::new(expr.span, self.file), @@ -1524,8 +1528,6 @@ impl<'a> Resolver<'a> { } } - // let generics = - // generics.map(|generics| vecmap(generics, |typ| self.resolve_type(typ))); HirExpression::Ident(hir_ident, generics) } } diff --git a/compiler/noirc_frontend/src/hir/type_check/expr.rs b/compiler/noirc_frontend/src/hir/type_check/expr.rs index 25caa73a62d..daf7d021f2b 100644 --- a/compiler/noirc_frontend/src/hir/type_check/expr.rs +++ b/compiler/noirc_frontend/src/hir/type_check/expr.rs @@ -249,14 +249,36 @@ impl<'interner> TypeChecker<'interner> { } } + // Need to borrow `generics` here as `method_call` is moved when calling `into_function_call` + // We don't save the expected generic count just yet though as `into_function_call` also accounts + // for any implicit generics such as from a generic impl. + let has_turbofish_generics = method_call.generics.is_some(); // TODO: update object_type here? - let (_, function_call) = method_call.into_function_call( + let ((ident_expr_id, _), function_call, implicit_generics_count) = method_call.into_function_call( &method_ref, object_type, location, self.interner, ); + if has_turbofish_generics { + // dbg!(function_call.clone()); + // dbg!(expr_id); + // dbg!(ident_expr_id); + // let expr = self.interner.expression(&function_call.func); + // dbg!(expr.clone()); + // dbg!(implicit_generics_count); + self.method_call_implicit_generic_counts.insert(function_call.func, implicit_generics_count); + } + // let x = function_call. + + // if let HirExpression::Ident(func_var, generics) = self.interner.expression(&function_call.func) { + // // self.expected_generic_counts.insert(ident_expr_id, generics.un) + // } else { + // panic!("HirCallExpression func field must be an HirExpression::Ident"); + // } + // self.expected_generic_counts.insert(ident_expr_id, hir_ident.) + self.interner.replace_expr(expr_id, HirExpression::Call(function_call)); // Type check the new call now that it has been changed from a method call @@ -381,17 +403,35 @@ impl<'interner> TypeChecker<'interner> { let span = self.interner.expr_span(expr_id); + // if generics.is_some() { + // self.expected_generic_counts.get(expr_id).expect("Should have saved expected generic count earlier") + // } + + // let definition = self.interner.try_definition(ident.id); - let expected_generic_count = definition.map_or(0, |definition| match &definition.kind { - DefinitionKind::Function(function) => { - self.interner.function_modifiers(function).generic_count - } - _ => 0, + let function_generic_count = definition.map_or(0, |definition| match &definition.kind { + DefinitionKind::Function(function) => { + self.interner.function_modifiers(function).generic_count + } + _ => 0, }); + + let expected_generic_count = function_generic_count + self.method_call_implicit_generic_counts.get(expr_id).copied().unwrap_or_default(); + // let expected_generic_count = function_generic_count; + if generics.is_some() { + dbg!(function_generic_count); + dbg!(expr_id); + dbg!(expected_generic_count); + dbg!(bindings.len()); + dbg!(generics.clone()); + dbg!(ident.impl_kind.clone()); + dbg!(t.clone()); + } + // This instantiates a trait's generics as well which need to be set // when the constraint below is later solved for when the function is // finished. How to link the two? - let (typ, bindings) = self.instantiate(t, bindings, generics, expected_generic_count, span); + let (typ, bindings) = self.instantiate(t, bindings, generics, expected_generic_count, self.method_call_implicit_generic_counts.get(expr_id).copied().unwrap_or_default(), span); // Push any trait constraints required by this definition to the context // to be checked later when the type of this variable is further constrained. @@ -432,14 +472,19 @@ impl<'interner> TypeChecker<'interner> { bindings: TypeBindings, generics: Option>, expected_generic_count: usize, + implicit_generic_count: usize, span: Span, ) -> (Type, TypeBindings) { match generics { Some(generics) => { if generics.len() != expected_generic_count { + // The list of generics as well as the `expected_generic_count` represent + // the total number of generics on a function, including implicit generics. + // In a user facing error we want to display only the number of function generics though + // as this is what the user will actually be specifying. self.errors.push(TypeCheckError::IncorrectTurbofishGenericCount { - expected_count: expected_generic_count, - actual_count: generics.len(), + expected_count: expected_generic_count - implicit_generic_count, + actual_count: generics.len() - implicit_generic_count, span, }); typ.instantiate_with_bindings(bindings, self.interner) diff --git a/compiler/noirc_frontend/src/hir/type_check/mod.rs b/compiler/noirc_frontend/src/hir/type_check/mod.rs index 5518bcf2dec..cc7a82dc629 100644 --- a/compiler/noirc_frontend/src/hir/type_check/mod.rs +++ b/compiler/noirc_frontend/src/hir/type_check/mod.rs @@ -24,6 +24,7 @@ use crate::{ node_interner::{ExprId, FuncId, GlobalId, NodeInterner}, Type, TypeBindings, }; +use fxhash::FxHashMap as HashMap; pub use self::errors::Source; @@ -42,6 +43,10 @@ pub struct TypeChecker<'interner> { /// This map is used to default any integer type variables at the end of /// a function (before checking trait constraints) if a type wasn't already chosen. type_variables: Vec, + + /// Method calls can have implicit generics from generic impls. + /// These need to be tracked separately for accurate type checking. + method_call_implicit_generic_counts: HashMap, } /// Type checks a function and assigns the @@ -374,6 +379,7 @@ impl<'interner> TypeChecker<'interner> { trait_constraints: Vec::new(), type_variables: Vec::new(), current_function: None, + method_call_implicit_generic_counts: HashMap::default(), } } @@ -391,6 +397,7 @@ impl<'interner> TypeChecker<'interner> { trait_constraints: Vec::new(), type_variables: Vec::new(), current_function: None, + method_call_implicit_generic_counts: HashMap::default(), }; let statement = this.interner.get_global(id).let_statement; this.check_statement(&statement); diff --git a/compiler/noirc_frontend/src/hir_def/expr.rs b/compiler/noirc_frontend/src/hir_def/expr.rs index 51b48d2fc5c..bcfc8765c1c 100644 --- a/compiler/noirc_frontend/src/hir_def/expr.rs +++ b/compiler/noirc_frontend/src/hir_def/expr.rs @@ -212,7 +212,29 @@ impl HirMethodCallExpression { object_type: Type, location: Location, interner: &mut NodeInterner, - ) -> ((ExprId, HirIdent), HirCallExpression) { + ) -> ((ExprId, HirIdent), HirCallExpression, usize) { + // Attach any generics which may not be a part of the function call's explicit generics + // This includes types such as implicit generics if the function is part of an impl + let (generics, generics_count) = if let Some(mut function_generics) = self.generics.clone() { + let mut generics = Vec::new(); + match object_type.clone() { + Type::Struct(_, mut struct_generics) => { + dbg!("got struct"); + dbg!(struct_generics.clone()); + generics.append(&mut struct_generics); + } + _ => { + // TODO: Support other types that may contain generics such as `TraitAsType` + unreachable!("Only struct method calls are supported"); + } + }; + let implicit_generics_count = generics.len(); + generics.append(&mut function_generics); + (Some(generics), implicit_generics_count) + } else { + (None, 0) + }; + let mut arguments = vec![self.object]; arguments.append(&mut self.arguments); @@ -231,10 +253,10 @@ impl HirMethodCallExpression { } }; let func_var = HirIdent { location, id, impl_kind }; - let func = interner.push_expr(HirExpression::Ident(func_var.clone(), None)); + let func = interner.push_expr(HirExpression::Ident(func_var.clone(), generics)); interner.push_expr_location(func, location.span, location.file); let expr = HirCallExpression { func, arguments, location }; - ((func, func_var), expr) + ((func, func_var), expr, generics_count) } } diff --git a/test_programs/compile_failure/turbofish_generic_count/Nargo.toml b/test_programs/compile_failure/turbofish_generic_count/Nargo.toml new file mode 100644 index 00000000000..92be7dcb749 --- /dev/null +++ b/test_programs/compile_failure/turbofish_generic_count/Nargo.toml @@ -0,0 +1,7 @@ +[package] +name = "turbofish_generic_count" +type = "bin" +authors = [""] +compiler_version = ">=0.29.0" + +[dependencies] \ No newline at end of file diff --git a/test_programs/compile_failure/turbofish_generic_count/src/main.nr b/test_programs/compile_failure/turbofish_generic_count/src/main.nr new file mode 100644 index 00000000000..a360641fa15 --- /dev/null +++ b/test_programs/compile_failure/turbofish_generic_count/src/main.nr @@ -0,0 +1,22 @@ + +struct Bar { + one: Field, + two: Field, + other: T, +} + +impl Bar { + fn zeroed(_self: Self) -> A { + dep::std::unsafe::zeroed() + } +} + +fn foo(bar: Bar) { + assert(bar.one == bar.two); +} + +fn main(x: Field, y: pub Field) { + let bar1: Bar = Bar { one: x, two: y, other: 0 }; + + assert(bar1.zeroed::() == 0); +} diff --git a/test_programs/execution_success/generics/src/main.nr b/test_programs/execution_success/generics/src/main.nr index c8616960559..c47b68b159f 100644 --- a/test_programs/execution_success/generics/src/main.nr +++ b/test_programs/execution_success/generics/src/main.nr @@ -40,23 +40,23 @@ impl Bar { fn main(x: Field, y: Field) { let bar1: Bar = Bar { one: x, two: y, other: 0 }; - let bar2 = Bar { one: x, two: y, other: [0] }; + // let bar2 = Bar { one: x, two: y, other: [0] }; - foo(bar1); - foo(bar2); - // Test generic impls - let int1 = BigInt { limbs: [1] }; - let int2 = BigInt { limbs: [2] }; - let BigInt { limbs } = int1.second(int2).first(int1); - assert(limbs == int2.limbs); - // Test impl exclusively for Bar - assert(bar1.get_other() == bar1.other); - // Expected type error - // assert(bar2.get_other() == bar2.other); - let one = x; - let two = y; - let nested_generics: Bar> = Bar { one, two, other: Bar { one, two, other: 0 } }; - assert(nested_generics.other.other == bar1.get_other()); + // foo(bar1); + // foo(bar2); + // // Test generic impls + // let int1 = BigInt { limbs: [1] }; + // let int2 = BigInt { limbs: [2] }; + // let BigInt { limbs } = int1.second(int2).first(int1); + // assert(limbs == int2.limbs); + // // Test impl exclusively for Bar + // assert(bar1.get_other() == bar1.other); + // // Expected type error + // // assert(bar2.get_other() == bar2.other); + // let one = x; + // let two = y; + // let nested_generics: Bar> = Bar { one, two, other: Bar { one, two, other: 0 } }; + // assert(nested_generics.other.other == bar1.get_other()); // Test turbofish operator foo::(bar1); @@ -64,9 +64,9 @@ fn main(x: Field, y: Field) { // Test that turbofish works on methods and that it uses the generics on the methods // While still handling the generic on the impl (T in this case) that is implicitly added // to the method. - assert(bar1.zeroed::() == 0); + assert(bar1.zeroed::() == 0); - let _ = regression_2055([1, 2, 3]); + // let _ = regression_2055([1, 2, 3]); } fn regression_2055(bytes: [u8; LEN]) -> Field { diff --git a/test_programs/noir_test_success/turbofish/Nargo.toml b/test_programs/noir_test_success/turbofish/Nargo.toml new file mode 100644 index 00000000000..bdf1779cfb0 --- /dev/null +++ b/test_programs/noir_test_success/turbofish/Nargo.toml @@ -0,0 +1,7 @@ +[package] +name = "turbofish" +type = "bin" +authors = [""] +compiler_version = ">=0.29.0" + +[dependencies] \ No newline at end of file diff --git a/test_programs/noir_test_success/turbofish/src/main.nr b/test_programs/noir_test_success/turbofish/src/main.nr new file mode 100644 index 00000000000..62203091170 --- /dev/null +++ b/test_programs/noir_test_success/turbofish/src/main.nr @@ -0,0 +1,34 @@ + +struct Bar { + one: Field, + two: Field, + other: T, +} + +impl Bar { + // This is to test that we can use turbofish on methods as well + fn zeroed(_self: Self) -> A { + dep::std::unsafe::zeroed() + } +} + +fn foo(bar: Bar) { + assert(bar.one == bar.two); +} + +global bar1: Bar = Bar { one: 2, two: 2, other: 0 }; +global bar2: Bar = Bar { one: 2, two: 2, other: 10 }; + +#[test] +fn test_plain_function_call() { + // let bar1: Bar = Bar { one: 1, two: 2, other: 0 }; + + // Test turbofish operator + foo::(bar1); +} + +#[test] +fn test_method_call_with_implicit_generics() { + // assert(bar1.zeroed::() == bar2.other); + assert(bar1.zeroed::() == 0); +} \ No newline at end of file From 8f2be2ea6307f3687f105d4c6b4f4fb978da6825 Mon Sep 17 00:00:00 2001 From: Maxim Vezenov Date: Thu, 16 May 2024 17:01:26 +0000 Subject: [PATCH 19/55] separate out function and implicit generic counts --- .../src/elaborator/expressions.rs | 8 +-- .../src/hir/resolution/resolver.rs | 3 - .../noirc_frontend/src/hir/type_check/expr.rs | 71 +++++++------------ .../noirc_frontend/src/hir/type_check/mod.rs | 22 +----- compiler/noirc_frontend/src/hir_def/expr.rs | 5 +- .../execution_success/generics/src/main.nr | 36 +++++----- .../noir_test_success/turbofish/Nargo.toml | 7 -- .../noir_test_success/turbofish/src/main.nr | 34 --------- tooling/nargo_fmt/src/visitor/expr.rs | 2 +- 9 files changed, 48 insertions(+), 140 deletions(-) delete mode 100644 test_programs/noir_test_success/turbofish/Nargo.toml delete mode 100644 test_programs/noir_test_success/turbofish/src/main.nr diff --git a/compiler/noirc_frontend/src/elaborator/expressions.rs b/compiler/noirc_frontend/src/elaborator/expressions.rs index cf453fc60e8..c2721d85dc2 100644 --- a/compiler/noirc_frontend/src/elaborator/expressions.rs +++ b/compiler/noirc_frontend/src/elaborator/expressions.rs @@ -328,12 +328,8 @@ impl<'context> Elaborator<'context> { // Desugar the method call into a normal, resolved function call // so that the backend doesn't need to worry about methods // TODO: update object_type here? - let ((function_id, function_name), function_call, _) = method_call.into_function_call( - &method_ref, - object_type, - location, - self.interner, - ); + let ((function_id, function_name), function_call, _) = method_call + .into_function_call(&method_ref, object_type, location, self.interner); let func_type = self.type_check_variable(function_name, function_id); diff --git a/compiler/noirc_frontend/src/hir/resolution/resolver.rs b/compiler/noirc_frontend/src/hir/resolution/resolver.rs index 483ab3580fb..1f006697359 100644 --- a/compiler/noirc_frontend/src/hir/resolution/resolver.rs +++ b/compiler/noirc_frontend/src/hir/resolution/resolver.rs @@ -1481,9 +1481,6 @@ impl<'a> Resolver<'a> { if let Some((method, constraint, assumed)) = self.resolve_trait_generic_path(&path) { - if generics.is_some() { - - } HirExpression::Ident( HirIdent { location: Location::new(expr.span, self.file), diff --git a/compiler/noirc_frontend/src/hir/type_check/expr.rs b/compiler/noirc_frontend/src/hir/type_check/expr.rs index daf7d021f2b..3c334d4c9d4 100644 --- a/compiler/noirc_frontend/src/hir/type_check/expr.rs +++ b/compiler/noirc_frontend/src/hir/type_check/expr.rs @@ -249,35 +249,18 @@ impl<'interner> TypeChecker<'interner> { } } - // Need to borrow `generics` here as `method_call` is moved when calling `into_function_call` + // Need to borrow `generics` here as `method_call` is moved when calling `into_function_call` // We don't save the expected generic count just yet though as `into_function_call` also accounts // for any implicit generics such as from a generic impl. let has_turbofish_generics = method_call.generics.is_some(); // TODO: update object_type here? - let ((ident_expr_id, _), function_call, implicit_generics_count) = method_call.into_function_call( - &method_ref, - object_type, - location, - self.interner, - ); + let (_, function_call, implicit_generics_count) = method_call + .into_function_call(&method_ref, object_type, location, self.interner); if has_turbofish_generics { - // dbg!(function_call.clone()); - // dbg!(expr_id); - // dbg!(ident_expr_id); - // let expr = self.interner.expression(&function_call.func); - // dbg!(expr.clone()); - // dbg!(implicit_generics_count); - self.method_call_implicit_generic_counts.insert(function_call.func, implicit_generics_count); + self.method_call_implicit_generic_counts + .insert(function_call.func, implicit_generics_count); } - // let x = function_call. - - // if let HirExpression::Ident(func_var, generics) = self.interner.expression(&function_call.func) { - // // self.expected_generic_counts.insert(ident_expr_id, generics.un) - // } else { - // panic!("HirCallExpression func field must be an HirExpression::Ident"); - // } - // self.expected_generic_counts.insert(ident_expr_id, hir_ident.) self.interner.replace_expr(expr_id, HirExpression::Call(function_call)); @@ -403,35 +386,29 @@ impl<'interner> TypeChecker<'interner> { let span = self.interner.expr_span(expr_id); - // if generics.is_some() { - // self.expected_generic_counts.get(expr_id).expect("Should have saved expected generic count earlier") - // } - - // let definition = self.interner.try_definition(ident.id); let function_generic_count = definition.map_or(0, |definition| match &definition.kind { - DefinitionKind::Function(function) => { - self.interner.function_modifiers(function).generic_count - } - _ => 0, + DefinitionKind::Function(function) => { + self.interner.function_modifiers(function).generic_count + } + _ => 0, }); - - let expected_generic_count = function_generic_count + self.method_call_implicit_generic_counts.get(expr_id).copied().unwrap_or_default(); - // let expected_generic_count = function_generic_count; - if generics.is_some() { - dbg!(function_generic_count); - dbg!(expr_id); - dbg!(expected_generic_count); - dbg!(bindings.len()); - dbg!(generics.clone()); - dbg!(ident.impl_kind.clone()); - dbg!(t.clone()); - } + + let implicit_generic_count = + self.method_call_implicit_generic_counts.get(expr_id).copied().unwrap_or_default(); + // let expected_generic_count = function_generic_count + implicit_generic_count; // This instantiates a trait's generics as well which need to be set // when the constraint below is later solved for when the function is // finished. How to link the two? - let (typ, bindings) = self.instantiate(t, bindings, generics, expected_generic_count, self.method_call_implicit_generic_counts.get(expr_id).copied().unwrap_or_default(), span); + let (typ, bindings) = self.instantiate( + t, + bindings, + generics, + function_generic_count, + implicit_generic_count, + span, + ); // Push any trait constraints required by this definition to the context // to be checked later when the type of this variable is further constrained. @@ -477,13 +454,13 @@ impl<'interner> TypeChecker<'interner> { ) -> (Type, TypeBindings) { match generics { Some(generics) => { - if generics.len() != expected_generic_count { + if generics.len() != (expected_generic_count + implicit_generic_count) { // The list of generics as well as the `expected_generic_count` represent - // the total number of generics on a function, including implicit generics. + // the total number of generics on a function, including implicit generics. // In a user facing error we want to display only the number of function generics though // as this is what the user will actually be specifying. self.errors.push(TypeCheckError::IncorrectTurbofishGenericCount { - expected_count: expected_generic_count - implicit_generic_count, + expected_count: expected_generic_count, actual_count: generics.len() - implicit_generic_count, span, }); diff --git a/compiler/noirc_frontend/src/hir/type_check/mod.rs b/compiler/noirc_frontend/src/hir/type_check/mod.rs index cc7a82dc629..8a658cf82c7 100644 --- a/compiler/noirc_frontend/src/hir/type_check/mod.rs +++ b/compiler/noirc_frontend/src/hir/type_check/mod.rs @@ -44,7 +44,7 @@ pub struct TypeChecker<'interner> { /// a function (before checking trait constraints) if a type wasn't already chosen. type_variables: Vec, - /// Method calls can have implicit generics from generic impls. + /// Method calls can have implicit generics from generic impls. /// These need to be tracked separately for accurate type checking. method_call_implicit_generic_counts: HashMap, } @@ -84,9 +84,6 @@ pub fn type_check_func(interner: &mut NodeInterner, func_id: FuncId) -> Vec Vec Vec Vec ((ExprId, HirIdent), HirCallExpression, usize) { // Attach any generics which may not be a part of the function call's explicit generics // This includes types such as implicit generics if the function is part of an impl - let (generics, generics_count) = if let Some(mut function_generics) = self.generics.clone() { + let (generics, generics_count) = if let Some(mut function_generics) = self.generics.clone() + { let mut generics = Vec::new(); match object_type.clone() { Type::Struct(_, mut struct_generics) => { - dbg!("got struct"); - dbg!(struct_generics.clone()); generics.append(&mut struct_generics); } _ => { diff --git a/test_programs/execution_success/generics/src/main.nr b/test_programs/execution_success/generics/src/main.nr index c47b68b159f..c8616960559 100644 --- a/test_programs/execution_success/generics/src/main.nr +++ b/test_programs/execution_success/generics/src/main.nr @@ -40,23 +40,23 @@ impl Bar { fn main(x: Field, y: Field) { let bar1: Bar = Bar { one: x, two: y, other: 0 }; - // let bar2 = Bar { one: x, two: y, other: [0] }; + let bar2 = Bar { one: x, two: y, other: [0] }; - // foo(bar1); - // foo(bar2); - // // Test generic impls - // let int1 = BigInt { limbs: [1] }; - // let int2 = BigInt { limbs: [2] }; - // let BigInt { limbs } = int1.second(int2).first(int1); - // assert(limbs == int2.limbs); - // // Test impl exclusively for Bar - // assert(bar1.get_other() == bar1.other); - // // Expected type error - // // assert(bar2.get_other() == bar2.other); - // let one = x; - // let two = y; - // let nested_generics: Bar> = Bar { one, two, other: Bar { one, two, other: 0 } }; - // assert(nested_generics.other.other == bar1.get_other()); + foo(bar1); + foo(bar2); + // Test generic impls + let int1 = BigInt { limbs: [1] }; + let int2 = BigInt { limbs: [2] }; + let BigInt { limbs } = int1.second(int2).first(int1); + assert(limbs == int2.limbs); + // Test impl exclusively for Bar + assert(bar1.get_other() == bar1.other); + // Expected type error + // assert(bar2.get_other() == bar2.other); + let one = x; + let two = y; + let nested_generics: Bar> = Bar { one, two, other: Bar { one, two, other: 0 } }; + assert(nested_generics.other.other == bar1.get_other()); // Test turbofish operator foo::(bar1); @@ -64,9 +64,9 @@ fn main(x: Field, y: Field) { // Test that turbofish works on methods and that it uses the generics on the methods // While still handling the generic on the impl (T in this case) that is implicitly added // to the method. - assert(bar1.zeroed::() == 0); + assert(bar1.zeroed::() == 0); - // let _ = regression_2055([1, 2, 3]); + let _ = regression_2055([1, 2, 3]); } fn regression_2055(bytes: [u8; LEN]) -> Field { diff --git a/test_programs/noir_test_success/turbofish/Nargo.toml b/test_programs/noir_test_success/turbofish/Nargo.toml deleted file mode 100644 index bdf1779cfb0..00000000000 --- a/test_programs/noir_test_success/turbofish/Nargo.toml +++ /dev/null @@ -1,7 +0,0 @@ -[package] -name = "turbofish" -type = "bin" -authors = [""] -compiler_version = ">=0.29.0" - -[dependencies] \ No newline at end of file diff --git a/test_programs/noir_test_success/turbofish/src/main.nr b/test_programs/noir_test_success/turbofish/src/main.nr deleted file mode 100644 index 62203091170..00000000000 --- a/test_programs/noir_test_success/turbofish/src/main.nr +++ /dev/null @@ -1,34 +0,0 @@ - -struct Bar { - one: Field, - two: Field, - other: T, -} - -impl Bar { - // This is to test that we can use turbofish on methods as well - fn zeroed(_self: Self) -> A { - dep::std::unsafe::zeroed() - } -} - -fn foo(bar: Bar) { - assert(bar.one == bar.two); -} - -global bar1: Bar = Bar { one: 2, two: 2, other: 0 }; -global bar2: Bar = Bar { one: 2, two: 2, other: 10 }; - -#[test] -fn test_plain_function_call() { - // let bar1: Bar = Bar { one: 1, two: 2, other: 0 }; - - // Test turbofish operator - foo::(bar1); -} - -#[test] -fn test_method_call_with_implicit_generics() { - // assert(bar1.zeroed::() == bar2.other); - assert(bar1.zeroed::() == 0); -} \ No newline at end of file diff --git a/tooling/nargo_fmt/src/visitor/expr.rs b/tooling/nargo_fmt/src/visitor/expr.rs index f9fdc8b9882..1afbd234231 100644 --- a/tooling/nargo_fmt/src/visitor/expr.rs +++ b/tooling/nargo_fmt/src/visitor/expr.rs @@ -1,7 +1,7 @@ +use noirc_frontend::ast::Expression; use noirc_frontend::ast::{ BlockExpression, ConstructorExpression, ExpressionKind, IfExpression, Statement, StatementKind, }; -use noirc_frontend::ast::Expression; use noirc_frontend::{hir::resolution::errors::Span, lexer::Lexer, token::Token}; use super::{ExpressionType, FmtVisitor, Shape}; From 5ad8fb41e8f82d36b70d7ed7beb05017a81428db Mon Sep 17 00:00:00 2001 From: Maxim Vezenov Date: Thu, 16 May 2024 22:20:24 +0000 Subject: [PATCH 20/55] cleanup --- compiler/noirc_frontend/src/elaborator/mod.rs | 2 +- .../noirc_frontend/src/hir/type_check/expr.rs | 13 ++-- compiler/noirc_frontend/src/hir_def/types.rs | 76 ------------------- .../src/monomorphization/mod.rs | 6 +- compiler/noirc_frontend/src/parser/parser.rs | 18 ++--- tooling/nargo_fmt/src/items.rs | 2 - tooling/nargo_fmt/src/visitor/expr.rs | 4 +- 7 files changed, 20 insertions(+), 101 deletions(-) diff --git a/compiler/noirc_frontend/src/elaborator/mod.rs b/compiler/noirc_frontend/src/elaborator/mod.rs index 83b22148e9f..446e5b62ead 100644 --- a/compiler/noirc_frontend/src/elaborator/mod.rs +++ b/compiler/noirc_frontend/src/elaborator/mod.rs @@ -230,7 +230,7 @@ impl<'context> Elaborator<'context> { let cycle_errors = this.interner.check_for_dependency_cycles(); this.errors.extend(cycle_errors); - dbg!(this.errors.len()); + this.errors } diff --git a/compiler/noirc_frontend/src/hir/type_check/expr.rs b/compiler/noirc_frontend/src/hir/type_check/expr.rs index 3c334d4c9d4..a58805478a5 100644 --- a/compiler/noirc_frontend/src/hir/type_check/expr.rs +++ b/compiler/noirc_frontend/src/hir/type_check/expr.rs @@ -396,7 +396,6 @@ impl<'interner> TypeChecker<'interner> { let implicit_generic_count = self.method_call_implicit_generic_counts.get(expr_id).copied().unwrap_or_default(); - // let expected_generic_count = function_generic_count + implicit_generic_count; // This instantiates a trait's generics as well which need to be set // when the constraint below is later solved for when the function is @@ -448,19 +447,19 @@ impl<'interner> TypeChecker<'interner> { typ: Type, bindings: TypeBindings, generics: Option>, - expected_generic_count: usize, + function_generic_count: usize, implicit_generic_count: usize, span: Span, ) -> (Type, TypeBindings) { match generics { Some(generics) => { - if generics.len() != (expected_generic_count + implicit_generic_count) { - // The list of generics as well as the `expected_generic_count` represent - // the total number of generics on a function, including implicit generics. - // In a user facing error we want to display only the number of function generics though + if generics.len() != (function_generic_count + implicit_generic_count) { + // The list of `generics` represents the total number of generics on a function, including implicit generics. + // We calculate any implicit generics earlier when type checking a method call, + // In a user facing error we want to display only the number of function generics // as this is what the user will actually be specifying. self.errors.push(TypeCheckError::IncorrectTurbofishGenericCount { - expected_count: expected_generic_count, + expected_count: function_generic_count, actual_count: generics.len() - implicit_generic_count, span, }); diff --git a/compiler/noirc_frontend/src/hir_def/types.rs b/compiler/noirc_frontend/src/hir_def/types.rs index 8d72c3b8d8b..9f5e3a1ea1e 100644 --- a/compiler/noirc_frontend/src/hir_def/types.rs +++ b/compiler/noirc_frontend/src/hir_def/types.rs @@ -1503,82 +1503,6 @@ impl Type { } } - // /// Replace each NamedGeneric (and TypeVariable) in this type with a fresh type variable - // pub(crate) fn instantiate_type_variables( - // &self, - // interner: &NodeInterner, - // ) -> (Type, TypeBindings) { - // let mut type_variables = HashMap::new(); - // self.find_all_unbound_type_variables(&mut type_variables); - - // let substitutions = type_variables - // .into_iter() - // .map(|(id, type_var)| (id, (type_var, interner.next_type_variable()))) - // .collect(); - - // (self.substitute(&substitutions), substitutions) - // } - - // /// For each unbound type variable in the current type, add a type binding to the given list - // /// to bind the unbound type variable to a fresh type variable. - // fn find_all_unbound_type_variables( - // &self, - // type_variables: &mut HashMap, - // ) { - // match self { - // Type::FieldElement - // | Type::Integer(_, _) - // | Type::Bool - // | Type::Unit - // | Type::TraitAsType(_) - // | Type::Constant(_) - // | Type::NotConstant - // | Type::Error => (), - // Type::Array(length, elem) => { - // length.find_all_unbound_type_variables(type_variables); - // elem.find_all_unbound_type_variables(type_variables); - // } - // Type::String(length) => length.find_all_unbound_type_variables(type_variables), - // Type::FmtString(length, env) => { - // length.find_all_unbound_type_variables(type_variables); - // env.find_all_unbound_type_variables(type_variables); - // } - // Type::Struct(_, generics) => { - // for generic in generics { - // generic.find_all_unbound_type_variables(type_variables); - // } - // } - // Type::Tuple(fields) => { - // for field in fields { - // field.find_all_unbound_type_variables(type_variables); - // } - // } - // Type::Function(args, ret, env) => { - // for arg in args { - // arg.find_all_unbound_type_variables(type_variables); - // } - // ret.find_all_unbound_type_variables(type_variables); - // env.find_all_unbound_type_variables(type_variables); - // } - // Type::MutableReference(elem) => { - // elem.find_all_unbound_type_variables(type_variables); - // } - // Type::Forall(_, typ) => typ.find_all_unbound_type_variables(type_variables), - // Type::TypeVariable(type_variable, _) | Type::NamedGeneric(type_variable, _) => { - // match &*type_variable.0.borrow() { - // TypeBinding::Bound(binding) => { - // binding.find_all_unbound_type_variables(type_variables); - // } - // TypeBinding::Unbound(id) => { - // if !type_variables.contains_key(id) { - // type_variables.insert(*id, type_variable.clone()); - // } - // } - // } - // } - // } - // } - /// Substitute any type variables found within this type with the /// given bindings if found. If a type variable is not found within /// the given TypeBindings, it is unchanged. diff --git a/compiler/noirc_frontend/src/monomorphization/mod.rs b/compiler/noirc_frontend/src/monomorphization/mod.rs index d255342b2c7..9a20d0dd537 100644 --- a/compiler/noirc_frontend/src/monomorphization/mod.rs +++ b/compiler/noirc_frontend/src/monomorphization/mod.rs @@ -1181,8 +1181,8 @@ impl<'interner> Monomorphizer<'interner> { Type::FmtString(_, elements) => { match *elements { Type::Tuple(element_types) => { - for element in element_types { - Self::append_printable_type_info_inner(&element, arguments); + for typ in element_types { + Self::append_printable_type_info_inner(&typ, arguments); } } _ => unreachable!( @@ -1191,7 +1191,7 @@ impl<'interner> Monomorphizer<'interner> { } true } - typ => { + _ => { Self::append_printable_type_info_inner(&typ, arguments); false } diff --git a/compiler/noirc_frontend/src/parser/parser.rs b/compiler/noirc_frontend/src/parser/parser.rs index 7eac73dc44f..c6d10e42de3 100644 --- a/compiler/noirc_frontend/src/parser/parser.rs +++ b/compiler/noirc_frontend/src/parser/parser.rs @@ -905,12 +905,6 @@ where }) } -// fn turbofish<'a>( -// type_parser: impl NoirParser + 'a, -// ) -> impl NoirParser>> + 'a { -// just(Token::DoubleColon).ignore_then(generic_type_args(type_parser)).or_not() -// } - fn expression() -> impl ExprParser { recursive(|expr| { expression_with_precedence( @@ -1391,7 +1385,7 @@ mod test { fresh_statement(), true, ), - vec!["x as u8", "0 as Field", "(x + 3) as [Field; 8]"], + vec!["x as u8", "x as u16", "0 as Field", "(x + 3) as [Field; 8]"], ); parse_all_failing( atom_or_right_unary(expression(), expression_nc, fresh_statement(), true), @@ -1589,8 +1583,14 @@ mod test { #[test] fn parse_parenthesized_expression() { - parse_all(expression(), vec!["(0)", "(x+a)", "({(({{({(nested)})}}))})"]); - parse_all_failing(expression(), vec!["(x+a", "((x+a)", "(,)"]); + parse_all( + atom(expression(), expression_no_constructors(expression()), fresh_statement(), true), + vec!["(0)", "(x+a)", "({(({{({(nested)})}}))})"], + ); + parse_all_failing( + atom(expression(), expression_no_constructors(expression()), fresh_statement(), true), + vec!["(x+a", "((x+a)", "(,)"], + ); } #[test] diff --git a/tooling/nargo_fmt/src/items.rs b/tooling/nargo_fmt/src/items.rs index 8f1b9ac3fdc..7f998f45b59 100644 --- a/tooling/nargo_fmt/src/items.rs +++ b/tooling/nargo_fmt/src/items.rs @@ -38,8 +38,6 @@ impl<'me, T: HasItem> Items<'me, T> { span: Span, elements: Vec, ) -> Self { - dbg!(span.start()); - dbg!(span.end()); Self { visitor, shape, diff --git a/tooling/nargo_fmt/src/visitor/expr.rs b/tooling/nargo_fmt/src/visitor/expr.rs index 1afbd234231..18b962a7f85 100644 --- a/tooling/nargo_fmt/src/visitor/expr.rs +++ b/tooling/nargo_fmt/src/visitor/expr.rs @@ -207,10 +207,8 @@ pub(crate) fn format_seq( nested_indent.indent.block_indent(visitor.config); let exprs: Vec<_> = Items::new(&visitor, nested_indent, span, exprs).collect(); - // dbg!(exprs.clone()); - // dbg!("abotu format exprs"); let exprs = format_exprs(visitor.config, tactic, trailing_comma, exprs, nested_indent, reduce); - // dbg!("got here"); + wrap_exprs(prefix, suffix, exprs, nested_indent, shape, mode) } From cc135e752e5f47b1db69acf0992140ac7d95e506 Mon Sep 17 00:00:00 2001 From: Maxim Vezenov Date: Thu, 16 May 2024 22:28:50 +0000 Subject: [PATCH 21/55] cargo fmt --- compiler/noirc_frontend/src/hir/type_check/expr.rs | 8 ++++---- compiler/noirc_frontend/src/hir/type_check/mod.rs | 3 ++- compiler/noirc_frontend/src/hir_def/types.rs | 1 - 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/compiler/noirc_frontend/src/hir/type_check/expr.rs b/compiler/noirc_frontend/src/hir/type_check/expr.rs index c286b6a575d..29a868a6cb1 100644 --- a/compiler/noirc_frontend/src/hir/type_check/expr.rs +++ b/compiler/noirc_frontend/src/hir/type_check/expr.rs @@ -235,9 +235,9 @@ impl<'interner> TypeChecker<'interner> { let location = method_call.location; // if method_call.generics.is_none() { - // dbg!(method_ref.clone()); + // dbg!(method_ref.clone()); // } - + // Automatically add `&mut` if the method expects a mutable reference and // the object is not already one. // if let HirMethodReference::FuncId(func_id) = &method_ref { @@ -270,7 +270,7 @@ impl<'interner> TypeChecker<'interner> { match definition.kind { DefinitionKind::Function(func_id) => { let function_type = - self.interner.function_meta(&func_id).typ.clone(); + self.interner.function_meta(&func_id).typ.clone(); // dbg!(function_type.clone()); self.try_add_mutable_reference_to_object( &mut method_call, @@ -278,7 +278,7 @@ impl<'interner> TypeChecker<'interner> { &mut object_type, ); } - _ => {}, + _ => {} } } } diff --git a/compiler/noirc_frontend/src/hir/type_check/mod.rs b/compiler/noirc_frontend/src/hir/type_check/mod.rs index d40d8999ed4..789fb3bea84 100644 --- a/compiler/noirc_frontend/src/hir/type_check/mod.rs +++ b/compiler/noirc_frontend/src/hir/type_check/mod.rs @@ -152,7 +152,8 @@ pub fn type_check_func(interner: &mut NodeInterner, func_id: FuncId) -> Vec Date: Thu, 16 May 2024 22:38:40 +0000 Subject: [PATCH 22/55] cleanup --- .../noirc_frontend/src/hir/type_check/expr.rs | 51 ++++--------------- .../noirc_frontend/src/hir/type_check/mod.rs | 6 +-- compiler/noirc_frontend/src/hir_def/types.rs | 43 ++++++++-------- .../execution_success/generics/src/main.nr | 5 +- .../trait_method_mut_self/src/main.nr | 21 ++++---- 5 files changed, 46 insertions(+), 80 deletions(-) diff --git a/compiler/noirc_frontend/src/hir/type_check/expr.rs b/compiler/noirc_frontend/src/hir/type_check/expr.rs index 29a868a6cb1..9fe6294c863 100644 --- a/compiler/noirc_frontend/src/hir/type_check/expr.rs +++ b/compiler/noirc_frontend/src/hir/type_check/expr.rs @@ -234,23 +234,6 @@ impl<'interner> TypeChecker<'interner> { // so that the backend doesn't need to worry about methods let location = method_call.location; - // if method_call.generics.is_none() { - // dbg!(method_ref.clone()); - // } - - // Automatically add `&mut` if the method expects a mutable reference and - // the object is not already one. - // if let HirMethodReference::FuncId(func_id) = &method_ref { - // if *func_id != FuncId::dummy_id() { - // let function_type = - // self.interner.function_meta(func_id).typ.clone(); - // self.try_add_mutable_reference_to_object( - // &mut method_call, - // &function_type, - // &mut object_type, - // ); - // } - // } match &method_ref { HirMethodReference::FuncId(func_id) => { if *func_id != FuncId::dummy_id() { @@ -263,22 +246,17 @@ impl<'interner> TypeChecker<'interner> { ); } } - // _ => {}, HirMethodReference::TraitMethodId(method_id, _) => { let id = self.interner.trait_method_id(*method_id); let definition = self.interner.definition(id); - match definition.kind { - DefinitionKind::Function(func_id) => { - let function_type = - self.interner.function_meta(&func_id).typ.clone(); - // dbg!(function_type.clone()); - self.try_add_mutable_reference_to_object( - &mut method_call, - &function_type, - &mut object_type, - ); - } - _ => {} + if let DefinitionKind::Function(func_id) = definition.kind { + let function_type = + self.interner.function_meta(&func_id).typ.clone(); + self.try_add_mutable_reference_to_object( + &mut method_call, + &function_type, + &mut object_type, + ); } } } @@ -396,9 +374,6 @@ impl<'interner> TypeChecker<'interner> { ) -> Type { let mut bindings = TypeBindings::new(); - if generics.is_some() { - dbg!(ident.impl_kind.clone()); - } // Add type bindings from any constraints that were used. // We need to do this first since otherwise instantiating the type below // will replace each trait generic with a fresh type variable, rather than @@ -439,7 +414,7 @@ impl<'interner> TypeChecker<'interner> { let (typ, bindings) = self.instantiate( t, bindings, - generics.clone(), + generics, function_generic_count, implicit_generic_count, span, @@ -450,18 +425,14 @@ impl<'interner> TypeChecker<'interner> { if let Some(definition) = self.interner.try_definition(ident.id) { if let DefinitionKind::Function(function) = definition.kind { let function = self.interner.function_meta(&function); - if generics.is_some() { - dbg!(function.trait_constraints.clone()); - } + for mut constraint in function.trait_constraints.clone() { constraint.apply_bindings(&bindings); self.trait_constraints.push((constraint, *expr_id)); } } } - if generics.is_some() { - dbg!(ident.impl_kind.clone()); - } + if let ImplKind::TraitMethod(_, mut constraint, assumed) = ident.impl_kind { constraint.apply_bindings(&bindings); if assumed { diff --git a/compiler/noirc_frontend/src/hir/type_check/mod.rs b/compiler/noirc_frontend/src/hir/type_check/mod.rs index 789fb3bea84..71fb0663627 100644 --- a/compiler/noirc_frontend/src/hir/type_check/mod.rs +++ b/compiler/noirc_frontend/src/hir/type_check/mod.rs @@ -144,11 +144,9 @@ pub fn type_check_func(interner: &mut NodeInterner, func_id: FuncId) -> Vec (Type, TypeBindings) { + match self { + Type::Forall(typevars, typ) => { + let replacements = typevars + .iter() + .map(|var| { + let new = interner.next_type_variable(); + (var.id(), (var.clone(), new)) + }) + .collect(); + + let instantiated = typ.force_substitute(&replacements); + (instantiated, replacements) + } + other => (other.clone(), HashMap::new()), + } + } + /// Instantiates a type with the given types. /// This differs from substitute in that only the quantified type variables /// are matched against the type list and are eligible for substitution - similar @@ -1465,7 +1486,6 @@ impl Type { /// /// Expects the given type vector to be the same length as the Forall type variables. pub fn instantiate_with(&self, types: Vec) -> (Type, TypeBindings) { - dbg!(self.clone()); match self { Type::Forall(typevars, typ) => { assert_eq!(types.len(), typevars.len(), "Turbofish operator used with incorrect generic count which was not caught by name resolution"); @@ -1483,27 +1503,6 @@ impl Type { } } - /// Instantiate this type, replacing any type variables it is quantified - /// over with fresh type variables. If this type is not a Type::Forall, - /// it is unchanged. - pub fn instantiate(&self, interner: &NodeInterner) -> (Type, TypeBindings) { - match self { - Type::Forall(typevars, typ) => { - let replacements = typevars - .iter() - .map(|var| { - let new = interner.next_type_variable(); - (var.id(), (var.clone(), new)) - }) - .collect(); - - let instantiated = typ.force_substitute(&replacements); - (instantiated, replacements) - } - other => (other.clone(), HashMap::new()), - } - } - /// Substitute any type variables found within this type with the /// given bindings if found. If a type variable is not found within /// the given TypeBindings, it is unchanged. diff --git a/test_programs/execution_success/generics/src/main.nr b/test_programs/execution_success/generics/src/main.nr index 2fb122d5729..c8616960559 100644 --- a/test_programs/execution_success/generics/src/main.nr +++ b/test_programs/execution_success/generics/src/main.nr @@ -1,6 +1,3 @@ -use dep::std::hash::Hasher; -use dep::std::hash::poseidon2::Poseidon2Hasher; - struct Bar { one: Field, two: Field, @@ -42,7 +39,7 @@ impl Bar { } fn main(x: Field, y: Field) { - let mut bar1: Bar = Bar { one: x, two: y, other: 0 }; + let bar1: Bar = Bar { one: x, two: y, other: 0 }; let bar2 = Bar { one: x, two: y, other: [0] }; foo(bar1); diff --git a/test_programs/execution_success/trait_method_mut_self/src/main.nr b/test_programs/execution_success/trait_method_mut_self/src/main.nr index 6d183c96242..27375068f98 100644 --- a/test_programs/execution_success/trait_method_mut_self/src/main.nr +++ b/test_programs/execution_success/trait_method_mut_self/src/main.nr @@ -5,16 +5,16 @@ fn main(x: Field, y: pub Field) { let mut a_mut_ref = AType::new(x); pass_trait_by_value(a_mut_ref, y); assert(a_mut_ref.x == x); - pass_trait_by_mut_ref(&mut a_mut_ref, y); - assert(a_mut_ref.x == y); + // pass_trait_by_mut_ref(&mut a_mut_ref, y); + // assert(a_mut_ref.x == y); - let mut hasher = Poseidon2Hasher::default(); - hasher.write(x); - hasher.write(y); - let expected_hash = hasher.finish(); - // Check that we get the same result when using the hasher in a - // method that purely uses trait methods without a supplied implementation. - assert(hash_simple_array::([x, y]) == expected_hash); + // let mut hasher = Poseidon2Hasher::default(); + // hasher.write(x); + // hasher.write(y); + // let expected_hash = hasher.finish(); + // // Check that we get the same result when using the hasher in a + // // method that purely uses trait methods without a supplied implementation. + // assert(hash_simple_array::([x, y]) == expected_hash); } trait SomeTrait { @@ -43,7 +43,8 @@ impl SomeTrait for AType { } } -fn pass_trait_by_value(mut a_mut_ref: T, value: Field) where T: SomeTrait { +fn pass_trait_by_value(mut a_mut_ref: impl SomeTrait, value: Field) { +// where T: SomeTrait { // We auto add a mutable reference to the object type if the method call expects a mutable self a_mut_ref.set_value(value); assert(a_mut_ref.get_value() == value); From 954aba71177202349c3e62736b4defbdf03cdc60 Mon Sep 17 00:00:00 2001 From: Maxim Vezenov Date: Thu, 16 May 2024 22:44:53 +0000 Subject: [PATCH 23/55] nargo fmt --- .../trait_method_mut_self/src/main.nr | 34 ++++++++++++------- 1 file changed, 22 insertions(+), 12 deletions(-) diff --git a/test_programs/execution_success/trait_method_mut_self/src/main.nr b/test_programs/execution_success/trait_method_mut_self/src/main.nr index 27375068f98..8f1984a5afc 100644 --- a/test_programs/execution_success/trait_method_mut_self/src/main.nr +++ b/test_programs/execution_success/trait_method_mut_self/src/main.nr @@ -3,18 +3,23 @@ use dep::std::hash::poseidon2::Poseidon2Hasher; fn main(x: Field, y: pub Field) { let mut a_mut_ref = AType::new(x); + pass_trait_by_value(a_mut_ref, y); assert(a_mut_ref.x == x); - // pass_trait_by_mut_ref(&mut a_mut_ref, y); - // assert(a_mut_ref.x == y); - // let mut hasher = Poseidon2Hasher::default(); - // hasher.write(x); - // hasher.write(y); - // let expected_hash = hasher.finish(); - // // Check that we get the same result when using the hasher in a - // // method that purely uses trait methods without a supplied implementation. - // assert(hash_simple_array::([x, y]) == expected_hash); + pass_tarit_by_value_impl_param(a_mut_ref, y); + assert(a_mut_ref.x == x); + + pass_trait_by_mut_ref(&mut a_mut_ref, y); + assert(a_mut_ref.x == y); + + let mut hasher = Poseidon2Hasher::default(); + hasher.write(x); + hasher.write(y); + let expected_hash = hasher.finish(); + // Check that we get the same result when using the hasher in a + // method that purely uses trait methods without a supplied implementation. + assert(hash_simple_array::([x, y]) == expected_hash); } trait SomeTrait { @@ -43,8 +48,13 @@ impl SomeTrait for AType { } } -fn pass_trait_by_value(mut a_mut_ref: impl SomeTrait, value: Field) { -// where T: SomeTrait { +fn pass_tarit_by_value_impl_param(mut a_mut_ref: impl SomeTrait, value: Field) { + // We auto add a mutable reference to the object type if the method call expects a mutable self + a_mut_ref.set_value(value); + assert(a_mut_ref.get_value() == value); +} + +fn pass_trait_by_value(mut a_mut_ref: T, value: Field) where T: SomeTrait { // We auto add a mutable reference to the object type if the method call expects a mutable self a_mut_ref.set_value(value); assert(a_mut_ref.get_value() == value); @@ -65,4 +75,4 @@ fn hash_simple_array(input: [Field; 2]) -> Field where H: Hasher + Default { hasher.write(input[0]); hasher.write(input[1]); hasher.finish() -} \ No newline at end of file +} From 1e22003c61f8afd19eac9a78ef67a0284dba9e82 Mon Sep 17 00:00:00 2001 From: Maxim Vezenov Date: Thu, 16 May 2024 22:46:01 +0000 Subject: [PATCH 24/55] add todo comments --- .../execution_success/trait_method_mut_self/src/main.nr | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/test_programs/execution_success/trait_method_mut_self/src/main.nr b/test_programs/execution_success/trait_method_mut_self/src/main.nr index 8f1984a5afc..df41ca48689 100644 --- a/test_programs/execution_success/trait_method_mut_self/src/main.nr +++ b/test_programs/execution_success/trait_method_mut_self/src/main.nr @@ -67,7 +67,9 @@ fn pass_trait_by_mut_ref(a_mut_ref: &mut T, value: Field) where T: SomeTrait fn hash_simple_array(input: [Field; 2]) -> Field where H: Hasher + Default { // Check that we can a trait method to instead a trait implementation - let mut hasher: H = H::default(); + // TODO: Need to remove the need for this type annotation + // TODO: Curently, without the annotation we will get `Expression type is ambiguous` when trying to use the `hasher` + let mut hasher = H::default(); // Regression that the object is converted to a mutable reference type `&mut _`. // Otherwise will see `Expected type &mut _, found type H`. // Then we need to make sure to also auto dereference later in the type checking process From 44a4aee9a9eb2a727fd774cab61763f9fbfabd54 Mon Sep 17 00:00:00 2001 From: Maxim Vezenov Date: Fri, 17 May 2024 04:12:12 +0000 Subject: [PATCH 25/55] specify type annotation for hasher --- .../execution_success/trait_method_mut_self/src/main.nr | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test_programs/execution_success/trait_method_mut_self/src/main.nr b/test_programs/execution_success/trait_method_mut_self/src/main.nr index df41ca48689..fe06e97d207 100644 --- a/test_programs/execution_success/trait_method_mut_self/src/main.nr +++ b/test_programs/execution_success/trait_method_mut_self/src/main.nr @@ -69,7 +69,7 @@ fn hash_simple_array(input: [Field; 2]) -> Field where H: Hasher + Default { // Check that we can a trait method to instead a trait implementation // TODO: Need to remove the need for this type annotation // TODO: Curently, without the annotation we will get `Expression type is ambiguous` when trying to use the `hasher` - let mut hasher = H::default(); + let mut hasher: H = H::default(); // Regression that the object is converted to a mutable reference type `&mut _`. // Otherwise will see `Expected type &mut _, found type H`. // Then we need to make sure to also auto dereference later in the type checking process From 8d5c0721a478a082a4c361a3798437763263cab3 Mon Sep 17 00:00:00 2001 From: Maxim Vezenov Date: Fri, 17 May 2024 04:12:51 +0000 Subject: [PATCH 26/55] rename --- .../execution_success/trait_method_mut_self/src/main.nr | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test_programs/execution_success/trait_method_mut_self/src/main.nr b/test_programs/execution_success/trait_method_mut_self/src/main.nr index fe06e97d207..98ae7b1d90f 100644 --- a/test_programs/execution_success/trait_method_mut_self/src/main.nr +++ b/test_programs/execution_success/trait_method_mut_self/src/main.nr @@ -7,7 +7,7 @@ fn main(x: Field, y: pub Field) { pass_trait_by_value(a_mut_ref, y); assert(a_mut_ref.x == x); - pass_tarit_by_value_impl_param(a_mut_ref, y); + pass_trait_by_value_impl_param(a_mut_ref, y); assert(a_mut_ref.x == x); pass_trait_by_mut_ref(&mut a_mut_ref, y); @@ -48,7 +48,7 @@ impl SomeTrait for AType { } } -fn pass_tarit_by_value_impl_param(mut a_mut_ref: impl SomeTrait, value: Field) { +fn pass_trait_by_value_impl_param(mut a_mut_ref: impl SomeTrait, value: Field) { // We auto add a mutable reference to the object type if the method call expects a mutable self a_mut_ref.set_value(value); assert(a_mut_ref.get_value() == value); From c6f3057913f89bb776f318d7088507b1ff166fb5 Mon Sep 17 00:00:00 2001 From: Maxim Vezenov Date: Fri, 17 May 2024 04:18:51 +0000 Subject: [PATCH 27/55] brought back removed comment --- compiler/noirc_frontend/src/hir/type_check/expr.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/compiler/noirc_frontend/src/hir/type_check/expr.rs b/compiler/noirc_frontend/src/hir/type_check/expr.rs index 9fe6294c863..d1eae2622ee 100644 --- a/compiler/noirc_frontend/src/hir/type_check/expr.rs +++ b/compiler/noirc_frontend/src/hir/type_check/expr.rs @@ -234,6 +234,8 @@ impl<'interner> TypeChecker<'interner> { // so that the backend doesn't need to worry about methods let location = method_call.location; + // Automatically add `&mut` if the method expects a mutable reference and + // the object is not already one. match &method_ref { HirMethodReference::FuncId(func_id) => { if *func_id != FuncId::dummy_id() { From 384e26a015f1353091ac50b9cd1f789d19ae71b4 Mon Sep 17 00:00:00 2001 From: TomAFrench Date: Fri, 17 May 2024 17:06:52 +0000 Subject: [PATCH 28/55] chore: add test for specifying types on function with turbofish --- compiler/noirc_frontend/src/tests.rs | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/compiler/noirc_frontend/src/tests.rs b/compiler/noirc_frontend/src/tests.rs index fb80a7d8018..b93e5ee9206 100644 --- a/compiler/noirc_frontend/src/tests.rs +++ b/compiler/noirc_frontend/src/tests.rs @@ -1378,3 +1378,18 @@ fn deny_fold_attribute_on_unconstrained() { CompilationError::ResolverError(ResolverError::FoldAttributeOnUnconstrained { .. }) )); } + +#[test] +fn specify_function_types_with_turbofish() { + let src = r#" + fn generic_func() -> (T, U) where T: Default, U: Default { + (T::default(), U::default()) + } + + fn main() { + let _ = generic_func::(); + } + "#; + let errors = get_program_errors(src); + assert_eq!(errors.len(), 0); +} From 98c5d895940ee3ba36553e8af0dc8a5c933a6322 Mon Sep 17 00:00:00 2001 From: TomAFrench Date: Fri, 17 May 2024 17:35:53 +0000 Subject: [PATCH 29/55] chore: add test for using turbofish with generic methods --- compiler/noirc_frontend/src/tests.rs | 34 ++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/compiler/noirc_frontend/src/tests.rs b/compiler/noirc_frontend/src/tests.rs index b93e5ee9206..8822dd339b2 100644 --- a/compiler/noirc_frontend/src/tests.rs +++ b/compiler/noirc_frontend/src/tests.rs @@ -1393,3 +1393,37 @@ fn specify_function_types_with_turbofish() { let errors = get_program_errors(src); assert_eq!(errors.len(), 0); } + +#[test] +fn specify_method_types_with_turbofish() { + let src = r#" + trait Default { + fn default() -> Self; + } + + impl Default for Field { + fn default() -> Self { 0 } + } + + // Need the above as we don't have access to the stdlib here. + // We also need to construct a concrete value of `U` without giving away its type + // as otherwise the unspecified type is ignored. + + struct Foo { + inner: T + } + + impl Foo { + fn generic_method(_self: Self) where U: Default { + U::default() + } + } + + fn main() { + let foo: Foo = Foo { inner: 1 }; + foo.generic_method::(); + } + "#; + let errors = get_program_errors(src); + assert_eq!(errors.len(), 0); +} From 32d1714acb26b0d0e652d842ea720a538a830944 Mon Sep 17 00:00:00 2001 From: TomAFrench Date: Fri, 17 May 2024 17:36:04 +0000 Subject: [PATCH 30/55] chore: add turbofish to cspell --- cspell.json | 1 + 1 file changed, 1 insertion(+) diff --git a/cspell.json b/cspell.json index bf3040265c2..79ddc2df4e7 100644 --- a/cspell.json +++ b/cspell.json @@ -179,6 +179,7 @@ "termcolor", "thiserror", "tslog", + "turbofish", "typecheck", "typechecked", "typevar", From dde6d5a30b5bbe07f852bc51b85a0bfa82427fde Mon Sep 17 00:00:00 2001 From: Maxim Vezenov Date: Fri, 17 May 2024 21:33:01 +0000 Subject: [PATCH 31/55] correctly monomorphize turbofish functions --- .../src/monomorphization/mod.rs | 48 ++++++++++++++----- .../turbofish_call_func_diff_types/Nargo.toml | 7 +++ .../Prover.toml | 2 + .../src/main.nr | 36 ++++++++++++++ 4 files changed, 81 insertions(+), 12 deletions(-) create mode 100644 test_programs/execution_success/turbofish_call_func_diff_types/Nargo.toml create mode 100644 test_programs/execution_success/turbofish_call_func_diff_types/Prover.toml create mode 100644 test_programs/execution_success/turbofish_call_func_diff_types/src/main.nr diff --git a/compiler/noirc_frontend/src/monomorphization/mod.rs b/compiler/noirc_frontend/src/monomorphization/mod.rs index 9a20d0dd537..54a6af97744 100644 --- a/compiler/noirc_frontend/src/monomorphization/mod.rs +++ b/compiler/noirc_frontend/src/monomorphization/mod.rs @@ -55,9 +55,12 @@ struct LambdaContext { struct Monomorphizer<'interner> { /// Functions are keyed by their unique ID and expected type so that we can monomorphize /// a new version of the function for each type. + /// We also key by any turbofish generics that are specified. + /// This is necessary for a case where we may have a trait generic that can be instantiated + /// outside of a function parameter or return value. /// /// Using nested HashMaps here lets us avoid cloning HirTypes when calling .get() - functions: HashMap>, + functions: HashMap), FuncId>>, /// Unlike functions, locals are only keyed by their unique ID because they are never /// duplicated during monomorphization. Doing so would allow them to be used polymorphically @@ -198,10 +201,15 @@ impl<'interner> Monomorphizer<'interner> { id: node_interner::FuncId, expr_id: node_interner::ExprId, typ: &HirType, + turbofish_generics: Vec, trait_method: Option, ) -> Definition { let typ = typ.follow_bindings(); - match self.functions.get(&id).and_then(|inner_map| inner_map.get(&typ)) { + match self + .functions + .get(&id) + .and_then(|inner_map| inner_map.get(&(typ.clone(), turbofish_generics.clone()))) + { Some(id) => Definition::Function(*id), None => { // Function has not been monomorphized yet @@ -222,7 +230,8 @@ impl<'interner> Monomorphizer<'interner> { Definition::Builtin(opcode) } FunctionKind::Normal => { - let id = self.queue_function(id, expr_id, typ, trait_method); + let id = + self.queue_function(id, expr_id, typ, turbofish_generics, trait_method); Definition::Function(id) } FunctionKind::Oracle => { @@ -249,8 +258,14 @@ impl<'interner> Monomorphizer<'interner> { } /// Prerequisite: typ = typ.follow_bindings() - fn define_function(&mut self, id: node_interner::FuncId, typ: HirType, new_id: FuncId) { - self.functions.entry(id).or_default().insert(typ, new_id); + fn define_function( + &mut self, + id: node_interner::FuncId, + typ: HirType, + turbofish_generics: Vec, + new_id: FuncId, + ) { + self.functions.entry(id).or_default().insert((typ, turbofish_generics), new_id); } fn compile_main( @@ -393,7 +408,7 @@ impl<'interner> Monomorphizer<'interner> { use ast::Literal::*; let expr = match self.interner.expression(&expr) { - HirExpression::Ident(ident, _) => self.ident(ident, expr)?, + HirExpression::Ident(ident, generics) => self.ident(ident, expr, generics)?, HirExpression::Literal(HirLiteral::Str(contents)) => Literal(Str(contents)), HirExpression::Literal(HirLiteral::FmtStr(contents, idents)) => { let fields = try_vecmap(idents, |ident| self.expr(ident))?; @@ -825,6 +840,7 @@ impl<'interner> Monomorphizer<'interner> { &mut self, ident: HirIdent, expr_id: node_interner::ExprId, + generics: Option>, ) -> Result { let typ = self.interner.id_type(expr_id); @@ -838,7 +854,13 @@ impl<'interner> Monomorphizer<'interner> { let mutable = definition.mutable; let location = Some(ident.location); let name = definition.name.clone(); - let definition = self.lookup_function(*func_id, expr_id, &typ, None); + let definition = self.lookup_function( + *func_id, + expr_id, + &typ, + generics.unwrap_or_default(), + None, + ); let typ = Self::convert_type(&typ, ident.location)?; let ident = ast::Ident { location, mutable, definition, name, typ: typ.clone() }; let ident_expression = ast::Expression::Ident(ident); @@ -1063,10 +1085,11 @@ impl<'interner> Monomorphizer<'interner> { } }; - let func_id = match self.lookup_function(func_id, expr_id, &function_type, Some(method)) { - Definition::Function(func_id) => func_id, - _ => unreachable!(), - }; + let func_id = + match self.lookup_function(func_id, expr_id, &function_type, vec![], Some(method)) { + Definition::Function(func_id) => func_id, + _ => unreachable!(), + }; let the_trait = self.interner.get_trait(method.trait_id); let location = self.interner.expr_location(&expr_id); @@ -1292,10 +1315,11 @@ impl<'interner> Monomorphizer<'interner> { id: node_interner::FuncId, expr_id: node_interner::ExprId, function_type: HirType, + turbofish_generics: Vec, trait_method: Option, ) -> FuncId { let new_id = self.next_function_id(); - self.define_function(id, function_type.clone(), new_id); + self.define_function(id, function_type.clone(), turbofish_generics, new_id); let bindings = self.interner.get_instantiation_bindings(expr_id); let bindings = self.follow_bindings(bindings); diff --git a/test_programs/execution_success/turbofish_call_func_diff_types/Nargo.toml b/test_programs/execution_success/turbofish_call_func_diff_types/Nargo.toml new file mode 100644 index 00000000000..8624cda646b --- /dev/null +++ b/test_programs/execution_success/turbofish_call_func_diff_types/Nargo.toml @@ -0,0 +1,7 @@ +[package] +name = "turbofish_call_func_diff_types" +type = "bin" +authors = [""] +compiler_version = ">=0.29.0" + +[dependencies] \ No newline at end of file diff --git a/test_programs/execution_success/turbofish_call_func_diff_types/Prover.toml b/test_programs/execution_success/turbofish_call_func_diff_types/Prover.toml new file mode 100644 index 00000000000..f28f2f8cc48 --- /dev/null +++ b/test_programs/execution_success/turbofish_call_func_diff_types/Prover.toml @@ -0,0 +1,2 @@ +x = "5" +y = "10" diff --git a/test_programs/execution_success/turbofish_call_func_diff_types/src/main.nr b/test_programs/execution_success/turbofish_call_func_diff_types/src/main.nr new file mode 100644 index 00000000000..5c73c250beb --- /dev/null +++ b/test_programs/execution_success/turbofish_call_func_diff_types/src/main.nr @@ -0,0 +1,36 @@ +use dep::std::hash::Hasher; +use dep::std::hash::poseidon2::Poseidon2Hasher; +use dep::std::hash::poseidon::PoseidonHasher; + +fn main(x: Field, y: pub Field) { + let mut hasher = PoseidonHasher::default(); + hasher.write(x); + hasher.write(y); + let poseidon_expected_hash = hasher.finish(); + // Check that we get the same result when using the hasher in a + // method that purely uses trait methods without a supplied implementation. + assert(hash_simple_array::([x, y]) == poseidon_expected_hash); + + // Now let's do the same logic but with a different `Hasher` supplied to the turbofish operator + // We want to make sure that we have correctly monomorphized a function with a trait generic + // where the generic is not used on any function parameters or the return value. + let mut hasher = Poseidon2Hasher::default(); + hasher.write(x); + hasher.write(y); + let poseidon2_expected_hash = hasher.finish(); + assert(hash_simple_array::([x, y]) == poseidon2_expected_hash); +} + +fn hash_simple_array(input: [Field; 2]) -> Field where H: Hasher + Default { + // Check that we can a trait method to instead a trait implementation + // TODO: Need to remove the need for this type annotation + // TODO: Curently, without the annotation we will get `Expression type is ambiguous` when trying to use the `hasher` + let mut hasher: H = H::default(); + // Regression that the object is converted to a mutable reference type `&mut _`. + // Otherwise will see `Expected type &mut _, found type H`. + // Then we need to make sure to also auto dereference later in the type checking process + // when searching for a matching impl or else we will get `No matching impl found for `&mut H: Hasher` + hasher.write(input[0]); + hasher.write(input[1]); + hasher.finish() +} From e26ed3639597643ff478ef616566b2be7fb749f1 Mon Sep 17 00:00:00 2001 From: Maxim Vezenov Date: Fri, 17 May 2024 21:39:59 +0000 Subject: [PATCH 32/55] missed save --- .../trait_method_mut_self/src/main.nr | 22 ------------------- 1 file changed, 22 deletions(-) diff --git a/test_programs/execution_success/trait_method_mut_self/src/main.nr b/test_programs/execution_success/trait_method_mut_self/src/main.nr index 98ae7b1d90f..319129ee655 100644 --- a/test_programs/execution_success/trait_method_mut_self/src/main.nr +++ b/test_programs/execution_success/trait_method_mut_self/src/main.nr @@ -12,14 +12,6 @@ fn main(x: Field, y: pub Field) { pass_trait_by_mut_ref(&mut a_mut_ref, y); assert(a_mut_ref.x == y); - - let mut hasher = Poseidon2Hasher::default(); - hasher.write(x); - hasher.write(y); - let expected_hash = hasher.finish(); - // Check that we get the same result when using the hasher in a - // method that purely uses trait methods without a supplied implementation. - assert(hash_simple_array::([x, y]) == expected_hash); } trait SomeTrait { @@ -64,17 +56,3 @@ fn pass_trait_by_mut_ref(a_mut_ref: &mut T, value: Field) where T: SomeTrait // We auto add a mutable reference to the object type if the method call expects a mutable self a_mut_ref.set_value(value); } - -fn hash_simple_array(input: [Field; 2]) -> Field where H: Hasher + Default { - // Check that we can a trait method to instead a trait implementation - // TODO: Need to remove the need for this type annotation - // TODO: Curently, without the annotation we will get `Expression type is ambiguous` when trying to use the `hasher` - let mut hasher: H = H::default(); - // Regression that the object is converted to a mutable reference type `&mut _`. - // Otherwise will see `Expected type &mut _, found type H`. - // Then we need to make sure to also auto dereference later in the type checking process - // when searching for a matching impl or else we will get `No matching impl found for `&mut H: Hasher` - hasher.write(input[0]); - hasher.write(input[1]); - hasher.finish() -} From 794c0826fef45e6e80f8b0fb43948d3a87c525bb Mon Sep 17 00:00:00 2001 From: Maxim Vezenov Date: Fri, 17 May 2024 21:46:31 +0000 Subject: [PATCH 33/55] update eddsa function to use turbofish --- noir_stdlib/src/eddsa.nr | 25 +++++++++---------- .../execution_success/eddsa/src/main.nr | 5 ++-- 2 files changed, 15 insertions(+), 15 deletions(-) diff --git a/noir_stdlib/src/eddsa.nr b/noir_stdlib/src/eddsa.nr index 3aff6043ffd..a658d528258 100644 --- a/noir_stdlib/src/eddsa.nr +++ b/noir_stdlib/src/eddsa.nr @@ -3,6 +3,7 @@ use crate::ec::consts::te::baby_jubjub; use crate::ec::tecurve::affine::Point as TEPoint; use crate::hash::{Hash, Hasher, BuildHasher, BuildHasherDefault}; use crate::hash::poseidon::PoseidonHasher; +use crate::default::Default; // Returns true if signature is valid pub fn eddsa_poseidon_verify( @@ -13,15 +14,13 @@ pub fn eddsa_poseidon_verify( signature_r8_y: Field, message: Field ) -> bool { - let mut hasher = PoseidonHasher::default(); - eddsa_verify_with_hasher( + eddsa_verify_with_hasher::( pub_key_x, pub_key_y, signature_s, signature_r8_x, signature_r8_y, - message, - &mut hasher + message ) } @@ -31,10 +30,9 @@ pub fn eddsa_verify_with_hasher( signature_s: Field, signature_r8_x: Field, signature_r8_y: Field, - message: Field, - hasher: &mut H + message: Field ) -> bool -where H: Hasher { +where H: Hasher + Default { // Verifies by testing: // S * B8 = R8 + H(R8, A, m) * A8 let bjj = baby_jubjub(); @@ -47,12 +45,13 @@ where H: Hasher { // Ensure S < Subgroup Order assert(signature_s.lt(bjj.suborder)); // Calculate the h = H(R, A, msg) - signature_r8_x.hash(hasher); - signature_r8_y.hash(hasher); - pub_key_x.hash(hasher); - pub_key_y.hash(hasher); - message.hash(hasher); - let hash: Field = (*hasher).finish(); + let mut hasher: H = H::default(); + hasher.write(signature_r8_x); + hasher.write(signature_r8_y); + hasher.write(pub_key_x); + hasher.write(pub_key_y); + hasher.write(message); + let hash: Field = hasher.finish(); // Calculate second part of the right side: right2 = h*8*A // Multiply by 8 by doubling 3 times. This also ensures that the result is in the subgroup. let pub_key_mul_2 = bjj.curve.add(pub_key, pub_key); diff --git a/test_programs/execution_success/eddsa/src/main.nr b/test_programs/execution_success/eddsa/src/main.nr index 012c8466f2f..2b60d8df3a0 100644 --- a/test_programs/execution_success/eddsa/src/main.nr +++ b/test_programs/execution_success/eddsa/src/main.nr @@ -50,7 +50,8 @@ fn main(msg: pub Field, _priv_key_a: Field, _priv_key_b: Field) { // User A's signature over the message can't be used with another message assert(!eddsa_poseidon_verify(pub_key_a.x, pub_key_a.y, s_a, r8_a.x, r8_a.y, msg + 1)); // Using a different hash should fail - let mut hasher = Poseidon2Hasher::default(); - assert(!eddsa_verify_with_hasher(pub_key_a.x, pub_key_a.y, s_a, r8_a.x, r8_a.y, msg, &mut hasher)); + assert( + !eddsa_verify_with_hasher::(pub_key_a.x, pub_key_a.y, s_a, r8_a.x, r8_a.y, msg) + ); } } From ae12b054a3071aa397c51d9c0bfb7b3c14dd6921 Mon Sep 17 00:00:00 2001 From: Maxim Vezenov Date: Fri, 17 May 2024 21:57:58 +0000 Subject: [PATCH 34/55] update docs --- .../standard_library/cryptographic_primitives/eddsa.mdx | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/docs/noir/standard_library/cryptographic_primitives/eddsa.mdx b/docs/docs/noir/standard_library/cryptographic_primitives/eddsa.mdx index c2c0624dfad..c45325265d7 100644 --- a/docs/docs/noir/standard_library/cryptographic_primitives/eddsa.mdx +++ b/docs/docs/noir/standard_library/cryptographic_primitives/eddsa.mdx @@ -15,12 +15,12 @@ Verifier for EdDSA signatures fn eddsa_poseidon_verify(public_key_x : Field, public_key_y : Field, signature_s: Field, signature_r8_x: Field, signature_r8_y: Field, message: Field) -> bool ``` -It is also possible to specify the hash algorithm used for the signature by using the `eddsa_verify_with_hasher` function with a parameter implementing the Hasher trait. For instance, if you want to use Poseidon2 instead, you can do the following: +It is also possible to specify the hash algorithm used for the signature by using the `eddsa_verify_with_hasher` function by passing a type implementing the Hasher trait with the turbofish operator. +For instance, if you want to use Poseidon2 instead, you can do the following: ```rust use dep::std::hash::poseidon2::Poseidon2Hasher; -let mut hasher = Poseidon2Hasher::default(); -eddsa_verify_with_hasher(pub_key_a.x, pub_key_a.y, s_a, r8_a.x, r8_a.y, msg, &mut hasher); +eddsa_verify_with_hasher::(pub_key_a.x, pub_key_a.y, s_a, r8_a.x, r8_a.y, msg); ``` From 9ab7351f0bf9faf7c7b081d2d69ebaa21356b2ed Mon Sep 17 00:00:00 2001 From: Maxim Vezenov Date: Fri, 17 May 2024 22:11:22 +0000 Subject: [PATCH 35/55] nargo fmt tests --- tooling/nargo_fmt/tests/expected/turbofish_call.nr | 8 ++++++++ .../tests/expected/turbofish_method_call.nr | 12 ++++++++++++ tooling/nargo_fmt/tests/input/turbofish_call.nr | 7 +++++++ .../nargo_fmt/tests/input/turbofish_method_call.nr | 5 +++++ 4 files changed, 32 insertions(+) create mode 100644 tooling/nargo_fmt/tests/expected/turbofish_call.nr create mode 100644 tooling/nargo_fmt/tests/expected/turbofish_method_call.nr create mode 100644 tooling/nargo_fmt/tests/input/turbofish_call.nr create mode 100644 tooling/nargo_fmt/tests/input/turbofish_method_call.nr diff --git a/tooling/nargo_fmt/tests/expected/turbofish_call.nr b/tooling/nargo_fmt/tests/expected/turbofish_call.nr new file mode 100644 index 00000000000..8d165c8aafc --- /dev/null +++ b/tooling/nargo_fmt/tests/expected/turbofish_call.nr @@ -0,0 +1,8 @@ +fn foo() { + my_function::(10, some_value, another_func(20, 30)); + + outer_function::( + some_function(), // Original inner function call + another_function() // Original inner function call + ); +} \ No newline at end of file diff --git a/tooling/nargo_fmt/tests/expected/turbofish_method_call.nr b/tooling/nargo_fmt/tests/expected/turbofish_method_call.nr new file mode 100644 index 00000000000..ab7022e235d --- /dev/null +++ b/tooling/nargo_fmt/tests/expected/turbofish_method_call.nr @@ -0,0 +1,12 @@ +fn foo() { + my_object.some_method::(10, var_value, inner_method::(20, 30)); + + assert( + p4_affine.eq::( + Gaffine::new::( + 6890855772600357754907169075114257697580319025794532037257385534741338397365, + 4338620300185947561074059802482547481416142213883829469920100239455078257889 + ) + ) + ); +} \ No newline at end of file diff --git a/tooling/nargo_fmt/tests/input/turbofish_call.nr b/tooling/nargo_fmt/tests/input/turbofish_call.nr new file mode 100644 index 00000000000..03abde789fe --- /dev/null +++ b/tooling/nargo_fmt/tests/input/turbofish_call.nr @@ -0,0 +1,7 @@ +fn foo() { + my_function :: ( 10,some_value,another_func( 20 , 30) ); + + outer_function :: (some_function(), // Original inner function call + another_function(), // Original inner function call + ); +} \ No newline at end of file diff --git a/tooling/nargo_fmt/tests/input/turbofish_method_call.nr b/tooling/nargo_fmt/tests/input/turbofish_method_call.nr new file mode 100644 index 00000000000..aa7ae87f23a --- /dev/null +++ b/tooling/nargo_fmt/tests/input/turbofish_method_call.nr @@ -0,0 +1,5 @@ +fn foo() { + my_object . some_method :: ( 10,var_value,inner_method:: ( 20 , 30) ); + + assert(p4_affine.eq::(Gaffine::new::(6890855772600357754907169075114257697580319025794532037257385534741338397365, 4338620300185947561074059802482547481416142213883829469920100239455078257889))); +} \ No newline at end of file From b73d26398f5c975610c6e06970ded722749c3de0 Mon Sep 17 00:00:00 2001 From: Maxim Vezenov Date: Fri, 17 May 2024 22:17:06 +0000 Subject: [PATCH 36/55] fix noirc_frontend tests --- compiler/noirc_frontend/src/tests.rs | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/compiler/noirc_frontend/src/tests.rs b/compiler/noirc_frontend/src/tests.rs index 8822dd339b2..7bf5655486b 100644 --- a/compiler/noirc_frontend/src/tests.rs +++ b/compiler/noirc_frontend/src/tests.rs @@ -1382,6 +1382,22 @@ fn deny_fold_attribute_on_unconstrained() { #[test] fn specify_function_types_with_turbofish() { let src = r#" + trait Default { + fn default() -> Self; + } + + impl Default for Field { + fn default() -> Self { 0 } + } + + impl Default for u64 { + fn default() -> Self { 0 } + } + + // Need the above as we don't have access to the stdlib here. + // We also need to construct a concrete value of `U` without giving away its type + // as otherwise the unspecified type is ignored. + fn generic_func() -> (T, U) where T: Default, U: Default { (T::default(), U::default()) } From a886946038d3ca1ec64597119f288dbbf9cb3084 Mon Sep 17 00:00:00 2001 From: Tom French Date: Mon, 20 May 2024 13:37:00 +0100 Subject: [PATCH 37/55] chore: update formatter test outputs --- tooling/nargo_fmt/tests/expected/turbofish_call.nr | 6 +++--- tooling/nargo_fmt/tests/expected/turbofish_method_call.nr | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/tooling/nargo_fmt/tests/expected/turbofish_call.nr b/tooling/nargo_fmt/tests/expected/turbofish_call.nr index 8d165c8aafc..8f061df422e 100644 --- a/tooling/nargo_fmt/tests/expected/turbofish_call.nr +++ b/tooling/nargo_fmt/tests/expected/turbofish_call.nr @@ -1,8 +1,8 @@ fn foo() { - my_function::(10, some_value, another_func(20, 30)); + my_function ::(10, some_value, another_func(20, 30)); - outer_function::( + outer_function ::( some_function(), // Original inner function call another_function() // Original inner function call ); -} \ No newline at end of file +} diff --git a/tooling/nargo_fmt/tests/expected/turbofish_method_call.nr b/tooling/nargo_fmt/tests/expected/turbofish_method_call.nr index ab7022e235d..7c5bc20090a 100644 --- a/tooling/nargo_fmt/tests/expected/turbofish_method_call.nr +++ b/tooling/nargo_fmt/tests/expected/turbofish_method_call.nr @@ -1,5 +1,5 @@ fn foo() { - my_object.some_method::(10, var_value, inner_method::(20, 30)); + my_object.some_method::(10, var_value, inner_method:: (20, 30)); assert( p4_affine.eq::( @@ -9,4 +9,4 @@ fn foo() { ) ) ); -} \ No newline at end of file +} From 41258046a4f3730dd20e48a8e0fab1cb72c406fa Mon Sep 17 00:00:00 2001 From: Tom French Date: Mon, 20 May 2024 14:40:33 +0100 Subject: [PATCH 38/55] Revert "chore: update formatter test outputs" This reverts commit a886946038d3ca1ec64597119f288dbbf9cb3084. --- tooling/nargo_fmt/tests/expected/turbofish_call.nr | 6 +++--- tooling/nargo_fmt/tests/expected/turbofish_method_call.nr | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/tooling/nargo_fmt/tests/expected/turbofish_call.nr b/tooling/nargo_fmt/tests/expected/turbofish_call.nr index 8f061df422e..8d165c8aafc 100644 --- a/tooling/nargo_fmt/tests/expected/turbofish_call.nr +++ b/tooling/nargo_fmt/tests/expected/turbofish_call.nr @@ -1,8 +1,8 @@ fn foo() { - my_function ::(10, some_value, another_func(20, 30)); + my_function::(10, some_value, another_func(20, 30)); - outer_function ::( + outer_function::( some_function(), // Original inner function call another_function() // Original inner function call ); -} +} \ No newline at end of file diff --git a/tooling/nargo_fmt/tests/expected/turbofish_method_call.nr b/tooling/nargo_fmt/tests/expected/turbofish_method_call.nr index 7c5bc20090a..ab7022e235d 100644 --- a/tooling/nargo_fmt/tests/expected/turbofish_method_call.nr +++ b/tooling/nargo_fmt/tests/expected/turbofish_method_call.nr @@ -1,5 +1,5 @@ fn foo() { - my_object.some_method::(10, var_value, inner_method:: (20, 30)); + my_object.some_method::(10, var_value, inner_method::(20, 30)); assert( p4_affine.eq::( @@ -9,4 +9,4 @@ fn foo() { ) ) ); -} +} \ No newline at end of file From bc7abf84a53dc9f5883445d9633962361e729129 Mon Sep 17 00:00:00 2001 From: Maxim Vezenov Date: Mon, 20 May 2024 18:45:09 +0100 Subject: [PATCH 39/55] Update compiler/noirc_frontend/src/hir_def/expr.rs Co-authored-by: jfecher --- compiler/noirc_frontend/src/hir_def/expr.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/noirc_frontend/src/hir_def/expr.rs b/compiler/noirc_frontend/src/hir_def/expr.rs index 5d4858a5c08..cac9240ffe3 100644 --- a/compiler/noirc_frontend/src/hir_def/expr.rs +++ b/compiler/noirc_frontend/src/hir_def/expr.rs @@ -215,7 +215,7 @@ impl HirMethodCallExpression { ) -> ((ExprId, HirIdent), HirCallExpression, usize) { // Attach any generics which may not be a part of the function call's explicit generics // This includes types such as implicit generics if the function is part of an impl - let (generics, generics_count) = if let Some(mut function_generics) = self.generics.clone() + let (generics, generics_count) = if let Some(mut function_generics) = self.generics { let mut generics = Vec::new(); match object_type.clone() { From 8b1bbc4d9bbe84e2947a7a442cf8426f6aa8f49b Mon Sep 17 00:00:00 2001 From: Maxim Vezenov Date: Mon, 20 May 2024 17:54:44 +0000 Subject: [PATCH 40/55] have atom_or_right_unary accept a type parser --- compiler/noirc_frontend/src/parser/parser.rs | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/compiler/noirc_frontend/src/parser/parser.rs b/compiler/noirc_frontend/src/parser/parser.rs index c6d10e42de3..890ab795e00 100644 --- a/compiler/noirc_frontend/src/parser/parser.rs +++ b/compiler/noirc_frontend/src/parser/parser.rs @@ -1044,6 +1044,7 @@ where expr_no_constructors, statement, allow_constructors, + parse_type(), )) }) } @@ -1064,6 +1065,7 @@ fn atom_or_right_unary<'a, P, P2, S>( expr_no_constructors: P2, statement: S, allow_constructors: bool, + type_parser: impl NoirParser + 'a, ) -> impl NoirParser + 'a where P: ExprParser + 'a, @@ -1088,12 +1090,12 @@ where // `as Type` in `atom as Type` let cast_rhs = keyword(Keyword::As) - .ignore_then(parse_type()) + .ignore_then(type_parser.clone()) .map(UnaryRhs::Cast) .labelled(ParsingRuleLabel::Cast); // A turbofish operator is optional in a method call to specify generic types - let turbofish = primitives::turbofish(parse_type()); + let turbofish = primitives::turbofish(type_parser); // `.foo` or `.foo(args)` in `atom.foo` or `atom.foo(args)` let member_rhs = just(Token::Dot) @@ -1384,11 +1386,12 @@ mod test { expression_no_constructors(expression()), fresh_statement(), true, + parse_type(), ), vec!["x as u8", "x as u16", "0 as Field", "(x + 3) as [Field; 8]"], ); parse_all_failing( - atom_or_right_unary(expression(), expression_nc, fresh_statement(), true), + atom_or_right_unary(expression(), expression_nc, fresh_statement(), true, parse_type()), vec!["x as pub u8"], ); } @@ -1408,6 +1411,7 @@ mod test { expression_no_constructors(expression()), fresh_statement(), true, + parse_type(), ), valid, ); From bd075fab6ca74b758b35e42fb014f9a43b09607c Mon Sep 17 00:00:00 2001 From: Maxim Vezenov Date: Mon, 20 May 2024 19:28:57 +0000 Subject: [PATCH 41/55] clippy and fmt --- compiler/noirc_frontend/src/hir_def/expr.rs | 3 +- tooling/nargo_fmt/src/rewrite/expr.rs | 48 +++++++++++++-------- 2 files changed, 31 insertions(+), 20 deletions(-) diff --git a/compiler/noirc_frontend/src/hir_def/expr.rs b/compiler/noirc_frontend/src/hir_def/expr.rs index cac9240ffe3..46dd0de1917 100644 --- a/compiler/noirc_frontend/src/hir_def/expr.rs +++ b/compiler/noirc_frontend/src/hir_def/expr.rs @@ -215,8 +215,7 @@ impl HirMethodCallExpression { ) -> ((ExprId, HirIdent), HirCallExpression, usize) { // Attach any generics which may not be a part of the function call's explicit generics // This includes types such as implicit generics if the function is part of an impl - let (generics, generics_count) = if let Some(mut function_generics) = self.generics - { + let (generics, generics_count) = if let Some(mut function_generics) = self.generics { let mut generics = Vec::new(); match object_type.clone() { Type::Struct(_, mut struct_generics) => { diff --git a/tooling/nargo_fmt/src/rewrite/expr.rs b/tooling/nargo_fmt/src/rewrite/expr.rs index 0c339e57fb8..e5b30f99b7b 100644 --- a/tooling/nargo_fmt/src/rewrite/expr.rs +++ b/tooling/nargo_fmt/src/rewrite/expr.rs @@ -1,5 +1,5 @@ use noirc_frontend::ast::{ - ArrayLiteral, BlockExpression, Expression, ExpressionKind, Literal, UnaryOp, + ArrayLiteral, BlockExpression, Expression, ExpressionKind, Literal, UnaryOp, UnresolvedType, }; use noirc_frontend::{macros_api::Span, token::Token}; @@ -73,6 +73,7 @@ pub(crate) fn rewrite( let object = rewrite_sub_expr(visitor, shape, method_call_expr.object); let method = method_call_expr.method_name.to_string(); + let turbofish = rewrite_turbofish(visitor, shape, method_call_expr.generics); let args = format_parens( visitor.config.fn_call_width.into(), visitor.fork(), @@ -84,21 +85,7 @@ pub(crate) fn rewrite( NewlineMode::IfContainsNewLineAndWidth, ); - if let Some(generics) = method_call_expr.generics { - let mut turbofish = "".to_owned(); - for (i, generic) in generics.into_iter().enumerate() { - let generic = rewrite::typ(visitor, shape, generic); - turbofish = if i == 0 { - format!("<{}", generic) - } else { - format!("{turbofish}, {}", generic) - }; - } - turbofish = format!("{turbofish}>"); - format!("{object}.{method}::{turbofish}{args}") - } else { - format!("{object}.{method}{args}") - } + format!("{object}.{method}{turbofish}{args}") } ExpressionKind::MemberAccess(member_access_expr) => { let lhs_str = rewrite_sub_expr(visitor, shape, member_access_expr.lhs); @@ -172,9 +159,13 @@ pub(crate) fn rewrite( visitor.format_if(*if_expr) } - ExpressionKind::Lambda(_) | ExpressionKind::Variable(_, _) => { - visitor.slice(span).to_string() + ExpressionKind::Variable(path, generics) => { + let path_string = visitor.slice(path.span); + + let turbofish = rewrite_turbofish(visitor, shape, generics); + format!("{path_string}{turbofish}") } + ExpressionKind::Lambda(_) => visitor.slice(span).to_string(), ExpressionKind::Quote(block) => format!("quote {}", rewrite_block(visitor, block, span)), ExpressionKind::Comptime(block) => { format!("comptime {}", rewrite_block(visitor, block, span)) @@ -188,3 +179,24 @@ fn rewrite_block(visitor: &FmtVisitor, block: BlockExpression, span: Span) -> St visitor.visit_block(block, span); visitor.finish() } + +fn rewrite_turbofish( + visitor: &FmtVisitor, + shape: Shape, + generics: Option>, +) -> String { + if let Some(generics) = generics { + let mut turbofish = "".to_owned(); + for (i, generic) in generics.into_iter().enumerate() { + let generic = rewrite::typ(visitor, shape, generic); + turbofish = if i == 0 { + format!("::<{}", generic) + } else { + format!("{turbofish}, {}", generic) + }; + } + format!("{turbofish}>") + } else { + "".to_owned() + } +} From b2100ad7c4af4ddca57c23507f149a16510d50aa Mon Sep 17 00:00:00 2001 From: Maxim Vezenov Date: Mon, 20 May 2024 22:09:14 +0100 Subject: [PATCH 42/55] Update compiler/noirc_frontend/src/hir/type_check/expr.rs Co-authored-by: jfecher --- .../noirc_frontend/src/hir/type_check/expr.rs | 39 +++++++++---------- 1 file changed, 18 insertions(+), 21 deletions(-) diff --git a/compiler/noirc_frontend/src/hir/type_check/expr.rs b/compiler/noirc_frontend/src/hir/type_check/expr.rs index d1eae2622ee..8cdeb889ec0 100644 --- a/compiler/noirc_frontend/src/hir/type_check/expr.rs +++ b/compiler/noirc_frontend/src/hir/type_check/expr.rs @@ -236,30 +236,27 @@ impl<'interner> TypeChecker<'interner> { // Automatically add `&mut` if the method expects a mutable reference and // the object is not already one. - match &method_ref { - HirMethodReference::FuncId(func_id) => { - if *func_id != FuncId::dummy_id() { - let function_type = - self.interner.function_meta(func_id).typ.clone(); - self.try_add_mutable_reference_to_object( - &mut method_call, - &function_type, - &mut object_type, - ); - } - } + let func_id = match &method_ref { + HirMethodReference::FuncId(func_id) => *func_id, HirMethodReference::TraitMethodId(method_id, _) => { let id = self.interner.trait_method_id(*method_id); let definition = self.interner.definition(id); - if let DefinitionKind::Function(func_id) = definition.kind { - let function_type = - self.interner.function_meta(&func_id).typ.clone(); - self.try_add_mutable_reference_to_object( - &mut method_call, - &function_type, - &mut object_type, - ); - } + let DefinitionKind::Function(func_id) = definition.kind else { + unreachable!("Expected trait function to be a DefinitionKind::Function") + }; + func_id + } + }; + + if *func_id != FuncId::dummy_id() { + let function_type = + self.interner.function_meta(func_id).typ.clone(); + self.try_add_mutable_reference_to_object( + &mut method_call, + &function_type, + &mut object_type, + ); + } } } From 0d7fb67e8d42c56a83fe399a0be887278e1e98f8 Mon Sep 17 00:00:00 2001 From: Maxim Vezenov Date: Mon, 20 May 2024 22:09:21 +0100 Subject: [PATCH 43/55] Update test_programs/execution_success/trait_method_mut_self/src/main.nr Co-authored-by: jfecher --- .../execution_success/trait_method_mut_self/src/main.nr | 6 ------ 1 file changed, 6 deletions(-) diff --git a/test_programs/execution_success/trait_method_mut_self/src/main.nr b/test_programs/execution_success/trait_method_mut_self/src/main.nr index 98ae7b1d90f..8f2a58ff84e 100644 --- a/test_programs/execution_success/trait_method_mut_self/src/main.nr +++ b/test_programs/execution_success/trait_method_mut_self/src/main.nr @@ -32,12 +32,6 @@ struct AType { x: Field } -impl AType { - fn new(x: Field) -> Self { - AType { x } - } -} - impl SomeTrait for AType { fn set_value(&mut self, new_value: Field) -> () { self.x = new_value; From c045540dba1dfa12c7cf0863baeb0eea49b936e9 Mon Sep 17 00:00:00 2001 From: Maxim Vezenov Date: Mon, 20 May 2024 22:09:26 +0100 Subject: [PATCH 44/55] Update test_programs/execution_success/trait_method_mut_self/src/main.nr Co-authored-by: jfecher --- .../execution_success/trait_method_mut_self/src/main.nr | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test_programs/execution_success/trait_method_mut_self/src/main.nr b/test_programs/execution_success/trait_method_mut_self/src/main.nr index 8f2a58ff84e..4bc20baa6b5 100644 --- a/test_programs/execution_success/trait_method_mut_self/src/main.nr +++ b/test_programs/execution_success/trait_method_mut_self/src/main.nr @@ -2,7 +2,7 @@ use dep::std::hash::Hasher; use dep::std::hash::poseidon2::Poseidon2Hasher; fn main(x: Field, y: pub Field) { - let mut a_mut_ref = AType::new(x); + let mut a_mut_ref = AType { x }; pass_trait_by_value(a_mut_ref, y); assert(a_mut_ref.x == x); From c6aed8c7e03b1ea07aad3d375e712602aa9bc424 Mon Sep 17 00:00:00 2001 From: Maxim Vezenov Date: Mon, 20 May 2024 22:14:49 +0000 Subject: [PATCH 45/55] fetch implicit generic count from node interner --- .../src/elaborator/expressions.rs | 2 +- .../noirc_frontend/src/hir/type_check/expr.rs | 36 +++++++++---------- compiler/noirc_frontend/src/hir_def/expr.rs | 26 ++------------ compiler/noirc_frontend/src/hir_def/types.rs | 18 ++++++---- 4 files changed, 32 insertions(+), 50 deletions(-) diff --git a/compiler/noirc_frontend/src/elaborator/expressions.rs b/compiler/noirc_frontend/src/elaborator/expressions.rs index c2721d85dc2..18ffe3290e9 100644 --- a/compiler/noirc_frontend/src/elaborator/expressions.rs +++ b/compiler/noirc_frontend/src/elaborator/expressions.rs @@ -328,7 +328,7 @@ impl<'context> Elaborator<'context> { // Desugar the method call into a normal, resolved function call // so that the backend doesn't need to worry about methods // TODO: update object_type here? - let ((function_id, function_name), function_call, _) = method_call + let ((function_id, function_name), function_call) = method_call .into_function_call(&method_ref, object_type, location, self.interner); let func_type = self.type_check_variable(function_name, function_id); diff --git a/compiler/noirc_frontend/src/hir/type_check/expr.rs b/compiler/noirc_frontend/src/hir/type_check/expr.rs index a58805478a5..e3d85071ae4 100644 --- a/compiler/noirc_frontend/src/hir/type_check/expr.rs +++ b/compiler/noirc_frontend/src/hir/type_check/expr.rs @@ -249,19 +249,10 @@ impl<'interner> TypeChecker<'interner> { } } - // Need to borrow `generics` here as `method_call` is moved when calling `into_function_call` - // We don't save the expected generic count just yet though as `into_function_call` also accounts - // for any implicit generics such as from a generic impl. - let has_turbofish_generics = method_call.generics.is_some(); // TODO: update object_type here? - let (_, function_call, implicit_generics_count) = method_call + let (_, function_call) = method_call .into_function_call(&method_ref, object_type, location, self.interner); - if has_turbofish_generics { - self.method_call_implicit_generic_counts - .insert(function_call.func, implicit_generics_count); - } - self.interner.replace_expr(expr_id, HirExpression::Call(function_call)); // Type check the new call now that it has been changed from a method call @@ -394,8 +385,19 @@ impl<'interner> TypeChecker<'interner> { _ => 0, }); - let implicit_generic_count = - self.method_call_implicit_generic_counts.get(expr_id).copied().unwrap_or_default(); + // Fetch the count of any implicit generics on the function + // This includes generics + let implicit_generic_count = if generics.is_some() { + let definition_type = self.interner.definition_type(ident.id); + match &definition_type { + Type::Forall(generics, _) => { + generics.len() - function_generic_count + } + _ => 0, + } + } else { + 0 + }; // This instantiates a trait's generics as well which need to be set // when the constraint below is later solved for when the function is @@ -453,19 +455,15 @@ impl<'interner> TypeChecker<'interner> { ) -> (Type, TypeBindings) { match generics { Some(generics) => { - if generics.len() != (function_generic_count + implicit_generic_count) { - // The list of `generics` represents the total number of generics on a function, including implicit generics. - // We calculate any implicit generics earlier when type checking a method call, - // In a user facing error we want to display only the number of function generics - // as this is what the user will actually be specifying. + if generics.len() != function_generic_count { self.errors.push(TypeCheckError::IncorrectTurbofishGenericCount { expected_count: function_generic_count, - actual_count: generics.len() - implicit_generic_count, + actual_count: generics.len(), span, }); typ.instantiate_with_bindings(bindings, self.interner) } else { - typ.instantiate_with(generics) + typ.instantiate_with(generics, self.interner, implicit_generic_count) } } None => typ.instantiate_with_bindings(bindings, self.interner), diff --git a/compiler/noirc_frontend/src/hir_def/expr.rs b/compiler/noirc_frontend/src/hir_def/expr.rs index 46dd0de1917..163ea10ee02 100644 --- a/compiler/noirc_frontend/src/hir_def/expr.rs +++ b/compiler/noirc_frontend/src/hir_def/expr.rs @@ -212,27 +212,7 @@ impl HirMethodCallExpression { object_type: Type, location: Location, interner: &mut NodeInterner, - ) -> ((ExprId, HirIdent), HirCallExpression, usize) { - // Attach any generics which may not be a part of the function call's explicit generics - // This includes types such as implicit generics if the function is part of an impl - let (generics, generics_count) = if let Some(mut function_generics) = self.generics { - let mut generics = Vec::new(); - match object_type.clone() { - Type::Struct(_, mut struct_generics) => { - generics.append(&mut struct_generics); - } - _ => { - // TODO: Support other types that may contain generics such as `TraitAsType` - unreachable!("Only struct method calls are supported"); - } - }; - let implicit_generics_count = generics.len(); - generics.append(&mut function_generics); - (Some(generics), implicit_generics_count) - } else { - (None, 0) - }; - + ) -> ((ExprId, HirIdent), HirCallExpression) { let mut arguments = vec![self.object]; arguments.append(&mut self.arguments); @@ -251,10 +231,10 @@ impl HirMethodCallExpression { } }; let func_var = HirIdent { location, id, impl_kind }; - let func = interner.push_expr(HirExpression::Ident(func_var.clone(), generics)); + let func = interner.push_expr(HirExpression::Ident(func_var.clone(), self.generics)); interner.push_expr_location(func, location.span, location.file); let expr = HirCallExpression { func, arguments, location }; - ((func, func_var), expr, generics_count) + ((func, func_var), expr) } } diff --git a/compiler/noirc_frontend/src/hir_def/types.rs b/compiler/noirc_frontend/src/hir_def/types.rs index 9f5e3a1ea1e..61740167e4a 100644 --- a/compiler/noirc_frontend/src/hir_def/types.rs +++ b/compiler/noirc_frontend/src/hir_def/types.rs @@ -1485,16 +1485,20 @@ impl Type { /// is used and generic substitutions are provided manually by users. /// /// Expects the given type vector to be the same length as the Forall type variables. - pub fn instantiate_with(&self, types: Vec) -> (Type, TypeBindings) { + pub fn instantiate_with(&self, types: Vec, interner: &NodeInterner, implicit_generic_count: usize) -> (Type, TypeBindings) { match self { Type::Forall(typevars, typ) => { - assert_eq!(types.len(), typevars.len(), "Turbofish operator used with incorrect generic count which was not caught by name resolution"); + assert_eq!(types.len() + implicit_generic_count, typevars.len(), "Turbofish operator used with incorrect generic count which was not caught by name resolution"); - let replacements = typevars - .iter() - .zip(types) - .map(|(var, binding)| (var.id(), (var.clone(), binding))) - .collect(); + let mut replacements = typevars[..implicit_generic_count].iter().map(|var| { + let new = interner.next_type_variable(); + (var.id(), (var.clone(), new)) + }).collect::>(); + + // `zip` ends at the shorter iterator so we skip any implicit generics included in the `Forall` type + for (var, binding) in typevars.iter().skip(implicit_generic_count).zip(types) { + replacements.insert(var.id(), (var.clone(), binding)); + } let instantiated = typ.substitute(&replacements); (instantiated, replacements) From 9bf4ac2fd3c910afca2f5bc8a2fe90640f021bb6 Mon Sep 17 00:00:00 2001 From: Maxim Vezenov Date: Mon, 20 May 2024 22:28:09 +0000 Subject: [PATCH 46/55] remove unused method implicit generics map --- Cargo.lock | 1 - compiler/noirc_frontend/Cargo.toml | 1 - .../src/elaborator/expressions.rs | 8 ++++++-- .../noirc_frontend/src/hir/type_check/expr.rs | 16 +++++++++------- .../noirc_frontend/src/hir/type_check/mod.rs | 7 ------- compiler/noirc_frontend/src/hir_def/types.rs | 18 +++++++++++++----- 6 files changed, 28 insertions(+), 23 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 1befb7a6e86..a8c63c032aa 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3170,7 +3170,6 @@ dependencies = [ "base64 0.21.2", "chumsky", "fm", - "fxhash", "im", "iter-extended", "lalrpop", diff --git a/compiler/noirc_frontend/Cargo.toml b/compiler/noirc_frontend/Cargo.toml index e1b67393064..0430d214d53 100644 --- a/compiler/noirc_frontend/Cargo.toml +++ b/compiler/noirc_frontend/Cargo.toml @@ -14,7 +14,6 @@ noirc_arena.workspace = true noirc_errors.workspace = true noirc_printable_type.workspace = true fm.workspace = true -fxhash.workspace = true iter-extended.workspace = true chumsky.workspace = true thiserror.workspace = true diff --git a/compiler/noirc_frontend/src/elaborator/expressions.rs b/compiler/noirc_frontend/src/elaborator/expressions.rs index 18ffe3290e9..75c95c06d09 100644 --- a/compiler/noirc_frontend/src/elaborator/expressions.rs +++ b/compiler/noirc_frontend/src/elaborator/expressions.rs @@ -328,8 +328,12 @@ impl<'context> Elaborator<'context> { // Desugar the method call into a normal, resolved function call // so that the backend doesn't need to worry about methods // TODO: update object_type here? - let ((function_id, function_name), function_call) = method_call - .into_function_call(&method_ref, object_type, location, self.interner); + let ((function_id, function_name), function_call) = method_call.into_function_call( + &method_ref, + object_type, + location, + self.interner, + ); let func_type = self.type_check_variable(function_name, function_id); diff --git a/compiler/noirc_frontend/src/hir/type_check/expr.rs b/compiler/noirc_frontend/src/hir/type_check/expr.rs index e3d85071ae4..a99b1a417ff 100644 --- a/compiler/noirc_frontend/src/hir/type_check/expr.rs +++ b/compiler/noirc_frontend/src/hir/type_check/expr.rs @@ -250,8 +250,12 @@ impl<'interner> TypeChecker<'interner> { } // TODO: update object_type here? - let (_, function_call) = method_call - .into_function_call(&method_ref, object_type, location, self.interner); + let (_, function_call) = method_call.into_function_call( + &method_ref, + object_type, + location, + self.interner, + ); self.interner.replace_expr(expr_id, HirExpression::Call(function_call)); @@ -386,15 +390,13 @@ impl<'interner> TypeChecker<'interner> { }); // Fetch the count of any implicit generics on the function - // This includes generics + // This includes generics let implicit_generic_count = if generics.is_some() { let definition_type = self.interner.definition_type(ident.id); match &definition_type { - Type::Forall(generics, _) => { - generics.len() - function_generic_count - } + Type::Forall(generics, _) => generics.len() - function_generic_count, _ => 0, - } + } } else { 0 }; diff --git a/compiler/noirc_frontend/src/hir/type_check/mod.rs b/compiler/noirc_frontend/src/hir/type_check/mod.rs index 8a658cf82c7..9335b8297fe 100644 --- a/compiler/noirc_frontend/src/hir/type_check/mod.rs +++ b/compiler/noirc_frontend/src/hir/type_check/mod.rs @@ -24,7 +24,6 @@ use crate::{ node_interner::{ExprId, FuncId, GlobalId, NodeInterner}, Type, TypeBindings, }; -use fxhash::FxHashMap as HashMap; pub use self::errors::Source; @@ -43,10 +42,6 @@ pub struct TypeChecker<'interner> { /// This map is used to default any integer type variables at the end of /// a function (before checking trait constraints) if a type wasn't already chosen. type_variables: Vec, - - /// Method calls can have implicit generics from generic impls. - /// These need to be tracked separately for accurate type checking. - method_call_implicit_generic_counts: HashMap, } /// Type checks a function and assigns the @@ -359,7 +354,6 @@ impl<'interner> TypeChecker<'interner> { trait_constraints: Vec::new(), type_variables: Vec::new(), current_function: None, - method_call_implicit_generic_counts: HashMap::default(), } } @@ -377,7 +371,6 @@ impl<'interner> TypeChecker<'interner> { trait_constraints: Vec::new(), type_variables: Vec::new(), current_function: None, - method_call_implicit_generic_counts: HashMap::default(), }; let statement = this.interner.get_global(id).let_statement; this.check_statement(&statement); diff --git a/compiler/noirc_frontend/src/hir_def/types.rs b/compiler/noirc_frontend/src/hir_def/types.rs index 61740167e4a..64b73d3f01b 100644 --- a/compiler/noirc_frontend/src/hir_def/types.rs +++ b/compiler/noirc_frontend/src/hir_def/types.rs @@ -1485,15 +1485,23 @@ impl Type { /// is used and generic substitutions are provided manually by users. /// /// Expects the given type vector to be the same length as the Forall type variables. - pub fn instantiate_with(&self, types: Vec, interner: &NodeInterner, implicit_generic_count: usize) -> (Type, TypeBindings) { + pub fn instantiate_with( + &self, + types: Vec, + interner: &NodeInterner, + implicit_generic_count: usize, + ) -> (Type, TypeBindings) { match self { Type::Forall(typevars, typ) => { assert_eq!(types.len() + implicit_generic_count, typevars.len(), "Turbofish operator used with incorrect generic count which was not caught by name resolution"); - let mut replacements = typevars[..implicit_generic_count].iter().map(|var| { - let new = interner.next_type_variable(); - (var.id(), (var.clone(), new)) - }).collect::>(); + let mut replacements = typevars[..implicit_generic_count] + .iter() + .map(|var| { + let new = interner.next_type_variable(); + (var.id(), (var.clone(), new)) + }) + .collect::>(); // `zip` ends at the shorter iterator so we skip any implicit generics included in the `Forall` type for (var, binding) in typevars.iter().skip(implicit_generic_count).zip(types) { From 22e3cda7914b647acfdfe354dd558543e2e11a0f Mon Sep 17 00:00:00 2001 From: Maxim Vezenov Date: Mon, 20 May 2024 22:47:45 +0000 Subject: [PATCH 47/55] add new line to expected formatter tests --- tooling/nargo_fmt/tests/expected/turbofish_call.nr | 2 +- tooling/nargo_fmt/tests/expected/turbofish_method_call.nr | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/tooling/nargo_fmt/tests/expected/turbofish_call.nr b/tooling/nargo_fmt/tests/expected/turbofish_call.nr index 8d165c8aafc..bcf0df9a969 100644 --- a/tooling/nargo_fmt/tests/expected/turbofish_call.nr +++ b/tooling/nargo_fmt/tests/expected/turbofish_call.nr @@ -5,4 +5,4 @@ fn foo() { some_function(), // Original inner function call another_function() // Original inner function call ); -} \ No newline at end of file +} diff --git a/tooling/nargo_fmt/tests/expected/turbofish_method_call.nr b/tooling/nargo_fmt/tests/expected/turbofish_method_call.nr index ab7022e235d..52fa3db2ac9 100644 --- a/tooling/nargo_fmt/tests/expected/turbofish_method_call.nr +++ b/tooling/nargo_fmt/tests/expected/turbofish_method_call.nr @@ -9,4 +9,4 @@ fn foo() { ) ) ); -} \ No newline at end of file +} From af2c858e9fec78864c46396dccce3ebdd26a1ccd Mon Sep 17 00:00:00 2001 From: Maxim Vezenov Date: Mon, 20 May 2024 22:49:54 +0000 Subject: [PATCH 48/55] fixup after code review --- compiler/noirc_frontend/src/hir/type_check/expr.rs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/compiler/noirc_frontend/src/hir/type_check/expr.rs b/compiler/noirc_frontend/src/hir/type_check/expr.rs index 8cdeb889ec0..2eb44e6af75 100644 --- a/compiler/noirc_frontend/src/hir/type_check/expr.rs +++ b/compiler/noirc_frontend/src/hir/type_check/expr.rs @@ -248,17 +248,15 @@ impl<'interner> TypeChecker<'interner> { } }; - if *func_id != FuncId::dummy_id() { + if func_id != FuncId::dummy_id() { let function_type = - self.interner.function_meta(func_id).typ.clone(); + self.interner.function_meta(&func_id).typ.clone(); self.try_add_mutable_reference_to_object( &mut method_call, &function_type, &mut object_type, ); } - } - } // Need to borrow `generics` here as `method_call` is moved when calling `into_function_call` // We don't save the expected generic count just yet though as `into_function_call` also accounts From aa8e49ebbd346878681e6280400c68c5cd4e6006 Mon Sep 17 00:00:00 2001 From: Maxim Vezenov Date: Mon, 20 May 2024 22:50:43 +0000 Subject: [PATCH 49/55] fmy and clippy --- compiler/noirc_frontend/src/hir/type_check/expr.rs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/compiler/noirc_frontend/src/hir/type_check/expr.rs b/compiler/noirc_frontend/src/hir/type_check/expr.rs index 2eb44e6af75..beac08b0ce5 100644 --- a/compiler/noirc_frontend/src/hir/type_check/expr.rs +++ b/compiler/noirc_frontend/src/hir/type_check/expr.rs @@ -242,15 +242,16 @@ impl<'interner> TypeChecker<'interner> { let id = self.interner.trait_method_id(*method_id); let definition = self.interner.definition(id); let DefinitionKind::Function(func_id) = definition.kind else { - unreachable!("Expected trait function to be a DefinitionKind::Function") + unreachable!( + "Expected trait function to be a DefinitionKind::Function" + ) }; func_id } }; if func_id != FuncId::dummy_id() { - let function_type = - self.interner.function_meta(&func_id).typ.clone(); + let function_type = self.interner.function_meta(&func_id).typ.clone(); self.try_add_mutable_reference_to_object( &mut method_call, &function_type, From e6850e952363613e832be64ecbaf4a22197a30e5 Mon Sep 17 00:00:00 2001 From: Maxim Vezenov Date: Mon, 20 May 2024 22:54:54 +0000 Subject: [PATCH 50/55] update comment --- .../execution_success/trait_method_mut_self/src/main.nr | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test_programs/execution_success/trait_method_mut_self/src/main.nr b/test_programs/execution_success/trait_method_mut_self/src/main.nr index 4bc20baa6b5..fa47fd5d881 100644 --- a/test_programs/execution_success/trait_method_mut_self/src/main.nr +++ b/test_programs/execution_success/trait_method_mut_self/src/main.nr @@ -60,7 +60,7 @@ fn pass_trait_by_mut_ref(a_mut_ref: &mut T, value: Field) where T: SomeTrait } fn hash_simple_array(input: [Field; 2]) -> Field where H: Hasher + Default { - // Check that we can a trait method to instead a trait implementation + // Check that we can call a trait method instead of a trait implementation // TODO: Need to remove the need for this type annotation // TODO: Curently, without the annotation we will get `Expression type is ambiguous` when trying to use the `hasher` let mut hasher: H = H::default(); From bacb36a7447baf627d409b6daa3e02fff25fd721 Mon Sep 17 00:00:00 2001 From: Maxim Vezenov Date: Mon, 20 May 2024 23:00:14 +0000 Subject: [PATCH 51/55] rename to eddsa_verify --- .../standard_library/cryptographic_primitives/eddsa.mdx | 4 ++-- noir_stdlib/src/eddsa.nr | 4 ++-- test_programs/execution_success/eddsa/src/main.nr | 6 ++---- 3 files changed, 6 insertions(+), 8 deletions(-) diff --git a/docs/docs/noir/standard_library/cryptographic_primitives/eddsa.mdx b/docs/docs/noir/standard_library/cryptographic_primitives/eddsa.mdx index c45325265d7..789d26ce426 100644 --- a/docs/docs/noir/standard_library/cryptographic_primitives/eddsa.mdx +++ b/docs/docs/noir/standard_library/cryptographic_primitives/eddsa.mdx @@ -15,12 +15,12 @@ Verifier for EdDSA signatures fn eddsa_poseidon_verify(public_key_x : Field, public_key_y : Field, signature_s: Field, signature_r8_x: Field, signature_r8_y: Field, message: Field) -> bool ``` -It is also possible to specify the hash algorithm used for the signature by using the `eddsa_verify_with_hasher` function by passing a type implementing the Hasher trait with the turbofish operator. +It is also possible to specify the hash algorithm used for the signature by using the `eddsa_verify` function by passing a type implementing the Hasher trait with the turbofish operator. For instance, if you want to use Poseidon2 instead, you can do the following: ```rust use dep::std::hash::poseidon2::Poseidon2Hasher; -eddsa_verify_with_hasher::(pub_key_a.x, pub_key_a.y, s_a, r8_a.x, r8_a.y, msg); +eddsa_verify::(pub_key_a.x, pub_key_a.y, s_a, r8_a.x, r8_a.y, msg); ``` diff --git a/noir_stdlib/src/eddsa.nr b/noir_stdlib/src/eddsa.nr index a658d528258..1ab0158af8f 100644 --- a/noir_stdlib/src/eddsa.nr +++ b/noir_stdlib/src/eddsa.nr @@ -14,7 +14,7 @@ pub fn eddsa_poseidon_verify( signature_r8_y: Field, message: Field ) -> bool { - eddsa_verify_with_hasher::( + eddsa_verify::( pub_key_x, pub_key_y, signature_s, @@ -24,7 +24,7 @@ pub fn eddsa_poseidon_verify( ) } -pub fn eddsa_verify_with_hasher( +pub fn eddsa_verify( pub_key_x: Field, pub_key_y: Field, signature_s: Field, diff --git a/test_programs/execution_success/eddsa/src/main.nr b/test_programs/execution_success/eddsa/src/main.nr index 2b60d8df3a0..ada15d5405c 100644 --- a/test_programs/execution_success/eddsa/src/main.nr +++ b/test_programs/execution_success/eddsa/src/main.nr @@ -2,7 +2,7 @@ use dep::std::compat; use dep::std::ec::consts::te::baby_jubjub; use dep::std::ec::tecurve::affine::Point as TEPoint; use dep::std::hash; -use dep::std::eddsa::{eddsa_to_pub, eddsa_poseidon_verify, eddsa_verify_with_hasher}; +use dep::std::eddsa::{eddsa_to_pub, eddsa_poseidon_verify, eddsa_verify}; use dep::std::hash::poseidon2::Poseidon2Hasher; fn main(msg: pub Field, _priv_key_a: Field, _priv_key_b: Field) { @@ -50,8 +50,6 @@ fn main(msg: pub Field, _priv_key_a: Field, _priv_key_b: Field) { // User A's signature over the message can't be used with another message assert(!eddsa_poseidon_verify(pub_key_a.x, pub_key_a.y, s_a, r8_a.x, r8_a.y, msg + 1)); // Using a different hash should fail - assert( - !eddsa_verify_with_hasher::(pub_key_a.x, pub_key_a.y, s_a, r8_a.x, r8_a.y, msg) - ); + assert(!eddsa_verify::(pub_key_a.x, pub_key_a.y, s_a, r8_a.x, r8_a.y, msg)); } } From bf07ee802e3347db7605cfb8d2ce44383afc2aec Mon Sep 17 00:00:00 2001 From: Maxim Vezenov Date: Tue, 21 May 2024 13:20:40 +0000 Subject: [PATCH 52/55] move where implciit_generic_count is computed --- .../noirc_frontend/src/hir/type_check/expr.rs | 28 +++++-------------- 1 file changed, 7 insertions(+), 21 deletions(-) diff --git a/compiler/noirc_frontend/src/hir/type_check/expr.rs b/compiler/noirc_frontend/src/hir/type_check/expr.rs index a99b1a417ff..2f134c05ec6 100644 --- a/compiler/noirc_frontend/src/hir/type_check/expr.rs +++ b/compiler/noirc_frontend/src/hir/type_check/expr.rs @@ -389,29 +389,10 @@ impl<'interner> TypeChecker<'interner> { _ => 0, }); - // Fetch the count of any implicit generics on the function - // This includes generics - let implicit_generic_count = if generics.is_some() { - let definition_type = self.interner.definition_type(ident.id); - match &definition_type { - Type::Forall(generics, _) => generics.len() - function_generic_count, - _ => 0, - } - } else { - 0 - }; - // This instantiates a trait's generics as well which need to be set // when the constraint below is later solved for when the function is // finished. How to link the two? - let (typ, bindings) = self.instantiate( - t, - bindings, - generics, - function_generic_count, - implicit_generic_count, - span, - ); + let (typ, bindings) = self.instantiate(t, bindings, generics, function_generic_count, span); // Push any trait constraints required by this definition to the context // to be checked later when the type of this variable is further constrained. @@ -452,7 +433,6 @@ impl<'interner> TypeChecker<'interner> { bindings: TypeBindings, generics: Option>, function_generic_count: usize, - implicit_generic_count: usize, span: Span, ) -> (Type, TypeBindings) { match generics { @@ -465,6 +445,12 @@ impl<'interner> TypeChecker<'interner> { }); typ.instantiate_with_bindings(bindings, self.interner) } else { + // Fetch the count of any implicit generics on the function, such as + // for a method within a generic impl. + let implicit_generic_count = match &typ { + Type::Forall(generics, _) => generics.len() - function_generic_count, + _ => 0, + }; typ.instantiate_with(generics, self.interner, implicit_generic_count) } } From a9cb6c4f04f3f8ca79ec76bae2049f82fbc885b7 Mon Sep 17 00:00:00 2001 From: Maxim Vezenov Date: Tue, 21 May 2024 13:53:45 +0000 Subject: [PATCH 53/55] switch to single loop in instantiate_with --- compiler/noirc_frontend/src/hir_def/types.rs | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/compiler/noirc_frontend/src/hir_def/types.rs b/compiler/noirc_frontend/src/hir_def/types.rs index 64b73d3f01b..cf9aafbb308 100644 --- a/compiler/noirc_frontend/src/hir_def/types.rs +++ b/compiler/noirc_frontend/src/hir_def/types.rs @@ -1495,18 +1495,18 @@ impl Type { Type::Forall(typevars, typ) => { assert_eq!(types.len() + implicit_generic_count, typevars.len(), "Turbofish operator used with incorrect generic count which was not caught by name resolution"); - let mut replacements = typevars[..implicit_generic_count] + let replacements = typevars .iter() - .map(|var| { - let new = interner.next_type_variable(); - (var.id(), (var.clone(), new)) + .enumerate() + .map(|(i, var)| { + let binding = if i < implicit_generic_count { + interner.next_type_variable() + } else { + types[i - implicit_generic_count].clone() + }; + (var.id(), (var.clone(), binding)) }) - .collect::>(); - - // `zip` ends at the shorter iterator so we skip any implicit generics included in the `Forall` type - for (var, binding) in typevars.iter().skip(implicit_generic_count).zip(types) { - replacements.insert(var.id(), (var.clone(), binding)); - } + .collect(); let instantiated = typ.substitute(&replacements); (instantiated, replacements) From daf151ba480e9b3bbd3a4e8551e1b4edc0bdbf40 Mon Sep 17 00:00:00 2001 From: Maxim Vezenov Date: Tue, 21 May 2024 14:30:13 +0000 Subject: [PATCH 54/55] rename to turbofish_generics --- compiler/noirc_frontend/src/hir/type_check/expr.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/compiler/noirc_frontend/src/hir/type_check/expr.rs b/compiler/noirc_frontend/src/hir/type_check/expr.rs index 2f134c05ec6..c4f80591bf8 100644 --- a/compiler/noirc_frontend/src/hir/type_check/expr.rs +++ b/compiler/noirc_frontend/src/hir/type_check/expr.rs @@ -431,16 +431,16 @@ impl<'interner> TypeChecker<'interner> { &mut self, typ: Type, bindings: TypeBindings, - generics: Option>, + turbofish_generics: Option>, function_generic_count: usize, span: Span, ) -> (Type, TypeBindings) { - match generics { - Some(generics) => { - if generics.len() != function_generic_count { + match turbofish_generics { + Some(turbofish_generics) => { + if turbofish_generics.len() != function_generic_count { self.errors.push(TypeCheckError::IncorrectTurbofishGenericCount { expected_count: function_generic_count, - actual_count: generics.len(), + actual_count: turbofish_generics.len(), span, }); typ.instantiate_with_bindings(bindings, self.interner) @@ -451,7 +451,7 @@ impl<'interner> TypeChecker<'interner> { Type::Forall(generics, _) => generics.len() - function_generic_count, _ => 0, }; - typ.instantiate_with(generics, self.interner, implicit_generic_count) + typ.instantiate_with(turbofish_generics, self.interner, implicit_generic_count) } } None => typ.instantiate_with_bindings(bindings, self.interner), From 0123ac8531bdc535631b0b425bc27c5fa4d90fe8 Mon Sep 17 00:00:00 2001 From: Maxim Vezenov Date: Tue, 21 May 2024 19:35:34 +0000 Subject: [PATCH 55/55] delete duplicated function-generic_count --- compiler/noirc_frontend/src/hir/type_check/expr.rs | 9 --------- 1 file changed, 9 deletions(-) diff --git a/compiler/noirc_frontend/src/hir/type_check/expr.rs b/compiler/noirc_frontend/src/hir/type_check/expr.rs index 781ed2c25b8..abff466e1d5 100644 --- a/compiler/noirc_frontend/src/hir/type_check/expr.rs +++ b/compiler/noirc_frontend/src/hir/type_check/expr.rs @@ -388,15 +388,6 @@ impl<'interner> TypeChecker<'interner> { // We must instantiate identifiers at every call site to replace this T with a new type // variable to handle generic functions. let t = self.interner.id_type_substitute_trait_as_type(ident.id); - let span = self.interner.expr_span(expr_id); - - let definition = self.interner.try_definition(ident.id); - let function_generic_count = definition.map_or(0, |definition| match &definition.kind { - DefinitionKind::Function(function) => { - self.interner.function_modifiers(function).generic_count - } - _ => 0, - }); let span = self.interner.expr_span(expr_id);