feat: Add pub modifier

compiler/noirc_frontend/src/ast/expression.rs

@@ -364,6 +364,9 @@ pub struct FunctionDefinition {
     /// True if this function was defined with the 'unconstrained' keyword
     pub is_unconstrained: bool,
 
+    /// True if this function was defined with the 'pub' keyword
+    pub is_public: bool,
+
     pub generics: UnresolvedGenerics,
     pub parameters: Vec<(Pattern, UnresolvedType, Visibility)>,
     pub body: BlockExpression,
@@ -649,6 +652,7 @@ impl FunctionDefinition {
             is_open: false,
             is_internal: false,
             is_unconstrained: false,
+            is_public: false,
             generics: generics.clone(),
             parameters: p,
             body: body.clone(),

compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs

@@ -841,8 +841,7 @@ fn resolve_function_set(
 
     vecmap(unresolved_functions.functions, |(mod_id, func_id, func)| {
         let module_id = ModuleId { krate: crate_id, local_id: mod_id };
-        let path_resolver =
-            StandardPathResolver::new(ModuleId { local_id: mod_id, krate: crate_id });
+        let path_resolver = StandardPathResolver::new(module_id);
 
         let mut resolver = Resolver::new(interner, &path_resolver, def_maps, file_id);
         // Must use set_generics here to ensure we re-use the same generics from when
@@ -851,7 +850,7 @@ fn resolve_function_set(
         resolver.set_generics(impl_generics.clone());
         resolver.set_self_type(self_type.clone());
 
-        let (hir_func, func_meta, errs) = resolver.resolve_function(func, func_id, module_id);
+        let (hir_func, func_meta, errs) = resolver.resolve_function(func, func_id);
         interner.push_fn_meta(func_meta, func_id);
         interner.update_fn(func_id, hir_func);
         extend_errors(errors, file_id, errs);

compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs

@@ -68,11 +68,11 @@ pub fn collect_defs(
 
     collector.collect_type_aliases(context, ast.type_aliases, errors);
 
-    collector.collect_functions(context, ast.functions, errors);
+    collector.collect_functions(context, ast.functions, crate_id, errors);
 
-    collector.collect_trait_impls(context, ast.trait_impls, errors);
+    collector.collect_trait_impls(context, ast.trait_impls, crate_id, errors);
 
-    collector.collect_impls(context, ast.impls);
+    collector.collect_impls(context, ast.impls, crate_id);
 }
 
 impl<'a> ModCollector<'a> {
@@ -111,14 +111,23 @@ impl<'a> ModCollector<'a> {
         }
     }
 
-    fn collect_impls(&mut self, context: &mut Context, impls: Vec<TypeImpl>) {
+    fn collect_impls(&mut self, context: &mut Context, impls: Vec<TypeImpl>, krate: CrateId) {
+        let module_id = ModuleId { krate, local_id: self.module_id };
+
         for r#impl in impls {
             let mut unresolved_functions =
                 UnresolvedFunctions { file_id: self.file_id, functions: Vec::new() };
 
             for method in r#impl.methods {
                 let func_id = context.def_interner.push_empty_fn();
-                context.def_interner.push_function_definition(method.name().to_owned(), func_id);
+                let is_public = method.def.is_public;
+
+                context.def_interner.push_function_definition(
+                    method.name().to_owned(),
+                    func_id,
+                    is_public,
+                    module_id,
+                );
                 unresolved_functions.push_fn(self.module_id, func_id, method);
             }
 
@@ -132,25 +141,33 @@ impl<'a> ModCollector<'a> {
         &mut self,
         context: &mut Context,
         impls: Vec<TraitImpl>,
+        krate: CrateId,
         errors: &mut Vec<FileDiagnostic>,
     ) {
+        let module_id = ModuleId { krate, local_id: self.module_id };
+
         for trait_impl in impls {
             let trait_name = trait_impl.trait_name.clone();
             let module = &self.def_collector.def_map.modules[self.module_id.0];
+
             match module.find_name(&trait_name).types {
                 Some((module_def_id, _visibility)) => {
                     if let Some(collected_trait) = self.get_unresolved_trait(module_def_id) {
                         let unresolved_functions = self.collect_trait_implementations(
                             context,
                             &trait_impl,
                             &collected_trait.trait_def,
+                            krate,
                             errors,
                         );
 
                         for (_, func_id, noir_function) in &unresolved_functions.functions {
                             let name = noir_function.name().to_owned();
+                            let is_public = noir_function.def.is_public;
 
-                            context.def_interner.push_function_definition(name, *func_id);
+                            context
+                                .def_interner
+                                .push_function_definition(name, *func_id, is_public, module_id);
                         }
 
                         let unresolved_trait_impl = UnresolvedTraitImpl {
@@ -199,17 +216,23 @@ impl<'a> ModCollector<'a> {
         context: &mut Context,
         trait_impl: &TraitImpl,
         trait_def: &NoirTrait,
+        krate: CrateId,
         errors: &mut Vec<FileDiagnostic>,
     ) -> UnresolvedFunctions {
         let mut unresolved_functions =
             UnresolvedFunctions { file_id: self.file_id, functions: Vec::new() };
 
+        let module = ModuleId { krate, local_id: self.module_id };
+
         for item in &trait_impl.items {
             if let TraitImplItem::Function(impl_method) = item {
                 let func_id = context.def_interner.push_empty_fn();
-                context
-                    .def_interner
-                    .push_function_definition(impl_method.name().to_owned(), func_id);
+                context.def_interner.push_function_definition(
+                    impl_method.name().to_owned(),
+                    func_id,
+                    impl_method.def.is_public,
+                    module,
+                );
                 unresolved_functions.push_fn(self.module_id, func_id, impl_method.clone());
             }
         }
@@ -243,7 +266,13 @@ impl<'a> ModCollector<'a> {
                             // if there's a default implementation for the method, use it
                             let method_name = name.0.contents.clone();
                             let func_id = context.def_interner.push_empty_fn();
-                            context.def_interner.push_function_definition(method_name, func_id);
+                            let is_public = false; // trait functions are always public
+                            context.def_interner.push_function_definition(
+                                method_name,
+                                func_id,
+                                is_public,
+                                module,
+                            );
                             let impl_method = NoirFunction::normal(FunctionDefinition::normal(
                                 name,
                                 generics,
@@ -291,18 +320,23 @@ impl<'a> ModCollector<'a> {
         &mut self,
         context: &mut Context,
         functions: Vec<NoirFunction>,
+        krate: CrateId,
         errors: &mut Vec<FileDiagnostic>,
     ) {
         let mut unresolved_functions =
             UnresolvedFunctions { file_id: self.file_id, functions: Vec::new() };
 
+        let module = ModuleId { krate, local_id: self.module_id };
+
         for function in functions {
             let name = function.name_ident().clone();
+            let func_id = context.def_interner.push_empty_fn();
+            let is_public = function.def.is_public;
+            let name_string = name.0.contents.clone();
 
             // First create dummy function in the DefInterner
             // So that we can get a FuncId
-            let func_id = context.def_interner.push_empty_fn();
-            context.def_interner.push_function_definition(name.0.contents.clone(), func_id);
+            context.def_interner.push_function_definition(name_string, func_id, is_public, module);
 
             // Now link this func_id to a crate level map with the noir function and the module id
             // Encountering a NoirFunction, we retrieve it's module_data to get the namespace

compiler/noirc_frontend/src/hir/mod.rs

@@ -79,11 +79,11 @@ impl Context {
 
         let name = self.def_interner.function_name(id);
 
-        let meta = self.def_interner.function_meta(id);
-        let module = self.module(meta.module_id);
+        let module_id = self.def_interner.function_module(*id);
+        let module = self.module(module_id);
 
         let parent =
-            def_map.get_module_path_with_separator(meta.module_id.local_id.0, module.parent, "::");
+            def_map.get_module_path_with_separator(module_id.local_id.0, module.parent, "::");
 
         if parent.is_empty() {
             name.into()

compiler/noirc_frontend/src/hir/resolution/errors.rs

@@ -76,6 +76,8 @@ pub enum ResolverError {
     NumericConstantInFormatString { name: String, span: Span },
     #[error("Closure environment must be a tuple or unit type")]
     InvalidClosureEnvironment { typ: Type, span: Span },
+    #[error("{name} is private and not visible from the current module")]
+    PrivateFunctionCalled { name: String, span: Span },
 }
 
 impl ResolverError {
@@ -288,6 +290,10 @@ impl From<ResolverError> for Diagnostic {
             ResolverError::InvalidClosureEnvironment { span, typ } => Diagnostic::simple_error(
                 format!("{typ} is not a valid closure environment type"),
                 "Closure environment must be a tuple or unit type".to_string(), span),
+            // This will be upgraded to an error in future versions
+            ResolverError::PrivateFunctionCalled { span, name } => Diagnostic::simple_warning(
+                format!("{name} is private and not visible from the current module"),
+                format!("{name} is private"), span),
         }
     }
 }

compiler/noirc_frontend/src/hir/resolution/resolver.rs

@@ -24,7 +24,7 @@ use std::collections::{BTreeMap, HashSet};
 use std::rc::Rc;
 
 use crate::graph::CrateId;
-use crate::hir::def_map::{ModuleDefId, ModuleId, TryFromModuleDefId, MAIN_FUNCTION};
+use crate::hir::def_map::{LocalModuleId, ModuleDefId, TryFromModuleDefId, MAIN_FUNCTION};
 use crate::hir_def::stmt::{HirAssignStatement, HirLValue, HirPattern};
 use crate::node_interner::{
     DefinitionId, DefinitionKind, ExprId, FuncId, NodeInterner, StmtId, StructId, TraitId,
@@ -145,7 +145,6 @@ impl<'a> Resolver<'a> {
         mut self,
         func: NoirFunction,
         func_id: FuncId,
-        module_id: ModuleId,
     ) -> (HirFunction, FuncMeta, Vec<ResolverError>) {
         self.scopes.start_function();
 
@@ -154,7 +153,7 @@ impl<'a> Resolver<'a> {
 
         self.add_generics(&func.def.generics);
 
-        let (hir_func, func_meta) = self.intern_function(func, func_id, module_id);
+        let (hir_func, func_meta) = self.intern_function(func, func_id);
         let func_scope_tree = self.scopes.end_function();
 
         self.check_for_unused_variables_in_scope_tree(func_scope_tree);
@@ -313,13 +312,8 @@ impl<'a> Resolver<'a> {
         }
     }
 
-    fn intern_function(
-        &mut self,
-        func: NoirFunction,
-        id: FuncId,
-        module_id: ModuleId,
-    ) -> (HirFunction, FuncMeta) {
-        let func_meta = self.extract_meta(&func, id, module_id);
+    fn intern_function(&mut self, func: NoirFunction, id: FuncId) -> (HirFunction, FuncMeta) {
+        let func_meta = self.extract_meta(&func, id);
         let hir_func = match func.kind {
             FunctionKind::Builtin | FunctionKind::LowLevel | FunctionKind::Oracle => {
                 HirFunction::empty()
@@ -673,12 +667,7 @@ impl<'a> Resolver<'a> {
     /// to be used in analysis and intern the function parameters
     /// Prerequisite: self.add_generics() has already been called with the given
     /// function's generics, including any generics from the impl, if any.
-    fn extract_meta(
-        &mut self,
-        func: &NoirFunction,
-        func_id: FuncId,
-        module_id: ModuleId,
-    ) -> FuncMeta {
+    fn extract_meta(&mut self, func: &NoirFunction, func_id: FuncId) -> FuncMeta {
         let location = Location::new(func.name_ident().span(), self.file);
         let id = self.interner.function_definition_id(func_id);
         let name_ident = HirIdent { id, location };
@@ -756,7 +745,6 @@ impl<'a> Resolver<'a> {
             name: name_ident,
             kind: func.kind,
             attributes,
-            module_id,
             contract_function_type: self.handle_function_type(func),
             is_internal: self.handle_is_function_internal(func),
             is_unconstrained: func.def.is_unconstrained,
@@ -982,6 +970,40 @@ impl<'a> Resolver<'a> {
         }
     }
 
+    // Issue an error if the given private function is being called from a non-child module
+    fn check_can_reference_private_function(&mut self, func: FuncId, span: Span) {
+        let function_module = self.interner.function_module(func);
+        let current_module = self.path_resolver.module_id();
+
+        let same_crate = function_module.krate == current_module.krate;
+        let krate = function_module.krate;
+        let current_module = current_module.local_id;
+
+        if !same_crate
+            || !self.module_descendent_of_target(krate, function_module.local_id, current_module)
+        {
+            let name = self.interner.function_name(&func).to_string();
+            self.errors.push(ResolverError::PrivateFunctionCalled { span, name });
+        }
+    }
+
+    // Returns true if `current` is a (potentially nested) child module of `target`.
+    // This is also true if `current == target`.
+    fn module_descendent_of_target(
+        &self,
+        krate: CrateId,
+        target: LocalModuleId,
+        current: LocalModuleId,
+    ) -> bool {
+        if current == target {
+            return true;
+        }
+
+        self.def_maps[&krate].modules[current.0]
+            .parent
+            .map_or(false, |parent| self.module_descendent_of_target(krate, target, parent))
+    }
+
     fn resolve_local_variable(&mut self, hir_ident: HirIdent, var_scope_index: usize) {
         let mut transitive_capture_index: Option<usize> = None;
 
@@ -1051,7 +1073,12 @@ impl<'a> Resolver<'a> {
 
                 if hir_ident.id != DefinitionId::dummy_id() {
                     match self.interner.definition(hir_ident.id).kind {
-                        DefinitionKind::Function(_) => {}
+                        DefinitionKind::Function(id) => {
+                            if self.interner.function_visibility(id) == Visibility::Private {
+                                let span = hir_ident.location.span;
+                                self.check_can_reference_private_function(id, span);
+                            }
+                        }
                         DefinitionKind::Global(_) => {}
                         DefinitionKind::GenericType(_) => {
                             // Initialize numeric generics to a polymorphic integer type in case
@@ -1594,7 +1621,7 @@ mod test {
 
         let func_ids = vecmap(&func_namespace, |name| {
             let id = interner.push_fn(HirFunction::empty());
-            interner.push_function_definition(name.to_string(), id);
+            interner.push_function_definition(name.to_string(), id, true, ModuleId::dummy_id());
             id
         });
 
@@ -1605,10 +1632,11 @@ mod test {
         let mut errors = Vec::new();
         for func in program.functions {
             let id = interner.push_fn(HirFunction::empty());
-            interner.push_function_definition(func.name().to_string(), id);
+            let name = func.name().to_string();
+            interner.push_function_definition(name, id, true, ModuleId::dummy_id());
 
             let resolver = Resolver::new(&mut interner, &path_resolver, &def_maps, file);
-            let (_, _, err) = resolver.resolve_function(func, id, ModuleId::dummy_id());
+            let (_, _, err) = resolver.resolve_function(func, id);
             errors.extend(err);
         }
 
@@ -1622,11 +1650,12 @@ mod test {
         let mut all_captures: Vec<Vec<String>> = Vec::new();
         for func in program.functions {
             let id = interner.push_fn(HirFunction::empty());
-            interner.push_function_definition(func.name().to_string(), id);
+            let name = func.name().to_string();
+            interner.push_function_definition(name, id, true, ModuleId::dummy_id());
             path_resolver.insert_func(func.name().to_owned(), id);
 
             let resolver = Resolver::new(&mut interner, &path_resolver, &def_maps, file);
-            let (hir_func, _, _) = resolver.resolve_function(func, id, ModuleId::dummy_id());
+            let (hir_func, _, _) = resolver.resolve_function(func, id);
 
             // Iterate over function statements and apply filtering function
             parse_statement_blocks(