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
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