Skip to content

Fix: resolution structs and import aliases to support separated namespaces#287

Merged
KyrylR merged 1 commit intoBlockstreamResearch:dev/importsfrom
LesterEvSe:fix/aliases
Apr 20, 2026
Merged

Fix: resolution structs and import aliases to support separated namespaces#287
KyrylR merged 1 commit intoBlockstreamResearch:dev/importsfrom
LesterEvSe:fix/aliases

Conversation

@LesterEvSe
Copy link
Copy Markdown
Collaborator

Currently, SimplicityHL allows separated namespaces. A user should be able to use the exact same name for both a type and a function in the same scope. For example:

type temp = u32;
fn temp() {}

The previous alias tracking functionality broke this behavior during the use import resolution phase.
So, This PR refactors the alias architecture and the process_use_item logic.

@LesterEvSe LesterEvSe requested a review from KyrylR April 17, 2026 14:43
@LesterEvSe LesterEvSe self-assigned this Apr 17, 2026
@LesterEvSe LesterEvSe requested a review from delta1 as a code owner April 17, 2026 14:43
@LesterEvSe LesterEvSe added the bug Something isn't working label Apr 17, 2026
@LesterEvSe LesterEvSe force-pushed the fix/aliases branch 2 times, most recently from 6a899a5 to 763086b Compare April 17, 2026 15:16
@KyrylR
Copy link
Copy Markdown
Collaborator

KyrylR commented Apr 18, 2026

Was this thing allowed in simplicity 0.4.1?

@KyrylR
Copy link
Copy Markdown
Collaborator

KyrylR commented Apr 18, 2026

Was this thing allowed in simplicity 0.4.1?

I was able to compile this:

fn foo() -> bool { false }
type foo = bool;

fn main() {
    let x: foo = false;
    assert!(foo());
}

Comment thread src/driver/mod.rs Outdated
/// This serves as the exact inverse of the `lookup` map.
paths: Vec<CanonPath>,

// NOTE: Consider to optimising this with `Vec` instead of `HashMap`
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// NOTE: Consider to optimising this with `Vec` instead of `HashMap`
// TODO: Consider to optimising this with `Vec` instead of `HashMap`

@KyrylR
Copy link
Copy Markdown
Collaborator

KyrylR commented Apr 19, 2026

At the same time the following test passes:

    #[test]
    fn test_private_alias_error_does_not_mask_duplicate_function_import() {
        let (graph, _ids, _dir) = setup_graph(vec![
            ("libs/lib/A.simf", "pub fn foo() {}"),
            ("libs/lib/B.simf", "pub fn foo() {} type foo = u32;"),
            ("main.simf", "use lib::A::foo; use lib::B::foo;"),
        ]);

        let mut error_handler = ErrorCollector::new();
        let program_option = graph.linearize_and_build(&mut error_handler).unwrap();
        let errors = error_handler.to_string();

        assert!(
            program_option.is_none(),
            "build should fail when a second import reuses the function name `foo`"
        );
    }

@KyrylR
Copy link
Copy Markdown
Collaborator

KyrylR commented Apr 19, 2026

ACK 763086b

So I would mention the behavior above in the md doc

@LesterEvSe
Copy link
Copy Markdown
Collaborator Author

Was this thing allowed in simplicity 0.4.1?

I was able to compile this:

fn foo() -> bool { false }
type foo = bool;

fn main() {
    let x: foo = false;
    assert!(foo());
}

Yes, SimplicityHL 0.4.1 allows this

@LesterEvSe
Copy link
Copy Markdown
Collaborator Author

At the same time the following test passes:

    #[test]
    fn test_private_alias_error_does_not_mask_duplicate_function_import() {
        let (graph, _ids, _dir) = setup_graph(vec![
            ("libs/lib/A.simf", "pub fn foo() {}"),
            ("libs/lib/B.simf", "pub fn foo() {} type foo = u32;"),
            ("main.simf", "use lib::A::foo; use lib::B::foo;"),
        ]);

        let mut error_handler = ErrorCollector::new();
        let program_option = graph.linearize_and_build(&mut error_handler).unwrap();
        let errors = error_handler.to_string();

        assert!(
            program_option.is_none(),
            "build should fail when a second import reuses the function name `foo`"
        );
    }

It is okay too. I added this test and explain why it passed

@KyrylR KyrylR merged commit feb901e into BlockstreamResearch:dev/imports Apr 20, 2026
11 checks passed
Copy link
Copy Markdown
Collaborator

@KyrylR KyrylR left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK feb901e

@KyrylR KyrylR mentioned this pull request Apr 20, 2026
6 tasks
@LesterEvSe LesterEvSe deleted the fix/aliases branch April 20, 2026 14:15
delta1 added a commit that referenced this pull request Apr 23, 2026
8b12ac6 fix: internal error inside resolution.rs and add error tests (Sdoba16)
da1240a test: add functional tests (Sdoba16)
7df36e7 feat: add `--dep` flag and connect driver to overall execution flow, add examples (LesterEvSe)
857cc5a feat: change ast.rs to match the functionality of the driver (LesterEvSe)
fda737f fix: resolution data structure and alias architecture (LesterEvSe)
260d844 feat: add and test import aliases (LesterEvSe)
8300fe5 feat: add linearize and build functionality to the driver, fix `Error` enum a bit (LesterEvSe)
f5999f2 feat: Added linearization algorithm (Sdoba16)
056a359 feat: implement `DependencyGraph` inside `driver.rs` file and change parse trait to the `SourceFile` (LesterEvSe)
e2604f3 feat: add data structure for dependency resolution (LesterEvSe)
2064542 feat: add `SourceFile` to the `RichError` and change the `ErrorCollector` (LesterEvSe)
80ae099 feat: add import errors (LesterEvSe)
3c7d4c3 feat: implement resolution.rs utils (LesterEvSe)
7ddeed2 feat: add pub and use keywords and parse them (LesterEvSe)

Pull request description:

  ## Motivation (copied from #258)

  The main goal of this PR is to enable an analogue of the OpenZeppelin library to exist in the SimplicityHL ecosystem. By introducing official compiler support for imports, we significantly increase code reusability and speed up application development. For projects heavily focused on security, this also allows developers to eventually "flatten" the code and audit all functions without external dependencies.

  ## Checklist

  - [x] **Visibility:** Added `pub` and `pub use` to control local scope versus re-exporting.
  - [x] **Import Syntax:** Implemented parsing and resolution for `use m::name;` and explicit aliasing (`as`).
  - [x] **Collision Handling:** Enforced strict errors for local scope collisions, utilizing linearization for resolving re-export conflicts.
  - [x] **Transitive Dependencies**: Add support for libraries importing other libraries.
  - [x] **Documentation:** Сode comments are updated accordingly.
  - [x] **Quality Assurance:** Code is formatted, all tests pass, and CI shows no errors.

  Related PRs (in order of the merge): #260, #256, #265, #266, #269, #278, #279, #280, #283, #287, #284, #289

ACKs for top commit:
  KyrylR:
    ACK 8b12ac6; code review and test runs (see #260, #256, #265, #266, #269, #278, #279, #280, #283, #287, #284, #289)
  delta1:
    ACK 8b12ac6; tested locally

Tree-SHA512: bec0d7437279dd45810cd8b13b48ba3921739d8c38f61d2f61401e419b32a2f4b31328df2c5596375fe8c69cabadc3e931a639ad833fe446918d1360091a447f
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants