-
Notifications
You must be signed in to change notification settings - Fork 70
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
Add diagnostics and handle errors gracefully / Upgrade Cairo. #1590
Conversation
74d5183
to
f614f01
Compare
Hi! I've managed to run some benchmarks. For
I tried to run parallel diagnostics warmup (inspired with https://github.com/starkware-libs/cairo/blob/e4136a412b03aff3ba1dec3eb8d8d22356cd67ae/crates/cairo-lang-compiler/src/diagnostics.rs#L220), but it didn't really help to bring the time down at all, resulting in Please note that compilation of the Given that, the cost of running diagnostics is obviously high, but nothing concerning. We should do that. I would opt for trying to expose an API in the compiler repository, that does not require |
A PR was created in Cairo repository, to extend the API of diagnostics, so the LoweringGroup will no longer be required |
I've tried hard to find some input which would result in diagnostics, and this caused it
Do you think we should have a test case like that? 🤔 |
Done |
5c0e4ce
to
02e96d6
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great stuff!
Minor comments attached below.
795b66e
to
35c6dee
Compare
Signed-off-by: maciektr <[email protected]>
Signed-off-by: maciektr <[email protected]>
Signed-off-by: maciektr <[email protected]>
Signed-off-by: maciektr <[email protected]>
Please, during review check if the speed of diagnostics during doc generation is acceptable.