Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Separate glob imports from item imports #5911

Closed
jjcnn opened this issue Apr 23, 2024 · 0 comments · Fixed by #5914
Closed

Separate glob imports from item imports #5911

jjcnn opened this issue Apr 23, 2024 · 0 comments · Fixed by #5914
Labels
compiler: frontend Everything to do with type checking, control flow analysis, and everything between parsing and IRgen

Comments

@jjcnn
Copy link
Contributor

jjcnn commented Apr 23, 2024

Single-item imports shadow glob imports with the same name:

// a.sw
struct Foo = { ... }

// b.sw
struct Foo = { ... }

// c.sw
use a::*;  // Foo is now in scope and refers to a::Foo
use b::Foo; // This is legal, and shadows the previous import so that Foo now refers to b::Foo

We currently represent all imported names in a single map Items::use_synonyms (sway-core/src/semantic_analysis/namespace/lexical_scope.rs), and shadowing is handled when that map is populated.

The code would be clearer if item imports and glob imports lived in separate maps use_glob_synonyms and use_item_synonyms, with the shadowing implemented in the name resolution logic.

@jjcnn jjcnn added the compiler: frontend Everything to do with type checking, control flow analysis, and everything between parsing and IRgen label Apr 23, 2024
IGI-111 pushed a commit that referenced this issue Apr 24, 2024
…orts (#5914)

## Description

Fixes #5911 .

So far all imported items in a module have been collected in single map,
with a flag in each value indicating whether it was the result of a
glob/star import or an item import (this distinction matters because
item imports shadow glob imports).

This PR splits the map in two: One for glob imports and one for item
imports. This simplifies name resolution, and eliminates the
`GlobImport` flag.

When another module imports all reexported items the logic is now
slightly more complex, but this will be resolved once the excessive
cloning of namespaces, modules and typed declarations are eliminated.

## Checklist

- [x] I have linked to any relevant issues.
- [x] I have commented my code, particularly in hard-to-understand
areas.
- [x] 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)
- [x] I have added tests that prove my fix is effective or that my
feature works.
- [x] 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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compiler: frontend Everything to do with type checking, control flow analysis, and everything between parsing and IRgen
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant