-
Notifications
You must be signed in to change notification settings - Fork 518
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
bug: Crash in Cairo compiler(WASM-Cairo) with Rust Analyzer's Salsa. #6629
Comments
Relative issue: #6171 |
Moving to a newer salsa version was required for making the language server viable for normal user usage. Having a wasm build for a fork of the compiler wasn't part of the considerations. |
Thanks for your reply! @orizi |
In any case - from looking at the failure stack - it seems like a salsa issue. have you tried rebasing over the latest on main? |
@cryptonerdcn you could give https://doc.rust-lang.org/cargo/reference/overriding-dependencies.html a try to forcefully compile with a stable salsa release, but I recall there were some source-wise differences in cycles handling (and there are definitely breaking changes regarding cancellation, but that one is scoped to LS only) I am currently trying to debug this also |
Well - you can see in the PR the adaptations we did to handle the changes, so the opposite will be required for a revert, but do note we needed to do some logical adaptations to handle the difference between these (their cycle handling logic is somewhat different) - which i assume you really won't want to do. |
@cryptonerdcn to me it looks like one of the unwraps here is failing: e7d5aa9#diff-04d31e27b9f6f114064562aa726da3f89794fc84843b002862ead0d5832a25ecR255-R267 I have analysed the stack trace you got vs the actual source that is built, and it looks like some stuff was aggressively inlined by Rust between these two frames:
The |
@mkaput Thank you! Because wasm can not really read a file from disk, the |
Nope, Salsa shouldn't be doing any I/O. Instead, my guess is that it changes some order of operations or smth, that is causing |
Good news: As @mkaput said, "there are definitely breaking changes regarding cancellation, but that one is scoped to LS only". |
Bug Report
Cairo version:
2.8.0
Current behavior:
I am cryptonerdcn, author of WASM-Cairo. After this PR(#6173), the WASM-Cairo version Cairo compiler will crash when the code uses
println!
.Expected behavior:
Steps to reproduce:
Checkout this branch: https://github.com/cryptonerdcn/cairo/tree/tmp/test_PR6173
Build:
Then run:
The code will run normally.
But if you run:
Will crash with this output:
Related code:
The previous PR #6245 (fb3d11c ) works normally. So I am sure the problem is from #6173 (3be4896).
Other information:
The text was updated successfully, but these errors were encountered: