-
Notifications
You must be signed in to change notification settings - Fork 5.4k
[PRISM] Provide runtime flag for prism in iseq #9934
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
Conversation
962dbb8
to
3dfc306
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me, but I would like @ko1 to review this too.
Two comments:
|
@ko1 no problem, I've asked for matz's approval on the redmine ticket. I'll wait on merging this until we get that. |
@kddnewton I just remembered! There is already https://bugs.ruby-lang.org/issues/19083 It would be good to create a separate ticket to introduce an alias |
Ahh that's great @mame thanks. I don't mind it being on |
3dfc306
to
f85d0d7
Compare
How about that?
|
I'm sorry if I misunderstood, I thought you were saying you preferred parser_prism but were okay with just prism. I'm happy to change it if you'd like. The reason I wanted to go with this was because it's coming from the prism compiler, not the prism parser. In the future even if prism isn't chosen matz has said the AST will be prism's, so there won't be a need for multiple compilers. So I thought having a parser flag would be confusing, since it's more about the compiler that generated the ISEQ. That being said, I am happy to change it! |
@kddnewton I do not blame you for misinterpreting his explanation, but what ko1 wanted to say is that But I am unsure what |
Oh @mame thank you I understand now! I will change that first thing tomorrow. I am also happy to change the binary representation, whatever you think makes sense, just let me know. |
… by prism" if Ruby parse is Prism This commit addresses these CI failures. https://buildkite.com/rails/rails-nightly/builds/1023#0191ed31-1d8d-41e5-906b-e3b88aab79da Ruby 3.4.0dev default parser has been switched to Prism, After the default parser is switched from parse.y to Prism some of Active Support tests get failed with "cannot get AST for ISEQ compiled by prism" message. According to ruby/ruby#9934 , it looks an expected behavior of Ruby with Prism parser then this commit adds the `prism_skip` method following the naming `jruby_skip` According to ruby/ruby#9934, this seems to be the expected behavior of Ruby with the Prism parser. This commit adds the `prism_skip` method following the naming convention of `jruby_skip` that has been removed via rails#49454 . - This commit addresses these failures against Ruby 3.4.0dev with Prism parser. ```ruby $ ruby -v ruby 3.4.0dev (2024-09-15T01:06:11Z master 532af89e3b) +PRISM [x86_64-linux] $ cd activesupport $ bin/test -n "/^(?:TestOrderTest#(?:test_test_order_is_global)|AssertionsTest#(?:test_assert_changes_message_with_lambda|test_assert_difference_message_with_lambda|test_assert_no_changes_message_with_lambda|test_assert_no_changes_message_with_multi_line_lambda|test_assert_no_difference_with_multiple_expressions_fail)|ExceptionsInsideAssertionsTest#(?:test_warning_is_not_logged_if_assertions_are_nested_correctly))$/" /home/yahonda/.gem/ruby/3.4.0+0/gems/json-2.7.1/lib/json/common.rb:3: warning: ostruct was loaded from the standard library, but will no longer be part of the default gems starting from Ruby 3.5.0. You can add ostruct to your Gemfile or gemspec to silence this warning. WARNING: Nokogiri was built against libxml version 2.12.9, but has dynamically loaded 2.9.14 Run options: -n "/^(?:TestOrderTest#(?:test_test_order_is_global)|AssertionsTest#(?:test_assert_changes_message_with_lambda|test_assert_difference_message_with_lambda|test_assert_no_changes_message_with_lambda|test_assert_no_changes_message_with_multi_line_lambda|test_assert_no_difference_with_multiple_expressions_fail)|ExceptionsInsideAssertionsTest#(?:test_warning_is_not_logged_if_assertions_are_nested_correctly))$/" --seed 65502 F Failure: ExceptionsInsideAssertionsTest#test_warning_is_not_logged_if_assertions_are_nested_correctly [test/test_case_test.rb:508]: [Minitest::Assertion] exception expected, not Class: <RuntimeError> Message: <"cannot get AST for ISEQ compiled by prism"> ---Backtrace--- <internal:ast>:97:in 'RubyVM::AbstractSyntaxTree.of' /home/yahonda/src/github.com/rails/rails/activesupport/lib/active_support/testing/assertions.rb:304:in 'ActiveSupport::Testing::Assertions#_callable_to_source_string' /home/yahonda/src/github.com/rails/rails/activesupport/lib/active_support/testing/assertions.rb:270:in 'block in ActiveSupport::Testing::Assertions#assert_no_changes' --------------- bin/test test/test_case_test.rb:507 F Failure: AssertionsTest#test_assert_changes_message_with_lambda [test/test_case_test.rb:252]: [Minitest::Assertion] exception expected, not Class: <RuntimeError> Message: <"cannot get AST for ISEQ compiled by prism"> ---Backtrace--- <internal:ast>:97:in 'RubyVM::AbstractSyntaxTree.of' /home/yahonda/src/github.com/rails/rails/activesupport/lib/active_support/testing/assertions.rb:304:in 'ActiveSupport::Testing::Assertions#_callable_to_source_string' /home/yahonda/src/github.com/rails/rails/activesupport/lib/active_support/testing/assertions.rb:213:in 'block in ActiveSupport::Testing::Assertions#assert_changes' --------------- bin/test test/test_case_test.rb:249 F Failure: AssertionsTest#test_assert_no_changes_message_with_multi_line_lambda [test/test_case_test.rb:427]: [Minitest::Assertion] exception expected, not Class: <RuntimeError> Message: <"cannot get AST for ISEQ compiled by prism"> ---Backtrace--- <internal:ast>:97:in 'RubyVM::AbstractSyntaxTree.of' /home/yahonda/src/github.com/rails/rails/activesupport/lib/active_support/testing/assertions.rb:304:in 'ActiveSupport::Testing::Assertions#_callable_to_source_string' /home/yahonda/src/github.com/rails/rails/activesupport/lib/active_support/testing/assertions.rb:270:in 'block in ActiveSupport::Testing::Assertions#assert_no_changes' --------------- bin/test test/test_case_test.rb:422 F Failure: AssertionsTest#test_assert_no_changes_message_with_lambda [test/test_case_test.rb:380]: [Minitest::Assertion] exception expected, not Class: <RuntimeError> Message: <"cannot get AST for ISEQ compiled by prism"> ---Backtrace--- <internal:ast>:97:in 'RubyVM::AbstractSyntaxTree.of' /home/yahonda/src/github.com/rails/rails/activesupport/lib/active_support/testing/assertions.rb:304:in 'ActiveSupport::Testing::Assertions#_callable_to_source_string' /home/yahonda/src/github.com/rails/rails/activesupport/lib/active_support/testing/assertions.rb:270:in 'block in ActiveSupport::Testing::Assertions#assert_no_changes' --------------- bin/test test/test_case_test.rb:377 F Failure: AssertionsTest#test_assert_no_difference_with_multiple_expressions_fail [test/test_case_test.rb:78]: [Minitest::Assertion] exception expected, not Class: <RuntimeError> Message: <"cannot get AST for ISEQ compiled by prism"> ---Backtrace--- <internal:ast>:97:in 'RubyVM::AbstractSyntaxTree.of' /home/yahonda/src/github.com/rails/rails/activesupport/lib/active_support/testing/assertions.rb:304:in 'ActiveSupport::Testing::Assertions#_callable_to_source_string' /home/yahonda/src/github.com/rails/rails/activesupport/lib/active_support/testing/assertions.rb:122:in 'block (2 levels) in ActiveSupport::Testing::Assertions#assert_difference' --------------- bin/test test/test_case_test.rb:76 F Failure: AssertionsTest#test_assert_difference_message_with_lambda [test/test_case_test.rb:176]: [Minitest::Assertion] exception expected, not Class: <RuntimeError> Message: <"cannot get AST for ISEQ compiled by prism"> ---Backtrace--- <internal:ast>:97:in 'RubyVM::AbstractSyntaxTree.of' /home/yahonda/src/github.com/rails/rails/activesupport/lib/active_support/testing/assertions.rb:304:in 'ActiveSupport::Testing::Assertions#_callable_to_source_string' /home/yahonda/src/github.com/rails/rails/activesupport/lib/active_support/testing/assertions.rb:122:in 'block (2 levels) in ActiveSupport::Testing::Assertions#assert_difference' --------------- bin/test test/test_case_test.rb:173 . Finished in 0.044451s, 157.4784 runs/s, 629.9136 assertions/s. 7 runs, 28 assertions, 6 failures, 0 errors, 0 skips $ ``` Refer to ruby/ruby#9934 ruby/ruby#11497 https://bugs.ruby-lang.org/issues/20564
… Ruby parse is Prism This commit addresses these CI failures. https://buildkite.com/rails/rails-nightly/builds/1023#0191ed31-1d8d-41e5-906b-e3b88aab79da Ruby 3.4.0dev default parser has been switched to Prism, After the default parser is switched from parse.y to Prism some of Active Support tests get failed with "cannot get AST for ISEQ compiled by prism" message. According to ruby/ruby#9934 , it looks an expected behavior of Ruby with Prism parser then this commit adds the `prism_skip` method following the naming `jruby_skip` According to ruby/ruby#9934, this seems to be the expected behavior of Ruby with the Prism parser. This commit adds the `prism_skip` method following the naming convention of `jruby_skip` that has been removed via rails#49454 . - This commit addresses these failures against Ruby 3.4.0dev with Prism parser. ```ruby $ ruby -v ruby 3.4.0dev (2024-09-15T01:06:11Z master 532af89e3b) +PRISM [x86_64-linux] $ cd activesupport $ bin/test -n "/^(?:TestOrderTest#(?:test_test_order_is_global)|AssertionsTest#(?:test_assert_changes_message_with_lambda|test_assert_difference_message_with_lambda|test_assert_no_changes_message_with_lambda|test_assert_no_changes_message_with_multi_line_lambda|test_assert_no_difference_with_multiple_expressions_fail)|ExceptionsInsideAssertionsTest#(?:test_warning_is_not_logged_if_assertions_are_nested_correctly))$/" /home/yahonda/.gem/ruby/3.4.0+0/gems/json-2.7.1/lib/json/common.rb:3: warning: ostruct was loaded from the standard library, but will no longer be part of the default gems starting from Ruby 3.5.0. You can add ostruct to your Gemfile or gemspec to silence this warning. WARNING: Nokogiri was built against libxml version 2.12.9, but has dynamically loaded 2.9.14 Run options: -n "/^(?:TestOrderTest#(?:test_test_order_is_global)|AssertionsTest#(?:test_assert_changes_message_with_lambda|test_assert_difference_message_with_lambda|test_assert_no_changes_message_with_lambda|test_assert_no_changes_message_with_multi_line_lambda|test_assert_no_difference_with_multiple_expressions_fail)|ExceptionsInsideAssertionsTest#(?:test_warning_is_not_logged_if_assertions_are_nested_correctly))$/" --seed 65502 F Failure: ExceptionsInsideAssertionsTest#test_warning_is_not_logged_if_assertions_are_nested_correctly [test/test_case_test.rb:508]: [Minitest::Assertion] exception expected, not Class: <RuntimeError> Message: <"cannot get AST for ISEQ compiled by prism"> ---Backtrace--- <internal:ast>:97:in 'RubyVM::AbstractSyntaxTree.of' /home/yahonda/src/github.com/rails/rails/activesupport/lib/active_support/testing/assertions.rb:304:in 'ActiveSupport::Testing::Assertions#_callable_to_source_string' /home/yahonda/src/github.com/rails/rails/activesupport/lib/active_support/testing/assertions.rb:270:in 'block in ActiveSupport::Testing::Assertions#assert_no_changes' --------------- bin/test test/test_case_test.rb:507 F Failure: AssertionsTest#test_assert_changes_message_with_lambda [test/test_case_test.rb:252]: [Minitest::Assertion] exception expected, not Class: <RuntimeError> Message: <"cannot get AST for ISEQ compiled by prism"> ---Backtrace--- <internal:ast>:97:in 'RubyVM::AbstractSyntaxTree.of' /home/yahonda/src/github.com/rails/rails/activesupport/lib/active_support/testing/assertions.rb:304:in 'ActiveSupport::Testing::Assertions#_callable_to_source_string' /home/yahonda/src/github.com/rails/rails/activesupport/lib/active_support/testing/assertions.rb:213:in 'block in ActiveSupport::Testing::Assertions#assert_changes' --------------- bin/test test/test_case_test.rb:249 F Failure: AssertionsTest#test_assert_no_changes_message_with_multi_line_lambda [test/test_case_test.rb:427]: [Minitest::Assertion] exception expected, not Class: <RuntimeError> Message: <"cannot get AST for ISEQ compiled by prism"> ---Backtrace--- <internal:ast>:97:in 'RubyVM::AbstractSyntaxTree.of' /home/yahonda/src/github.com/rails/rails/activesupport/lib/active_support/testing/assertions.rb:304:in 'ActiveSupport::Testing::Assertions#_callable_to_source_string' /home/yahonda/src/github.com/rails/rails/activesupport/lib/active_support/testing/assertions.rb:270:in 'block in ActiveSupport::Testing::Assertions#assert_no_changes' --------------- bin/test test/test_case_test.rb:422 F Failure: AssertionsTest#test_assert_no_changes_message_with_lambda [test/test_case_test.rb:380]: [Minitest::Assertion] exception expected, not Class: <RuntimeError> Message: <"cannot get AST for ISEQ compiled by prism"> ---Backtrace--- <internal:ast>:97:in 'RubyVM::AbstractSyntaxTree.of' /home/yahonda/src/github.com/rails/rails/activesupport/lib/active_support/testing/assertions.rb:304:in 'ActiveSupport::Testing::Assertions#_callable_to_source_string' /home/yahonda/src/github.com/rails/rails/activesupport/lib/active_support/testing/assertions.rb:270:in 'block in ActiveSupport::Testing::Assertions#assert_no_changes' --------------- bin/test test/test_case_test.rb:377 F Failure: AssertionsTest#test_assert_no_difference_with_multiple_expressions_fail [test/test_case_test.rb:78]: [Minitest::Assertion] exception expected, not Class: <RuntimeError> Message: <"cannot get AST for ISEQ compiled by prism"> ---Backtrace--- <internal:ast>:97:in 'RubyVM::AbstractSyntaxTree.of' /home/yahonda/src/github.com/rails/rails/activesupport/lib/active_support/testing/assertions.rb:304:in 'ActiveSupport::Testing::Assertions#_callable_to_source_string' /home/yahonda/src/github.com/rails/rails/activesupport/lib/active_support/testing/assertions.rb:122:in 'block (2 levels) in ActiveSupport::Testing::Assertions#assert_difference' --------------- bin/test test/test_case_test.rb:76 F Failure: AssertionsTest#test_assert_difference_message_with_lambda [test/test_case_test.rb:176]: [Minitest::Assertion] exception expected, not Class: <RuntimeError> Message: <"cannot get AST for ISEQ compiled by prism"> ---Backtrace--- <internal:ast>:97:in 'RubyVM::AbstractSyntaxTree.of' /home/yahonda/src/github.com/rails/rails/activesupport/lib/active_support/testing/assertions.rb:304:in 'ActiveSupport::Testing::Assertions#_callable_to_source_string' /home/yahonda/src/github.com/rails/rails/activesupport/lib/active_support/testing/assertions.rb:122:in 'block (2 levels) in ActiveSupport::Testing::Assertions#assert_difference' --------------- bin/test test/test_case_test.rb:173 . Finished in 0.044451s, 157.4784 runs/s, 629.9136 assertions/s. 7 runs, 28 assertions, 6 failures, 0 errors, 0 skips $ ``` Refer to ruby/ruby#9934 ruby/ruby#11497 https://bugs.ruby-lang.org/issues/20564
… Ruby parse is Prism This commit addresses these CI failures. https://buildkite.com/rails/rails-nightly/builds/1023#0191ed31-1d8d-41e5-906b-e3b88aab79da Ruby 3.4.0dev default parser has been switched to Prism, After the default parser is switched from parse.y to Prism some of Active Support tests get failed with "cannot get AST for ISEQ compiled by prism" message. According to ruby/ruby#9934, this seems to be the expected behavior of Ruby with the Prism parser. This commit adds the `prism_skip` method following the naming convention of `jruby_skip` that has been removed via rails#49454 . - This commit addresses these failures against Ruby 3.4.0dev with Prism parser. ```ruby $ ruby -v ruby 3.4.0dev (2024-09-15T01:06:11Z master 532af89e3b) +PRISM [x86_64-linux] $ cd activesupport $ bin/test -n "/^(?:TestOrderTest#(?:test_test_order_is_global)|AssertionsTest#(?:test_assert_changes_message_with_lambda|test_assert_difference_message_with_lambda|test_assert_no_changes_message_with_lambda|test_assert_no_changes_message_with_multi_line_lambda|test_assert_no_difference_with_multiple_expressions_fail)|ExceptionsInsideAssertionsTest#(?:test_warning_is_not_logged_if_assertions_are_nested_correctly))$/" /home/yahonda/.gem/ruby/3.4.0+0/gems/json-2.7.1/lib/json/common.rb:3: warning: ostruct was loaded from the standard library, but will no longer be part of the default gems starting from Ruby 3.5.0. You can add ostruct to your Gemfile or gemspec to silence this warning. WARNING: Nokogiri was built against libxml version 2.12.9, but has dynamically loaded 2.9.14 Run options: -n "/^(?:TestOrderTest#(?:test_test_order_is_global)|AssertionsTest#(?:test_assert_changes_message_with_lambda|test_assert_difference_message_with_lambda|test_assert_no_changes_message_with_lambda|test_assert_no_changes_message_with_multi_line_lambda|test_assert_no_difference_with_multiple_expressions_fail)|ExceptionsInsideAssertionsTest#(?:test_warning_is_not_logged_if_assertions_are_nested_correctly))$/" --seed 65502 F Failure: ExceptionsInsideAssertionsTest#test_warning_is_not_logged_if_assertions_are_nested_correctly [test/test_case_test.rb:508]: [Minitest::Assertion] exception expected, not Class: <RuntimeError> Message: <"cannot get AST for ISEQ compiled by prism"> ---Backtrace--- <internal:ast>:97:in 'RubyVM::AbstractSyntaxTree.of' /home/yahonda/src/github.com/rails/rails/activesupport/lib/active_support/testing/assertions.rb:304:in 'ActiveSupport::Testing::Assertions#_callable_to_source_string' /home/yahonda/src/github.com/rails/rails/activesupport/lib/active_support/testing/assertions.rb:270:in 'block in ActiveSupport::Testing::Assertions#assert_no_changes' --------------- bin/test test/test_case_test.rb:507 F Failure: AssertionsTest#test_assert_changes_message_with_lambda [test/test_case_test.rb:252]: [Minitest::Assertion] exception expected, not Class: <RuntimeError> Message: <"cannot get AST for ISEQ compiled by prism"> ---Backtrace--- <internal:ast>:97:in 'RubyVM::AbstractSyntaxTree.of' /home/yahonda/src/github.com/rails/rails/activesupport/lib/active_support/testing/assertions.rb:304:in 'ActiveSupport::Testing::Assertions#_callable_to_source_string' /home/yahonda/src/github.com/rails/rails/activesupport/lib/active_support/testing/assertions.rb:213:in 'block in ActiveSupport::Testing::Assertions#assert_changes' --------------- bin/test test/test_case_test.rb:249 F Failure: AssertionsTest#test_assert_no_changes_message_with_multi_line_lambda [test/test_case_test.rb:427]: [Minitest::Assertion] exception expected, not Class: <RuntimeError> Message: <"cannot get AST for ISEQ compiled by prism"> ---Backtrace--- <internal:ast>:97:in 'RubyVM::AbstractSyntaxTree.of' /home/yahonda/src/github.com/rails/rails/activesupport/lib/active_support/testing/assertions.rb:304:in 'ActiveSupport::Testing::Assertions#_callable_to_source_string' /home/yahonda/src/github.com/rails/rails/activesupport/lib/active_support/testing/assertions.rb:270:in 'block in ActiveSupport::Testing::Assertions#assert_no_changes' --------------- bin/test test/test_case_test.rb:422 F Failure: AssertionsTest#test_assert_no_changes_message_with_lambda [test/test_case_test.rb:380]: [Minitest::Assertion] exception expected, not Class: <RuntimeError> Message: <"cannot get AST for ISEQ compiled by prism"> ---Backtrace--- <internal:ast>:97:in 'RubyVM::AbstractSyntaxTree.of' /home/yahonda/src/github.com/rails/rails/activesupport/lib/active_support/testing/assertions.rb:304:in 'ActiveSupport::Testing::Assertions#_callable_to_source_string' /home/yahonda/src/github.com/rails/rails/activesupport/lib/active_support/testing/assertions.rb:270:in 'block in ActiveSupport::Testing::Assertions#assert_no_changes' --------------- bin/test test/test_case_test.rb:377 F Failure: AssertionsTest#test_assert_no_difference_with_multiple_expressions_fail [test/test_case_test.rb:78]: [Minitest::Assertion] exception expected, not Class: <RuntimeError> Message: <"cannot get AST for ISEQ compiled by prism"> ---Backtrace--- <internal:ast>:97:in 'RubyVM::AbstractSyntaxTree.of' /home/yahonda/src/github.com/rails/rails/activesupport/lib/active_support/testing/assertions.rb:304:in 'ActiveSupport::Testing::Assertions#_callable_to_source_string' /home/yahonda/src/github.com/rails/rails/activesupport/lib/active_support/testing/assertions.rb:122:in 'block (2 levels) in ActiveSupport::Testing::Assertions#assert_difference' --------------- bin/test test/test_case_test.rb:76 F Failure: AssertionsTest#test_assert_difference_message_with_lambda [test/test_case_test.rb:176]: [Minitest::Assertion] exception expected, not Class: <RuntimeError> Message: <"cannot get AST for ISEQ compiled by prism"> ---Backtrace--- <internal:ast>:97:in 'RubyVM::AbstractSyntaxTree.of' /home/yahonda/src/github.com/rails/rails/activesupport/lib/active_support/testing/assertions.rb:304:in 'ActiveSupport::Testing::Assertions#_callable_to_source_string' /home/yahonda/src/github.com/rails/rails/activesupport/lib/active_support/testing/assertions.rb:122:in 'block (2 levels) in ActiveSupport::Testing::Assertions#assert_difference' --------------- bin/test test/test_case_test.rb:173 . Finished in 0.044451s, 157.4784 runs/s, 629.9136 assertions/s. 7 runs, 28 assertions, 6 failures, 0 errors, 0 skips $ ``` Refer to ruby/ruby#9934 ruby/ruby#11497 https://bugs.ruby-lang.org/issues/20564
The reason we want this is that we want to store prism-specific node ids in the iseq in order to support error highlight. So we need to be able to map from the compiler that produced the iseq to the node ids that are contained in the iseq.
This adds a single bool to
rb_iseq_constant_body
and adds:prism
to themisc
hash in the serialized array form of an iseq.Note that this PR includes ruby/error_highlight#41.