feat: catch imports of private definitions in resolution

compiler/noirc_frontend/src/ast/mod.rs

@@ -385,6 +385,18 @@ pub enum ItemVisibility {
     Public,
     Private,
     PublicCrate,
+    PublicSuper,
+}
+
+impl std::fmt::Display for ItemVisibility {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        match self {
+            Self::Public => write!(f, "pub"),
+            Self::Private => write!(f, "priv"),
+            Self::PublicCrate => write!(f, "pub(crate)"),
+            Self::PublicSuper => write!(f, "pub(super)"),
+        }
+    }
 }
 
 #[derive(Clone, Copy, Debug, PartialEq, Eq, Hash, Serialize, Deserialize)]

compiler/noirc_frontend/src/ast/statement.rs

@@ -280,10 +280,15 @@ pub trait Recoverable {
 #[derive(Debug, PartialEq, Eq, Clone)]
 pub struct ModuleDeclaration {
     pub ident: Ident,
+    pub visibility: ItemVisibility,
 }
 
 impl std::fmt::Display for ModuleDeclaration {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        if self.visibility != ItemVisibility::Private {
+            write!(f, "{} ", self.visibility)?;
+        };
+
         write!(f, "mod {}", self.ident)
     }
 }

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

@@ -285,6 +285,7 @@ impl<'a> ModCollector<'a> {
             let id = match self.push_child_module(
                 context,
                 &name,
+                ItemVisibility::Public,
                 Location::new(name.span(), self.file_id),
                 false,
                 false,
@@ -386,6 +387,7 @@ impl<'a> ModCollector<'a> {
             let trait_id = match self.push_child_module(
                 context,
                 &name,
+                ItemVisibility::Public,
                 Location::new(name.span(), self.file_id),
                 false,
                 false,
@@ -552,6 +554,7 @@ impl<'a> ModCollector<'a> {
             match self.push_child_module(
                 context,
                 &submodule.name,
+                ItemVisibility::Public,
                 Location::new(submodule.name.span(), file_id),
                 true,
                 submodule.is_contract,
@@ -643,6 +646,7 @@ impl<'a> ModCollector<'a> {
         match self.push_child_module(
             context,
             &mod_decl.ident,
+            ItemVisibility::Public,
             Location::new(Span::empty(0), child_file_id),
             true,
             false,
@@ -676,6 +680,7 @@ impl<'a> ModCollector<'a> {
         &mut self,
         context: &mut Context,
         mod_name: &Ident,
+        visibility: ItemVisibility,
         mod_location: Location,
         add_to_parent_scope: bool,
         is_contract: bool,
@@ -711,9 +716,11 @@ impl<'a> ModCollector<'a> {
         // to a child module containing its methods) since the module name should not shadow
         // the struct name.
         if add_to_parent_scope {
-            if let Err((first_def, second_def)) =
-                modules[self.module_id.0].declare_child_module(mod_name.to_owned(), mod_id)
-            {
+            if let Err((first_def, second_def)) = modules[self.module_id.0].declare_child_module(
+                mod_name.to_owned(),
+                visibility,
+                mod_id,
+            ) {
                 let err = DefCollectorErrorKind::Duplicate {
                     typ: DuplicateType::Module,
                     first_def,

compiler/noirc_frontend/src/hir/def_map/module_data.rs

@@ -102,9 +102,10 @@ impl ModuleData {
     pub fn declare_child_module(
         &mut self,
         name: Ident,
+        visibility: ItemVisibility,
         child_id: ModuleId,
     ) -> Result<(), (Ident, Ident)> {
-        self.declare(name, ItemVisibility::Public, child_id.into(), None)
+        self.declare(name, visibility, child_id.into(), None)
     }
 
     pub fn find_func_with_name(&self, name: &Ident) -> Option<FuncId> {

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

@@ -359,6 +359,18 @@ fn can_reference_module_id(
     match visibility {
         ItemVisibility::Public => true,
         ItemVisibility::PublicCrate => same_crate,
+        ItemVisibility::PublicSuper => {
+            same_crate
+                && (module_descendent_of_target(
+                    target_crate_def_map,
+                    target_module.local_id,
+                    current_module,
+                ) || module_parent_of_target(
+                    target_crate_def_map,
+                    target_module.local_id,
+                    current_module,
+                ))
+        }
         ItemVisibility::Private => {
             same_crate
                 && module_descendent_of_target(
@@ -385,3 +397,12 @@ fn module_descendent_of_target(
         .parent
         .map_or(false, |parent| module_descendent_of_target(def_map, target, parent))
 }
+
+// Returns true if `target` is a direct child module of `current`.
+fn module_parent_of_target(
+    def_map: &CrateDefMap,
+    target: LocalModuleId,
+    current: LocalModuleId,
+) -> bool {
+    def_map.modules[target.0].parent.map_or(false, |parent| parent == current)
+}

compiler/noirc_frontend/src/parser/parser.rs

@@ -461,10 +461,37 @@ fn optional_type_annotation<'a>() -> impl NoirParser<UnresolvedType> + 'a {
         .map(|r#type| r#type.unwrap_or_else(UnresolvedType::unspecified))
 }
 
+/// import_visibility: 'pub(crate)' | 'pub' | %empty
+fn import_visibility() -> impl NoirParser<ItemVisibility> {
+    let is_pub_crate = (keyword(Keyword::Pub)
+        .then_ignore(just(Token::LeftParen))
+        .then_ignore(keyword(Keyword::Crate))
+        .then_ignore(just(Token::RightParen)))
+    .map(|_| ItemVisibility::PublicCrate);
+
+    let is_pub_super = (keyword(Keyword::Pub)
+        .then_ignore(just(Token::LeftParen))
+        .then_ignore(keyword(Keyword::Super))
+        .then_ignore(just(Token::RightParen)))
+    .map(|_| ItemVisibility::PublicSuper);
+
+    let is_pub = keyword(Keyword::Pub).map(|_| ItemVisibility::Public);
+
+    let is_private = empty().map(|_| ItemVisibility::Private);
+
+    choice((is_pub_crate, is_pub_super, is_pub, is_private))
+}
+
 fn module_declaration() -> impl NoirParser<TopLevelStatement> {
-    keyword(Keyword::Mod)
-        .ignore_then(ident())
-        .map(|ident| TopLevelStatement::Module(ModuleDeclaration { ident }))
+    import_visibility().then_ignore(keyword(Keyword::Mod)).then(ident()).map(
+        |(mut visibility, ident)| {
+            // A module's contents are visible to the parent module unless they themselves are marked private.
+            if visibility == ItemVisibility::Private {
+                visibility = ItemVisibility::PublicSuper;
+            }
+            TopLevelStatement::Module(ModuleDeclaration { ident, visibility })
+        },
+    )
 }
 
 fn use_statement() -> impl NoirParser<TopLevelStatement> {

compiler/noirc_frontend/src/parser/parser/function.rs

@@ -51,28 +51,13 @@ pub(super) fn function_definition(allow_self: bool) -> impl NoirParser<NoirFunct
         })
 }
 
-/// visibility_modifier: 'pub(crate)'? 'pub'? ''
-fn visibility_modifier() -> impl NoirParser<ItemVisibility> {
-    let is_pub_crate = (keyword(Keyword::Pub)
-        .then_ignore(just(Token::LeftParen))
-        .then_ignore(keyword(Keyword::Crate))
-        .then_ignore(just(Token::RightParen)))
-    .map(|_| ItemVisibility::PublicCrate);
-
-    let is_pub = keyword(Keyword::Pub).map(|_| ItemVisibility::Public);
-
-    let is_private = empty().map(|_| ItemVisibility::Private);
-
-    choice((is_pub_crate, is_pub, is_private))
-}
-
 /// function_modifiers: 'unconstrained'? (visibility)?
 ///
 /// returns (is_unconstrained, visibility) for whether each keyword was present
 fn function_modifiers() -> impl NoirParser<(bool, ItemVisibility, bool)> {
     keyword(Keyword::Unconstrained)
         .or_not()
-        .then(visibility_modifier())
+        .then(import_visibility())
         .then(maybe_comp_time())
         .map(|((unconstrained, visibility), comptime)| {
             (unconstrained.is_some(), visibility, comptime)

compiler/wasm/test/fixtures/deps/lib-c/src/lib.nr

@@ -1 +1 @@
-mod module;
+pub mod module;

compiler/wasm/test/fixtures/deps/lib-c/src/module.nr

@@ -1 +1 @@
-mod foo;
+pub mod foo;

noir_stdlib/src/collections/mod.nr

@@ -1,3 +1,3 @@
-mod vec;
-mod bounded_vec;
-mod map;
+pub mod vec;
+pub mod bounded_vec;
+pub mod map;

noir_stdlib/src/ec/consts/mod.nr

@@ -1 +1 @@
-mod te;
+pub mod te;

noir_stdlib/src/ec/mod.nr

@@ -2,10 +2,10 @@
 // Overview
 // ========
 // The following three elliptic curve representations are admissible:
-mod tecurve; // Twisted Edwards curves
-mod swcurve; // Elliptic curves in Short Weierstraß form
-mod montcurve; // Montgomery curves
-mod consts; // Commonly used curve presets
+pub mod tecurve; // Twisted Edwards curves
+pub mod swcurve; // Elliptic curves in Short Weierstraß form
+pub mod montcurve; // Montgomery curves
+pub mod consts; // Commonly used curve presets
 //
 // Note that Twisted Edwards and Montgomery curves are (birationally) equivalent, so that
 // they may be freely converted between one another, whereas Short Weierstraß curves are

noir_stdlib/src/field/mod.nr

@@ -1,4 +1,4 @@
-mod bn254;
+pub mod bn254;
 use bn254::lt as bn254_lt;
 
 impl Field {

noir_stdlib/src/hash/mod.nr

@@ -1,6 +1,7 @@
-mod poseidon;
-mod mimc;
-mod poseidon2;
+pub mod poseidon;
+pub mod mimc;
+pub mod poseidon2;
+pub mod pedersen;
 
 use crate::default::Default;
 use crate::uint128::U128;

noir_stdlib/src/hash/poseidon/bn254.nr

@@ -1,6 +1,6 @@
 // Instantiations of Poseidon constants, permutations and sponge for prime field of the same order as BN254
-mod perm;
-mod consts;
+pub mod perm;
+pub mod consts;
 
 use crate::hash::poseidon::{PoseidonConfig, absorb};
 

noir_stdlib/src/hash/poseidon/mod.nr

@@ -1,4 +1,4 @@
-mod bn254; // Instantiations of Poseidon for prime field of the same order as BN254
+pub mod bn254; // Instantiations of Poseidon for prime field of the same order as BN254
 use crate::field::modulus_num_bits;
 use crate::hash::Hasher;
 use crate::default::Default;

noir_stdlib/src/lib.nr

@@ -1,33 +1,33 @@
-mod hash;
-mod aes128;
-mod array;
-mod slice;
-mod merkle;
-mod schnorr;
-mod ecdsa_secp256k1;
-mod ecdsa_secp256r1;
-mod eddsa;
-mod embedded_curve_ops;
-mod sha256;
-mod sha512;
-mod field;
-mod ec;
-mod unsafe;
-mod collections;
-mod compat;
-mod convert;
-mod option;
-mod string;
-mod test;
-mod cmp;
-mod ops;
-mod default;
-mod prelude;
-mod uint128;
-mod bigint;
-mod runtime;
-mod meta;
-mod append;
+pub mod hash;
+pub mod aes128;
+pub mod array;
+pub mod slice;
+pub mod merkle;
+pub mod schnorr;
+pub mod ecdsa_secp256k1;
+pub mod ecdsa_secp256r1;
+pub mod eddsa;
+pub mod embedded_curve_ops;
+pub mod sha256;
+pub mod sha512;
+pub mod field;
+pub mod ec;
+pub mod unsafe;
+pub mod collections;
+pub mod compat;
+pub(crate) mod convert;
+pub mod option;
+pub mod string;
+pub mod test;
+pub mod cmp;
+pub mod ops;
+pub(crate) mod default;
+pub mod prelude;
+pub mod uint128;
+pub mod bigint;
+pub mod runtime;
+pub mod meta;
+pub mod append;
 
 // Oracle calls are required to be wrapped in an unconstrained function
 // Thus, the only argument to the `println` oracle is expected to always be an ident

test_programs/execution_success/global_consts/src/foo.nr

@@ -1,4 +1,4 @@
-mod bar;
+pub(crate) mod bar;
 
 global N: u64 = 5;
 global MAGIC_NUMBER: u64 = 3;

test_programs/execution_success/import/src/import.nr

@@ -1,3 +1,5 @@
-pub fn hello(x: Field) -> Field {
+pub(crate) mod foo;
+
+pub(crate) fn hello(x: Field) -> Field {
     x
 }

test_programs/execution_success/import/src/import/foo.nr

@@ -0,0 +1,3 @@
+pub fn goodbye(x: Field) -> Field {
+    x
+}

test_programs/execution_success/import/src/main.nr

@@ -1,9 +1,12 @@
 mod import;
 use crate::import::hello;
+use import::foo::goodbye;
 
 fn main(x: Field, y: Field) {
     let _k = std::hash::pedersen_commitment([x]);
     let _l = hello(x);
+    let _l = goodbye(x);
 
     assert(x != import::hello(y));
+    assert(x != goodbye(y));
 }

test_programs/execution_success/modules_more/src/foo.nr

@@ -1,4 +1,4 @@
-mod bar;
+pub(crate) mod bar;
 
 fn hello(x: Field) -> Field {
     x

test_programs/execution_success/struct_inputs/src/foo.nr

@@ -1,4 +1,4 @@
-mod bar;
+pub(crate) mod bar;
 
 struct fooStruct {
     bar_struct: bar::barStruct,