From 916b61507dc0dc91ef57abfb4090169506f76be7 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Fri, 27 Sep 2024 11:23:36 -0300 Subject: [PATCH 1/9] feat: visibility for modules --- compiler/noirc_frontend/src/ast/statement.rs | 1 + .../src/hir/def_collector/dc_mod.rs | 9 ++- .../src/hir/def_map/module_data.rs | 3 +- compiler/noirc_frontend/src/parser/mod.rs | 3 + compiler/noirc_frontend/src/parser/parser.rs | 18 ++++-- noir_stdlib/src/collections/mod.nr | 8 +-- noir_stdlib/src/ec/consts/mod.nr | 2 +- noir_stdlib/src/ec/mod.nr | 8 +-- noir_stdlib/src/ec/montcurve.nr | 4 +- noir_stdlib/src/ec/swcurve.nr | 4 +- noir_stdlib/src/ec/tecurve.nr | 4 +- noir_stdlib/src/field/mod.nr | 2 +- noir_stdlib/src/hash/mod.nr | 17 +++-- noir_stdlib/src/hash/poseidon/bn254.nr | 4 +- noir_stdlib/src/hash/poseidon/mod.nr | 2 +- noir_stdlib/src/lib.nr | 62 +++++++++---------- noir_stdlib/src/meta/mod.nr | 28 ++++----- noir_stdlib/src/ops/mod.nr | 4 +- tooling/nargo_fmt/src/visitor/item.rs | 8 ++- tooling/nargo_fmt/tests/expected/module.nr | 4 ++ tooling/nargo_fmt/tests/input/module.nr | 4 ++ 21 files changed, 114 insertions(+), 85 deletions(-) diff --git a/compiler/noirc_frontend/src/ast/statement.rs b/compiler/noirc_frontend/src/ast/statement.rs index 38cf2cb1d80..4abea8cebb4 100644 --- a/compiler/noirc_frontend/src/ast/statement.rs +++ b/compiler/noirc_frontend/src/ast/statement.rs @@ -294,6 +294,7 @@ pub trait Recoverable { #[derive(Debug, PartialEq, Eq, Clone)] pub struct ModuleDeclaration { + pub visibility: ItemVisibility, pub ident: Ident, pub outer_attributes: Vec, } diff --git a/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs b/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs index 162d39e0adb..b530e023152 100644 --- a/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs +++ b/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs @@ -381,6 +381,7 @@ impl<'a> ModCollector<'a> { let trait_id = match self.push_child_module( context, &name, + ItemVisibility::Public, Location::new(name.span(), self.file_id), Vec::new(), Vec::new(), @@ -612,6 +613,7 @@ impl<'a> ModCollector<'a> { match self.push_child_module( context, &submodule.name, + submodule.visibility, Location::new(submodule.name.span(), file_id), submodule.outer_attributes.clone(), submodule.contents.inner_attributes.clone(), @@ -711,6 +713,7 @@ impl<'a> ModCollector<'a> { match self.push_child_module( context, &mod_decl.ident, + mod_decl.visibility, Location::new(Span::empty(0), child_file_id), mod_decl.outer_attributes.clone(), ast.inner_attributes.clone(), @@ -761,6 +764,7 @@ impl<'a> ModCollector<'a> { &mut self, context: &mut Context, mod_name: &Ident, + visibility: ItemVisibility, mod_location: Location, outer_attributes: Vec, inner_attributes: Vec, @@ -772,6 +776,7 @@ impl<'a> ModCollector<'a> { &mut self.def_collector.def_map, self.module_id, mod_name, + visibility, mod_location, outer_attributes, inner_attributes, @@ -806,6 +811,7 @@ fn push_child_module( def_map: &mut CrateDefMap, parent: LocalModuleId, mod_name: &Ident, + visibility: ItemVisibility, mod_location: Location, outer_attributes: Vec, inner_attributes: Vec, @@ -840,7 +846,7 @@ fn push_child_module( // the struct name. if add_to_parent_scope { if let Err((first_def, second_def)) = - modules[parent.0].declare_child_module(mod_name.to_owned(), mod_id) + modules[parent.0].declare_child_module(mod_name.to_owned(), visibility, mod_id) { let err = DefCollectorErrorKind::Duplicate { typ: DuplicateType::Module, @@ -952,6 +958,7 @@ pub fn collect_struct( def_map, module_id, &name, + ItemVisibility::Public, location, Vec::new(), Vec::new(), diff --git a/compiler/noirc_frontend/src/hir/def_map/module_data.rs b/compiler/noirc_frontend/src/hir/def_map/module_data.rs index 149b1977925..ff36bcf27d6 100644 --- a/compiler/noirc_frontend/src/hir/def_map/module_data.rs +++ b/compiler/noirc_frontend/src/hir/def_map/module_data.rs @@ -135,9 +135,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 { diff --git a/compiler/noirc_frontend/src/parser/mod.rs b/compiler/noirc_frontend/src/parser/mod.rs index 1b6d573560d..ad6dd1459e9 100644 --- a/compiler/noirc_frontend/src/parser/mod.rs +++ b/compiler/noirc_frontend/src/parser/mod.rs @@ -366,6 +366,7 @@ pub enum ItemKind { /// These submodules always share the same file as some larger ParsedModule #[derive(Clone, Debug)] pub struct ParsedSubModule { + pub visibility: ItemVisibility, pub name: Ident, pub contents: ParsedModule, pub outer_attributes: Vec, @@ -375,6 +376,7 @@ pub struct ParsedSubModule { impl ParsedSubModule { pub fn into_sorted(self) -> SortedSubModule { SortedSubModule { + visibility: self.visibility, name: self.name, contents: self.contents.into_sorted(), outer_attributes: self.outer_attributes, @@ -398,6 +400,7 @@ impl std::fmt::Display for SortedSubModule { #[derive(Clone)] pub struct SortedSubModule { pub name: Ident, + pub visibility: ItemVisibility, pub contents: SortedModule, pub outer_attributes: Vec, pub is_contract: bool, diff --git a/compiler/noirc_frontend/src/parser/parser.rs b/compiler/noirc_frontend/src/parser/parser.rs index d902f0a3b5a..df656dc5a7d 100644 --- a/compiler/noirc_frontend/src/parser/parser.rs +++ b/compiler/noirc_frontend/src/parser/parser.rs @@ -262,14 +262,16 @@ fn submodule( module_parser: impl NoirParser, ) -> impl NoirParser { attributes() + .then(item_visibility()) .then_ignore(keyword(Keyword::Mod)) .then(ident()) .then_ignore(just(Token::LeftBrace)) .then(module_parser) .then_ignore(just(Token::RightBrace)) - .validate(|((attributes, name), contents), span, emit| { + .validate(|(((attributes, visibility), name), contents), span, emit| { let attributes = validate_secondary_attributes(attributes, span, emit); TopLevelStatementKind::SubModule(ParsedSubModule { + visibility, name, contents, outer_attributes: attributes, @@ -283,14 +285,16 @@ fn contract( module_parser: impl NoirParser, ) -> impl NoirParser { attributes() + .then(item_visibility()) .then_ignore(keyword(Keyword::Contract)) .then(ident()) .then_ignore(just(Token::LeftBrace)) .then(module_parser) .then_ignore(just(Token::RightBrace)) - .validate(|((attributes, name), contents), span, emit| { + .validate(|(((attributes, visibility), name), contents), span, emit| { let attributes = validate_secondary_attributes(attributes, span, emit); TopLevelStatementKind::SubModule(ParsedSubModule { + visibility, name, contents, outer_attributes: attributes, @@ -431,10 +435,14 @@ fn optional_type_annotation<'a>() -> impl NoirParser + 'a { } fn module_declaration() -> impl NoirParser { - attributes().then_ignore(keyword(Keyword::Mod)).then(ident()).validate( - |(attributes, ident), span, emit| { + attributes().then(item_visibility()).then_ignore(keyword(Keyword::Mod)).then(ident()).validate( + |((attributes, visibility), ident), span, emit| { let attributes = validate_secondary_attributes(attributes, span, emit); - TopLevelStatementKind::Module(ModuleDeclaration { ident, outer_attributes: attributes }) + TopLevelStatementKind::Module(ModuleDeclaration { + visibility, + ident, + outer_attributes: attributes, + }) }, ) } diff --git a/noir_stdlib/src/collections/mod.nr b/noir_stdlib/src/collections/mod.nr index 29f3e8cc854..d11e9f2febb 100644 --- a/noir_stdlib/src/collections/mod.nr +++ b/noir_stdlib/src/collections/mod.nr @@ -1,4 +1,4 @@ -mod vec; -mod bounded_vec; -mod map; -mod umap; +pub mod vec; +pub mod bounded_vec; +pub mod map; +pub mod umap; diff --git a/noir_stdlib/src/ec/consts/mod.nr b/noir_stdlib/src/ec/consts/mod.nr index f4d67e7a92c..73c594c6a26 100644 --- a/noir_stdlib/src/ec/consts/mod.nr +++ b/noir_stdlib/src/ec/consts/mod.nr @@ -1 +1 @@ -mod te; +pub mod te; diff --git a/noir_stdlib/src/ec/mod.nr b/noir_stdlib/src/ec/mod.nr index 3c1ba87eb9f..112f39f74eb 100644 --- a/noir_stdlib/src/ec/mod.nr +++ b/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 Weierstrass form -mod montcurve; // Montgomery curves -mod consts; // Commonly used curve presets +pub mod tecurve; // Twisted Edwards curves +pub mod swcurve; // Elliptic curves in Short Weierstrass 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 Weierstrass curves are diff --git a/noir_stdlib/src/ec/montcurve.nr b/noir_stdlib/src/ec/montcurve.nr index 395e8528b45..b8077b6b639 100644 --- a/noir_stdlib/src/ec/montcurve.nr +++ b/noir_stdlib/src/ec/montcurve.nr @@ -1,4 +1,4 @@ -mod affine { +pub mod affine { // Affine representation of Montgomery curves // Points are represented by two-dimensional Cartesian coordinates. // All group operations are induced by those of the corresponding Twisted Edwards curve. @@ -210,7 +210,7 @@ mod affine { } } } -mod curvegroup { +pub mod curvegroup { // Affine representation of Montgomery curves // Points are represented by three-dimensional projective (homogeneous) coordinates. // All group operations are induced by those of the corresponding Twisted Edwards curve. diff --git a/noir_stdlib/src/ec/swcurve.nr b/noir_stdlib/src/ec/swcurve.nr index 839069e1fd8..9c40ddd1adc 100644 --- a/noir_stdlib/src/ec/swcurve.nr +++ b/noir_stdlib/src/ec/swcurve.nr @@ -1,4 +1,4 @@ -mod affine { +pub mod affine { // Affine representation of Short Weierstrass curves // Points are represented by two-dimensional Cartesian coordinates. // Group operations are implemented in terms of those in CurveGroup (in this case, extended Twisted Edwards) coordinates @@ -186,7 +186,7 @@ mod affine { } } -mod curvegroup { +pub mod curvegroup { // CurveGroup representation of Weierstrass curves // Points are represented by three-dimensional Jacobian coordinates. // See for details. diff --git a/noir_stdlib/src/ec/tecurve.nr b/noir_stdlib/src/ec/tecurve.nr index f9cdf83aab9..c37b7c94a54 100644 --- a/noir_stdlib/src/ec/tecurve.nr +++ b/noir_stdlib/src/ec/tecurve.nr @@ -1,4 +1,4 @@ -mod affine { +pub mod affine { // Affine coordinate representation of Twisted Edwards curves // Points are represented by two-dimensional Cartesian coordinates. // Group operations are implemented in terms of those in CurveGroup (in this case, extended Twisted Edwards) coordinates @@ -193,7 +193,7 @@ mod affine { } } } -mod curvegroup { +pub mod curvegroup { // CurveGroup coordinate representation of Twisted Edwards curves // Points are represented by four-dimensional projective coordinates, viz. extended Twisted Edwards coordinates. // See section 3 of for details. diff --git a/noir_stdlib/src/field/mod.nr b/noir_stdlib/src/field/mod.nr index e1d08c6c230..93245e18072 100644 --- a/noir_stdlib/src/field/mod.nr +++ b/noir_stdlib/src/field/mod.nr @@ -1,4 +1,4 @@ -mod bn254; +pub mod bn254; use bn254::lt as bn254_lt; impl Field { diff --git a/noir_stdlib/src/hash/mod.nr b/noir_stdlib/src/hash/mod.nr index c69c3f9c49e..1b37a07f6c9 100644 --- a/noir_stdlib/src/hash/mod.nr +++ b/noir_stdlib/src/hash/mod.nr @@ -1,9 +1,9 @@ -mod poseidon; -mod mimc; -mod poseidon2; -mod keccak; -mod sha256; -mod sha512; +pub mod poseidon; +pub mod mimc; +pub mod poseidon2; +pub mod keccak; +pub mod sha256; +pub mod sha512; use crate::default::Default; use crate::uint128::U128; @@ -96,10 +96,7 @@ pub fn derive_generators(domain_separator_bytes: [u8; M] #[builtin(derive_pedersen_generators)] #[field(bn254)] -fn __derive_generators( - domain_separator_bytes: [u8; M], - starting_index: u32 -) -> [EmbeddedCurvePoint; N] {} +fn __derive_generators(domain_separator_bytes: [u8; M], starting_index: u32) -> [EmbeddedCurvePoint; N] {} #[field(bn254)] // Same as from_field but: diff --git a/noir_stdlib/src/hash/poseidon/bn254.nr b/noir_stdlib/src/hash/poseidon/bn254.nr index a1c78a9b31c..ce4b904ed5c 100644 --- a/noir_stdlib/src/hash/poseidon/bn254.nr +++ b/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::absorb; diff --git a/noir_stdlib/src/hash/poseidon/mod.nr b/noir_stdlib/src/hash/poseidon/mod.nr index 8a1025ada71..9553d352d28 100644 --- a/noir_stdlib/src/hash/poseidon/mod.nr +++ b/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::hash::Hasher; use crate::default::Default; diff --git a/noir_stdlib/src/lib.nr b/noir_stdlib/src/lib.nr index 714079d306a..3d1dd3e90eb 100644 --- a/noir_stdlib/src/lib.nr +++ b/noir_stdlib/src/lib.nr @@ -1,34 +1,34 @@ -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 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; -mod mem; -mod panic; +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 collections; +pub mod compat; +pub mod convert; +pub mod option; +pub mod string; +pub mod test; +pub mod cmp; +pub mod ops; +pub mod default; +pub mod prelude; +pub mod uint128; +pub mod bigint; +pub mod runtime; +pub mod meta; +pub mod append; +pub mod mem; +pub mod panic; // 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 diff --git a/noir_stdlib/src/meta/mod.nr b/noir_stdlib/src/meta/mod.nr index 0dc2e5fa714..378f0df1ba8 100644 --- a/noir_stdlib/src/meta/mod.nr +++ b/noir_stdlib/src/meta/mod.nr @@ -1,17 +1,17 @@ -mod ctstring; -mod expr; -mod format_string; -mod function_def; -mod module; -mod op; -mod struct_def; -mod trait_constraint; -mod trait_def; -mod trait_impl; -mod typ; -mod typed_expr; -mod quoted; -mod unresolved_type; +pub mod ctstring; +pub mod expr; +pub mod format_string; +pub mod function_def; +pub mod module; +pub mod op; +pub mod struct_def; +pub mod trait_constraint; +pub mod trait_def; +pub mod trait_impl; +pub mod typ; +pub mod typed_expr; +pub mod quoted; +pub mod unresolved_type; /// Calling unquote as a macro (via `unquote!(arg)`) will unquote /// its argument. Since this is the effect `!` already does, `unquote` diff --git a/noir_stdlib/src/ops/mod.nr b/noir_stdlib/src/ops/mod.nr index 6cf20432468..09f9f1eb9e6 100644 --- a/noir_stdlib/src/ops/mod.nr +++ b/noir_stdlib/src/ops/mod.nr @@ -1,5 +1,5 @@ -mod arith; -mod bit; +pub mod arith; +pub mod bit; pub use arith::{Add, Sub, Mul, Div, Rem, Neg}; pub use bit::{Not, BitOr, BitAnd, BitXor, Shl, Shr}; diff --git a/tooling/nargo_fmt/src/visitor/item.rs b/tooling/nargo_fmt/src/visitor/item.rs index b2a689919bd..2feae4b390c 100644 --- a/tooling/nargo_fmt/src/visitor/item.rs +++ b/tooling/nargo_fmt/src/visitor/item.rs @@ -7,7 +7,7 @@ use crate::{ visitor::expr::{format_seq, NewlineMode}, }; use noirc_frontend::{ - ast::{NoirFunction, TraitImplItemKind, Visibility}, + ast::{ItemVisibility, NoirFunction, TraitImplItemKind, Visibility}, macros_api::UnresolvedTypeData, }; use noirc_frontend::{ @@ -179,8 +179,12 @@ impl super::FmtVisitor<'_> { let after_brace = self.span_after(span, Token::LeftBrace).start(); self.last_position = after_brace; - let keyword = if module.is_contract { "contract" } else { "mod" }; + let visibility = module.visibility; + if visibility != ItemVisibility::Private { + self.push_str(&format!("{visibility} ")); + } + let keyword = if module.is_contract { "contract" } else { "mod" }; self.push_str(&format!("{keyword} {name} ")); if module.contents.items.is_empty() { diff --git a/tooling/nargo_fmt/tests/expected/module.nr b/tooling/nargo_fmt/tests/expected/module.nr index 0a051a1b50f..1d153b02078 100644 --- a/tooling/nargo_fmt/tests/expected/module.nr +++ b/tooling/nargo_fmt/tests/expected/module.nr @@ -33,3 +33,7 @@ mod a { #![inner] } } + +pub(crate) mod one {} + +pub mod two {} diff --git a/tooling/nargo_fmt/tests/input/module.nr b/tooling/nargo_fmt/tests/input/module.nr index 0a051a1b50f..3e01b81bcf0 100644 --- a/tooling/nargo_fmt/tests/input/module.nr +++ b/tooling/nargo_fmt/tests/input/module.nr @@ -33,3 +33,7 @@ mod a { #![inner] } } + +pub(crate) mod one { } + +pub mod two { } From bfd9c3f8972b6a7c54675991828923434d047fa6 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Fri, 27 Sep 2024 11:24:53 -0300 Subject: [PATCH 2/9] Add docs --- docs/docs/noir/modules_packages_crates/modules.md | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/docs/docs/noir/modules_packages_crates/modules.md b/docs/docs/noir/modules_packages_crates/modules.md index d21b009be3b..05399c38b4c 100644 --- a/docs/docs/noir/modules_packages_crates/modules.md +++ b/docs/docs/noir/modules_packages_crates/modules.md @@ -208,4 +208,14 @@ fn main() { } ``` -In this example, the module `some_module` re-exports two public names defined in `foo`. \ No newline at end of file +In this example, the module `some_module` re-exports two public names defined in `foo`. + +### Visibility + +By default, like functions, modules are private to the module (or crate) the exist in. You can use `pub` +to make the module public or `pub(crate)` to make it public to just its crate: + +```rust +// This module is now public and can be seen by other crates. +pub mod foo; +``` \ No newline at end of file From ffb2cf437321e567aa6c8e3bce1d966fe0ed3444 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Fri, 27 Sep 2024 11:51:57 -0300 Subject: [PATCH 3/9] Fix test program --- .../compile_success_empty/comptime_module/src/main.nr | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/test_programs/compile_success_empty/comptime_module/src/main.nr b/test_programs/compile_success_empty/comptime_module/src/main.nr index 9443b59fb0a..dc19fd20f7d 100644 --- a/test_programs/compile_success_empty/comptime_module/src/main.nr +++ b/test_programs/compile_success_empty/comptime_module/src/main.nr @@ -4,7 +4,7 @@ mod foo { pub fn x() {} pub fn y() {} - struct Struct1 {} + pub struct Struct1 {} } contract bar {} @@ -61,6 +61,8 @@ comptime fn add_function(m: Module) { } fn main() { + let _ = foo::Struct1 {}; + comptime { // Check Module::is_contract From 1fff52a4a50593a1301ea25f18ed1b7c47f4e6f4 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Fri, 27 Sep 2024 12:04:17 -0300 Subject: [PATCH 4/9] Fix LSP tests --- .../code_action/remove_unused_import.rs | 4 +-- tooling/lsp/src/requests/completion/tests.rs | 30 +++++++++---------- 2 files changed, 17 insertions(+), 17 deletions(-) diff --git a/tooling/lsp/src/requests/code_action/remove_unused_import.rs b/tooling/lsp/src/requests/code_action/remove_unused_import.rs index f1e12d64ef5..f5f0b520149 100644 --- a/tooling/lsp/src/requests/code_action/remove_unused_import.rs +++ b/tooling/lsp/src/requests/code_action/remove_unused_import.rs @@ -189,7 +189,7 @@ mod tests { let title = "Remove the whole `use` item"; let src = r#" - mod moo { + pub(crate) mod moo { pub fn foo() {} } @@ -202,7 +202,7 @@ mod tests { "#; let expected = r#" - mod moo { + pub(crate) mod moo { pub fn foo() {} } diff --git a/tooling/lsp/src/requests/completion/tests.rs b/tooling/lsp/src/requests/completion/tests.rs index 68cb3870f63..147fb5be4f3 100644 --- a/tooling/lsp/src/requests/completion/tests.rs +++ b/tooling/lsp/src/requests/completion/tests.rs @@ -149,8 +149,8 @@ mod completion_tests { async fn test_use_second_segment() { let src = r#" mod foo { - mod bar {} - mod baz {} + pub mod bar {} + pub mod baz {} } use foo::>|< "#; @@ -163,8 +163,8 @@ mod completion_tests { async fn test_use_second_segment_after_typing() { let src = r#" mod foo { - mod bar {} - mod brave {} + pub mod bar {} + pub mod brave {} } use foo::ba>|< "#; @@ -239,7 +239,7 @@ mod completion_tests { async fn test_use_in_tree_after_letter() { let src = r#" mod foo { - mod bar {} + pub mod bar {} } use foo::{b>|<} "#; @@ -251,8 +251,8 @@ mod completion_tests { async fn test_use_in_tree_after_colons() { let src = r#" mod foo { - mod bar { - mod baz {} + pub mod bar { + pub mod baz {} } } use foo::{bar::>|<} @@ -265,8 +265,8 @@ mod completion_tests { async fn test_use_in_tree_after_colons_after_another_segment() { let src = r#" mod foo { - mod bar {} - mod qux {} + pub mod bar {} + pub mod qux {} } use foo::{bar, q>|<} "#; @@ -350,7 +350,7 @@ mod completion_tests { async fn test_complete_path_after_colons_shows_submodule() { let src = r#" mod foo { - mod bar {} + pub mod bar {} } fn main() { @@ -364,7 +364,7 @@ mod completion_tests { async fn test_complete_path_after_colons_and_letter_shows_submodule() { let src = r#" mod foo { - mod qux {} + pub mod qux {} } fn main() { @@ -1809,8 +1809,8 @@ mod completion_tests { async fn test_suggests_pub_use() { let src = r#" mod bar { - mod baz { - mod coco {} + pub mod baz { + pub mod coco {} } pub use baz::coco; @@ -1827,8 +1827,8 @@ mod completion_tests { async fn test_auto_import_suggests_pub_use_for_module() { let src = r#" mod bar { - mod baz { - mod coco {} + pub mod baz { + pub mod coco {} } pub use baz::coco as foobar; From 8b257f3dac1ab91b11d657a47a68296d9dfa51a2 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Fri, 27 Sep 2024 12:13:17 -0300 Subject: [PATCH 5/9] Update noir_stdlib/src/ops/mod.nr Co-authored-by: Tom French <15848336+TomAFrench@users.noreply.github.com> --- noir_stdlib/src/ops/mod.nr | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/noir_stdlib/src/ops/mod.nr b/noir_stdlib/src/ops/mod.nr index 09f9f1eb9e6..6cf20432468 100644 --- a/noir_stdlib/src/ops/mod.nr +++ b/noir_stdlib/src/ops/mod.nr @@ -1,5 +1,5 @@ -pub mod arith; -pub mod bit; +mod arith; +mod bit; pub use arith::{Add, Sub, Mul, Div, Rem, Neg}; pub use bit::{Not, BitOr, BitAnd, BitXor, Shl, Shr}; From f808f7ffbd33986d0f1a164098f3d41a7b3593b4 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Fri, 27 Sep 2024 12:33:33 -0300 Subject: [PATCH 6/9] Make a couple of mods pub(crate) --- noir_stdlib/src/ops/mod.nr | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/noir_stdlib/src/ops/mod.nr b/noir_stdlib/src/ops/mod.nr index 6cf20432468..bf908ea4b27 100644 --- a/noir_stdlib/src/ops/mod.nr +++ b/noir_stdlib/src/ops/mod.nr @@ -1,5 +1,5 @@ -mod arith; -mod bit; +pub(crate) mod arith; +pub(crate) mod bit; pub use arith::{Add, Sub, Mul, Div, Rem, Neg}; pub use bit::{Not, BitOr, BitAnd, BitXor, Shl, Shr}; From e09fdf1e02fc4624e4710b4d8649a461cab5186b Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Fri, 27 Sep 2024 13:20:24 -0300 Subject: [PATCH 7/9] Fix test --- test_programs/compile_success_empty/comptime_module/src/main.nr | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test_programs/compile_success_empty/comptime_module/src/main.nr b/test_programs/compile_success_empty/comptime_module/src/main.nr index dc19fd20f7d..4c48c1b1a76 100644 --- a/test_programs/compile_success_empty/comptime_module/src/main.nr +++ b/test_programs/compile_success_empty/comptime_module/src/main.nr @@ -103,7 +103,7 @@ fn main() { // docs:start:as_module_example mod baz { - mod qux {} + pub mod qux {} } #[test] From 1fb63da037657b7ebca7ce579b92daeee5279b58 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Fri, 27 Sep 2024 13:44:07 -0300 Subject: [PATCH 8/9] Move visibility tests to a separate file --- compiler/noirc_frontend/src/tests.rs | 81 +---------------- .../noirc_frontend/src/tests/visibility.rs | 87 +++++++++++++++++++ 2 files changed, 88 insertions(+), 80 deletions(-) create mode 100644 compiler/noirc_frontend/src/tests/visibility.rs diff --git a/compiler/noirc_frontend/src/tests.rs b/compiler/noirc_frontend/src/tests.rs index 9b55e689eed..63013036ca9 100644 --- a/compiler/noirc_frontend/src/tests.rs +++ b/compiler/noirc_frontend/src/tests.rs @@ -6,6 +6,7 @@ mod name_shadowing; mod references; mod turbofish; mod unused_items; +mod visibility; // XXX: These tests repeat a lot of code // what we should do is have test cases which are passed to a test harness @@ -3092,29 +3093,6 @@ fn use_numeric_generic_in_trait_method() { assert_eq!(errors.len(), 0); } -#[test] -fn errors_once_on_unused_import_that_is_not_accessible() { - // Tests that we don't get an "unused import" here given that the import is not accessible - let src = r#" - mod moo { - struct Foo {} - } - use moo::Foo; - fn main() { - let _ = Foo {}; - } - "#; - - let errors = get_program_errors(src); - assert_eq!(errors.len(), 1); - assert!(matches!( - errors[0].0, - CompilationError::DefinitionError(DefCollectorErrorKind::PathResolutionError( - PathResolutionError::Private { .. } - )) - )); -} - #[test] fn trait_unconstrained_methods_typechecked_correctly() { // This test checks that we properly track whether a method has been declared as unconstrained on the trait definition @@ -3143,60 +3121,3 @@ fn trait_unconstrained_methods_typechecked_correctly() { println!("{errors:?}"); assert_eq!(errors.len(), 0); } - -#[test] -fn errors_if_type_alias_aliases_more_private_type() { - let src = r#" - struct Foo {} - pub type Bar = Foo; - - pub fn no_unused_warnings(_b: Bar) { - let _ = Foo {}; - } - - fn main() {} - "#; - - let errors = get_program_errors(src); - assert_eq!(errors.len(), 1); - - let CompilationError::ResolverError(ResolverError::TypeIsMorePrivateThenItem { - typ, item, .. - }) = &errors[0].0 - else { - panic!("Expected an unused item error"); - }; - - assert_eq!(typ, "Foo"); - assert_eq!(item, "Bar"); -} - -#[test] -fn errors_if_type_alias_aliases_more_private_type_in_generic() { - let src = r#" - pub struct Generic { value: T } - - struct Foo {} - pub type Bar = Generic; - - pub fn no_unused_warnings(_b: Bar) { - let _ = Foo {}; - let _ = Generic { value: 1 }; - } - - fn main() {} - "#; - - let errors = get_program_errors(src); - assert_eq!(errors.len(), 1); - - let CompilationError::ResolverError(ResolverError::TypeIsMorePrivateThenItem { - typ, item, .. - }) = &errors[0].0 - else { - panic!("Expected an unused item error"); - }; - - assert_eq!(typ, "Foo"); - assert_eq!(item, "Bar"); -} diff --git a/compiler/noirc_frontend/src/tests/visibility.rs b/compiler/noirc_frontend/src/tests/visibility.rs new file mode 100644 index 00000000000..2fb25d2062b --- /dev/null +++ b/compiler/noirc_frontend/src/tests/visibility.rs @@ -0,0 +1,87 @@ +use crate::{ + hir::{ + def_collector::{dc_crate::CompilationError, errors::DefCollectorErrorKind}, + resolution::{errors::ResolverError, import::PathResolutionError}, + }, + tests::get_program_errors, +}; + +#[test] +fn errors_once_on_unused_import_that_is_not_accessible() { + // Tests that we don't get an "unused import" here given that the import is not accessible + let src = r#" + mod moo { + struct Foo {} + } + use moo::Foo; + fn main() { + let _ = Foo {}; + } + "#; + + let errors = get_program_errors(src); + assert_eq!(errors.len(), 1); + assert!(matches!( + errors[0].0, + CompilationError::DefinitionError(DefCollectorErrorKind::PathResolutionError( + PathResolutionError::Private { .. } + )) + )); +} + +#[test] +fn errors_if_type_alias_aliases_more_private_type() { + let src = r#" + struct Foo {} + pub type Bar = Foo; + + pub fn no_unused_warnings(_b: Bar) { + let _ = Foo {}; + } + + fn main() {} + "#; + + let errors = get_program_errors(src); + assert_eq!(errors.len(), 1); + + let CompilationError::ResolverError(ResolverError::TypeIsMorePrivateThenItem { + typ, item, .. + }) = &errors[0].0 + else { + panic!("Expected an unused item error"); + }; + + assert_eq!(typ, "Foo"); + assert_eq!(item, "Bar"); +} + +#[test] +fn errors_if_type_alias_aliases_more_private_type_in_generic() { + let src = r#" + pub struct Generic { value: T } + + struct Foo {} + pub type Bar = Generic; + + pub fn no_unused_warnings(_b: Bar) { + let _ = Foo {}; + let _ = Generic { value: 1 }; + } + + fn main() {} + "#; + + let errors = get_program_errors(src); + assert_eq!(errors.len(), 1); + + let CompilationError::ResolverError(ResolverError::TypeIsMorePrivateThenItem { + typ, item, .. + }) = &errors[0].0 + else { + panic!("Expected an unused item error"); + }; + + assert_eq!(typ, "Foo"); + assert_eq!(item, "Bar"); +} From fed72a9485de87f7a487fa5a5d163f67dd2218e8 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Fri, 27 Sep 2024 13:47:19 -0300 Subject: [PATCH 9/9] Add test --- .../noirc_frontend/src/tests/visibility.rs | 26 +++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/compiler/noirc_frontend/src/tests/visibility.rs b/compiler/noirc_frontend/src/tests/visibility.rs index 2fb25d2062b..e6c2680ea19 100644 --- a/compiler/noirc_frontend/src/tests/visibility.rs +++ b/compiler/noirc_frontend/src/tests/visibility.rs @@ -85,3 +85,29 @@ fn errors_if_type_alias_aliases_more_private_type_in_generic() { assert_eq!(typ, "Foo"); assert_eq!(item, "Bar"); } + +#[test] +fn errors_if_trying_to_access_public_function_inside_private_module() { + let src = r#" + mod foo { + mod bar { + pub fn baz() {} + } + } + fn main() { + foo::bar::baz() + } + "#; + + let errors = get_program_errors(src); + assert_eq!(errors.len(), 2); // There's a bug that duplicates this error + + let CompilationError::ResolverError(ResolverError::PathResolutionError( + PathResolutionError::Private(ident), + )) = &errors[0].0 + else { + panic!("Expected a private error"); + }; + + assert_eq!(ident.to_string(), "bar"); +}