From 82c335d3e36365695eccc1c4af63e58dd0633328 Mon Sep 17 00:00:00 2001 From: Maxim Vezenov Date: Tue, 9 Jul 2024 11:20:57 -0400 Subject: [PATCH] fix: Account for the expected kind when resolving turbofish generics (#5448) # Description ## Problem\* Resolves #5442 ## Summary\* In #5155 we make sure that we make sure we resolve against the correct type kind in various places such as when resolving trait bound generics or trait impl generics. This was not done for function calls. Now when resolving turbofish generics, rather than calling `resolve_type` in all cases aside type expressions, we check whether the variable we are elaborating is a function and fetch its already resolved generics and pass that in as the expected kind. To keep the resolution logic the simple (I just zipped together the `direct_generics` from the function meta and the user specified turbofish generics), as `zip` uses the shortest iterator, I also added two checks for whether we have the correct turbofish count. We now have multiple of these checks rather than in just `instantiate`, but I felt it was the simplest logic and would lead to the least potential future confusion. I also have expanded the bug identified in the issue to also include turbofish generics for method calls. I have also included this as part of my test for this PR. ## Additional Context ## Documentation\* Check one: - [X] No documentation needed. - [ ] Documentation included in this PR. - [ ] **[For Experimental Features]** Documentation to be submitted in a separate PR. # PR Checklist\* - [X] I have tested the changes locally. - [X] I have formatted the changes with [Prettier](https://prettier.io/) and/or `cargo fmt` on default settings. --------- Co-authored-by: jfecher --- .../src/elaborator/expressions.rs | 31 ++--- compiler/noirc_frontend/src/elaborator/mod.rs | 3 +- .../noirc_frontend/src/elaborator/patterns.rs | 46 ++++++- .../src/hir/resolution/resolver.rs | 3 +- .../noirc_frontend/src/hir/type_check/mod.rs | 8 +- .../noirc_frontend/src/hir_def/function.rs | 6 +- compiler/noirc_frontend/src/tests.rs | 126 ++++++++++++++++++ 7 files changed, 186 insertions(+), 37 deletions(-) diff --git a/compiler/noirc_frontend/src/elaborator/expressions.rs b/compiler/noirc_frontend/src/elaborator/expressions.rs index 5c3866816a6..d4e0eb9fe0b 100644 --- a/compiler/noirc_frontend/src/elaborator/expressions.rs +++ b/compiler/noirc_frontend/src/elaborator/expressions.rs @@ -29,7 +29,7 @@ use crate::{ }, node_interner::{DefinitionKind, ExprId, FuncId, ReferenceId}, token::Tokens, - Kind, QuotedType, Shared, StructType, Type, + QuotedType, Shared, StructType, Type, }; use super::Elaborator; @@ -51,23 +51,7 @@ 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| { - // All type expressions should resolve to a `Type::Constant` - if generic.is_type_expression() { - self.resolve_type_inner( - generic, - &Kind::Numeric(Box::new(Type::default_int_type())), - ) - } else { - self.resolve_type(generic) - } - }) - .collect() - }); - return self.elaborate_variable(variable, generics); + return self.elaborate_variable(variable, generics) } ExpressionKind::Tuple(tuple) => self.elaborate_tuple(tuple), ExpressionKind::Lambda(lambda) => self.elaborate_lambda(*lambda), @@ -342,14 +326,18 @@ impl<'context> Elaborator<'context> { } }; - if func_id != FuncId::dummy_id() { + let generics = if func_id != FuncId::dummy_id() { let function_type = self.interner.function_meta(&func_id).typ.clone(); self.try_add_mutable_reference_to_object( &function_type, &mut object_type, &mut object, ); - } + + self.resolve_turbofish_generics(&func_id, method_call.generics, span) + } else { + None + }; // These arguments will be given to the desugared function call. // Compared to the method arguments, they also contain the object. @@ -367,9 +355,6 @@ 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 turbofish_generics = generics.clone(); let method_call = HirMethodCallExpression { method, object, arguments, location, generics }; diff --git a/compiler/noirc_frontend/src/elaborator/mod.rs b/compiler/noirc_frontend/src/elaborator/mod.rs index 7d436c01ebe..b6c4b7a7393 100644 --- a/compiler/noirc_frontend/src/elaborator/mod.rs +++ b/compiler/noirc_frontend/src/elaborator/mod.rs @@ -702,8 +702,7 @@ impl<'context> Elaborator<'context> { let direct_generics = func.def.generics.iter(); let direct_generics = direct_generics - .filter_map(|generic| self.find_generic(&generic.ident().0.contents)) - .map(|ResolvedGeneric { name, type_var, .. }| (name.clone(), type_var.clone())) + .filter_map(|generic| self.find_generic(&generic.ident().0.contents).cloned()) .collect(); let statements = std::mem::take(&mut func.def.body.statements); diff --git a/compiler/noirc_frontend/src/elaborator/patterns.rs b/compiler/noirc_frontend/src/elaborator/patterns.rs index 81fffb522bf..45b7e2d498a 100644 --- a/compiler/noirc_frontend/src/elaborator/patterns.rs +++ b/compiler/noirc_frontend/src/elaborator/patterns.rs @@ -3,7 +3,7 @@ use noirc_errors::{Location, Span}; use rustc_hash::FxHashSet as HashSet; use crate::{ - ast::ERROR_IDENT, + ast::{UnresolvedType, ERROR_IDENT}, hir::{ comptime::Interpreter, def_collector::dc_crate::CompilationError, @@ -15,7 +15,9 @@ use crate::{ stmt::HirPattern, }, macros_api::{HirExpression, Ident, Path, Pattern}, - node_interner::{DefinitionId, DefinitionKind, ExprId, GlobalId, ReferenceId, TraitImplKind}, + node_interner::{ + DefinitionId, DefinitionKind, ExprId, FuncId, GlobalId, ReferenceId, TraitImplKind, + }, Shared, StructType, Type, TypeBindings, }; @@ -398,15 +400,53 @@ impl<'context> Elaborator<'context> { } } + /// Resolve generics using the expected kinds of the function we are calling + pub(super) fn resolve_turbofish_generics( + &mut self, + func_id: &FuncId, + unresolved_turbofish: Option>, + span: Span, + ) -> Option> { + let direct_generics = self.interner.function_meta(func_id).direct_generics.clone(); + + unresolved_turbofish.map(|option_inner| { + if option_inner.len() != direct_generics.len() { + let type_check_err = TypeCheckError::IncorrectTurbofishGenericCount { + expected_count: direct_generics.len(), + actual_count: option_inner.len(), + span, + }; + self.push_err(type_check_err); + } + + let generics_with_types = direct_generics.iter().zip(option_inner); + vecmap(generics_with_types, |(generic, unresolved_type)| { + self.resolve_type_inner(unresolved_type, &generic.kind) + }) + }) + } + pub(super) fn elaborate_variable( &mut self, variable: Path, - generics: Option>, + unresolved_turbofish: Option>, ) -> (ExprId, Type) { let span = variable.span; let expr = self.resolve_variable(variable); let definition_id = expr.id; + let definition_kind = + self.interner.try_definition(definition_id).map(|definition| definition.kind.clone()); + + // Resolve any generics if we the variable we have resolved is a function + // and if the turbofish operator was used. + let generics = definition_kind.and_then(|definition_kind| match &definition_kind { + DefinitionKind::Function(function) => { + self.resolve_turbofish_generics(function, unresolved_turbofish, span) + } + _ => None, + }); + let id = self.interner.push_expr(HirExpression::Ident(expr.clone(), generics.clone())); self.interner.push_expr_location(id, span, self.file); diff --git a/compiler/noirc_frontend/src/hir/resolution/resolver.rs b/compiler/noirc_frontend/src/hir/resolution/resolver.rs index 364d694462b..793362bb3d6 100644 --- a/compiler/noirc_frontend/src/hir/resolution/resolver.rs +++ b/compiler/noirc_frontend/src/hir/resolution/resolver.rs @@ -1096,8 +1096,7 @@ impl<'a> Resolver<'a> { let direct_generics = func.def.generics.iter(); let direct_generics = direct_generics - .filter_map(|generic| self.find_generic(&generic.ident().0.contents)) - .map(|ResolvedGeneric { name, type_var, .. }| (name.clone(), type_var.clone())) + .filter_map(|generic| self.find_generic(&generic.ident().0.contents).cloned()) .collect(); FuncMeta { diff --git a/compiler/noirc_frontend/src/hir/type_check/mod.rs b/compiler/noirc_frontend/src/hir/type_check/mod.rs index a9a51b636a8..1a70bade863 100644 --- a/compiler/noirc_frontend/src/hir/type_check/mod.rs +++ b/compiler/noirc_frontend/src/hir/type_check/mod.rs @@ -22,7 +22,7 @@ use crate::{ traits::TraitConstraint, }, node_interner::{ExprId, FuncId, GlobalId, NodeInterner}, - Kind, Type, TypeBindings, + Kind, ResolvedGeneric, Type, TypeBindings, }; pub use self::errors::Source; @@ -281,8 +281,10 @@ pub(crate) fn check_trait_impl_method_matches_declaration( } // Substitute each generic on the trait function with the corresponding generic on the impl function - for ((_, trait_fn_generic), (name, impl_fn_generic)) in - trait_fn_meta.direct_generics.iter().zip(&meta.direct_generics) + for ( + ResolvedGeneric { type_var: trait_fn_generic, .. }, + ResolvedGeneric { name, type_var: impl_fn_generic, .. }, + ) in trait_fn_meta.direct_generics.iter().zip(&meta.direct_generics) { let arg = Type::NamedGeneric(impl_fn_generic.clone(), name.clone(), Kind::Normal); bindings.insert(trait_fn_generic.id(), (trait_fn_generic.clone(), arg)); diff --git a/compiler/noirc_frontend/src/hir_def/function.rs b/compiler/noirc_frontend/src/hir_def/function.rs index a4a9f855c62..fa8bb55abee 100644 --- a/compiler/noirc_frontend/src/hir_def/function.rs +++ b/compiler/noirc_frontend/src/hir_def/function.rs @@ -1,8 +1,6 @@ use iter_extended::vecmap; use noirc_errors::{Location, Span}; -use std::rc::Rc; - use super::expr::{HirBlockExpression, HirExpression, HirIdent}; use super::stmt::HirPattern; use super::traits::TraitConstraint; @@ -10,7 +8,7 @@ use crate::ast::{FunctionKind, FunctionReturnType, Visibility}; use crate::graph::CrateId; use crate::macros_api::BlockExpression; use crate::node_interner::{ExprId, NodeInterner, TraitImplId}; -use crate::{ResolvedGeneric, Type, TypeVariable}; +use crate::{ResolvedGeneric, Type}; /// A Hir function is a block expression /// with a list of statements @@ -113,7 +111,7 @@ pub struct FuncMeta { /// This does not include generics from an outer scope, like those introduced by /// an `impl` block. This also does not include implicit generics added by the compiler /// such as a trait's `Self` type variable. - pub direct_generics: Vec<(Rc, TypeVariable)>, + pub direct_generics: Vec, /// All the generics used by this function, which includes any implicit generics or generics /// from outer scopes, such as those introduced by an impl. diff --git a/compiler/noirc_frontend/src/tests.rs b/compiler/noirc_frontend/src/tests.rs index 10183ae0ac9..70f8f785d68 100644 --- a/compiler/noirc_frontend/src/tests.rs +++ b/compiler/noirc_frontend/src/tests.rs @@ -1455,6 +1455,79 @@ fn specify_method_types_with_turbofish() { assert_eq!(errors.len(), 0); } +#[test] +fn incorrect_turbofish_count_function_call() { + 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()) + } + + fn main() { + let _ = generic_func::(); + } + "#; + let errors = get_program_errors(src); + assert_eq!(errors.len(), 1); + assert!(matches!( + errors[0].0, + CompilationError::TypeError(TypeCheckError::IncorrectTurbofishGenericCount { .. }), + )); +} + +#[test] +fn incorrect_turbofish_count_method_call() { + 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) -> U where U: Default { + U::default() + } + } + + fn main() { + let foo: Foo = Foo { inner: 1 }; + let _ = foo.generic_method::(); + } + "#; + let errors = get_program_errors(src); + assert_eq!(errors.len(), 1); + assert!(matches!( + errors[0].0, + CompilationError::TypeError(TypeCheckError::IncorrectTurbofishGenericCount { .. }), + )); +} + #[test] fn struct_numeric_generic_in_function() { let src = r#" @@ -1983,3 +2056,56 @@ fn underflowing_i8() { panic!("Expected OverflowingAssignment error, got {:?}", errors[0].0); } } + +#[test] +fn turbofish_numeric_generic_nested_call() { + // Check for turbofish numeric generics used with function calls + let src = r#" + fn foo() -> [u8; N] { + [0; N] + } + + fn bar() -> [u8; N] { + foo::() + } + + global M: u32 = 3; + + fn main() { + let _ = bar::(); + } + "#; + let errors = get_program_errors(src); + assert!(errors.is_empty()); + + // Check for turbofish numeric generics used with method calls + let src = r#" + struct Foo { + a: T + } + + impl Foo { + fn static_method() -> [u8; N] { + [0; N] + } + + fn impl_method(self) -> [T; N] { + [self.a; N] + } + } + + fn bar() -> [u8; N] { + let _ = Foo::static_method::(); + let x: Foo = Foo { a: 0 }; + x.impl_method::() + } + + global M: u32 = 3; + + fn main() { + let _ = bar::(); + } + "#; + let errors = get_program_errors(src); + assert!(errors.is_empty()); +}