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

Supporting TruffleRuby through using RubyNext #168

Closed
eregon opened this issue Oct 13, 2022 · 9 comments
Closed

Supporting TruffleRuby through using RubyNext #168

eregon opened this issue Oct 13, 2022 · 9 comments

Comments

@eregon
Copy link
Contributor

eregon commented Oct 13, 2022

From phlex-ruby/phlex#157

https://github.com/ruby-next/ruby-next could be a nice way to support TruffleRuby in syntax_tree until TruffleRuby has full pattern matching support (which will likely require the new parser, the existing one seems too buggy to support hash patterns and other cases).

@kddnewton Would you be open to a PR using https://github.com/ruby-next/ruby-next to make syntax_tree on TruffleRuby and potentially other Rubies? I might give it a try.
It's not ideal because Ripper is rather slow on TruffleRuby (it's a lot of C ext code to warmup), but it should work.

P.S.: I guess longer-term the new parser would probably also be used for syntax_tree or instead of it? Although I guess if Ripper uses the new parser there might not need to do anything and it'd still use the new parser under the hood, it'd just be an extra indirection in between.

@kddnewton
Copy link
Member

Yeah for sure. I'd welcome it if you could restrict the platform to just truffleruby. (I don't want to inflict a dependency on folks that aren't on it.)

Long-term yeah syntax tree is going to drop its dependency on ripper and just used the shared library.

@eregon
Copy link
Contributor Author

eregon commented Oct 13, 2022

I'd welcome it if you could restrict the platform to just truffleruby. (I don't want to inflict a dependency on folks that aren't on it.)

The workflow seems to be to generate some extra files just before packaging/releasing a gem: https://github.com/ruby-next/ruby-next#integrating-into-a-gem-development
But indeed that require "ruby-next/language/setup" probably needs a dependency, unless such files are also generated.
Seems unfortunate to add a runtime dependency for so little, maybe there is a way to avoid it.
Anyway, I'll investigate when I get some time to look at this.

@kddnewton
Copy link
Member

I think we could probably check RUBY_PLATFORM and only require it if necessary. And then also restrict it in the bundle? Or we could just require that folks add ruby-next to their bundles if they want to use it on truffleruby.

@kddnewton
Copy link
Member

@eregon truffleruby should now be working except for the language server (I still need to remove the pattern matching there). Could you give it a shot?

@eregon
Copy link
Contributor Author

eregon commented Oct 18, 2022

Yes, stree format works now.

I tried to run bundle exec rake test but some tests use pattern matching and as you say the language server too (that can be worked around with rm -r test/language_server*)

@eregon
Copy link
Contributor Author

eregon commented Oct 18, 2022

Let's close this since the gem loads on TruffleRuby now.
Ideally we'd also add TruffleRuby in CI but that seems difficult due to the tests using pattern matching (also inside eval).

@eregon eregon closed this as completed Oct 18, 2022
@eregon
Copy link
Contributor Author

eregon commented Oct 18, 2022

Playing a bit more with RubyNext and desugaring some test files I'm down to 1894 runs, 12788 assertions, 1 failures, 73 errors, 0 skips, after commenting out the eval in test/test_helper.rb.

Many of them are Polyglot::ForeignException: External LLVMFunction rb_warning_category_enabled_p cannot be found., something we should fix in TruffleRuby.

Also some of these, which I'm not sure why it happens:

 63) Error:
SyntaxTree::FormattingTest#test_formatted_unless_2:
NoMethodError: undefined method `deconstruct_keys' for nil:NilClass
    /home/eregon/code/syntax_tree/lib/.rbnext/syntax_tree/node.rb:5235:in `call'
    /home/eregon/code/syntax_tree/lib/.rbnext/syntax_tree/node.rb:5287:in `format'
    /home/eregon/code/syntax_tree/lib/.rbnext/syntax_tree/node.rb:9239:in `format'
    /home/eregon/code/syntax_tree/lib/syntax_tree/formatter.rb:93:in `format'
    /home/eregon/code/syntax_tree/lib/.rbnext/syntax_tree/node.rb:8204:in `block in format'
    <internal:core> core/enumerable.rb:358:in `each_with_index'
    /home/eregon/code/syntax_tree/lib/.rbnext/syntax_tree/node.rb:8200:in `format'
    /home/eregon/code/syntax_tree/lib/syntax_tree/formatter.rb:93:in `format'
    /home/eregon/code/syntax_tree/lib/.rbnext/syntax_tree/node.rb:7118:in `format'
    /home/eregon/code/syntax_tree/lib/syntax_tree.rb:59:in `format'
    /home/eregon/code/syntax_tree/test/formatting_test.rb:9:in `test_formatted_unless_2'

@eregon
Copy link
Contributor Author

eregon commented Oct 18, 2022

Running the tests on latest main (so without needing to rewrite the pattern matching in lib), I get only 38 errors, all Polyglot::ForeignException: External LLVMFunction rb_warning_category_enabled_p cannot be found.. So probably everything passes once that's implemented: oracle/truffleruby#2764
My changes to run that: main...eregon:syntax_tree:truffleruby-tests

@eregon
Copy link
Contributor Author

eregon commented Oct 28, 2022

Adding TruffleRuby in CI in #183

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

No branches or pull requests

2 participants