Skip to content

Commit

Permalink
Fix resolution of local items that shadow imported ones (#5418)
Browse files Browse the repository at this point in the history
## Description

Closes #5383.

#5383 blocks PR #5306.

## Checklist

- [x] I have linked to any relevant issues.
- [x] 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).
- [x] 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.
- [x] 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).
- [x] I have requested a review from the relevant team or maintainers.
  • Loading branch information
ironcev authored Dec 22, 2023
1 parent b963086 commit a676a3d
Show file tree
Hide file tree
Showing 21 changed files with 353 additions and 44 deletions.
1 change: 1 addition & 0 deletions sway-core/src/semantic_analysis/namespace/items.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ pub(crate) enum GlobImport {
}

pub(super) type SymbolMap = im::OrdMap<Ident, ty::TyDecl>;
// The final `bool` field of `UseSynonyms` is true if the `Vec<Ident>` path is absolute.
pub(super) type UseSynonyms = im::HashMap<Ident, (Vec<Ident>, GlobImport, ty::TyDecl, bool)>;
pub(super) type UseAliases = im::HashMap<String, Ident>;

Expand Down
15 changes: 13 additions & 2 deletions sway-core/src/semantic_analysis/namespace/root.rs
Original file line number Diff line number Diff line change
Expand Up @@ -342,8 +342,19 @@ impl Root {
match module.use_synonyms.get(symbol) {
Some((_, _, decl @ ty::TyDecl::EnumVariantDecl { .. }, _)) => Ok(decl.clone()),
Some((src_path, _, _, _)) if mod_path != src_path => {
// TODO: check that the symbol import is public?
self.resolve_symbol(handler, engines, src_path, true_symbol, self_type)
// If the symbol is imported, before resolving to it,
// we need to check if there is a local symbol withing the module with
// the same name, and if yes resolve to the local symbol, because it
// shadows the import.
// Note that we can have two situations here:
// - glob-import, in which case the local symbol simply shadows the glob-imported one.
// - non-glob import, in which case we will already have a name clash reported
// as an error, but still have to resolve to the local module symbol
// if it exists.
match module.symbols.get(true_symbol) {
Some(decl) => Ok(decl.clone()),
None => self.resolve_symbol(handler, engines, src_path, true_symbol, self_type),
}
}
_ => module
.check_symbol(true_symbol)
Expand Down
2 changes: 1 addition & 1 deletion sway-core/src/semantic_analysis/type_check_context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -606,7 +606,7 @@ impl<'a> TypeCheckContext<'a> {
///
/// The `mod_path` is significant here as we assume the resolution is done within the
/// context of the module pointed to by `mod_path` and will only check the call path prefixes
/// and the symbol's own visibility
/// and the symbol's own visibility.
pub(crate) fn resolve_call_path_with_visibility_check_and_modpath(
&self,
handler: &Handler,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
[[package]]
name = "core"
source = "path+from-root-8AA4CB15A1A05A28"

[[package]]
name = "resolve_local_items_that_shadow_imports"
source = "member"
dependencies = ["std"]

[[package]]
name = "std"
source = "path+from-root-8AA4CB15A1A05A28"
dependencies = ["core"]
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
[project]
name = "resolve_local_items_that_shadow_imports"
authors = ["Fuel Labs <contact@fuel.sh>"]
entry = "main.sw"
license = "Apache-2.0"

[dependencies]
std = { path = "../../../../../../sway-lib-std" }
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": [],
"type": "()",
"typeId": 0,
"typeParameters": null
}
]
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
library;

pub enum Enum {
A: (),
}

pub struct Struct {
x: u64,
}

pub struct PubStruct {
x: u64,
}

pub struct GenericStruct<T> {
x: T,
}

pub const X: u64 = 0u64;
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
// This test proves that https://github.com/FuelLabs/sway/issues/5383 is fixed.

script;

mod lib;

use lib::Enum;
use lib::Struct;
use lib::PubStruct;
use lib::GenericStruct;

enum Enum {
A: (),
B: (),
}

struct Struct {
x: u64,
y: u64,
}

impl Struct {
fn use_me(self) {
poke(self.x);
poke(self.y);
}
}

pub struct PubStruct {
x: u64,
y: u64,
}

impl PubStruct {
fn use_me(self) {
poke(self.x);
poke(self.y);
}
}

struct GenericStruct<T> {
x: T,
y: u64,
}

impl<T> GenericStruct<T> {
fn use_me(self) {
poke(self.x);
poke(self.y);
}
}

pub const X: bool = true;

fn main() {
// We must get errors for defining items multiple times,
// but that should be all errors.
// We shouldn't have any errors below, because the
// below items will resolve to local ones.
let _ = Struct { x: 0, y: 0 };
let _ = PubStruct { x: 0, y: 0 };
let _ = GenericStruct { x: 0, y: 0 };
let _ = Enum::B;
let _: bool = X;

Struct { x: 0, y: 0 }.use_me();
PubStruct { x: 0, y: 0 }.use_me();
GenericStruct { x: 0, y: 0 }.use_me();
poke(Enum::A);
}

fn poke<T>(_x: T) { }
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
category = "fail"

#check: $()error
#check: $()enum Enum {
#nextln: $()Name "Enum" is defined multiple times.

#check: $()error
#check: $()struct Struct {
#nextln: $()Name "Struct" is defined multiple times.

#check: $()error
#check: $()pub struct PubStruct {
#nextln: $()Name "PubStruct" is defined multiple times.

#check: $()error
#check: $()struct GenericStruct<T> {
#nextln: $()Name "GenericStruct" is defined multiple times.

#not: $()error
Original file line number Diff line number Diff line change
@@ -1,10 +1,5 @@
script;

fn revert<T>() -> T {
let code = 1u64;
__revert(code)
}

fn diverge_in_let_body() -> u64 {
let _x: bool = {
return 5;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,37 +12,14 @@
}
}
],
"loggedTypes": [
{
"logId": 0,
"loggedType": {
"name": "",
"type": 1,
"typeArguments": null
}
},
{
"logId": 1,
"loggedType": {
"name": "",
"type": 1,
"typeArguments": null
}
}
],
"loggedTypes": [],
"messagesTypes": [],
"types": [
{
"components": [],
"type": "()",
"typeId": 0,
"typeParameters": null
},
{
"components": null,
"type": "u64",
"typeId": 1,
"typeParameters": null
}
]
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,12 @@ struct CustomType {
name: str,
}

impl CustomType {
fn use_me(self) {
poke(self.name);
}
}

enum MyResult<T, E> {
Ok: T,
Err: E,
Expand Down Expand Up @@ -56,12 +62,15 @@ fn test_try_from() {
}

fn main() {
sell_product();
let _ = sell_product();
simple_vec_test();
complex_vec_test();
simple_option_generics_test();
test_assert_eq_u64();
test_try_from();

// Suppress DCA warnings.
CustomType { name: "" }.use_me();
}

fn sell_product() -> MyResult<bool, CustomType> {
Expand All @@ -73,3 +82,5 @@ fn sell_product() -> MyResult<bool, CustomType> {

return MyResult::Ok(false);
}

fn poke<T>(_x: T) { }
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
[[package]]
name = "core"
source = "path+from-root-8AA4CB15A1A05A28"

[[package]]
name = "resolve_local_items_that_shadow_imports"
source = "member"
dependencies = ["std"]

[[package]]
name = "std"
source = "path+from-root-8AA4CB15A1A05A28"
dependencies = ["core"]
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
[project]
name = "resolve_local_items_that_shadow_imports"
authors = ["Fuel Labs <contact@fuel.sh>"]
entry = "main.sw"
license = "Apache-2.0"

[dependencies]
std = { path = "../../../../../../sway-lib-std" }
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": [],
"type": "()",
"typeId": 0,
"typeParameters": null
}
]
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
library;

pub enum Enum {
A: (),
}

pub struct Struct {
x: u64,
}

pub struct PubStruct {
x: u64,
}

pub struct GenericStruct<T> {
x: T,
}

pub const X: u64 = 0u64;
Loading

0 comments on commit a676a3d

Please sign in to comment.