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

Improve the quality of the emscripten_set_main_loop abstraction #608

Merged
merged 5 commits into from
Aug 5, 2021

Conversation

lopopolo
Copy link
Member

@lopopolo lopopolo commented Aug 5, 2021

Lint the playground under the wasm32-unknown-emscripten target, remove raw pointers.

- No more raw pointers.
- Don't cast closures to `*mut c_void`.
- Store `RefCell<Option<Box<dyn FnMut()>>>` in the thread local.
- Add `'static` bound to closure type parameter, which is required for
  soundness.
- Don't panic or segfault if the thread local does not contain a valid
  `FnMut`.
- Remove unused imports and dead code. Remove lint `allow` pragmas.
- Address clippy lints re: pointer safety and item declaration order.
- Remove `F` type parameter from wrapper funtion and instead delegate to
  the trait object `FnMut` in the thread local.
@lopopolo lopopolo added A-wasm-build-target Area: Support for wasm build targets. O-wasm-emscripten Target: Support for building the `wasm32-unknown-emscripten` target. A-security Area: Security vulnerabilities and unsoundness issues. A-build Area: CI build infrastructure. labels Aug 5, 2021
@lopopolo lopopolo force-pushed the lint-emscripten-target branch from 8bf3f3a to 1fd6cbd Compare August 5, 2021 23:03
@lopopolo lopopolo merged commit 7e80f7e into trunk Aug 5, 2021
@lopopolo lopopolo deleted the lint-emscripten-target branch August 5, 2021 23:10
@lopopolo
Copy link
Member Author

lopopolo commented Aug 5, 2021

trunk builds started failing after this PR was merged due to mymindstorm/setup-emsdk#20.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-build Area: CI build infrastructure. A-security Area: Security vulnerabilities and unsoundness issues. A-wasm-build-target Area: Support for wasm build targets. O-wasm-emscripten Target: Support for building the `wasm32-unknown-emscripten` target.
Development

Successfully merging this pull request may close these issues.

1 participant