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

WIP: implement use for single file .wit #864

Closed
wants to merge 1 commit into from
Closed

WIP: implement use for single file .wit #864

wants to merge 1 commit into from

Conversation

fibonacci1729
Copy link
Contributor

This PR implements resolution of use for single file .wit. Interfaces are defined and resolved topologically.

Signed-off-by: Brian H brian.hardock@fermyon.com

Copy link
Member

@alexcrichton alexcrichton left a comment

Choose a reason for hiding this comment

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

Looking good to me!

}

pub struct Use<'a> {
pub names: Option<Vec<UseName<'a>>>,
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 it's ok to remove the * syntax since I don't think we really need that much any more and can always add that in later. For now I'm leaving that out of WebAssembly/component-model#141

Copy link
Contributor Author

@fibonacci1729 fibonacci1729 Dec 14, 2022

Choose a reason for hiding this comment

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

Should i also make a pass through the tests and delete any use * ... related ones?

Comment on lines 80 to 82
SortResults::Partial(_) => {
bail!("interface `use` cycle detected");
},
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 it's definitely ok to punt "really good error messages" to later, but I do think this should at least mention one interface (with a span) in the chain of cyclic interfaces

Copy link
Contributor Author

Choose a reason for hiding this comment

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

How does this look? The following wit ...

interface x {
    type m = string
    use { m } from y
}

interface y {
    use { m } from x
}

... produces the error:

Error: interface y contains a `use` cycle
     --> test.wit:6:11
      |
    6 | interface y {
      |

crates/wit-parser/src/ast/resolve.rs Outdated Show resolved Hide resolved
crates/wit-parser/src/ast/resolve.rs Outdated Show resolved Hide resolved
Signed-off-by: Brian H <brian.hardock@fermyon.com>
@alexcrichton
Copy link
Member

I've folded this into #867, although moreso conceptually than literally as I'm not doing a great job of persisting history and such. I wanted to note here though the progress on that!

@fibonacci1729
Copy link
Contributor Author

Excellent. I close this for now! Thanks @alexcrichton

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.

2 participants