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

[hl/eval/cpp/neko] Fix exception stack when wrapping native exceptions #11249

Merged
merged 7 commits into from Jul 5, 2023

Conversation

kLabz
Copy link
Contributor

@kLabz kLabz commented Jun 8, 2023

Fixes #11247 (caused by #10519), where auto wrapped exceptions on eval/hl/neko/cpp were missing their first entry, removed automatically by all haxe.Exception subclasses' constructor (__shiftStack call added by compiler itself) while we don't want it in this case.

Current implementation is hacky; an alternative could be to handle that directly in filters/exceptions.ml but I'm not sure how to proceed there.

This should also cover #10926, but since there was no issue/test for the commit that introduced the issue, it's hard to say if this covers everything from it.
Closes #10926

Added a related fix, closes #11265

@skial skial mentioned this pull request Jun 14, 2023
1 task
@giuppe giuppe mentioned this pull request Jun 27, 2023
13 tasks
@kLabz kLabz added this to the 4.3 Hotfix milestone Jun 27, 2023
@kLabz kLabz marked this pull request as ready for review July 4, 2023 06:52
@kLabz kLabz requested a review from Simn July 4, 2023 06:52
@Simn
Copy link
Member

Simn commented Jul 5, 2023

I'm not very familiar with the exception handling code either, but if you think this is the right thing to do then feel free to merge it!

@kLabz kLabz merged commit a8e181c into development Jul 5, 2023
78 of 80 checks passed
@kLabz kLabz deleted the fix/issue11247 branch July 7, 2023 07:53
kLabz added a commit that referenced this pull request Aug 31, 2023
…11249)

* [tests] add test for #11247

* Fix for #11247

* [hl] restore callstack fix

* [tests] cpp doesn't like native exceptions in those tests either

* [tests] only run new test for eval/hl/neko

* Fix #11265

* [tests] don't compile test on target that won't run it
@kLabz kLabz removed this from the 4.3 Hotfix milestone Sep 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants