Skip to content

Commit

Permalink
fix: operator overloading & static trait method references resolving …
Browse files Browse the repository at this point in the history
…to generic impls (#3967)

# Description

## Problem\*

Resolves #3964

## Summary\*

The core problem in #3964 stems from a trait method's generics being
different from the corresponding impl's generics. So when the former was
instantiated, and later tried to apply to the latter during
monomorphization after an impl was selected, the latter would be
unchanged. This was fixed by remembering the `TraitMethodId` during
monomorphization and calling `try_unify` between the trait method's type
and the impl method's type before monomorphizing the impl function. This
gives type bindings to translate between the two.

## Additional Context

- In creating the above type bindings, we need to bind to the original
type variables on the trait without instantiating them. This meant any
further impl searches would see that trait as having its monomorphized
type, rather than its generic type. This would break impl search since
that trait/impl would no longer apply to all the types it used to. I've
fixed this by allowing `Type::substitute` to substitute to already-bound
type variables so that when the impl search later occurs and the type is
instantiated, the instantiation automatically sheds the bindings after
its (currently bound) type variables are swapped to fresh type
variables.

Allowing substitution on already bound type variables is a bit
concerning though. It's unclear if this could cause unsoundness later
on. ~~I'll open an issue to limit the scope of this by only allowing it
during `instantiate` calls instead of all `substitute` calls.~~ I've
just implemented the change directly in this PR instead. I split
`substitute` into `substitute` and `force_substitute`.

- There's also a mostly unrelated change in this PR. `function_meta` now
returns a reference instead of cloning the entire `FunctionMeta` which
was quite wasteful. I'll work on breaking this out into a separate PR.
(Edit: #3968)

## Documentation\*

Check one:
- [x] No documentation needed.
- [ ] Documentation included in this PR.
- [ ] **[Exceptional Case]** 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.
  • Loading branch information
jfecher committed Jan 8, 2024
1 parent 67201bf commit f1de8fa
Show file tree
Hide file tree
Showing 7 changed files with 203 additions and 81 deletions.
10 changes: 5 additions & 5 deletions compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -474,18 +474,18 @@ pub(crate) fn check_methods_signatures(
let trait_methods = std::mem::take(&mut the_trait.methods);

for (file_id, func_id) in impl_methods {
let impl_method = resolver.interner.function_meta(func_id);
let func_name = resolver.interner.function_name(func_id).to_owned();

let mut typecheck_errors = Vec::new();

// This is None in the case where the impl block has a method that's not part of the trait.
// If that's the case, a `MethodNotInTrait` error has already been thrown, and we can ignore
// the impl method, since there's nothing in the trait to match its signature against.
if let Some(trait_method) =
trait_methods.iter().find(|method| method.name.0.contents == func_name)
{
let impl_function_type = impl_method.typ.instantiate(resolver.interner);
let mut typecheck_errors = Vec::new();
let impl_method = resolver.interner.function_meta(func_id);

let (impl_function_type, _) = impl_method.typ.instantiate(resolver.interner);

let impl_method_generic_count =
impl_method.typ.generic_count() - trait_impl_generic_count;
Expand All @@ -505,7 +505,7 @@ pub(crate) fn check_methods_signatures(
errors.push((error.into(), *file_id));
}

if let Type::Function(impl_params, _, _) = impl_function_type.0 {
if let Type::Function(impl_params, _, _) = impl_function_type {
if trait_method.arguments().len() == impl_params.len() {
// Check the parameters of the impl method against the parameters of the trait method
let args = trait_method.arguments().iter();
Expand Down
1 change: 1 addition & 0 deletions compiler/noirc_frontend/src/hir_def/traits.rs
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ pub struct Trait {
pub self_type_typevar_id: TypeVariableId,
pub self_type_typevar: TypeVariable,
}

#[derive(Debug)]
pub struct TraitImpl {
pub ident: Ident,
Expand Down
Loading

0 comments on commit f1de8fa

Please sign in to comment.