feat!: require trait primitive functions/calls to have their trait in scope

compiler/noirc_frontend/src/elaborator/types.rs

@@ -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};
@@ -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(..) => {
@@ -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
@@ -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
@@ -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?

compiler/noirc_frontend/src/hir_def/types.rs

@@ -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();

compiler/noirc_frontend/src/node_interner.rs

@@ -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)>,
@@ -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(),
@@ -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 {
@@ -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)
@@ -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))
     }
@@ -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`
@@ -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)
     }
 
@@ -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;
@@ -2462,6 +2421,7 @@ enum TypeMethodKey {
     Function,
     Generic,
     Quoted(QuotedType),
+    Struct(StructId),
 }
 
 fn get_type_method_key(typ: &Type) -> Option<TypeMethodKey> {
@@ -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,