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

Introduce import statements #1469

Open
wants to merge 40 commits into
base: alex/export-statements
Choose a base branch
from

Conversation

sezna
Copy link
Contributor

@sezna sezna commented May 3, 2024

This PR introduces specific item imports, as opposed to our previous open-style imports which just glob-import an entire namespace.

For example:

import Foo.Bar;
import Foo.{Baz, Quux};
import Foo.{Bar.{Baz, Quux}, Corge as Grault};

@sezna sezna marked this pull request as ready for review May 15, 2024 17:38
@sezna sezna marked this pull request as draft May 20, 2024 17:43
@sezna sezna marked this pull request as ready for review May 21, 2024 21:56
if scope.terms.contains_key(&local_name.name)
|| scope.tys.contains_key(&local_name.name)
{
let err = Error::ImportedDuplicate(local_name.name.to_string(), local_name.span);
Copy link
Member

Choose a reason for hiding this comment

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

Not sure why but I'm getting the wrong span on this error in practice.

image

(Running the alex/glob-imports branch so not 100% sure which PR the bug is coming from).

if scope.terms.contains_key(&local_name.name)
|| scope.tys.contains_key(&local_name.name)
{
let err = Error::ImportedDuplicate(local_name.name.to_string(), local_name.span);
Copy link
Member

Choose a reason for hiding this comment

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

Where exactly should this error be thrown? I think technically, everything below the first open statement is a duplicate import, but I'm only getting errors for the last two.

open Bar;
import Bar.*;
import Bar.BarOperation;
import Bar.BarOperation;  // name error: imported symbol that already exists in scope
import BarOperation; // name error: imported symbol that already exists in scope

Copy link
Member

Choose a reason for hiding this comment

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

Something about the asymmetry between the import and export syntaxes bothers me.

import Foo; // doesn't allow multiple items, allows globs, no braces required
export { Foo, Bar }; // allows multiple items, doesn't allow globs, braces required

In practice, it took me a few tries to arrive at the correct syntax as I was typing in the editor - knowing the export syntax was no help in figuring out the import syntax. Is there an opportunity to bring these a little closer together?

Copy link
Member

Choose a reason for hiding this comment

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

I think there may be a problem with newtype imports:

namespace Foo {
    import Bar.NewType; // no error
    
    operation FooOperation() : Unit {
        let x: NewType = NewType("a");  // The constructor name is resolved, but the type name isn't
//             ^^^^^^^  name error: `NewType` not found
    }
}

namespace Bar {
    export { NewType }; // not sure if this is required, but it doesn't seem to matter
    newtype NewType = String;
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants