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

fix: Allow trait method references from the trait name #3774

Merged
merged 14 commits into from
Dec 13, 2023
1 change: 1 addition & 0 deletions compiler/noirc_driver/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -334,6 +334,7 @@ pub fn compile_no_check(
force_compile: bool,
) -> Result<CompiledProgram, RuntimeError> {
let program = monomorphize(main_function, &context.def_interner);
println!("{}", program);
jfecher marked this conversation as resolved.
Show resolved Hide resolved

let hash = fxhash::hash64(&program);
let hashes_match = cached_program.as_ref().map_or(false, |program| program.hash == hash);
Expand Down
1 change: 1 addition & 0 deletions compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@
pub module_id: LocalModuleId,
pub crate_id: CrateId,
pub trait_def: NoirTrait,
pub method_ids: HashMap<String, FuncId>,
pub fns_with_default_impl: UnresolvedFunctions,
}

Expand Down Expand Up @@ -402,7 +403,7 @@
.collect()
}

// TODO(vitkov): Move this out of here and into type_check

Check warning on line 406 in compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs

View workflow job for this annotation

GitHub Actions / Spellcheck / Spellcheck

Unknown word (vitkov)
pub(crate) fn check_methods_signatures(
resolver: &mut Resolver,
impl_methods: &Vec<(FileId, FuncId)>,
Expand Down
7 changes: 6 additions & 1 deletion compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use std::vec;
use std::{collections::HashMap, vec};

use acvm::acir::acir_field::FieldOptions;
use fm::FileId;
Expand Down Expand Up @@ -376,6 +376,8 @@
functions: Vec::new(),
trait_id: None,
};

let mut method_ids = HashMap::new();
for trait_item in &trait_definition.items {
match trait_item {
TraitItem::Function {
Expand All @@ -387,10 +389,12 @@
body,
} => {
let func_id = context.def_interner.push_empty_fn();
method_ids.insert(name.to_string(), func_id);

let modifiers = FunctionModifiers {
name: name.to_string(),
visibility: crate::FunctionVisibility::Public,
// TODO(Maddiaa): Investigate trait implementations with attributes see: https://github.com/noir-lang/noir/issues/2629

Check warning on line 397 in compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs

View workflow job for this annotation

GitHub Actions / Spellcheck / Spellcheck

Unknown word (Maddiaa)
attributes: crate::token::Attributes::empty(),
is_unconstrained: false,
contract_function_type: None,
Expand Down Expand Up @@ -449,7 +453,7 @@
}
}
TraitItem::Type { name } => {
// TODO(nickysn or alexvitkov): implement context.def_interner.push_empty_type_alias and get an id, instead of using TypeAliasId::dummy_id()

Check warning on line 456 in compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs

View workflow job for this annotation

GitHub Actions / Spellcheck / Spellcheck

Unknown word (nickysn)

Check warning on line 456 in compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs

View workflow job for this annotation

GitHub Actions / Spellcheck / Spellcheck

Unknown word (alexvitkov)
if let Err((first_def, second_def)) = self.def_collector.def_map.modules
[trait_id.0.local_id.0]
.declare_type_alias(name.clone(), TypeAliasId::dummy_id())
Expand All @@ -471,6 +475,7 @@
module_id: self.module_id,
crate_id: krate,
trait_def: trait_definition,
method_ids,
fns_with_default_impl: unresolved_functions,
};
self.def_collector.collected_traits.insert(trait_id, unresolved);
Expand Down
77 changes: 71 additions & 6 deletions compiler/noirc_frontend/src/hir/resolution/resolver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ use crate::hir_def::expr::{
};

use crate::hir_def::traits::{Trait, TraitConstraint};
use crate::token::FunctionAttribute;
use crate::token::{Attributes, FunctionAttribute};
use regex::Regex;
use std::collections::{BTreeMap, HashSet};
use std::rc::Rc;
Expand All @@ -37,11 +37,11 @@ use crate::{
StatementKind,
};
use crate::{
ArrayLiteral, ContractFunctionType, Distinctness, ForRange, FunctionVisibility, Generics,
LValue, NoirStruct, NoirTypeAlias, Param, Path, PathKind, Pattern, Shared, StructType, Type,
TypeAliasType, TypeBinding, TypeVariable, UnaryOp, UnresolvedGenerics,
UnresolvedTraitConstraint, UnresolvedType, UnresolvedTypeData, UnresolvedTypeExpression,
Visibility, ERROR_IDENT,
ArrayLiteral, ContractFunctionType, Distinctness, ForRange, FunctionDefinition,
FunctionReturnType, FunctionVisibility, Generics, LValue, NoirStruct, NoirTypeAlias, Param,
Path, PathKind, Pattern, Shared, StructType, Type, TypeAliasType, TypeBinding, TypeVariable,
UnaryOp, UnresolvedGenerics, UnresolvedTraitConstraint, UnresolvedType, UnresolvedTypeData,
UnresolvedTypeExpression, Visibility, ERROR_IDENT,
};
use fm::FileId;
use iter_extended::vecmap;
Expand Down Expand Up @@ -200,6 +200,52 @@ impl<'a> Resolver<'a> {
(hir_func, func_meta, self.errors)
}

pub fn resolve_trait_function(
&mut self,
name: &Ident,
parameters: &[(Ident, UnresolvedType)],
return_type: &FunctionReturnType,
where_clause: &[UnresolvedTraitConstraint],
func_id: FuncId,
) -> (HirFunction, FuncMeta) {
self.scopes.start_function();

// Check whether the function has globals in the local module and add them to the scope
self.resolve_local_globals();

self.trait_bounds = where_clause.to_vec();

let kind = FunctionKind::Normal;
let def = FunctionDefinition {
name: name.clone(),
attributes: Attributes::empty(),
is_open: false,
is_internal: false,
is_unconstrained: false,
visibility: FunctionVisibility::Public, // Trait functions are always public
generics: Vec::new(), // self.generics should already be set
parameters: vecmap(parameters, |(name, typ)| Param {
visibility: Visibility::Private,
pattern: Pattern::Identifier(name.clone()),
typ: typ.clone(),
span: name.span(),
}),
body: BlockExpression(Vec::new()),
span: name.span(),
where_clause: where_clause.to_vec(),
return_type: return_type.clone(),
return_visibility: Visibility::Private,
return_distinctness: Distinctness::DuplicationAllowed,
};

let (hir_func, func_meta) = self.intern_function(NoirFunction { kind, def }, func_id);
let func_scope_tree = self.scopes.end_function();
self.check_for_unused_variables_in_scope_tree(func_scope_tree);

self.trait_bounds.clear();
(hir_func, func_meta)
}

fn check_for_unused_variables_in_scope_tree(&mut self, scope_decls: ScopeTree) {
let mut unused_vars = Vec::new();
for scope in scope_decls.0.into_iter() {
Expand Down Expand Up @@ -1605,6 +1651,24 @@ impl<'a> Resolver<'a> {
None
}

// this resolves TraitName::some_static_method
fn resolve_trait_static_method(&mut self, path: &Path) -> Option<(HirExpression, Type)> {
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);

if let Some(method) = the_trait.find_method(method.0.contents.as_str()) {
let self_type = Type::type_variable(the_trait.self_type_typevar_id);
return Some((HirExpression::TraitMethodReference(method), self_type));
}
}
None
}

// this resolves a static trait method T::trait_method by iterating over the where clause
fn resolve_trait_method_by_named_generic(
&mut self,
Expand Down Expand Up @@ -1641,6 +1705,7 @@ impl<'a> Resolver<'a> {

fn resolve_trait_generic_path(&mut self, path: &Path) -> Option<(HirExpression, Type)> {
self.resolve_trait_static_method_by_self(path)
.or_else(|| self.resolve_trait_static_method(path))
.or_else(|| self.resolve_trait_method_by_named_generic(path))
}

Expand Down
54 changes: 22 additions & 32 deletions compiler/noirc_frontend/src/hir/resolution/traits.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use std::collections::{BTreeMap, HashSet};

use fm::FileId;
use iter_extended::vecmap;
use noirc_errors::{Location, Span};
use noirc_errors::Location;

use crate::{
graph::CrateId,
Expand All @@ -22,9 +22,7 @@ use crate::{
};

use super::{
errors::ResolverError,
functions, get_module_mut, get_struct_type,
import::PathResolutionError,
path_resolver::{PathResolver, StandardPathResolver},
resolver::Resolver,
take_errors,
Expand Down Expand Up @@ -92,13 +90,14 @@ fn resolve_trait_methods(

let mut functions = vec![];
let mut resolver_errors = vec![];

for item in &unresolved_trait.trait_def.items {
if let TraitItem::Function {
name,
generics,
parameters,
return_type,
where_clause: _,
where_clause,
body: _,
} = item
{
Expand All @@ -110,6 +109,16 @@ fn resolve_trait_methods(
resolver.add_generics(generics);
resolver.set_self_type(Some(self_type));

let func_id = unresolved_trait.method_ids[&name.0.contents];
let (_, func_meta) = resolver.resolve_trait_function(
name,
parameters,
return_type,
where_clause,
func_id,
);
resolver.interner.push_fn_meta(func_meta, func_id);

let arguments = vecmap(parameters, |param| resolver.resolve_type(param.1.clone()));
let return_type = resolver.resolve_type(return_type.get_type().into_owned());

Expand All @@ -124,14 +133,13 @@ fn resolve_trait_methods(
let the_trait = resolver.interner.get_trait(trait_id);
generics.push((the_trait.self_type_typevar_id, the_trait.self_type_typevar.clone()));

let name = name.clone();
let span: Span = name.span();
let default_impl_list: Vec<_> = unresolved_trait
.fns_with_default_impl
.functions
.iter()
.filter(|(_, _, q)| q.name() == name.0.contents)
.collect();

let default_impl = if default_impl_list.len() == 1 {
Some(Box::new(default_impl_list[0].2.clone()))
} else {
Expand All @@ -140,18 +148,18 @@ fn resolve_trait_methods(

let no_environment = Box::new(Type::Unit);
let function_type = Type::Function(arguments, Box::new(return_type), no_environment);
let typ = Type::Forall(generics, Box::new(function_type));

let f = TraitFunction {
name,
typ,
span,
functions.push(TraitFunction {
name: name.clone(),
typ: Type::Forall(generics, Box::new(function_type)),
span: name.span(),
default_impl,
default_impl_file_id: unresolved_trait.file_id,
default_impl_module_id: unresolved_trait.module_id,
};
functions.push(f);
resolver_errors.extend(take_errors_filter_self_not_resolved(file, resolver));
});

let errors = resolver.take_errors().into_iter();
resolver_errors.extend(errors.map(|resolution_error| (resolution_error.into(), file)));
}
}
(functions, resolver_errors)
Expand Down Expand Up @@ -451,21 +459,3 @@ pub(crate) fn resolve_trait_impls(

methods
}

pub(crate) fn take_errors_filter_self_not_resolved(
file_id: FileId,
resolver: Resolver<'_>,
) -> Vec<(CompilationError, FileId)> {
resolver
.take_errors()
.iter()
.filter(|resolution_error| match resolution_error {
ResolverError::PathResolutionError(PathResolutionError::Unresolved(ident)) => {
&ident.0.contents != "Self"
}
_ => true,
})
.cloned()
.map(|resolution_error| (resolution_error.into(), file_id))
.collect()
}
10 changes: 9 additions & 1 deletion compiler/noirc_frontend/src/hir/type_check/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ use crate::{
},
types::Type,
},
node_interner::{DefinitionKind, ExprId, FuncId, TraitId, TraitMethodId},
node_interner::{DefinitionKind, ExprId, FuncId, TraitId, TraitImplKind, TraitMethodId},
BinaryOpKind, Signedness, TypeBinding, TypeBindings, TypeVariableKind, UnaryOp,
};

Expand Down Expand Up @@ -291,6 +291,14 @@ impl<'interner> TypeChecker<'interner> {
let the_trait = self.interner.get_trait(method.trait_id);
let typ = &the_trait.methods[method.method_index].typ;
let (typ, bindings) = typ.instantiate(self.interner);

// We must also remember to apply these substitutions to the object_type
// referenced by the selected trait impl, if one has yet to be selected.
let impl_kind = self.interner.get_selected_impl_for_ident_mut(*expr_id);
if let Some(TraitImplKind::Assumed { object_type }) = impl_kind {
*object_type = object_type.substitute(&bindings);
}

self.interner.store_instantiation_bindings(*expr_id, bindings);
typ
}
Expand Down
31 changes: 8 additions & 23 deletions compiler/noirc_frontend/src/hir_def/traits.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
use std::collections::HashMap;

use crate::{
graph::CrateId,
node_interner::{FuncId, TraitId, TraitMethodId},
Expand Down Expand Up @@ -43,6 +45,12 @@ pub struct Trait {

pub methods: Vec<TraitFunction>,

/// Maps method_name -> method id.
/// This map is separate from methods since TraitFunction ids
/// are created during collection where we don't yet have all
/// the information needed to create the full TraitFunction.
pub method_ids: HashMap<String, FuncId>,

pub constants: Vec<TraitConstant>,
pub types: Vec<TraitType>,

Expand Down Expand Up @@ -98,29 +106,6 @@ impl PartialEq for Trait {
}

impl Trait {
pub fn new(
id: TraitId,
name: Ident,
crate_id: CrateId,
span: Span,
generics: Generics,
self_type_typevar_id: TypeVariableId,
self_type_typevar: TypeVariable,
) -> Trait {
Trait {
id,
name,
crate_id,
span,
methods: Vec::new(),
constants: Vec::new(),
types: Vec::new(),
generics,
self_type_typevar_id,
self_type_typevar,
}
}

pub fn set_methods(&mut self, methods: Vec<TraitFunction>) {
self.methods = methods;
}
Expand Down
Loading
Loading