From cfaa8130d3acc11678067cd062330dd5d79aea5c Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Fri, 14 Feb 2025 14:57:34 -0300 Subject: [PATCH 1/5] feat: allow unquoting TraitConstraint in trait impl position --- compiler/noirc_frontend/src/ast/traits.rs | 6 +- compiler/noirc_frontend/src/ast/visitor.rs | 2 +- .../noirc_frontend/src/elaborator/comptime.rs | 3 +- compiler/noirc_frontend/src/elaborator/mod.rs | 101 +++++++++++++++--- .../noirc_frontend/src/hir/comptime/value.rs | 5 + .../src/hir/def_collector/dc_crate.rs | 7 +- .../src/hir/def_collector/dc_mod.rs | 5 +- .../src/hir/resolution/errors.rs | 9 ++ compiler/noirc_frontend/src/parser/errors.rs | 2 - .../noirc_frontend/src/parser/parser/impls.rs | 89 ++++++++------- .../src/parser/parser/traits.rs | 44 +++++--- .../Nargo.toml | 7 ++ .../Prover.toml | 3 + .../src/main.nr | 24 +++++ .../code_action/implement_missing_members.rs | 6 +- tooling/lsp/src/requests/completion.rs | 8 +- tooling/lsp/src/requests/document_symbol.rs | 34 ++---- tooling/lsp/src/requests/inlay_hint.rs | 2 +- .../src/trait_impl_method_stub_generator.rs | 13 ++- tooling/nargo_fmt/src/formatter/trait_impl.rs | 3 +- 20 files changed, 252 insertions(+), 121 deletions(-) create mode 100644 test_programs/compile_success_empty/unquote_trait_constraint_in_trait_impl_position/Nargo.toml create mode 100644 test_programs/compile_success_empty/unquote_trait_constraint_in_trait_impl_position/Prover.toml create mode 100644 test_programs/compile_success_empty/unquote_trait_constraint_in_trait_impl_position/src/main.nr diff --git a/compiler/noirc_frontend/src/ast/traits.rs b/compiler/noirc_frontend/src/ast/traits.rs index 475e3ff1be9..e5040463d17 100644 --- a/compiler/noirc_frontend/src/ast/traits.rs +++ b/compiler/noirc_frontend/src/ast/traits.rs @@ -69,9 +69,7 @@ pub struct TypeImpl { pub struct NoirTraitImpl { pub impl_generics: UnresolvedGenerics, - pub trait_name: Path, - - pub trait_generics: GenericTypeArgs, + pub r#trait: UnresolvedType, pub object_type: UnresolvedType, @@ -247,7 +245,7 @@ impl Display for NoirTraitImpl { )?; } - write!(f, " {}{} for {}", self.trait_name, self.trait_generics, self.object_type)?; + write!(f, " {} for {}", self.r#trait, self.object_type)?; if !self.where_clause.is_empty() { write!( f, diff --git a/compiler/noirc_frontend/src/ast/visitor.rs b/compiler/noirc_frontend/src/ast/visitor.rs index e40c534c3b9..ad25aa9025e 100644 --- a/compiler/noirc_frontend/src/ast/visitor.rs +++ b/compiler/noirc_frontend/src/ast/visitor.rs @@ -598,7 +598,7 @@ impl NoirTraitImpl { } pub fn accept_children(&self, visitor: &mut impl Visitor) { - self.trait_name.accept(visitor); + self.r#trait.accept(visitor); self.object_type.accept(visitor); for item in &self.items { diff --git a/compiler/noirc_frontend/src/elaborator/comptime.rs b/compiler/noirc_frontend/src/elaborator/comptime.rs index c13c74f44cb..b78787249cb 100644 --- a/compiler/noirc_frontend/src/elaborator/comptime.rs +++ b/compiler/noirc_frontend/src/elaborator/comptime.rs @@ -397,8 +397,7 @@ impl<'context> Elaborator<'context> { generated_items.trait_impls.push(UnresolvedTraitImpl { file_id: self.file, module_id: self.local_module, - trait_generics: trait_impl.trait_generics, - trait_path: trait_impl.trait_name, + r#trait: trait_impl.r#trait, object_type: trait_impl.object_type, methods, generics: trait_impl.impl_generics, diff --git a/compiler/noirc_frontend/src/elaborator/mod.rs b/compiler/noirc_frontend/src/elaborator/mod.rs index a8e722a9205..b2662011e3d 100644 --- a/compiler/noirc_frontend/src/elaborator/mod.rs +++ b/compiler/noirc_frontend/src/elaborator/mod.rs @@ -1458,9 +1458,25 @@ impl<'context> Elaborator<'context> { } let trait_generics = trait_impl.resolved_trait_generics.clone(); + let ident = match &trait_impl.r#trait.typ { + UnresolvedTypeData::Named(trait_path, _, _) => trait_path.last_ident(), + UnresolvedTypeData::Resolved(quoted_type_id) => { + let typ = self.interner.get_quoted_type(*quoted_type_id); + if let Type::TraitAsType(_, name, _) = typ { + Ident::new(name.as_ref().clone(), trait_impl.r#trait.span) + } else { + Ident::new(typ.to_string(), trait_impl.r#trait.span) + } + } + _ => { + // We don't error in this case because an error will be produced later on when + // solving the trait impl trait type + Ident::new(trait_impl.r#trait.to_string(), trait_impl.r#trait.span) + } + }; let resolved_trait_impl = Shared::new(TraitImpl { - ident: trait_impl.trait_path.last_ident(), + ident, typ: self_type.clone(), trait_id, trait_generics, @@ -1960,6 +1976,13 @@ impl<'context> Elaborator<'context> { impls: &mut ImplMap, trait_impls: &mut [UnresolvedTraitImpl], ) { + // Event though the trait type of an impl is `UnresolvedType`, only a couple of types are actually + // valid for a trait impl, namely named types and resolved types that point to a trait constraint. + enum TraitType { + Named(Path, GenericTypeArgs), + Trait(TraitId, TraitGenerics), + } + for function_set in functions { self.define_function_metas_for_functions(function_set); } @@ -1983,7 +2006,31 @@ impl<'context> Elaborator<'context> { self.file = trait_impl.file_id; self.local_module = trait_impl.module_id; - let trait_id = self.resolve_trait_by_path(trait_impl.trait_path.clone()); + let trait_type = match &trait_impl.r#trait.typ { + UnresolvedTypeData::Named(trait_path, trait_generics, _) => { + TraitType::Named(trait_path.clone(), trait_generics.clone()) + } + UnresolvedTypeData::Resolved(quoted_type_id) => { + let typ = self.interner.get_quoted_type(*quoted_type_id); + if let Type::TraitAsType(trait_id, _, trait_generics) = typ { + TraitType::Trait(*trait_id, trait_generics.clone()) + } else { + self.push_err(ResolverError::ExpectedTrait { + span: trait_impl.r#trait.span, + }); + continue; + } + } + _ => { + self.push_err(ResolverError::ExpectedTrait { span: trait_impl.r#trait.span }); + continue; + } + }; + + let trait_id = match &trait_type { + TraitType::Named(trait_path, _) => self.resolve_trait_by_path(trait_path.clone()), + TraitType::Trait(trait_id, _) => Some(*trait_id), + }; trait_impl.trait_id = trait_id; let unresolved_type = trait_impl.object_type.clone(); @@ -2006,14 +2053,39 @@ impl<'context> Elaborator<'context> { method.def.where_clause.append(&mut trait_impl.where_clause.clone()); } - // Add each associated type to the list of named type arguments - let mut trait_generics = trait_impl.trait_generics.clone(); - trait_generics.named_args.extend(self.take_unresolved_associated_types(trait_impl)); - let impl_id = self.interner.next_trait_impl_id(); self.current_trait_impl = Some(impl_id); - let path_span = trait_impl.trait_path.span; + let path_span = match &trait_type { + TraitType::Named(path, _) => path.span, + TraitType::Trait(..) => trait_impl.r#trait.span, + }; + + let mut trait_generics = match &trait_type { + TraitType::Named(_, trait_generics) => trait_generics.clone(), + TraitType::Trait(_, trait_generics) => { + // In order to take associated types into account we turn these resolved generics + // into unresolved ones, but ones that point to solved types. + GenericTypeArgs { + ordered_args: vecmap(&trait_generics.ordered, |typ| { + let quoted_type_id = self.interner.push_quoted_type(typ.clone()); + let typ = UnresolvedTypeData::Resolved(quoted_type_id); + UnresolvedType { typ, span: path_span } + }), + named_args: vecmap(&trait_generics.named, |named_type| { + let quoted_type_id = + self.interner.push_quoted_type(named_type.typ.clone()); + let typ = UnresolvedTypeData::Resolved(quoted_type_id); + (named_type.name.clone(), UnresolvedType { typ, span: path_span }) + }), + kinds: Vec::new(), + } + } + }; + + // Add each associated type to the list of named type arguments + trait_generics.named_args.extend(self.take_unresolved_associated_types(trait_impl)); + let (ordered_generics, named_generics) = trait_impl .trait_id .map(|trait_id| { @@ -2038,12 +2110,15 @@ impl<'context> Elaborator<'context> { self.generics.clear(); if let Some(trait_id) = trait_id { - let trait_name = trait_impl.trait_path.last_ident(); - self.interner.add_trait_reference( - trait_id, - Location::new(trait_name.span(), trait_impl.file_id), - trait_name.is_self_type_name(), - ); + let (span, is_self_type_name) = match &trait_impl.r#trait.typ { + UnresolvedTypeData::Named(trait_path, _, _) => { + let trait_name = trait_path.last_ident(); + (trait_name.span(), trait_name.is_self_type_name()) + } + _ => (trait_impl.r#trait.span, false), + }; + let location = Location::new(span, trait_impl.file_id); + self.interner.add_trait_reference(trait_id, location, is_self_type_name); } } } diff --git a/compiler/noirc_frontend/src/hir/comptime/value.rs b/compiler/noirc_frontend/src/hir/comptime/value.rs index a58a80fbd91..be87b7a8f26 100644 --- a/compiler/noirc_frontend/src/hir/comptime/value.rs +++ b/compiler/noirc_frontend/src/hir/comptime/value.rs @@ -495,6 +495,11 @@ impl Value { Value::UnresolvedType(typ) => { Token::InternedUnresolvedTypeData(interner.push_unresolved_type_data(typ)) } + Value::TraitConstraint(trait_id, generics) => { + let name = Rc::new(interner.get_trait(trait_id).name.0.contents.clone()); + let typ = Type::TraitAsType(trait_id, name, generics); + Token::QuotedType(interner.push_quoted_type(typ)) + } Value::TypedExpr(TypedExpr::ExprId(expr_id)) => Token::UnquoteMarker(expr_id), Value::U1(bool) => Token::Bool(bool), Value::U8(value) => Token::Int((value as u128).into()), diff --git a/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs b/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs index 73c6c5a5dd2..ee52646072c 100644 --- a/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs +++ b/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs @@ -21,8 +21,8 @@ use crate::node_interner::{ }; use crate::ast::{ - ExpressionKind, GenericTypeArgs, Ident, ItemVisibility, LetStatement, Literal, NoirFunction, - NoirStruct, NoirTrait, NoirTypeAlias, Path, PathKind, PathSegment, UnresolvedGenerics, + ExpressionKind, Ident, ItemVisibility, LetStatement, Literal, NoirFunction, NoirStruct, + NoirTrait, NoirTypeAlias, Path, PathKind, PathSegment, UnresolvedGenerics, UnresolvedTraitConstraint, UnresolvedType, UnsupportedNumericGenericType, }; @@ -83,8 +83,7 @@ pub struct UnresolvedTrait { pub struct UnresolvedTraitImpl { pub file_id: FileId, pub module_id: LocalModuleId, - pub trait_generics: GenericTypeArgs, - pub trait_path: Path, + pub r#trait: UnresolvedType, pub object_type: UnresolvedType, pub methods: UnresolvedFunctions, pub generics: UnresolvedGenerics, 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 f6f31638557..ec97bca0b88 100644 --- a/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs +++ b/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs @@ -198,8 +198,6 @@ impl<'a> ModCollector<'a> { let mut errors = Vec::new(); for mut trait_impl in impls { - let trait_name = trait_impl.trait_name.clone(); - let (mut unresolved_functions, associated_types, associated_constants) = collect_trait_impl_items( &mut context.def_interner, @@ -233,12 +231,11 @@ impl<'a> ModCollector<'a> { let unresolved_trait_impl = UnresolvedTraitImpl { file_id: self.file_id, module_id: self.module_id, - trait_path: trait_name, + r#trait: trait_impl.r#trait, methods: unresolved_functions, object_type: trait_impl.object_type, generics: trait_impl.impl_generics, where_clause: trait_impl.where_clause, - trait_generics: trait_impl.trait_generics, associated_constants, associated_types, diff --git a/compiler/noirc_frontend/src/hir/resolution/errors.rs b/compiler/noirc_frontend/src/hir/resolution/errors.rs index 68ac84d42c6..142c3d8975c 100644 --- a/compiler/noirc_frontend/src/hir/resolution/errors.rs +++ b/compiler/noirc_frontend/src/hir/resolution/errors.rs @@ -178,6 +178,8 @@ pub enum ResolverError { }, #[error("`loop` statements are not yet implemented")] LoopNotYetSupported { span: Span }, + #[error("Expected a trait")] + ExpectedTrait { span: Span }, } impl ResolverError { @@ -678,6 +680,13 @@ impl<'a> From<&'a ResolverError> for Diagnostic { *span) } + ResolverError::ExpectedTrait { span } => { + Diagnostic::simple_error( + "Expected a trait".to_string(), + String::new(), + *span) + + } } } } diff --git a/compiler/noirc_frontend/src/parser/errors.rs b/compiler/noirc_frontend/src/parser/errors.rs index 508ed33857e..c80b088af8b 100644 --- a/compiler/noirc_frontend/src/parser/errors.rs +++ b/compiler/noirc_frontend/src/parser/errors.rs @@ -23,8 +23,6 @@ pub enum ParserErrorReason { ExpectedMutAfterAmpersand { found: Token }, #[error("Invalid left-hand side of assignment")] InvalidLeftHandSideOfAssignment, - #[error("Expected trait, found {found}")] - ExpectedTrait { found: String }, #[error("Visibility `{visibility}` is not followed by an item")] VisibilityNotFollowedByAnItem { visibility: ItemVisibility }, #[error("`unconstrained` is not followed by an item")] diff --git a/compiler/noirc_frontend/src/parser/parser/impls.rs b/compiler/noirc_frontend/src/parser/parser/impls.rs index 278c20e1e27..8d9e2058ccc 100644 --- a/compiler/noirc_frontend/src/parser/parser/impls.rs +++ b/compiler/noirc_frontend/src/parser/parser/impls.rs @@ -2,9 +2,9 @@ use noirc_errors::Span; use crate::{ ast::{ - Documented, Expression, ExpressionKind, GenericTypeArgs, Ident, ItemVisibility, - NoirFunction, NoirTraitImpl, Path, TraitImplItem, TraitImplItemKind, TypeImpl, - UnresolvedGeneric, UnresolvedType, UnresolvedTypeData, + Documented, Expression, ExpressionKind, Ident, ItemVisibility, NoirFunction, NoirTraitImpl, + TraitImplItem, TraitImplItemKind, TypeImpl, UnresolvedGeneric, UnresolvedType, + UnresolvedTypeData, }, parser::{labels::ParsingRuleLabel, ParserErrorReason}, token::{Keyword, Token}, @@ -29,24 +29,10 @@ impl<'a> Parser<'a> { let type_span = self.span_since(type_span_start); if self.eat_keyword(Keyword::For) { - if let UnresolvedTypeData::Named(trait_name, trait_generics, _) = object_type.typ { - return Impl::TraitImpl(self.parse_trait_impl( - generics, - trait_generics, - trait_name, - )); - } else { - self.push_error( - ParserErrorReason::ExpectedTrait { found: object_type.typ.to_string() }, - self.current_token_span, - ); - - // Error, but we continue parsing the type and assume this is going to be a regular type impl - self.parse_type(); - }; + Impl::TraitImpl(self.parse_trait_impl(generics, object_type)) + } else { + Impl::Impl(self.parse_type_impl(object_type, type_span, generics)) } - - self.parse_type_impl(object_type, type_span, generics) } /// TypeImpl = 'impl' Generics Type TypeImplBody @@ -55,11 +41,10 @@ impl<'a> Parser<'a> { object_type: UnresolvedType, type_span: Span, generics: Vec, - ) -> Impl { + ) -> TypeImpl { let where_clause = self.parse_where_clause(); let methods = self.parse_type_impl_body(); - - Impl::Impl(TypeImpl { object_type, type_span, generics, where_clause, methods }) + TypeImpl { object_type, type_span, generics, where_clause, methods } } /// TypeImplBody = '{' TypeImplItem* '}' @@ -107,23 +92,14 @@ impl<'a> Parser<'a> { fn parse_trait_impl( &mut self, impl_generics: Vec, - trait_generics: GenericTypeArgs, - trait_name: Path, + r#trait: UnresolvedType, ) -> NoirTraitImpl { let object_type = self.parse_type_or_error(); let where_clause = self.parse_where_clause(); let items = self.parse_trait_impl_body(); let is_synthetic = false; - NoirTraitImpl { - impl_generics, - trait_name, - trait_generics, - object_type, - where_clause, - items, - is_synthetic, - } + NoirTraitImpl { impl_generics, r#trait, object_type, where_clause, items, is_synthetic } } /// TraitImplBody = '{' TraitImplItem* '}' @@ -454,7 +430,12 @@ mod tests { fn parse_empty_trait_impl() { let src = "impl Foo for Field {}"; let trait_impl = parse_trait_impl_no_errors(src); - assert_eq!(trait_impl.trait_name.to_string(), "Foo"); + + let UnresolvedTypeData::Named(trait_name, _, _) = trait_impl.r#trait.typ else { + panic!("Expected name type"); + }; + + assert_eq!(trait_name.to_string(), "Foo"); assert!(matches!(trait_impl.object_type.typ, UnresolvedTypeData::FieldElement)); assert!(trait_impl.items.is_empty()); assert!(trait_impl.impl_generics.is_empty()); @@ -464,7 +445,12 @@ mod tests { fn parse_empty_trait_impl_with_generics() { let src = "impl Foo for Field {}"; let trait_impl = parse_trait_impl_no_errors(src); - assert_eq!(trait_impl.trait_name.to_string(), "Foo"); + + let UnresolvedTypeData::Named(trait_name, _, _) = trait_impl.r#trait.typ else { + panic!("Expected name type"); + }; + + assert_eq!(trait_name.to_string(), "Foo"); assert!(matches!(trait_impl.object_type.typ, UnresolvedTypeData::FieldElement)); assert!(trait_impl.items.is_empty()); assert_eq!(trait_impl.impl_generics.len(), 1); @@ -474,7 +460,12 @@ mod tests { fn parse_trait_impl_with_function() { let src = "impl Foo for Field { fn foo() {} }"; let mut trait_impl = parse_trait_impl_no_errors(src); - assert_eq!(trait_impl.trait_name.to_string(), "Foo"); + + let UnresolvedTypeData::Named(trait_name, _, _) = trait_impl.r#trait.typ else { + panic!("Expected name type"); + }; + + assert_eq!(trait_name.to_string(), "Foo"); assert_eq!(trait_impl.items.len(), 1); let item = trait_impl.items.remove(0).item; @@ -489,15 +480,26 @@ mod tests { fn parse_trait_impl_with_generic_type_args() { let src = "impl Foo for Field { }"; let trait_impl = parse_trait_impl_no_errors(src); - assert_eq!(trait_impl.trait_name.to_string(), "Foo"); - assert!(!trait_impl.trait_generics.is_empty()); + + let UnresolvedTypeData::Named(trait_name, trait_generics, _) = trait_impl.r#trait.typ + else { + panic!("Expected name type"); + }; + + assert_eq!(trait_name.to_string(), "Foo"); + assert!(!trait_generics.is_empty()); } #[test] fn parse_trait_impl_with_type() { let src = "impl Foo for Field { type Foo = i32; }"; let mut trait_impl = parse_trait_impl_no_errors(src); - assert_eq!(trait_impl.trait_name.to_string(), "Foo"); + + let UnresolvedTypeData::Named(trait_name, _, _) = trait_impl.r#trait.typ else { + panic!("Expected name type"); + }; + + assert_eq!(trait_name.to_string(), "Foo"); assert_eq!(trait_impl.items.len(), 1); let item = trait_impl.items.remove(0).item; @@ -512,7 +514,12 @@ mod tests { fn parse_trait_impl_with_let() { let src = "impl Foo for Field { let x: Field = 1; }"; let mut trait_impl = parse_trait_impl_no_errors(src); - assert_eq!(trait_impl.trait_name.to_string(), "Foo"); + + let UnresolvedTypeData::Named(trait_name, _, _) = trait_impl.r#trait.typ else { + panic!("Expected name type"); + }; + + assert_eq!(trait_name.to_string(), "Foo"); assert_eq!(trait_impl.items.len(), 1); let item = trait_impl.items.remove(0).item; diff --git a/compiler/noirc_frontend/src/parser/parser/traits.rs b/compiler/noirc_frontend/src/parser/parser/traits.rs index 6f6a9bab960..53df5cd00b1 100644 --- a/compiler/noirc_frontend/src/parser/parser/traits.rs +++ b/compiler/noirc_frontend/src/parser/parser/traits.rs @@ -92,6 +92,11 @@ impl<'a> Parser<'a> { }) .into(); + let r#trait = UnresolvedType { + typ: UnresolvedTypeData::Named(trait_name, trait_generics, false), + span, + }; + // bounds from trait let mut where_clause = where_clause.clone(); for bound in bounds.clone() { @@ -104,15 +109,7 @@ impl<'a> Parser<'a> { let items = vec![]; let is_synthetic = true; - NoirTraitImpl { - impl_generics, - trait_name, - trait_generics, - object_type, - where_clause, - items, - is_synthetic, - } + NoirTraitImpl { impl_generics, r#trait, object_type, where_clause, items, is_synthetic } }); let noir_trait = NoirTrait { @@ -287,7 +284,7 @@ fn empty_trait( #[cfg(test)] mod tests { use crate::{ - ast::{NoirTrait, NoirTraitImpl, TraitItem}, + ast::{NoirTrait, NoirTraitImpl, TraitItem, UnresolvedTypeData}, parser::{ parser::{ parse_program, @@ -374,9 +371,14 @@ mod tests { assert!(noir_trait_alias.items.is_empty()); assert!(noir_trait_alias.is_alias); - assert_eq!(noir_trait_impl.trait_name.to_string(), "Foo"); + let UnresolvedTypeData::Named(trait_name, trait_generics, _) = noir_trait_impl.r#trait.typ + else { + panic!("Expected name type"); + }; + + assert_eq!(trait_name.to_string(), "Foo"); assert_eq!(noir_trait_impl.impl_generics.len(), 3); - assert_eq!(noir_trait_impl.trait_generics.ordered_args.len(), 2); + assert_eq!(trait_generics.ordered_args.len(), 2); assert_eq!(noir_trait_impl.where_clause.len(), 2); assert_eq!(noir_trait_alias.bounds.len(), 2); assert_eq!(noir_trait_alias.bounds[0].to_string(), "Bar"); @@ -428,9 +430,14 @@ mod tests { assert!(noir_trait_alias.items.is_empty()); assert!(noir_trait_alias.is_alias); - assert_eq!(noir_trait_impl.trait_name.to_string(), "Foo"); + let UnresolvedTypeData::Named(trait_name, trait_generics, _) = noir_trait_impl.r#trait.typ + else { + panic!("Expected name type"); + }; + + assert_eq!(trait_name.to_string(), "Foo"); assert_eq!(noir_trait_impl.impl_generics.len(), 3); - assert_eq!(noir_trait_impl.trait_generics.ordered_args.len(), 2); + assert_eq!(trait_generics.ordered_args.len(), 2); assert_eq!(noir_trait_impl.where_clause.len(), 3); assert_eq!(noir_trait_impl.where_clause[0].to_string(), "A: Z"); assert_eq!(noir_trait_impl.where_clause[1].to_string(), "#T: Bar"); @@ -557,9 +564,14 @@ mod tests { assert_eq!(noir_trait_alias.to_string(), "trait Foo = Bar + Baz;"); assert!(noir_trait_alias.is_alias); - assert_eq!(noir_trait_impl.trait_name.to_string(), "Foo"); + let UnresolvedTypeData::Named(trait_name, trait_generics, _) = noir_trait_impl.r#trait.typ + else { + panic!("Expected name type"); + }; + + assert_eq!(trait_name.to_string(), "Foo"); assert_eq!(noir_trait_impl.impl_generics.len(), 1); - assert_eq!(noir_trait_impl.trait_generics.ordered_args.len(), 0); + assert_eq!(trait_generics.ordered_args.len(), 0); assert_eq!(noir_trait_impl.where_clause.len(), 2); assert_eq!(noir_trait_impl.where_clause[0].to_string(), "#T: Bar"); assert_eq!(noir_trait_impl.where_clause[1].to_string(), "#T: Baz"); diff --git a/test_programs/compile_success_empty/unquote_trait_constraint_in_trait_impl_position/Nargo.toml b/test_programs/compile_success_empty/unquote_trait_constraint_in_trait_impl_position/Nargo.toml new file mode 100644 index 00000000000..8651c5577a2 --- /dev/null +++ b/test_programs/compile_success_empty/unquote_trait_constraint_in_trait_impl_position/Nargo.toml @@ -0,0 +1,7 @@ +[package] +name = "unquote_trait_constraint_in_trait_impl_position" +version = "0.1.0" +type = "bin" +authors = [""] + +[dependencies] diff --git a/test_programs/compile_success_empty/unquote_trait_constraint_in_trait_impl_position/Prover.toml b/test_programs/compile_success_empty/unquote_trait_constraint_in_trait_impl_position/Prover.toml new file mode 100644 index 00000000000..9bff601c75a --- /dev/null +++ b/test_programs/compile_success_empty/unquote_trait_constraint_in_trait_impl_position/Prover.toml @@ -0,0 +1,3 @@ +x = "3" +y = "4" +z = "429981696" diff --git a/test_programs/compile_success_empty/unquote_trait_constraint_in_trait_impl_position/src/main.nr b/test_programs/compile_success_empty/unquote_trait_constraint_in_trait_impl_position/src/main.nr new file mode 100644 index 00000000000..92ddf99f1c0 --- /dev/null +++ b/test_programs/compile_success_empty/unquote_trait_constraint_in_trait_impl_position/src/main.nr @@ -0,0 +1,24 @@ +struct Struct {} + +trait Trait { + fn method(self) -> T; +} + +fn main() { + let st = Struct {}; + assert_eq(st.method(), 1); +} + +#[foo] +comptime fn foo(_: FunctionDefinition) -> Quoted { + let tr = quote { Trait }.as_trait_constraint(); + let st = quote { Struct }.as_type(); + quote { + impl $tr for $st { + fn method(self) -> Field { + 1 + } + } + } +} + diff --git a/tooling/lsp/src/requests/code_action/implement_missing_members.rs b/tooling/lsp/src/requests/code_action/implement_missing_members.rs index 1cd181966a2..c29caf79848 100644 --- a/tooling/lsp/src/requests/code_action/implement_missing_members.rs +++ b/tooling/lsp/src/requests/code_action/implement_missing_members.rs @@ -21,7 +21,11 @@ impl<'a> CodeActionFinder<'a> { return; } - let location = Location::new(noir_trait_impl.trait_name.span(), self.file); + let UnresolvedTypeData::Named(trait_name, _, _) = &noir_trait_impl.r#trait.typ else { + return; + }; + + let location = Location::new(trait_name.span(), self.file); let Some(ReferenceId::Trait(trait_id)) = self.interner.find_referenced(location) else { return; }; diff --git a/tooling/lsp/src/requests/completion.rs b/tooling/lsp/src/requests/completion.rs index b464c3e7adc..756885342f1 100644 --- a/tooling/lsp/src/requests/completion.rs +++ b/tooling/lsp/src/requests/completion.rs @@ -1024,7 +1024,7 @@ impl<'a> NodeFinder<'a> { noir_function: &NoirFunction, ) { // First find the trait - let location = Location::new(noir_trait_impl.trait_name.span(), self.file); + let location = Location::new(noir_trait_impl.r#trait.span, self.file); let Some(ReferenceId::Trait(trait_id)) = self.interner.find_referenced(location) else { return; }; @@ -1286,7 +1286,11 @@ impl<'a> Visitor for NodeFinder<'a> { } fn visit_noir_trait_impl(&mut self, noir_trait_impl: &NoirTraitImpl, _: Span) -> bool { - self.find_in_path(&noir_trait_impl.trait_name, RequestedItems::OnlyTypes); + let UnresolvedTypeData::Named(trait_name, _, _) = &noir_trait_impl.r#trait.typ else { + return false; + }; + + self.find_in_path(trait_name, RequestedItems::OnlyTypes); noir_trait_impl.object_type.accept(self); self.type_parameters.clear(); diff --git a/tooling/lsp/src/requests/document_symbol.rs b/tooling/lsp/src/requests/document_symbol.rs index 0fc2dc4622e..b32b2fc7ad7 100644 --- a/tooling/lsp/src/requests/document_symbol.rs +++ b/tooling/lsp/src/requests/document_symbol.rs @@ -10,7 +10,7 @@ use noirc_errors::Span; use noirc_frontend::{ ast::{ Expression, FunctionReturnType, Ident, LetStatement, NoirFunction, NoirStruct, NoirTrait, - NoirTraitImpl, TypeImpl, UnresolvedType, Visitor, + NoirTraitImpl, TypeImpl, UnresolvedType, UnresolvedTypeData, Visitor, }, parser::ParsedSubModule, ParsedModule, @@ -349,32 +349,18 @@ impl<'a> Visitor for DocumentSymbolCollector<'a> { return false; }; - let Some(name_location) = self.to_lsp_location(noir_trait_impl.trait_name.span) else { + let name_span = + if let UnresolvedTypeData::Named(trait_name, _, _) = &noir_trait_impl.r#trait.typ { + trait_name.span + } else { + noir_trait_impl.r#trait.span + }; + + let Some(name_location) = self.to_lsp_location(name_span) else { return false; }; - let mut trait_name = String::new(); - trait_name.push_str(&noir_trait_impl.trait_name.to_string()); - if !noir_trait_impl.trait_generics.is_empty() { - trait_name.push('<'); - for (index, generic) in noir_trait_impl.trait_generics.ordered_args.iter().enumerate() { - if index > 0 { - trait_name.push_str(", "); - } - trait_name.push_str(&generic.to_string()); - } - for (index, (name, generic)) in - noir_trait_impl.trait_generics.named_args.iter().enumerate() - { - if index > 0 { - trait_name.push_str(", "); - } - trait_name.push_str(&name.0.contents); - trait_name.push_str(" = "); - trait_name.push_str(&generic.to_string()); - } - trait_name.push('>'); - } + let trait_name = noir_trait_impl.r#trait.to_string(); let old_symbols = std::mem::take(&mut self.symbols); self.symbols = Vec::new(); diff --git a/tooling/lsp/src/requests/inlay_hint.rs b/tooling/lsp/src/requests/inlay_hint.rs index b9673755da6..12a7cb188a2 100644 --- a/tooling/lsp/src/requests/inlay_hint.rs +++ b/tooling/lsp/src/requests/inlay_hint.rs @@ -327,7 +327,7 @@ impl<'a> Visitor for InlayHintCollector<'a> { fn visit_noir_trait_impl(&mut self, noir_trait_impl: &NoirTraitImpl, span: Span) -> bool { self.show_closing_brace_hint(span, || { - format!(" impl {} for {}", noir_trait_impl.trait_name, noir_trait_impl.object_type) + format!(" impl {} for {}", noir_trait_impl.r#trait, noir_trait_impl.object_type) }); true diff --git a/tooling/lsp/src/trait_impl_method_stub_generator.rs b/tooling/lsp/src/trait_impl_method_stub_generator.rs index 4e505eb5e12..e4d4d7b1dc5 100644 --- a/tooling/lsp/src/trait_impl_method_stub_generator.rs +++ b/tooling/lsp/src/trait_impl_method_stub_generator.rs @@ -1,11 +1,10 @@ use std::collections::BTreeMap; use noirc_frontend::{ - ast::NoirTraitImpl, + ast::{NoirTraitImpl, UnresolvedTypeData}, graph::CrateId, - hir::def_map::ModuleDefId, hir::{ - def_map::{CrateDefMap, ModuleId}, + def_map::{CrateDefMap, ModuleDefId, ModuleId}, type_check::generics::TraitGenerics, }, hir_def::{function::FuncMeta, stmt::HirPattern, traits::Trait}, @@ -300,7 +299,13 @@ impl<'a> TraitImplMethodStubGenerator<'a> { if let Some(index) = generics.iter().position(|generic| generic.type_var.id() == typevar.id()) { - if let Some(typ) = self.noir_trait_impl.trait_generics.ordered_args.get(index) { + let UnresolvedTypeData::Named(_, trait_generics, _) = + &self.noir_trait_impl.r#trait.typ + else { + return; + }; + + if let Some(typ) = trait_generics.ordered_args.get(index) { self.string.push_str(&typ.to_string()); return; } diff --git a/tooling/nargo_fmt/src/formatter/trait_impl.rs b/tooling/nargo_fmt/src/formatter/trait_impl.rs index 5bb9a0d0025..896620c3bf8 100644 --- a/tooling/nargo_fmt/src/formatter/trait_impl.rs +++ b/tooling/nargo_fmt/src/formatter/trait_impl.rs @@ -18,8 +18,7 @@ impl<'a> Formatter<'a> { self.write_keyword(Keyword::Impl); self.format_generics(trait_impl.impl_generics); self.write_space(); - self.format_path(trait_impl.trait_name); - self.format_generic_type_args(trait_impl.trait_generics); + self.format_type(trait_impl.r#trait); self.write_space(); self.write_keyword(Keyword::For); self.write_space(); From f62d093322871032f76d5bb3de93c78955d017d2 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Fri, 14 Feb 2025 15:00:52 -0300 Subject: [PATCH 2/5] Apply suggestions from code review --- compiler/noirc_frontend/src/elaborator/mod.rs | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/compiler/noirc_frontend/src/elaborator/mod.rs b/compiler/noirc_frontend/src/elaborator/mod.rs index b2662011e3d..12d54e750ab 100644 --- a/compiler/noirc_frontend/src/elaborator/mod.rs +++ b/compiler/noirc_frontend/src/elaborator/mod.rs @@ -1462,11 +1462,12 @@ impl<'context> Elaborator<'context> { UnresolvedTypeData::Named(trait_path, _, _) => trait_path.last_ident(), UnresolvedTypeData::Resolved(quoted_type_id) => { let typ = self.interner.get_quoted_type(*quoted_type_id); - if let Type::TraitAsType(_, name, _) = typ { - Ident::new(name.as_ref().clone(), trait_impl.r#trait.span) + let name = if let Type::TraitAsType(_, name, _) = typ { + name.to_string() } else { - Ident::new(typ.to_string(), trait_impl.r#trait.span) - } + typ.to_string() + }; + Ident::new(name, trait_impl.r#trait.span) } _ => { // We don't error in this case because an error will be produced later on when @@ -1976,7 +1977,7 @@ impl<'context> Elaborator<'context> { impls: &mut ImplMap, trait_impls: &mut [UnresolvedTraitImpl], ) { - // Event though the trait type of an impl is `UnresolvedType`, only a couple of types are actually + // Even though the trait type of an impl is `UnresolvedType`, only a couple of types are actually // valid for a trait impl, namely named types and resolved types that point to a trait constraint. enum TraitType { Named(Path, GenericTypeArgs), From a4da6d31752107be526ff2a75b2af99fc9f8e14f Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Fri, 14 Feb 2025 15:31:29 -0300 Subject: [PATCH 3/5] Update grammar --- compiler/noirc_frontend/src/parser/parser/impls.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/noirc_frontend/src/parser/parser/impls.rs b/compiler/noirc_frontend/src/parser/parser/impls.rs index 8d9e2058ccc..4b5054984d4 100644 --- a/compiler/noirc_frontend/src/parser/parser/impls.rs +++ b/compiler/noirc_frontend/src/parser/parser/impls.rs @@ -88,7 +88,7 @@ impl<'a> Parser<'a> { }) } - /// TraitImpl = 'impl' Generics Path GenericTypeArgs 'for' Type TraitImplBody + /// TraitImpl = 'impl' Generics Type 'for' Type TraitImplBody fn parse_trait_impl( &mut self, impl_generics: Vec, From 5d008a7fa13ec74be34f59ea4e492d56d94aab4b Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Tue, 18 Feb 2025 15:47:51 -0300 Subject: [PATCH 4/5] No need for enum TraitType --- compiler/noirc_frontend/src/elaborator/mod.rs | 90 ++++++++----------- 1 file changed, 36 insertions(+), 54 deletions(-) diff --git a/compiler/noirc_frontend/src/elaborator/mod.rs b/compiler/noirc_frontend/src/elaborator/mod.rs index 12d54e750ab..e286d8dd0c7 100644 --- a/compiler/noirc_frontend/src/elaborator/mod.rs +++ b/compiler/noirc_frontend/src/elaborator/mod.rs @@ -11,14 +11,15 @@ use crate::{ }, graph::CrateId, hir::{ - def_collector::dc_crate::{ - filter_literal_globals, CollectedItems, CompilationError, ImplMap, UnresolvedEnum, - UnresolvedFunctions, UnresolvedGlobal, UnresolvedStruct, UnresolvedTraitImpl, - UnresolvedTypeAlias, + def_collector::{ + dc_crate::{ + filter_literal_globals, CollectedItems, CompilationError, ImplMap, UnresolvedEnum, + UnresolvedFunctions, UnresolvedGlobal, UnresolvedStruct, UnresolvedTraitImpl, + UnresolvedTypeAlias, + }, + errors::DefCollectorErrorKind, }, - def_collector::errors::DefCollectorErrorKind, - def_map::{DefMaps, ModuleData}, - def_map::{LocalModuleId, ModuleId, MAIN_FUNCTION}, + def_map::{DefMaps, LocalModuleId, ModuleData, ModuleId, MAIN_FUNCTION}, resolution::errors::ResolverError, scope::ScopeForest as GenericScopeForest, type_check::{generics::TraitGenerics, TypeCheckError}, @@ -1977,13 +1978,6 @@ impl<'context> Elaborator<'context> { impls: &mut ImplMap, trait_impls: &mut [UnresolvedTraitImpl], ) { - // Even though the trait type of an impl is `UnresolvedType`, only a couple of types are actually - // valid for a trait impl, namely named types and resolved types that point to a trait constraint. - enum TraitType { - Named(Path, GenericTypeArgs), - Trait(TraitId, TraitGenerics), - } - for function_set in functions { self.define_function_metas_for_functions(function_set); } @@ -2007,20 +2001,39 @@ impl<'context> Elaborator<'context> { self.file = trait_impl.file_id; self.local_module = trait_impl.module_id; - let trait_type = match &trait_impl.r#trait.typ { + let (trait_id, mut trait_generics, path_span) = match &trait_impl.r#trait.typ { UnresolvedTypeData::Named(trait_path, trait_generics, _) => { - TraitType::Named(trait_path.clone(), trait_generics.clone()) + let trait_id = self.resolve_trait_by_path(trait_path.clone()); + (trait_id, trait_generics.clone(), trait_path.span) } UnresolvedTypeData::Resolved(quoted_type_id) => { let typ = self.interner.get_quoted_type(*quoted_type_id); - if let Type::TraitAsType(trait_id, _, trait_generics) = typ { - TraitType::Trait(*trait_id, trait_generics.clone()) - } else { - self.push_err(ResolverError::ExpectedTrait { - span: trait_impl.r#trait.span, - }); + let span = trait_impl.r#trait.span; + let Type::TraitAsType(trait_id, _, trait_generics) = typ else { + self.push_err(ResolverError::ExpectedTrait { span }); continue; - } + }; + + // In order to take associated types into account we turn these resolved generics + // into unresolved ones, but ones that point to solved types. + let trait_id = *trait_id; + let trait_generics = trait_generics.clone(); + let trait_generics = GenericTypeArgs { + ordered_args: vecmap(&trait_generics.ordered, |typ| { + let quoted_type_id = self.interner.push_quoted_type(typ.clone()); + let typ = UnresolvedTypeData::Resolved(quoted_type_id); + UnresolvedType { typ, span } + }), + named_args: vecmap(&trait_generics.named, |named_type| { + let quoted_type_id = + self.interner.push_quoted_type(named_type.typ.clone()); + let typ = UnresolvedTypeData::Resolved(quoted_type_id); + (named_type.name.clone(), UnresolvedType { typ, span }) + }), + kinds: Vec::new(), + }; + + (Some(trait_id), trait_generics, span) } _ => { self.push_err(ResolverError::ExpectedTrait { span: trait_impl.r#trait.span }); @@ -2028,10 +2041,6 @@ impl<'context> Elaborator<'context> { } }; - let trait_id = match &trait_type { - TraitType::Named(trait_path, _) => self.resolve_trait_by_path(trait_path.clone()), - TraitType::Trait(trait_id, _) => Some(*trait_id), - }; trait_impl.trait_id = trait_id; let unresolved_type = trait_impl.object_type.clone(); @@ -2057,33 +2066,6 @@ impl<'context> Elaborator<'context> { let impl_id = self.interner.next_trait_impl_id(); self.current_trait_impl = Some(impl_id); - let path_span = match &trait_type { - TraitType::Named(path, _) => path.span, - TraitType::Trait(..) => trait_impl.r#trait.span, - }; - - let mut trait_generics = match &trait_type { - TraitType::Named(_, trait_generics) => trait_generics.clone(), - TraitType::Trait(_, trait_generics) => { - // In order to take associated types into account we turn these resolved generics - // into unresolved ones, but ones that point to solved types. - GenericTypeArgs { - ordered_args: vecmap(&trait_generics.ordered, |typ| { - let quoted_type_id = self.interner.push_quoted_type(typ.clone()); - let typ = UnresolvedTypeData::Resolved(quoted_type_id); - UnresolvedType { typ, span: path_span } - }), - named_args: vecmap(&trait_generics.named, |named_type| { - let quoted_type_id = - self.interner.push_quoted_type(named_type.typ.clone()); - let typ = UnresolvedTypeData::Resolved(quoted_type_id); - (named_type.name.clone(), UnresolvedType { typ, span: path_span }) - }), - kinds: Vec::new(), - } - } - }; - // Add each associated type to the list of named type arguments trait_generics.named_args.extend(self.take_unresolved_associated_types(trait_impl)); From 8fa91d5c9065a80a8b726b6e8203ceba00583da4 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Tue, 18 Feb 2025 15:50:04 -0300 Subject: [PATCH 5/5] Show what we found when we expected a trait --- compiler/noirc_frontend/src/elaborator/mod.rs | 7 +++++-- compiler/noirc_frontend/src/hir/resolution/errors.rs | 8 ++++---- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/compiler/noirc_frontend/src/elaborator/mod.rs b/compiler/noirc_frontend/src/elaborator/mod.rs index e286d8dd0c7..d007bee0d8d 100644 --- a/compiler/noirc_frontend/src/elaborator/mod.rs +++ b/compiler/noirc_frontend/src/elaborator/mod.rs @@ -2010,7 +2010,8 @@ impl<'context> Elaborator<'context> { let typ = self.interner.get_quoted_type(*quoted_type_id); let span = trait_impl.r#trait.span; let Type::TraitAsType(trait_id, _, trait_generics) = typ else { - self.push_err(ResolverError::ExpectedTrait { span }); + let found = typ.to_string(); + self.push_err(ResolverError::ExpectedTrait { span, found }); continue; }; @@ -2036,7 +2037,9 @@ impl<'context> Elaborator<'context> { (Some(trait_id), trait_generics, span) } _ => { - self.push_err(ResolverError::ExpectedTrait { span: trait_impl.r#trait.span }); + let span = trait_impl.r#trait.span; + let found = trait_impl.r#trait.typ.to_string(); + self.push_err(ResolverError::ExpectedTrait { span, found }); continue; } }; diff --git a/compiler/noirc_frontend/src/hir/resolution/errors.rs b/compiler/noirc_frontend/src/hir/resolution/errors.rs index 5ae7236cb01..21cedac6c9a 100644 --- a/compiler/noirc_frontend/src/hir/resolution/errors.rs +++ b/compiler/noirc_frontend/src/hir/resolution/errors.rs @@ -180,8 +180,8 @@ pub enum ResolverError { }, #[error("`loop` statements are not yet implemented")] LoopNotYetSupported { span: Span }, - #[error("Expected a trait")] - ExpectedTrait { span: Span }, + #[error("Expected a trait but found {found}")] + ExpectedTrait { found: String, span: Span }, } impl ResolverError { @@ -689,9 +689,9 @@ impl<'a> From<&'a ResolverError> for Diagnostic { *span) } - ResolverError::ExpectedTrait { span } => { + ResolverError::ExpectedTrait { found, span } => { Diagnostic::simple_error( - "Expected a trait".to_string(), + format!("Expected a trait, found {found}"), String::new(), *span)