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

emterpreter: support wildcards in EMTERPRETIFY_WHITELIST (#7988) #8056

Merged
merged 1 commit into from Feb 12, 2019

Conversation

Beuc
Copy link
Contributor

@Beuc Beuc commented Feb 10, 2019

Cf. #7988
I used wildcards (not regexs) as it seems simpler, though this can be changed.

Copy link
Member

@kripken kripken left a comment

Choose a reason for hiding this comment

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

Thanks, looks good, but please add a test. Can maybe make an existing whitelist test run twice, once with a wildcard.

ChangeLog.md Outdated
@@ -23,6 +23,10 @@ Current Trunk
ad hoc constructed rules around default Emscripten uses. The old behavior
will be deprecated and removed in the future. Build with -s ASSERTIONS=1
to get diagnostics messages related to this transition.

- Option -s EMTERPRETIFY_WHITELIST now accepts shell-style wildcards;
Copy link
Member

Choose a reason for hiding this comment

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

better to add new stuff on the top, as then when new releases are added, the new thing would not be at risk of being included by mistake in the wrong release. (I just added 1.38.27 now, so this will need to be moved)

@Beuc
Copy link
Contributor Author

Beuc commented Feb 11, 2019

Sure.
Any reason why we use

  @no_wasm_backend('uses EMTERPRETIFY')

?

@kripken
Copy link
Member

kripken commented Feb 12, 2019

Emterpretify runs on asm.js, and the wasm backend doesn't use asm.js as an intermediary stage, so it's not compatible with that.

@kripken kripken merged commit b007b1c into emscripten-core:incoming Feb 12, 2019
@Beuc Beuc deleted the patch-3 branch November 1, 2019 15:42
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