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

bugfix: bound subtyping depth to avoid reliance on unpredictable stack overflow #4798

Merged
merged 18 commits into from
Dec 5, 2024

Conversation

crusso
Copy link
Contributor

@crusso crusso commented Dec 4, 2024

The idea is to fail fast and deterministically, especially in VSCode extension. OCaml stack overflow detection is unreliable and sometimes issues OOM, sometimes stack overflow.

Fixes #3057.

  • turn failure into compiler error

@crusso crusso requested review from rvanasa and ggreif December 4, 2024 17:14
@crusso crusso changed the title bug: bound subtyping depth to avoid reliance on unpredictable stack overflow bugfix: bound subtyping depth to avoid reliance on unpredictable stack overflow Dec 4, 2024
@crusso crusso marked this pull request as draft December 4, 2024 18:43
Copy link

github-actions bot commented Dec 4, 2024

Comparing from a57dddc to a08c730:
The produced WebAssembly code seems to be completely unchanged.

@crusso crusso marked this pull request as ready for review December 5, 2024 12:51
rvanasa
rvanasa previously approved these changes Dec 5, 2024
ggreif
ggreif previously approved these changes Dec 5, 2024
Copy link
Contributor

@ggreif ggreif left a comment

Choose a reason for hiding this comment

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

LGTM, but can you check whether it catches #3057?

@crusso crusso dismissed stale reviews from ggreif and rvanasa via 612edf3 December 5, 2024 17:23
@crusso
Copy link
Contributor Author

crusso commented Dec 5, 2024

LGTM, but can you check whether it catches #3057?

Yeah, it does. Added test.

test/fail/issue-3057.mo Outdated Show resolved Hide resolved
Copy link
Contributor

@ggreif ggreif left a comment

Choose a reason for hiding this comment

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

LGTM!

@ggreif ggreif added the Bug Something isn't working label Dec 5, 2024
@crusso crusso merged commit 947438f into master Dec 5, 2024
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Segmentation fault: 11 when trying to define a method which returns a new “instance” of a class
3 participants