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

Type-Safe/Non-Boxing Global #130

Merged

Conversation

martindevans
Copy link
Contributor

Used the same mechanisms implemented for type-safe/non-boxing functions to wrap Global in the same way. I expected this to take longer to implement, but everything just fell into place perfectly 🎉

@martindevans
Copy link
Contributor Author

martindevans commented Jun 29, 2022

Some open questions with how this is implemented:

  • Name: Currently it's Global.Accessor<T> but Global.Wrapper<T> may be more consistent?
  • Mutability: Currently Set(T) does a runtime check and throws if the global is immutable. Some alternatives:
    • WrapGetter<T> and WrapSetter<T> methods, with WrapSetter returning null if the global is immutable.
    • Wrap<T> method which returns (Getter<T>, Setter<T>?) i.e. the setter is null if the global is immutable.

Copy link
Member

@peterhuene peterhuene left a comment

Choose a reason for hiding this comment

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

Thanks for putting this up! Just the one nit comment, but everything else looks good to go!

src/Global.cs Outdated Show resolved Hide resolved
@martindevans
Copy link
Contributor Author

I've filled in that doc comment :)

@peterhuene
Copy link
Member

Ignoring transient CI failure.

@peterhuene peterhuene merged commit e970107 into bytecodealliance:main Jul 5, 2022
@peterhuene
Copy link
Member

@martindevans thanks again for all these great contributions!

@martindevans martindevans deleted the feature/wrapped_globals branch July 5, 2022 20:43
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