Skip to content

Conversation

@dcodeIO
Copy link
Contributor

@dcodeIO dcodeIO commented Aug 30, 2019

This PR adds a new parameter to BinaryenAddGlobalImport (C) respectively Module#addGlobalImport (JS) to support importing of mutable globals.

@dcodeIO
Copy link
Contributor Author

dcodeIO commented Aug 30, 2019

Also thinking if it makes sense to add a new Default feature flag (and use it as the default) that represents finished proposals, like MVP | MutableGlobals as of today? Considering to do something similar over at AssemblyScript in that everything finished is on by default. Just an idea, can be a follow-up PR.

Copy link
Member

@tlively tlively left a comment

Choose a reason for hiding this comment

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

Nice!

@tlively
Copy link
Member

tlively commented Aug 30, 2019

Also thinking if it makes sense to add a new Default feature flag (and use it as the default) that represents finished proposals, like MVP | MutableGlobals as of today? Considering to do something similar over at AssemblyScript in that everything finished is on by default. Just an idea, can be a follow-up PR.

I wonder if instead of adding a new flag for the default set, we could just update the default? We already have an --mvp-features flag for getting back to no features, so if people don't want their feature sets changing they can just pass that and explicitly enable features.

@MaxGraey
Copy link
Contributor

Yeah, instead adding new feature what about generalize (rename) from Feature::MVP to Feature::Default?

@dcodeIO
Copy link
Contributor Author

dcodeIO commented Aug 30, 2019

The MVP flag might still be useful to disable all features. Could as well be named None I guess. A Default flag might be useful to easily undo previous modifications to features, but I agree that it isn't strictly necessary since one could instead copy the defaults early and remember that.

In general I think that if we have MVP and All (or any other combined set of features), it makes sense to also have Default.

@kripken
Copy link
Member

kripken commented Sep 3, 2019

Thanks @dcodeIO!

About the Default issue, this seems like a large topic that other toolchain projects might also want to coordinate on. Perhaps we should open an issue on tool-conventions?

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.

4 participants