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

Report errors when generating indexes #6123

Merged
merged 3 commits into from
Mar 30, 2023

Conversation

hubertp
Copy link
Collaborator

@hubertp hubertp commented Mar 29, 2023

Pull Request Description

--compile command would run the compilation pipeline but silently omit any encountered errors, thus skipping the serialization. This maybe was a good idea in the past but it was problematic now that we generate indexes on build time.
This resulted in rather obscure errors (#6092) for modules that were missing their caches.

The change should significantly improve developers' experience when working on stdlib.

Important Notes

Making compilation more resilient to sudden cache misses is a separate item to be worked on.

Checklist

Please ensure that the following checklist has been satisfied before submitting the PR:

  • The documentation has been updated, if necessary.
  • Screenshots/screencasts have been attached, if there are any visual changes. For interactive or animated visual changes, a screencast is preferred.
  • All code follows the
    Scala,
    Java,
    and
    Rust
    style guides. In case you are using a language not listed above, follow the Rust style guide.
  • All code has been tested:
    • Unit tests have been written where possible.
    • If GUI codebase was changed, the GUI was tested when built using ./run ide build.

@hubertp hubertp added the CI: No changelog needed Do not require a changelog entry for this PR. label Mar 29, 2023
@hubertp hubertp requested a review from radeusgd March 29, 2023 10:53
Copy link
Member

@radeusgd radeusgd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the fix! This will make our lives much easier.

I'm wondering if we could add one more layer of safety - when loading the IR cache, can we check if the loaded IR contains any errors and reject such a cache as 'invalid', to force a recompilation? Then, even if somehow a cache with errors becomes generated, our compiler will try its best to recompile to see if the errors go away - and if not - it will at least display good errors.

@hubertp
Copy link
Collaborator Author

hubertp commented Mar 29, 2023

Thanks for the fix! This will make our lives much easier.

I'm wondering if we could add one more layer of safety - when loading the IR cache, can we check if the loaded IR contains any errors and reject such a cache as 'invalid', to force a recompilation? Then, even if somehow a cache with errors becomes generated, our compiler will try its best to recompile to see if the errors go away - and if not - it will at least display good errors.

It's part of the follow up work.

@hubertp hubertp force-pushed the wip/hubert/6092-report-errors-when-generating-caches branch from f3ae5db to 449e474 Compare March 29, 2023 11:52
@hubertp hubertp added CI: Ready to merge This PR is eligible for automatic merge and removed CI: Ready to merge This PR is eligible for automatic merge labels Mar 29, 2023
hubertp added 2 commits March 29, 2023 16:07
`--compile` command would run the compilation pipeline but silently
omit any encountered errors, thus skipping the serialization.
This maybe was a good idea in the past but it was problematic now that
we generate indexes on build time.
This resulted in rather obscure errors (#6092) for modules that were
missing their caches.

Making compilation more resilient to sudden cache misses is a separate
item.
@hubertp hubertp force-pushed the wip/hubert/6092-report-errors-when-generating-caches branch from c488914 to 9f924bf Compare March 29, 2023 14:27
@hubertp hubertp added the CI: Ready to merge This PR is eligible for automatic merge label Mar 30, 2023
@mergify mergify bot merged commit 90612cf into develop Mar 30, 2023
@mergify mergify bot deleted the wip/hubert/6092-report-errors-when-generating-caches branch March 30, 2023 09:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI: No changelog needed Do not require a changelog entry for this PR. CI: Ready to merge This PR is eligible for automatic merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants