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

RubyVM usage #348

Open
eregon opened this issue Jul 30, 2020 · 8 comments · Fixed by #349
Open

RubyVM usage #348

eregon opened this issue Jul 30, 2020 · 8 comments · Fixed by #349

Comments

@eregon
Copy link
Member

eregon commented Jul 30, 2020

Hello, it seems that currently rbs uses RubyVM::AbstractSyntaxTree in a couple places:
https://github.com/ruby/rbs/search?q=RubyVM&unscoped_q=RubyVM

However, RubyVM by design is MRI-only and is expected to not exist as a constant on alternative Ruby implementations (documentation).

Do you plan to address this somehow?
If AbstractSyntaxTree is needed by rbs then I think it's time to move it somewhere outside of RubyVM.

For instance, when running on TruffleRuby:

% rbs prototype rb lib/person.rb lib/email.rb lib/phone.rb
/Users/bfish/Documents/truffleruby-ws/truffleruby/mxbuild/truffleruby-native/jre/languages/ruby/lib/gems/gems/rbs-0.7.0/lib/rbs/prototype/rb.rb:55:in `const_missing': uninitialized constant RBS::Prototype::RB::RubyVM (NameError)
	from /Users/bfish/Documents/truffleruby-ws/truffleruby/mxbuild/truffleruby-native/jre/languages/ruby/lib/gems/gems/rbs-0.7.0/lib/rbs/prototype/rb.rb:55:in `parse'
	from /Users/bfish/Documents/truffleruby-ws/truffleruby/mxbuild/truffleruby-native/jre/languages/ruby/lib/gems/gems/rbs-0.7.0/lib/rbs/cli.rb:629:in `block in run_prototype_file'
	from /Users/bfish/Documents/truffleruby-ws/truffleruby/mxbuild/truffleruby-native/jre/languages/ruby/lib/gems/gems/rbs-0.7.0/lib/rbs/cli.rb:628:in `each'
	from /Users/bfish/Documents/truffleruby-ws/truffleruby/mxbuild/truffleruby-native/jre/languages/ruby/lib/gems/gems/rbs-0.7.0/lib/rbs/cli.rb:628:in `run_prototype_file'
	from /Users/bfish/Documents/truffleruby-ws/truffleruby/mxbuild/truffleruby-native/jre/languages/ruby/lib/gems/gems/rbs-0.7.0/lib/rbs/cli.rb:526:in `run_prototype'
	from /Users/bfish/Documents/truffleruby-ws/truffleruby/mxbuild/truffleruby-native/jre/languages/ruby/lib/gems/gems/rbs-0.7.0/lib/rbs/cli.rb:92:in `run'
	from /Users/bfish/Documents/truffleruby-ws/truffleruby/mxbuild/truffleruby-native/jre/languages/ruby/lib/gems/gems/rbs-0.7.0/exe/rbs:7:in `<top (required)>'
	from <internal:core> core/kernel.rb:395:in `load'
	from <internal:core> core/kernel.rb:395:in `load'
	from /Users/bfish/.rbenv/versions/tr-native/bin/rbs:23:in `<main>'

cc @bjfish

Relates to oracle/truffleruby#1671

@soutaro
Copy link
Member

soutaro commented Jul 31, 2020

Good point!

I decided to use RubyVM::AbstractSyntaxTree because it's included MRuby and doesn't require additional dependencies. One option would be to make prototype commands an external library and uses parser gem (or something else if exists.)

@eregon
Copy link
Member Author

eregon commented Jul 31, 2020

because it's included MRuby

You mean CRuby, right? I think MRuby doesn't have RubyVM.

One option would be to make prototype commands an external library and uses parser gem (or something else if exists.)

Right, I think that would make sense.

I think moving and stabilizing AbstractSyntaxTree would also be valuable, it seems a few gems decided to use RubyVM::AbstractSyntaxTree but do not realize this is making the gem not portable across Ruby implementations, and using something experimental and unstable. (Just moving under ExperimentalFeatures would also be an option if not making it stable at the same time)

@soutaro
Copy link
Member

soutaro commented Jul 31, 2020

Oh sorry, I mixed CRuby and MRI...

It's great to me too if AbstractSyntaxTree is stable and portable.

I think showing a better error message and exit gracefully is the first step we can do now.

@soutaro
Copy link
Member

soutaro commented Aug 1, 2020

I don't think printing error messages is the best solution for this. Contributions for improvement are welcome.

@eregon
Copy link
Member Author

eregon commented Aug 1, 2020

Could we leave the issue opened?
The error message is an improvement but the original issue is still there: rbs prototype only works on CRuby.

@eregon
Copy link
Member Author

eregon commented Aug 1, 2020

I posted a comment related to this on the RubyVM::AbstractSyntaxTree on the CRuby tracker: https://bugs.ruby-lang.org/issues/14844#note-25

@soutaro
Copy link
Member

soutaro commented Aug 1, 2020

Okay. Reopening this issue. (I don't think I have bandwidth to do this for a while, but someone else would have!)

@eregon
Copy link
Member Author

eregon commented Jul 16, 2022

FYI, a report about this on the truffleruby tracker: oracle/truffleruby#2691

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

Successfully merging a pull request may close this issue.

2 participants