Failed to transpile ./lib/syntax_tree/node.rb: SyntaxError — unexpected token tRBRACK

[ruby-next]

Failed to transpile ./lib/syntax_tree/node.rb: SyntaxError — unexpected token tRBRACK

[kddnewton]

@palkan yeah unfortunately deconstruct_keys was showing up quite a lot in the profile. Switching back to a case..when gave me some performance back.

I think we can do a better job in CRuby of rewriting the iseqs to do some smarter matching when common patterns are found. Or just let YJIT do its thing. But in the meantime I decided to just roll with eliminating the pattern matching for now.

[eregon]

In general when using RubyNext in a gem I'd want to avoid the runtime dependency, hence I did the manual $LOAD_PATH.unshift "#{__dir__}/.rbnext" above.

[eregon]

I also tried:

$ ruby-next nextify --rewrite=pattern-matching,pattern-matching-find-pattern,pattern-matching-in,pattern-matching-oneline-parensless,pattern-matching-pin-vars ./lib
Rewriters not found: pattern-matching,pattern-matching-find-pattern,pattern-matching-in,pattern-matching-oneline-parensless,pattern-matching-pin-vars
Try --list-rewriters to see list of available rewriters

[eregon]

I noticed this succeeds:

$ ruby-next nextify -V --rewrite=pattern-matching ./lib                                        
RubyNext core strategy: refine
RubyNext transpile mode: rewrite
Remove old files: ./lib/.rbnext
Generated: ./lib/.rbnext/syntax_tree/language_server/inlay_hints.rb
Generated: ./lib/.rbnext/syntax_tree/language_server.rb
Generated: ./lib/.rbnext/syntax_tree/node.rb
Generated: ./lib/.rbnext/syntax_tree/parser.rb

but this fails:

$ ruby-next nextify -V --rewrite=pattern-matching --rewrite=pattern-matching-find-pattern ./lib
RubyNext core strategy: refine
RubyNext transpile mode: rewrite
Remove old files: ./lib/.rbnext
Generated: ./lib/.rbnext/syntax_tree/language_server/inlay_hints.rb
Generated: ./lib/.rbnext/syntax_tree/language_server.rb
Failed to transpile ./lib/syntax_tree/node.rb: SyntaxError — unexpected token tRBRACK

[eregon]

With the first one, then line 2147 becomes (which TruffleRuby cannot parse: syntax_tree/lib/.rbnext/syntax_tree/node.rb:2147: syntax error, unexpected '{ arg'):

when (__p_1__ && __p_3__ = (1 == __m_arr__.size) and (Paren(contents: {body: [ArrayLiteral(contents: {parts: [_, _, *]}) => array]}) === __m_arr__[0]))

the original is:

        in [
             Paren[
               contents: {
                 body: [ArrayLiteral[contents: { parts: [_, _, *] }] => array] }
             ]
           ]

[eregon]

I noticed that with commenting out diagnostics.all_errors_are_fatal = true, but I get the same error and translation for that line, there is still the ArrayLiteral(contents: {parts: [_, _, *]}) => array.
So maybe it is a parser gem issue?

[eregon]

Adding some debugging in Parser::Diagnostic::Engine#process:

    def process(diagnostic)
      if ignore?(diagnostic)
        # do nothing
      elsif @consumer
        @consumer.call(diagnostic)
      end

      if raise?(diagnostic)
        p diagnostic
        p diagnostic.message
        p diagnostic.location.line
        p diagnostic.location.source_line
        raise Parser::SyntaxError, diagnostic
      end

      self
    end

it shows

$ ruby-next nextify -V --rewrite=pattern-matching --rewrite=pattern-matching-find-pattern --rewrite=pattern-matching-in --rewrite=pattern-matching-oneline-parensless --rewrite=pattern-matching-pin-vars ./lib
...
#<Parser::Diagnostic:0x00007f2c344abc00 @level=:error, @reason=:unexpected_token, @arguments={:token=>"tRBRACK"}, @location=#<Parser::Source::Range (string) 55137...55138>, @highlights=[]>
"unexpected token tRBRACK"
2147
"        when (__p_1__ && __p_3__ = (1 == __m_arr__.size) and (Paren(contents: {body: [ArrayLiteral(contents: {parts: [_, _, *]}) => array]}) === __m_arr__[0]))"
Failed to transpile ./lib/syntax_tree/node.rb: SyntaxError — unexpected token tRBRACK

So it seems ruby-next tries to reparse what it produced, and that fails. So maybe the issue is that ruby-next doesn't desugar that ArrayLiteral(contents: {parts: [_, _, *]}) => array?

[eregon]

I guess [_, _, *] is actually "basic" pattern matching, not find pattern. And that should be the case since anyway syntax_tree supports CRuby 2.7 and its "basic" pattern matching.

[palkan]

Thanks for the investigation, @eregon!

when (p_1 && p_3 = (1 == m_arr.size) and (Paren(contents: {body: [ArrayLiteral(contents: {parts: [_, _, *]}) => array]}) === m_arr[0]))

I guess [_, _, *] is actually "basic" pattern matching, not find pattern

These two things look suspicious. I will take a look.

[palkan]

Fixed in 0.15.3.

Tested against syntax_tree main and 3.6.3 (since @kddnewton got rid of pattern matching recently—performance reasons? Curious to hear more about it 🙂)

[palkan]

unfortunately deconstruct_keys was showing up quite a lot in the profile

Yeah, it hasn't been optimized yet. We added some optimization for #deconstruct (ruby/ruby#3104) in 3.0, but doing the same for #deconstruct_keys is much trickier, since it depends on the args.

Switching back to a case..when gave me some performance back.

I'm curious, how the transpired code would perform (since transpiled pattern matching is better optimized than a native one). Do you have a benchmark script available somewhere, so I could try to experiment with it?

[eregon]

Fixed in 0.15.3.

Thanks!
I tried ruby-next nextify -V --rewrite=pattern-matching ./lib on ruby-syntax-tree/syntax_tree@cb72efc and that succeeded.
Together with the change below, the files load on TruffleRuby but the test suite can't be run because it itself uses pattern matching.
I also briefly tried require "ruby-next/language/runtime" but that didn't seem to handle eval (and it was a pretty high startup overhead).
FWIW I also tried ruby-next nextify -V --rewrite=pattern-matching ./test/*_test.rb but that doesn't work, it seems to ignore the 2nd and later files.
It'd be quite convenient to have a mode where the files are rewritten inplace actually for such experiments :)

This is the change to pick up the rewritten lib files under lib/.rbnext:

diff --git a/lib/syntax_tree.rb b/lib/syntax_tree.rb
index 88c6636..04fa0ae 100644
--- a/lib/syntax_tree.rb
+++ b/lib/syntax_tree.rb
@@ -1,5 +1,10 @@
 # frozen_string_literal: true

+if RUBY_ENGINE == "truffleruby"
+  module RubyNext; end
+  $LOAD_PATH.unshift "#{__dir__}/.rbnext"
+end
+
 require "delegate"
 require "etc"
 require "json"
@@ -9,7 +14,7 @@ require "ripper"
 require "stringio"

 require_relative "syntax_tree/formatter"
-require_relative "syntax_tree/node"
+require "syntax_tree/node"
 require_relative "syntax_tree/parser"
 require_relative "syntax_tree/version"

Ideally module RubyNext; end wouldn't be needed but RubyNext currently adds using RubyNext; even if it's only syntax desugaring and no method backports.

[eregon]

Ah and last thing I noticed this warning when using the runtime mode:

/home/eregon/.rubies/truffleruby-dev/lib/gems/gems/ruby-next-core-0.15.3/lib/.rbnext/2.7/ruby-next/language/rewriters/2.7/pattern_matching.rb:1010: warning: `_1' is reserved for numbered parameter; consider another name

@palkan Would you like I open separate issues with what I mentioned here? Or a single issue with various items?

[palkan]

Thanks @eregon!

Let's create separate issues, please. (I will add the "hacktoberfest" label, so the work could be parallelized 🙂).

[palkan]

I also briefly tried require "ruby-next/language/runtime" but that didn't seem to handle eval

Yeah, for eval support, a refinement must be activated explicitly: using RubyNext::Language::Eval.

but RubyNext currently adds using RubyNext; even if it's only syntax desugaring and no method backports.

You can run nextify with --no-refine to not inject using RubyNext

[palkan]

the test suite can't be run because it itself uses pattern matching.

One option to try adding -ruby-next to the Rake task's ruby_opts (or add it when running a single test file, ruby -ruby-next .... Haven't tried this though.

[eregon]

You can run nextify with --no-refine to not inject using RubyNext

Oh, right, somehow I missed that in the --help.