Skip to content

Cleanup the rb_catch_obj workaround#1035

Merged
byroot merged 1 commit into
ruby:masterfrom
byroot:update-truffleruby-workaround
Jun 21, 2026
Merged

Cleanup the rb_catch_obj workaround#1035
byroot merged 1 commit into
ruby:masterfrom
byroot:update-truffleruby-workaround

Conversation

@byroot

@byroot byroot commented Jun 21, 2026

Copy link
Copy Markdown
Member

Followup: #1031

Now that the bug has been fixed upstream, we can only apply it on unpatched version.

Also refactor the workaround to expose the exact same API as rb_catch_obj, making it easier to ignore and remove later.

Followup: ruby#1031

Now that the bug has been fixed upstream, we can only apply
it on unpatched version.

Also refactor the workaround to expose the exact same API
as `rb_catch_obj`, making it easier to ignore and remove later.
@byroot byroot merged commit 2afd1a9 into ruby:master Jun 21, 2026
42 checks passed
@byroot byroot deleted the update-truffleruby-workaround branch June 21, 2026 08:24
$defs << "-DJSON_DEBUG" if ENV.fetch("JSON_DEBUG", "0") != "0"
$defs << "-DJSON_WORKAROUND_RB_CATCH_BUG" if RUBY_ENGINE == 'truffleruby'

if RUBY_ENGINE == 'truffleruby' && RUBY_VERSION < '4.0'

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

FYI @eregon, let me know if the logic ain't correct here, I'm not very familiar with Truffle

@eregon eregon Jun 21, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

That works (in both cases the workaround is not applied on truffleruby-head which is now fixed) but I think RUBY_ENGINE_VERSION is little bit cleaner (explicitly mentions the version fixing the bug)

Suggested change
if RUBY_ENGINE == 'truffleruby' && RUBY_VERSION < '4.0'
if RUBY_ENGINE == 'truffleruby' && RUBY_ENGINE_VERSION < '40.0'

#1036

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants