Skip to content

Commit

Permalink
feat!: require trait primitive functions/calls to have their trait in…
Browse files Browse the repository at this point in the history 
… scope (#6901)

Co-authored-by: Tom French <[email protected]>
  • Loading branch information
@asterite @TomAFrench
asterite and TomAFrench authored Jan 9, 2025
1 parent 4d38a88 commit 56c931a
Show file tree
Hide file tree
Showing 11 changed files with 144 additions and 119 deletions.
77 changes: 33 additions & 44 deletions compiler/noirc_frontend/src/elaborator/types.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,7 @@ use crate::{
TraitImplKind, TraitMethodId,
},
token::SecondaryAttribute,
Generics, Kind, ResolvedGeneric, Shared, StructType, Type, TypeBinding, TypeBindings,
UnificationError,
Generics, Kind, ResolvedGeneric, Type, TypeBinding, TypeBindings, UnificationError,
};

use super::{lints, path_resolution::PathResolutionItem, Elaborator, UnsafeBlockStatus};
Expand Down Expand Up @@ -1320,9 +1319,6 @@ impl<'context> Elaborator<'context> {
has_self_arg: bool,
) -> Option<HirMethodReference> {
match object_type.follow_bindings() {
Type::Struct(typ, _args) => {
self.lookup_struct_method(object_type, method_name, span, has_self_arg, typ)
}
// TODO: We should allow method calls on `impl Trait`s eventually.
// For now it is fine since they are only allowed on return types.
Type::TraitAsType(..) => {
Expand All @@ -1338,11 +1334,9 @@ impl<'context> Elaborator<'context> {
}
// Mutable references to another type should resolve to methods of their element type.
// This may be a struct or a primitive type.
Type::MutableReference(element) => self
.interner
.lookup_primitive_trait_method_mut(element.as_ref(), method_name, has_self_arg)
.map(HirMethodReference::FuncId)
.or_else(|| self.lookup_method(&element, method_name, span, has_self_arg)),
Type::MutableReference(element) => {
self.lookup_method(&element, method_name, span, has_self_arg)
}

// If we fail to resolve the object to a struct type, we have no way of type
// checking its arguments as we can't even resolve the name of the function
Expand All @@ -1354,39 +1348,29 @@ impl<'context> Elaborator<'context> {
None
}

other => match self.interner.lookup_primitive_method(&other, method_name, has_self_arg)
{
Some(method_id) => Some(HirMethodReference::FuncId(method_id)),
None => {
// It could be that this type is a composite type that is bound to a trait,
// for example `x: (T, U) ... where (T, U): SomeTrait`
// (so this case is a generalization of the NamedGeneric case)
self.lookup_method_in_trait_constraints(object_type, method_name, span)
}
},
other => {
self.lookup_struct_or_primitive_method(&other, method_name, span, has_self_arg)
}
}
}

fn lookup_struct_method(
fn lookup_struct_or_primitive_method(
&mut self,
object_type: &Type,
method_name: &str,
span: Span,
has_self_arg: bool,
typ: Shared<StructType>,
) -> Option<HirMethodReference> {
let id = typ.borrow().id;

// First search in the struct methods
if let Some(method_id) =
self.interner.lookup_direct_method(object_type, id, method_name, has_self_arg)
self.interner.lookup_direct_method(object_type, method_name, has_self_arg)
{
return Some(HirMethodReference::FuncId(method_id));
}

// Next lookup all matching trait methods.
let trait_methods =
self.interner.lookup_trait_methods(object_type, id, method_name, has_self_arg);
self.interner.lookup_trait_methods(object_type, method_name, has_self_arg);

if trait_methods.is_empty() {
// If we couldn't find any trait methods, search in
Expand All @@ -1397,26 +1381,31 @@ impl<'context> Elaborator<'context> {
return Some(HirMethodReference::FuncId(func_id));
}

// Otherwise it's an error
let has_field_with_function_type = typ
.borrow()
.get_fields_as_written()
.into_iter()
.any(|field| field.name.0.contents == method_name && field.typ.is_function());
if has_field_with_function_type {
self.push_err(TypeCheckError::CannotInvokeStructFieldFunctionType {
method_name: method_name.to_string(),
object_type: object_type.clone(),
span,
});
if let Type::Struct(struct_type, _) = object_type {
let has_field_with_function_type =
struct_type.borrow().get_fields_as_written().into_iter().any(|field| {
field.name.0.contents == method_name && field.typ.is_function()
});
if has_field_with_function_type {
self.push_err(TypeCheckError::CannotInvokeStructFieldFunctionType {
method_name: method_name.to_string(),
object_type: object_type.clone(),
span,
});
} else {
self.push_err(TypeCheckError::UnresolvedMethodCall {
method_name: method_name.to_string(),
object_type: object_type.clone(),
span,
});
}
return None;
} else {
self.push_err(TypeCheckError::UnresolvedMethodCall {
method_name: method_name.to_string(),
object_type: object_type.clone(),
span,
});
// It could be that this type is a composite type that is bound to a trait,
// for example `x: (T, U) ... where (T, U): SomeTrait`
// (so this case is a generalization of the NamedGeneric case)
return self.lookup_method_in_trait_constraints(object_type, method_name, span);
}
return None;
}

// We found some trait methods... but is only one of them currently in scope?
Expand Down
2 changes: 1 addition & 1 deletion compiler/noirc_frontend/src/hir_def/types.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1862,7 +1862,7 @@ impl Type {
if let (Type::Array(_size, element1), Type::Slice(element2)) = (&this, &target) {
// We can only do the coercion if the `as_slice` method exists.
// This is usually true, but some tests don't have access to the standard library.
if let Some(as_slice) = interner.lookup_primitive_method(&this, "as_slice", true) {
if let Some(as_slice) = interner.lookup_direct_method(&this, "as_slice", true) {
// Still have to ensure the element types match.
// Don't need to issue an error here if not, it will be done in unify_with_coercions
let mut bindings = TypeBindings::new();
Expand Down
106 changes: 33 additions & 73 deletions compiler/noirc_frontend/src/node_interner.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -183,16 +183,13 @@ pub struct NodeInterner {

next_type_variable_id: std::cell::Cell<usize>,

/// A map from a struct type and method name to a function id for the method.
/// A map from a type and method name to a function id for the method.
/// This can resolve to potentially multiple methods if the same method name is
/// specialized for different generics on the same type. E.g. for `Struct<T>`, we
/// may have both `impl Struct<u32> { fn foo(){} }` and `impl Struct<u8> { fn foo(){} }`.
/// If this happens, the returned Vec will have 2 entries and we'll need to further
/// disambiguate them by checking the type of each function.
struct_methods: HashMap<StructId, HashMap<String, Methods>>,

/// Methods on primitive types defined in the stdlib.
primitive_methods: HashMap<TypeMethodKey, HashMap<String, Methods>>,
methods: HashMap<TypeMethodKey, HashMap<String, Methods>>,

// For trait implementation functions, this is their self type and trait they belong to
func_id_to_trait: HashMap<FuncId, (Type, TraitId)>,
Expand Down Expand Up @@ -666,8 +663,7 @@ impl Default for NodeInterner {
next_type_variable_id: std::cell::Cell::new(0),
globals: Vec::new(),
global_attributes: HashMap::default(),
struct_methods: HashMap::default(),
primitive_methods: HashMap::default(),
methods: HashMap::default(),
type_alias_ref: Vec::new(),
type_ref_locations: Vec::new(),
quoted_types: Default::default(),
Expand Down Expand Up @@ -1214,27 +1210,8 @@ impl NodeInterner {
self.structs[&id].clone()
}

pub fn get_struct_methods(&self, id: StructId) -> Option<&HashMap<String, Methods>> {
self.struct_methods.get(&id)
}

fn get_primitive_methods(&self, key: TypeMethodKey) -> Option<&HashMap<String, Methods>> {
self.primitive_methods.get(&key)
}

pub fn get_type_methods(&self, typ: &Type) -> Option<&HashMap<String, Methods>> {
match typ {
Type::Struct(struct_type, _) => {
let struct_type = struct_type.borrow();
self.get_struct_methods(struct_type.id)
}
Type::Alias(type_alias, generics) => {
let type_alias = type_alias.borrow();
let typ = type_alias.get_type(generics);
self.get_type_methods(&typ)
}
_ => get_type_method_key(typ).and_then(|key| self.get_primitive_methods(key)),
}
get_type_method_key(typ).and_then(|key| self.methods.get(&key))
}

pub fn get_trait(&self, id: TraitId) -> &Trait {
Expand Down Expand Up @@ -1394,39 +1371,27 @@ impl NodeInterner {
trait_id: Option<TraitId>,
) -> Option<FuncId> {
match self_type {
Type::Struct(struct_type, _generics) => {
let id = struct_type.borrow().id;

if trait_id.is_none() {
if let Some(existing) =
self.lookup_direct_method(self_type, id, &method_name, true)
{
return Some(existing);
}
}

self.struct_methods
.entry(id)
.or_default()
.entry(method_name)
.or_default()
.add_method(method_id, None, trait_id);
None
}
Type::Error => None,
Type::MutableReference(element) => {
self.add_method(element, method_name, method_id, trait_id)
}

other => {
_ => {
let key = get_type_method_key(self_type).unwrap_or_else(|| {
unreachable!("Cannot add a method to the unsupported type '{}'", other)
unreachable!("Cannot add a method to the unsupported type '{}'", self_type)
});

if trait_id.is_none() && matches!(self_type, Type::Struct(..)) {
if let Some(existing) = self.lookup_direct_method(self_type, &method_name, true)
{
return Some(existing);
}
}

// Only remember the actual type if it's FieldOrInt,
// so later we can disambiguate on calls like `u32::call`.
let typ =
if key == TypeMethodKey::FieldOrInt { Some(self_type.clone()) } else { None };
self.primitive_methods
self.methods
.entry(key)
.or_default()
.entry(method_name)
Expand Down Expand Up @@ -1785,12 +1750,13 @@ impl NodeInterner {
pub fn lookup_direct_method(
&self,
typ: &Type,
id: StructId,
method_name: &str,
has_self_arg: bool,
) -> Option<FuncId> {
self.struct_methods
.get(&id)
let key = get_type_method_key(typ)?;

self.methods
.get(&key)
.and_then(|h| h.get(method_name))
.and_then(|methods| methods.find_direct_method(typ, has_self_arg, self))
}
Expand All @@ -1799,15 +1765,19 @@ impl NodeInterner {
pub fn lookup_trait_methods(
&self,
typ: &Type,
id: StructId,
method_name: &str,
has_self_arg: bool,
) -> Vec<(FuncId, TraitId)> {
self.struct_methods
.get(&id)
.and_then(|h| h.get(method_name))
.map(|methods| methods.find_trait_methods(typ, has_self_arg, self))
.unwrap_or_default()
let key = get_type_method_key(typ);
if let Some(key) = key {
self.methods
.get(&key)
.and_then(|h| h.get(method_name))
.map(|methods| methods.find_trait_methods(typ, has_self_arg, self))
.unwrap_or_default()
} else {
Vec::new()
}
}

/// Select the 1 matching method with an object type matching `typ`
Expand All @@ -1833,8 +1803,7 @@ impl NodeInterner {
method_name: &str,
has_self_arg: bool,
) -> Option<FuncId> {
let global_methods =
self.primitive_methods.get(&TypeMethodKey::Generic)?.get(method_name)?;
let global_methods = self.methods.get(&TypeMethodKey::Generic)?.get(method_name)?;
global_methods.find_matching_method(typ, has_self_arg, self)
}

Expand All @@ -1846,20 +1815,10 @@ impl NodeInterner {
has_self_arg: bool,
) -> Option<FuncId> {
let key = get_type_method_key(typ)?;
let methods = self.primitive_methods.get(&key)?.get(method_name)?;
let methods = self.methods.get(&key)?.get(method_name)?;
self.find_matching_method(typ, Some(methods), method_name, has_self_arg)
}

pub fn lookup_primitive_trait_method_mut(
&self,
typ: &Type,
method_name: &str,
has_self_arg: bool,
) -> Option<FuncId> {
let typ = Type::MutableReference(Box::new(typ.clone()));
self.lookup_primitive_method(&typ, method_name, has_self_arg)
}

/// Returns what the next trait impl id is expected to be.
pub fn next_trait_impl_id(&mut self) -> TraitImplId {
let next_id = self.next_trait_implementation_id;
Expand Down Expand Up @@ -2462,6 +2421,7 @@ enum TypeMethodKey {
Function,
Generic,
Quoted(QuotedType),
Struct(StructId),
}

fn get_type_method_key(typ: &Type) -> Option<TypeMethodKey> {
Expand Down Expand Up @@ -2489,12 +2449,12 @@ fn get_type_method_key(typ: &Type) -> Option<TypeMethodKey> {
Type::Quoted(quoted) => Some(Quoted(*quoted)),
Type::MutableReference(element) => get_type_method_key(element),
Type::Alias(alias, _) => get_type_method_key(&alias.borrow().typ),
Type::Struct(struct_type, _) => Some(Struct(struct_type.borrow().id)),

// We do not support adding methods to these types
Type::Forall(_, _)
| Type::Constant(..)
| Type::Error
| Type::Struct(_, _)
| Type::InfixExpr(..)
| Type::CheckedCast { .. }
| Type::TraitAsType(..) => None,
Expand Down
Loading

0 comments on commit 56c931a

Please sign in to comment.