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

Refactor the wast component resolver #614

Merged

Conversation

alexcrichton
Copy link
Member

@alexcrichton alexcrichton commented May 18, 2022

More details in the two commits below, but this is entirely stylistic changes with no functional changes intended.

Closes #599

Currently `(start $f)` fails to compile because the loop that parses
values expected a `(result ..)` to always be present, but it is not
alwyas guaranteed to be present.
The purpose of this commit is to cut down on the complexity of reading
and understanding the resolver for components. These were some
refactorings I noticed during review that I think help make it a bit
more clear in each function what's happenign.

* Add a new `Resolver` type to encapsulate the `Vec<ComponentResolver>`
  stack
* Move freestanding functions to methods on `Resolver` and remove
  lifetime arguments on each function.
* Rename the `ComponentResolver` type to `ComponentState` to avoid
  conflicting with `Resolver`.
* Move `register*` functions to `ComponentState`
This refactors the injection of aliases to share the same code across
components themselves, component types, and instance types. This way the
drain of `aliases_to_insert` is only processed in one location to avoid
having to worry about it from multiple locations.
@alexcrichton alexcrichton merged commit 9d4a730 into bytecodealliance:main May 19, 2022
@alexcrichton alexcrichton deleted the refactor-component-resolve branch May 19, 2022 18:15
code-terror pushed a commit to code-terror/wasm-tools that referenced this pull request Aug 24, 2022
* Fix a failing test on `main`

Currently `(start $f)` fails to compile because the loop that parses
values expected a `(result ..)` to always be present, but it is not
alwyas guaranteed to be present.

* Refactor the `wast` resolver

The purpose of this commit is to cut down on the complexity of reading
and understanding the resolver for components. These were some
refactorings I noticed during review that I think help make it a bit
more clear in each function what's happenign.

* Add a new `Resolver` type to encapsulate the `Vec<ComponentResolver>`
  stack
* Move freestanding functions to methods on `Resolver` and remove
  lifetime arguments on each function.
* Rename the `ComponentResolver` type to `ComponentState` to avoid
  conflicting with `Resolver`.
* Move `register*` functions to `ComponentState`

* Refactor alias injection in component resolution

This refactors the injection of aliases to share the same code across
components themselves, component types, and instance types. This way the
drain of `aliases_to_insert` is only processed in one location to avoid
having to worry about it from multiple locations.
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.

components: Refactor the component resolver for readability
2 participants