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

Eval into a method, so method JITs can compile #1153

Closed
wants to merge 1 commit into from

Conversation

headius
Copy link
Contributor

@headius headius commented Jun 5, 2024

This replaces most evals of patterns with evaluation into and invoking of a temporary method. The remaining cases depend on surrounding state that would need to be moved into the eval string to work properly.

Partially addresses #1152

This replaces most evals of patterns with evaluation into and
invoking of a temporary method. The remaining cases depend on
surrounding state that would need to be moved into the eval string
to work properly.

Partially addresses ruby#1152
@headius
Copy link
Contributor Author

headius commented Jun 5, 2024

This allows JRuby's JIT set at a compilation threshold of 0 to exercise these specs, and showed several failures all the same cause as jruby/jruby#8283 (fixed by jruby/jruby#8284).

@eregon
Copy link
Member

eregon commented Jun 5, 2024

(see #1152 (comment))

@headius
Copy link
Contributor Author

headius commented Jun 6, 2024

Closing in favor of removing the evals.

@headius headius closed this Jun 6, 2024
@headius headius deleted the less_eval_in_pattern_specs branch June 6, 2024 21:35
@headius
Copy link
Contributor Author

headius commented Jun 6, 2024

See #1155 for the actual fix.

@eregon
Copy link
Member

eregon commented Jun 7, 2024

Another thought on this is there will likely always be a few eval's in ruby/spec for syntax which is not supported by the oldest supported Ruby version in ruby/spec.
These evals should almost always be literal strings though, so one way to compile them would be to actually remove the eval at parse time, but it's certainly hacky (e.g. hard to prove it's a call to Kernel#eval) and when we can remove the eval's like here, the better.
I'm mentioning it because this also applies e.g. for other usages of ruby/spec like testing parsers, rubocop, etc.

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

Successfully merging this pull request may close these issues.

2 participants