Skip to content

Commit

Permalink
Fix module ancestor check for imports (#6671)
Browse files Browse the repository at this point in the history
## Description

When importing items we allow imports from ancestor modules, even if the
ancestor module is declared as private.

The code that performs this check was moved from `module.rs` to
`root.rs` as part of #5697. During this move the current module path
(i.e., the path of the destination of the import) was mistakenly
replaced with the module path of the root module of the destination
module. Since root modules do not have ancestors this meant that no
source module was ever considered an ancestor of the destination module.

This PR fixes this problem, and adds an abscure test case that shows the
issue.


## Checklist

- [ ] I have linked to any relevant issues.
- [ ] I have commented my code, particularly in hard-to-understand
areas.
- [ ] I have updated the documentation where relevant (API docs, the
reference, and the Sway book).
- [ ] If my change requires substantial documentation changes, I have
[requested support from the DevRel
team](https://github.com/FuelLabs/devrel-requests/issues/new/choose)
- [ ] I have added tests that prove my fix is effective or that my
feature works.
- [ ] I have added (or requested a maintainer to add) the necessary
`Breaking*` or `New Feature` labels where relevant.
- [ ] I have done my best to ensure that my PR adheres to [the Fuel Labs
Code Review
Standards](https://github.com/FuelLabs/rfcs/blob/master/text/code-standards/external-contributors.md).
- [ ] I have requested a review from the relevant team or maintainers.

---------

Co-authored-by: Joshua Batty <[email protected]>
  • Loading branch information
jjcnn and JoshuaBatty authored Oct 24, 2024
1 parent cc842e3 commit a110a95
Show file tree
Hide file tree
Showing 11 changed files with 88 additions and 5 deletions.
10 changes: 5 additions & 5 deletions sway-core/src/semantic_analysis/namespace/root.rs
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,7 @@ impl Root {
dst: &ModulePath,
visibility: Visibility,
) -> Result<(), ErrorEmitted> {
self.check_module_privacy(handler, engines, src)?;
self.check_module_privacy(handler, engines, src, dst)?;

let src_mod = self.module.lookup_submodule(handler, engines, src)?;

Expand Down Expand Up @@ -358,7 +358,7 @@ impl Root {
alias: Option<Ident>,
visibility: Visibility,
) -> Result<(), ErrorEmitted> {
self.check_module_privacy(handler, engines, src)?;
self.check_module_privacy(handler, engines, src, dst)?;
let src_mod = self.module.lookup_submodule(handler, engines, src)?;

let (decl, path) = self.item_lookup(handler, engines, item, src, dst)?;
Expand Down Expand Up @@ -445,7 +445,7 @@ impl Root {
alias: Option<Ident>,
visibility: Visibility,
) -> Result<(), ErrorEmitted> {
self.check_module_privacy(handler, engines, src)?;
self.check_module_privacy(handler, engines, src, dst)?;

let decl_engine = engines.de();
let parsed_decl_engine = engines.pe();
Expand Down Expand Up @@ -610,7 +610,7 @@ impl Root {
enum_name: &Ident,
visibility: Visibility,
) -> Result<(), ErrorEmitted> {
self.check_module_privacy(handler, engines, src)?;
self.check_module_privacy(handler, engines, src, dst)?;

let parsed_decl_engine = engines.pe();
let decl_engine = engines.de();
Expand Down Expand Up @@ -689,8 +689,8 @@ impl Root {
handler: &Handler,
engines: &Engines,
src: &ModulePath,
dst: &ModulePath,
) -> Result<(), ErrorEmitted> {
let dst = self.module.mod_path();
// you are always allowed to access your ancestor's symbols
if !is_ancestor(src, dst) {
// we don't check the first prefix because direct children are always accessible
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
[[package]]
name = "core"
source = "path+from-root-044245A29CAD6C26"

[[package]]
name = "import_from_private_ancestor"
source = "member"
dependencies = ["core"]
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
[project]
authors = ["Fuel Labs <[email protected]>"]
license = "Apache-2.0"
name = "import_from_private_ancestor"
entry = "main.sw"
implicit-std = false

[dependencies]
core = { path = "../../../../../../../sway-lib-core" }
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
{
"configurables": [],
"functions": [
{
"attributes": null,
"inputs": [],
"name": "main",
"output": {
"name": "",
"type": 0,
"typeArguments": null
}
}
],
"loggedTypes": [],
"messagesTypes": [],
"types": [
{
"components": null,
"type": "bool",
"typeId": 0,
"typeParameters": null
}
]
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
{
"concreteTypes": [
{
"concreteTypeId": "b760f44fa5965c2474a3b471467a22c43185152129295af588b022ae50b50903",
"type": "bool"
}
],
"configurables": [],
"encodingVersion": "1",
"functions": [
{
"attributes": null,
"inputs": [],
"name": "main",
"output": "b760f44fa5965c2474a3b471467a22c43185152129295af588b022ae50b50903"
}
],
"loggedTypes": [],
"messagesTypes": [],
"metadataTypes": [],
"programType": "script",
"specVersion": "1"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
library;

mod foo;
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
library;

mod bar;
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
library;

mod baz;
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
library;

// This is legal even though foo is a private module, because foo is an ancestor of the current module
use ::foz::foo::*;
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
library;

mod foz;
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
category = "compile"
expected_warnings = 0

0 comments on commit a110a95

Please sign in to comment.