ethereum / fe

Emerging smart contract language for the Ethereum blockchain.
https://fe-lang.org
Other
1.6k stars 179 forks source link

Reconsider salsa recovery fn usage #939

Open Y-Nak opened 11 months ago

Y-Nak commented 11 months ago

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 https://github.com/salsa-rs/salsa/pull/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