Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Allow associated types to be ellided from trait constraints #7026

Merged
merged 14 commits into from
Jan 14, 2025
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
137 changes: 113 additions & 24 deletions compiler/noirc_frontend/src/elaborator/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -315,7 +315,7 @@

self.define_function_metas(&mut items.functions, &mut items.impls, &mut items.trait_impls);

self.collect_traits(&items.traits);
self.collect_traits(&mut items.traits);

// Before we resolve any function symbols we must go through our impls and
// re-collect the methods within into their proper module. This cannot be
Expand Down Expand Up @@ -354,6 +354,7 @@
self.current_trait = Some(trait_id);
self.elaborate_functions(unresolved_trait.fns_with_default_impl);
}

self.current_trait = None;

for impls in items.impls.into_values() {
Expand Down Expand Up @@ -475,7 +476,7 @@
self.add_existing_variable_to_scope(name, parameter.clone(), warn_if_unused);
}

self.add_trait_constraints_to_scope(&func_meta);
self.add_trait_constraints_to_scope(&func_meta.trait_constraints, func_meta.location.span);

let (hir_func, body_type) = match kind {
FunctionKind::Builtin
Expand All @@ -501,7 +502,7 @@
// when multiple impls are available. Instead we default first to choose the Field or u64 impl.
self.check_and_pop_function_context();

self.remove_trait_constraints_from_scope(&func_meta);
self.remove_trait_constraints_from_scope(&func_meta.trait_constraints);

let func_scope_tree = self.scopes.end_function();

Expand Down Expand Up @@ -733,24 +734,98 @@
None
}

/// TODO: This is currently only respected for generic free functions
/// there's a bunch of other places where trait constraints can pop up
fn resolve_trait_constraints(
&mut self,
where_clause: &[UnresolvedTraitConstraint],
add_constraints_to_scope: bool,
jfecher marked this conversation as resolved.
Show resolved Hide resolved
) -> Vec<TraitConstraint> {
where_clause
.iter()
.filter_map(|constraint| self.resolve_trait_constraint(constraint))
.filter_map(|constraint| {
self.resolve_trait_constraint(constraint, add_constraints_to_scope)
})
.collect()
}

/// Expands any traits in a where clause to mention all associated types if they were
/// ellided by the user. See `add_missing_named_generics` for more detail.

Check warning on line 751 in compiler/noirc_frontend/src/elaborator/mod.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (ellided)
michaeljklein marked this conversation as resolved.
Show resolved Hide resolved
jfecher marked this conversation as resolved.
Show resolved Hide resolved
///
/// Returns all newly created generics to be added to this function/trait/impl.
fn desugar_trait_constraints(
&mut self,
where_clause: &mut [UnresolvedTraitConstraint],
) -> Vec<ResolvedGeneric> {
where_clause
.iter_mut()
.flat_map(|constraint| self.add_missing_named_generics(&mut constraint.trait_bound))
.collect()
}

pub fn resolve_trait_constraint(
/// For each associated type that isn't mentioned in a trait bound, this adds
/// the type as an implicit generic to the where clause and returns the newly
/// created generics in a vector to add to the function/trait/impl later.
/// For example, this will turn a function using a trait with 2 associated types:
///
/// `fn foo<T>() where T: Foo { ... }`
///
/// into:
/// `fn foo<T>() where T: Foo<Bar = A, Baz = B> { ... }`
///
/// with a vector of `<A, B>` returned so that the caller can then modify the function to:
/// `fn foo<T, A, B>() where T: Foo<Bar = A, Baz = B> { ... }`
fn add_missing_named_generics(&mut self, bound: &mut TraitBound) -> Vec<ResolvedGeneric> {
let mut added_generics = Vec::new();

let Some(the_trait) = self.lookup_trait_or_error(bound.trait_path.clone()) else {
return added_generics;
};

if the_trait.associated_types.len() > bound.trait_generics.named_args.len() {
for associated_type in the_trait.associated_types.clone() {
jfecher marked this conversation as resolved.
Show resolved Hide resolved
if !bound
.trait_generics
.named_args
.iter()
.any(|(name, _)| name.0.contents == *associated_type.name.as_ref())
{
// This generic isn't contained in the bound's named arguments,
// so add it by creating a fresh type variable.
let new_generic_id = self.interner.next_type_variable_id();
let type_var = TypeVariable::unbound(new_generic_id, Kind::Normal);
jfecher marked this conversation as resolved.
Show resolved Hide resolved

let span = bound.trait_path.span;
let no_name = Rc::new("_".to_string());
jfecher marked this conversation as resolved.
Show resolved Hide resolved
let typ = Type::NamedGeneric(type_var.clone(), no_name.clone());
let typ = self.interner.push_quoted_type(typ);
let typ = UnresolvedTypeData::Resolved(typ).with_span(span);
let ident = Ident::new(associated_type.name.as_ref().clone(), span);

bound.trait_generics.named_args.push((ident, typ));
added_generics.push(ResolvedGeneric { name: no_name, span, type_var });
}
}
}

added_generics
}

fn resolve_trait_constraint(
&mut self,
constraint: &UnresolvedTraitConstraint,
add_constraints_to_scope: bool,
) -> Option<TraitConstraint> {
let typ = self.resolve_type(constraint.typ.clone());
let trait_bound = self.resolve_trait_bound(&constraint.trait_bound)?;

if add_constraints_to_scope {
self.add_trait_bound_to_scope(
constraint.trait_bound.trait_path.span,
&typ,
&trait_bound,
trait_bound.trait_id,
);
}

Some(TraitConstraint { typ, trait_bound })
}

Expand Down Expand Up @@ -800,10 +875,13 @@
let has_inline_attribute = has_no_predicates_attribute || should_fold;
let is_pub_allowed = self.pub_allowed(func, in_contract);
self.add_generics(&func.def.generics);
let mut generics = vecmap(&self.generics, |generic| generic.type_var.clone());

let mut trait_constraints = self.resolve_trait_constraints(&func.def.where_clause);
let new_generics = self.desugar_trait_constraints(&mut func.def.where_clause);
generics.extend(new_generics.into_iter().map(|generic| generic.type_var));

let mut trait_constraints = self.resolve_trait_constraints(&func.def.where_clause, true);

let mut generics = vecmap(&self.generics, |generic| generic.type_var.clone());
let mut parameters = Vec::new();
let mut parameter_types = Vec::new();
let mut parameter_idents = Vec::new();
Expand Down Expand Up @@ -874,6 +952,9 @@
None
};

// Remove the traits assumed by `resolve_trait_constraints` from scope
self.remove_trait_constraints_from_scope(&trait_constraints);

let meta = FuncMeta {
name: name_ident,
kind: func.kind,
Expand Down Expand Up @@ -1013,10 +1094,10 @@
}
}

fn add_trait_constraints_to_scope(&mut self, func_meta: &FuncMeta) {
for constraint in &func_meta.trait_constraints {
fn add_trait_constraints_to_scope(&mut self, constraints: &[TraitConstraint], span: Span) {
for constraint in constraints {
self.add_trait_bound_to_scope(
func_meta,
span,
&constraint.typ,
&constraint.trait_bound,
constraint.trait_bound.trait_id,
Expand All @@ -1030,16 +1111,16 @@
let self_type =
self.self_type.clone().expect("Expected a self type if there's a current trait");
self.add_trait_bound_to_scope(
func_meta,
span,
&self_type,
&constraint.trait_bound,
constraint.trait_bound.trait_id,
);
}
}

fn remove_trait_constraints_from_scope(&mut self, func_meta: &FuncMeta) {
for constraint in &func_meta.trait_constraints {
fn remove_trait_constraints_from_scope(&mut self, constraints: &[TraitConstraint]) {
for constraint in constraints {
self.interner
.remove_assumed_trait_implementations_for_trait(constraint.trait_bound.trait_id);
}
Expand All @@ -1052,7 +1133,7 @@

fn add_trait_bound_to_scope(
&mut self,
func_meta: &FuncMeta,
span: Span,
object: &Type,
trait_bound: &ResolvedTraitBound,
starting_trait_id: TraitId,
Expand All @@ -1064,7 +1145,6 @@
if let Some(the_trait) = self.interner.try_get_trait(trait_id) {
let trait_name = the_trait.name.to_string();
let typ = object.clone();
let span = func_meta.location.span;
self.push_err(TypeCheckError::UnneededTraitConstraint { trait_name, typ, span });
}
}
Expand All @@ -1081,12 +1161,7 @@

let parent_trait_bound =
self.instantiate_parent_trait_bound(trait_bound, &parent_trait_bound);
self.add_trait_bound_to_scope(
func_meta,
object,
&parent_trait_bound,
starting_trait_id,
);
self.add_trait_bound_to_scope(span, object, &parent_trait_bound, starting_trait_id);
}
}
}
Expand Down Expand Up @@ -1315,7 +1390,8 @@
if let Some(trait_id) = trait_impl.trait_id {
self.generics = trait_impl.resolved_generics.clone();

let where_clause = self.resolve_trait_constraints(&trait_impl.where_clause);
let where_clause = self.resolve_trait_constraints(&trait_impl.where_clause, true);
self.remove_trait_constraints_from_scope(&where_clause);

self.collect_trait_impl_methods(trait_id, trait_impl, &where_clause);

Expand Down Expand Up @@ -1805,6 +1881,17 @@
self.add_generics(&trait_impl.generics);
trait_impl.resolved_generics = self.generics.clone();

let new_generics = self.desugar_trait_constraints(&mut trait_impl.where_clause);
for new_generic in new_generics {
trait_impl.resolved_generics.push(new_generic.clone());
self.generics.push(new_generic);
}

// We need to resolve the where clause before any associated types to be
// able to resolve trait as type syntax, eg. `<T as Foo>` in case there
// is a where constraint for `T: Foo`.
let constraints = self.resolve_trait_constraints(&trait_impl.where_clause, true);

for (_, _, method) in trait_impl.methods.functions.iter_mut() {
// Attach any trait constraints on the impl to the function
method.def.where_clause.append(&mut trait_impl.where_clause.clone());
Expand All @@ -1828,6 +1915,8 @@
trait_impl.resolved_trait_generics = ordered_generics;
self.interner.set_associated_types_for_impl(impl_id, named_generics);

self.remove_trait_constraints_from_scope(&constraints);

let self_type = self.resolve_type(unresolved_type);
self.self_type = Some(self_type.clone());
trait_impl.methods.self_type = Some(self_type);
Expand Down
9 changes: 7 additions & 2 deletions compiler/noirc_frontend/src/elaborator/traits.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ use crate::{
use super::Elaborator;

impl<'context> Elaborator<'context> {
pub fn collect_traits(&mut self, traits: &BTreeMap<TraitId, UnresolvedTrait>) {
pub fn collect_traits(&mut self, traits: &mut BTreeMap<TraitId, UnresolvedTrait>) {
for (trait_id, unresolved_trait) in traits {
self.local_module = unresolved_trait.module_id;

Expand All @@ -39,8 +39,13 @@ impl<'context> Elaborator<'context> {
&resolved_generics,
);

let new_generics =
this.desugar_trait_constraints(&mut unresolved_trait.trait_def.where_clause);
this.generics.extend(new_generics);

let where_clause =
this.resolve_trait_constraints(&unresolved_trait.trait_def.where_clause);
this.resolve_trait_constraints(&unresolved_trait.trait_def.where_clause, true);
this.remove_trait_constraints_from_scope(&where_clause);

// Each associated type in this trait is also an implicit generic
for associated_type in &this.interner.get_trait(*trait_id).associated_types {
Expand Down
14 changes: 10 additions & 4 deletions compiler/noirc_frontend/src/elaborator/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -379,11 +379,13 @@ impl<'context> Elaborator<'context> {
resolved.push(NamedType { name, typ });
}

// Anything that hasn't been removed yet is missing
// Anything that hasn't been removed yet is missing, fill it in to avoid a panic.
// An error should already be issued earlier.
for generic in required_args {
let item = item.item_name(self.interner);
let name = generic.name.clone();
self.push_err(TypeCheckError::MissingNamedTypeArg { item, span, name });
let name = Ident::new(name.as_ref().clone(), span);
let typ = self.interner.next_type_variable();
resolved.push(NamedType { name, typ });
}

resolved
Expand Down Expand Up @@ -1943,7 +1945,7 @@ pub(crate) fn bind_named_generics(
args: &[NamedType],
bindings: &mut TypeBindings,
) {
assert_eq!(params.len(), args.len());
assert!(args.len() <= params.len());

for arg in args {
let i = params
Expand All @@ -1954,6 +1956,10 @@ pub(crate) fn bind_named_generics(
let param = params.swap_remove(i);
bind_generic(&param, &arg.typ, bindings);
}

for unbound_param in params {
bind_generic(&unbound_param, &Type::Error, bindings);
}
}

fn bind_generic(param: &ResolvedGeneric, arg: &Type, bindings: &mut TypeBindings) {
Expand Down
29 changes: 13 additions & 16 deletions docs/docs/noir/concepts/traits.md
Original file line number Diff line number Diff line change
Expand Up @@ -252,36 +252,33 @@ impl MyTrait for Field {
Since associated constants can also be used in a type position, its values are limited to only other
expression kinds allowed in numeric generics.

Note that currently all associated types and constants must be explicitly specified in a trait constraint.
If we leave out any, we'll get an error that we're missing one:
When writing a trait constraint, you can specify all associated types and constants explicitly if
you wish:

```rust
// Error! Constraint is missing associated constant for `Bar`
fn foo<T>(x: T) where T: MyTrait<Foo = i32> {
fn foo<T>(x: T) where T: MyTrait<Foo = i32, Bar = 11> {
...
}
```

Because all associated types and constants must be explicitly specified, they are essentially named generics,
although this is set to change in the future. Future versions of Noir will allow users to elide associated types
in trait constraints similar to Rust. When this is done, you may still refer to their value with the `<Type as Trait>::AssociatedType`
syntax:
Or you can also elide them since there should only be one `Foo` and `Bar` for a given implementation
of `MyTrait` for a type:

```rust
// Only valid in future versions of Noir:
fn foo<T>(x: T) where T: MyTrait {
let _: <T as MyTrait>::Foo = ...;
...
}
```

The type as trait syntax is possible in Noir today but is less useful when each type must be explicitly specified anyway:
If you elide associated types, you can still refer to them via the type as trait syntax `<T as MyTrait>`:

```rust
fn foo<T, F, let B: u32>(x: T) where T: MyTrait<Foo = F, Bar = B> {
// Works, but could just use F directly
let _: <T as MyTrait<Foo = F, Bar = B>>::Foo = ...;

let _: F = ...;
fn foo<T>(x: T) where
T: MyTrait,
<T as MyTrait>::Foo: Default + Eq
{
let foo_value: <T as MyTrait>::Foo = Default::default();
assert_eq(foo_value, foo_value);
}
```

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
[package]
name = "associated_types_implicit"
type = "bin"
authors = [""]

[dependencies]
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
a = [[0, 1], [2, 3]]
Loading
Loading