Skip to content

Commit

Permalink
fix: allow calling a trait method with paths that don't consist of ex…
Browse files Browse the repository at this point in the history
…actly two segments (#5577)

# Description

## Problem

Resolves #5557

## Summary

For something like `Add::add` the logic was checking if `Add` was a
trait, then looking for a method `add` in it. It only worked if the path
had exactly two segments.

I initially thought the fix was simple: also make it work if there are
more segments. However, it got tricky if there's just one segment: we
need to find out the trait associated with the found function. This
wasn't tracked in `FuncMeta` (well, there was a boolean for it but not
the optional `TraitId`) so in this PR this information is now tracked,
and the lookup works in all cases.

## Additional Context

None.

## 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.
  • Loading branch information
asterite authored Jul 22, 2024
1 parent df44919 commit 88c0a40
Show file tree
Hide file tree
Showing 7 changed files with 55 additions and 36 deletions.
6 changes: 3 additions & 3 deletions compiler/noirc_frontend/src/elaborator/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -647,7 +647,7 @@ impl<'context> Elaborator<'context> {
&mut self,
func: &mut NoirFunction,
func_id: FuncId,
is_trait_function: bool,
trait_id: Option<TraitId>,
) {
let in_contract = if self.self_type.is_some() {
// Without this, impl methods can accidentally be placed in contracts.
Expand Down Expand Up @@ -761,6 +761,7 @@ impl<'context> Elaborator<'context> {
direct_generics,
all_generics: self.generics.clone(),
struct_id,
trait_id,
trait_impl: self.current_trait_impl,
parameters: parameters.into(),
parameter_idents,
Expand All @@ -769,7 +770,6 @@ impl<'context> Elaborator<'context> {
has_body: !func.def.body.is_empty(),
trait_constraints,
is_entry_point,
is_trait_function,
has_inline_attribute,
source_crate: self.crate_id,
source_module: self.local_module,
Expand Down Expand Up @@ -1467,7 +1467,7 @@ impl<'context> Elaborator<'context> {
for (local_module, id, func) in &mut function_set.functions {
self.local_module = *local_module;
self.recover_generics(|this| {
this.define_function_meta(func, *id, false);
this.define_function_meta(func, *id, None);
});
}
}
Expand Down
2 changes: 1 addition & 1 deletion compiler/noirc_frontend/src/elaborator/trait_impls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ impl<'context> Elaborator<'context> {
let module = self.module_id();
let location = Location::new(default_impl.def.span, trait_impl.file_id);
self.interner.push_function(func_id, &default_impl.def, module, location);
self.define_function_meta(&mut default_impl_clone, func_id, false);
self.define_function_meta(&mut default_impl_clone, func_id, None);
func_ids_in_trait.insert(func_id);
ordered_methods.push((
method.default_impl_module_id,
Expand Down
5 changes: 4 additions & 1 deletion compiler/noirc_frontend/src/elaborator/traits.rs
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,7 @@ impl<'context> Elaborator<'context> {
let func_id = unresolved_trait.method_ids[&name.0.contents];

this.resolve_trait_function(
trait_id,
name,
generics,
parameters,
Expand Down Expand Up @@ -171,8 +172,10 @@ impl<'context> Elaborator<'context> {
functions
}

#[allow(clippy::too_many_arguments)]
pub fn resolve_trait_function(
&mut self,
trait_id: TraitId,
name: &Ident,
generics: &UnresolvedGenerics,
parameters: &[(Ident, UnresolvedType)],
Expand Down Expand Up @@ -206,7 +209,7 @@ impl<'context> Elaborator<'context> {
};

let mut function = NoirFunction { kind, def };
self.define_function_meta(&mut function, func_id, true);
self.define_function_meta(&mut function, func_id, Some(trait_id));
self.elaborate_function(func_id);
let _ = self.scopes.end_function();
// Don't check the scope tree for unused variables, they can't be used in a declaration anyway.
Expand Down
40 changes: 16 additions & 24 deletions compiler/noirc_frontend/src/elaborator/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,8 @@ use crate::{
Signedness, UnaryOp, UnresolvedType, UnresolvedTypeData,
},
node_interner::{
DefinitionKind, DependencyId, ExprId, GlobalId, TraitId, TraitImplKind, TraitMethodId,
DefinitionKind, DependencyId, ExprId, FuncId, GlobalId, TraitId, TraitImplKind,
TraitMethodId,
},
Generics, Kind, ResolvedGeneric, Type, TypeBinding, TypeVariable, TypeVariableKind,
};
Expand Down Expand Up @@ -444,29 +445,20 @@ impl<'context> Elaborator<'context> {
&mut self,
path: &Path,
) -> Option<(TraitMethodId, TraitConstraint, bool)> {
if path.kind == PathKind::Plain && path.segments.len() == 2 {
let method = &path.segments[1];

let mut trait_path = path.clone();
trait_path.pop();
let trait_id = self.lookup(trait_path).ok()?;
let the_trait = self.interner.get_trait(trait_id);

let method = the_trait.find_method(method.0.contents.as_str())?;
let constraint = TraitConstraint {
typ: Type::TypeVariable(
the_trait.self_type_typevar.clone(),
TypeVariableKind::Normal,
),
trait_generics: Type::from_generics(&vecmap(&the_trait.generics, |generic| {
generic.type_var.clone()
})),
trait_id,
span: path.span(),
};
return Some((method, constraint, false));
}
None
let func_id: FuncId = self.lookup(path.clone()).ok()?;
let meta = self.interner.function_meta(&func_id);
let trait_id = meta.trait_id?;
let the_trait = self.interner.get_trait(trait_id);
let method = the_trait.find_method(&path.last_segment().0.contents)?;
let constraint = TraitConstraint {
typ: Type::TypeVariable(the_trait.self_type_typevar.clone(), TypeVariableKind::Normal),
trait_generics: Type::from_generics(&vecmap(&the_trait.generics, |generic| {
generic.type_var.clone()
})),
trait_id,
span: path.span(),
};
Some((method, constraint, false))
}

// This resolves a static trait method T::trait_method by iterating over the where clause
Expand Down
12 changes: 5 additions & 7 deletions compiler/noirc_frontend/src/hir_def/function.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ use crate::ast::{FunctionKind, FunctionReturnType, Visibility};
use crate::graph::CrateId;
use crate::hir::def_map::LocalModuleId;
use crate::macros_api::{BlockExpression, StructId};
use crate::node_interner::{ExprId, NodeInterner, TraitImplId};
use crate::node_interner::{ExprId, NodeInterner, TraitId, TraitImplId};
use crate::{ResolvedGeneric, Type};

/// A Hir function is a block expression with a list of statements.
Expand Down Expand Up @@ -134,18 +134,16 @@ pub struct FuncMeta {
/// The struct this function belongs to, if any
pub struct_id: Option<StructId>,

// The trait this function belongs to, if any
pub trait_id: Option<TraitId>,

/// The trait impl this function belongs to, if any
pub trait_impl: Option<TraitImplId>,

/// True if this function is an entry point to the program.
/// For non-contracts, this means the function is `main`.
pub is_entry_point: bool,

/// True if this function was defined within a trait (not a trait impl!).
/// Trait functions are just stubs and shouldn't have their return type checked
/// against their body type, nor should unused variables be checked.
pub is_trait_function: bool,

/// True if this function is marked with an attribute
/// that indicates it should be inlined differently than the default (inline everything).
/// For example, such as `fold` (never inlined) or `no_predicates` (inlined after flattening)
Expand Down Expand Up @@ -174,7 +172,7 @@ impl FuncMeta {
/// We don't check the return type of these functions since it will always have
/// an empty body, and we don't check for unused parameters.
pub fn is_stub(&self) -> bool {
self.kind.can_ignore_return_type() || self.is_trait_function
self.kind.can_ignore_return_type() || self.trait_id.is_some()
}

pub fn function_signature(&self) -> FunctionSignature {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
[package]
name = "trait_call_full_path"
type = "bin"
authors = [""]

[dependencies]
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
mod foo {
trait Trait {
fn me(self) -> Self;
}

impl Trait for Field {
fn me(self) -> Self {
self
}
}
}

use foo::Trait;
use foo::Trait::me;

fn main(x: Field) {
let _ = foo::Trait::me(x);
let _ = Trait::me(x);
let _ = me(x);
}

0 comments on commit 88c0a40

Please sign in to comment.