Skip to content

Fix interpreter guard clauses for signal handling#15892

Merged
straight-shoota merged 2 commits intocrystal-lang:masterfrom
straight-shoota:fix/interpreter-signal
Jun 16, 2025
Merged

Fix interpreter guard clauses for signal handling#15892
straight-shoota merged 2 commits intocrystal-lang:masterfrom
straight-shoota:fix/interpreter-signal

Conversation

@straight-shoota
Copy link
Member

@straight-shoota straight-shoota commented Jun 11, 2025

The guard clauses introduced in #15178 to protect using the API for #14766 are wrong. The guards check for instance methods, which are never defined because we're using class methods instead.
As a result, the fix for #12241 was never actually applied.
The spec file uses the same guard clause, so it didn't run.

This was discovered by @jneen in https://discord.com/channels/591460182777790474/591597160492171264/1382014668784275457

@straight-shoota straight-shoota self-assigned this Jun 11, 2025
@straight-shoota straight-shoota added kind:bug A bug in the code. Does not apply to documentation, specs, etc. topic:compiler:interpreter labels Jun 11, 2025
@ysbaddaden
Copy link
Collaborator

To be honest I don't understand why we need to support previous compiler releases for the interpreter: it's unreleased, unstable, and it can't be forward compatible anyway.

@straight-shoota
Copy link
Member Author

We should support at least the latest stable release. That was the problem after merging #12241: In order to use the current standard library, you needed a build of the interpreter from master with that patch. It wouldn't work with 1.14. And that's annoying when you're working on the standard library itself.
I suppose at this point we could perhaps drop these restrictions. We don't need to support the interpreter at version 1.14 and below anymore.

@straight-shoota straight-shoota added this to the 1.17.0 milestone Jun 11, 2025
@straight-shoota
Copy link
Member Author

CI seems to be stuck 😢
The Test std_spec with interpreter jobs don't show any progress.

@straight-shoota straight-shoota removed this from the 1.17.0 milestone Jun 11, 2025
@ysbaddaden ysbaddaden added this to the 1.17.0 milestone Jun 14, 2025
@straight-shoota straight-shoota merged commit c5af983 into crystal-lang:master Jun 16, 2025
37 checks passed
@straight-shoota straight-shoota deleted the fix/interpreter-signal branch June 16, 2025 06:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

kind:bug A bug in the code. Does not apply to documentation, specs, etc. topic:compiler:interpreter

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants