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

Reconsider salsa recovery fn usage #939

Open
Y-Nak opened this issue Oct 10, 2023 · 0 comments
Open

Reconsider salsa recovery fn usage #939

Y-Nak opened this issue Oct 10, 2023 · 0 comments
Labels
v2 Issues in v2 implementation

Comments

@Y-Nak
Copy link
Collaborator

Y-Nak commented Oct 10, 2023

#931 introduced salsa recovery fn usage. However, the implementation of fallback in Salsa uses std::panic::catch_unwind, making it unworkable in WASM. This is due to the fact that WASM is in the standardization process for exceptions and also because support for WASM native exceptions on the Rust compiler side (rustc) is limited. Please refer to References section for more details.

NOTE: The old salsa doesn't use resume_unwind for the cycle recovering purpose, so it does work. But since salsa-rs/salsa#285 introduced resume/catch_unwind usage for cycle recovery, the wasm tests in the current analyzer will also fail when the new version that includes the PR is released.

How to fix it

Regarding this issue, the easiest approach would likely be to build a dependency graph(for types, aliases, traits, etc) for each ingot and calculate the SCC (Strongly Connected Components). However, considering the simplicity and performance of the implementation, and taking into account that Salsa internally builds a dependency graph, it would be ideal to use Salsa's recovery fallback if possible.

(TBW)

References

Support for native WASM exceptions
Implementation status in major browsers
Exception Handling Proposal

@Y-Nak Y-Nak added the v2 Issues in v2 implementation label Oct 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
v2 Issues in v2 implementation
Projects
None yet
Development

No branches or pull requests

1 participant