Feature #20257: Rearchitect Ripper

• Status: Closed
• Assignee: -
• Target version: -

[yui-knk (Kaneko Yuichiro)]

Abstract

Rearchitect Ripper to provide whole semantic analysis support for Ripper and improve maintainability of Ripper.
This rearchitecture is achieved by modifying Lrama parser generator.

Background and problem

Ripper is used for parsing ruby code, for example irb and rdoc use Ripper for parsing source codes.
Ripper and Ruby parser share the algorithm of parsing, however internal logic of Ripper is different from parser.
The differences cause three problems:

1. Ripper can not execute some semantic analysis. https://bugs.ruby-lang.org/issues/10436 is an example of this limitaion. m(&nil) {} raises syntax error but Ripper.sexp("m(&nil) {}") doesn't.
2. Ripper can not recognize regexp named capture. https://bugs.ruby-lang.org/issues/18988 is an example.
3. Makes prase.y complex. For example, the implementation of new_array_pattern is completely different between parser and ripper.

These problems come from the fact parser and Ripper use semantic value stack differently.
Parser stores Node on the stack in many rules but Ripper stores Ruby Object returned by callback method.
Therefore Ripper can not execute semantic analysis which requires Node (#1).
Values on the stack are different then they need to implement same name functions differently (#3).
This leads different behavior like #2 because they have different match_op function.

Proposal

Introduce new semantic value stack for Ripper so that Ripper can manage both Node and Ruby Object separately.
Lrama will provide some callback entry points and new special variable for actions.

Lrama will support these callback directives, specified function is called when the event happens

• %after-shift function_name
• %before-reduce function_name
• %after-reduce function_name
• %after-shift-error-token function_name
• %after-pop-stack function_name

Lrama also provides $:n variable to access index of each grammar symbols. The variable is translated to the minus index from the top of the stack.
For example

primary: k_if expr_value then compstmt if_tail k_end
          {
          /*% ripper: if!($:2, $:4, $:5) %*/
          /* $:2 = -5, $:4 = -3, $:5 = -2. */
          }

We can implement separated stack for Ruby Object by these features.

Implementation note

New fields of struct parser_params

• VALUE s_value: Holds Ruby Object returned by Ripper callback method call.
• VALUE s_lvalue: Holds Ruby Object responding to LHS of the rule.
• VALUE s_value_stack: Stack for Ruby Object. It's actually ruby array.

These fields are added only when it's Ripper.

The role of callback functions

• %after-shift: Push s_value to s_value_stack.
• %before-reduce: Assign the first Ruby Object of RHS to s_lvalue (similar with $$ = $1).
• %after-reduce: Pop s_value_stack rhs.len times then push s_lvalue to s_value_stack.
• %after-shift-error-token: Push nil to s_value_stack. This nil stands for error token.
• %after-pop-stack: Pop s_value_stack len times. This is needed to emulate panic mode.

Achievement

These bugs are fixed.

• Bug 10436 "ruby -c and ripper inconsistency: m(&nil) {}"
• Bug 18988 "Ripper cannot parse some code that has regexp named capture"
• Bug 20055 "Ripper seems to skip some checks like void value expression and duplicated variable name"

This means all of Ripper open bugs tickets which are related current architecture will be closed.

Links

• Lrama Implementaion: https://github.com/ruby/lrama/pull/367
• Ruby Implementaion: https://github.com/ruby/ruby/pull/9923

[yui-knk (Kaneko Yuichiro)]

• Description updated (diff)

[ko1 (Koichi Sasada)]

Do you think the proposal has impact for user code?

[yui-knk (Kaneko Yuichiro)]

No user interface changes other than lex state changes by making them to be aligned with parser's lex state transition.

[matz (Yukihiro Matsumoto)]

I am OK with it.

Matz.

[yui-knk (Kaneko Yuichiro)]

• Status changed from Open to Closed

Applied in changeset git|89cfc1520717257073012ec07105c551e4b8af7c.

---

[Feature #20257] Rearchitect Ripper

Introduce another semantic value stack for Ripper so that
Ripper can manage both Node and Ruby Object separately.
This rearchitectutre of Ripper solves these issues.
Therefore adding test cases for them.

• [Bug 10436] https://bugs.ruby-lang.org/issues/10436
• [Bug 18988] https://bugs.ruby-lang.org/issues/18988
• [Bug 20055] https://bugs.ruby-lang.org/issues/20055

Checked the differences of Ripper.sexp for files under /test/ruby
are only on test_pattern_matching.rb.
The differences comes from the differences between
new_hash_pattern_tail functions between parser and Ripper.
Ripper new_hash_pattern_tail didn’t call assignable then
kw_rest_arg wasn’t marked as local variable.
This is also fixed by this commit.

--- a/./tmp/before/test_pattern_matching.rb
+++ b/./tmp/after/test_pattern_matching.rb
@@ -3607,7 +3607,7 @@
                  [:in,
                   [:hshptn, nil, [], [:var_field, [:@ident, “a”, [984, 13]]]],
                   [[:binary,
-                    [:vcall, [:@ident, “a”, [985, 10]]],
+                    [:var_ref, [:@ident, “a”, [985, 10]]],
                     :==,
                     [:hash, nil]]],
                   nil]]],
@@ -3662,7 +3662,7 @@
                  [:in,
                   [:hshptn, nil, [], [:var_field, [:@ident, “a”, [993, 13]]]],
                   [[:binary,
-                    [:vcall, [:@ident, “a”, [994, 10]]],
+                    [:var_ref, [:@ident, “a”, [994, 10]]],
                     :==,
                     [:hash,
                      [:assoclist_from_args,
@@ -3813,7 +3813,7 @@
                    [:command,
                     [:@ident, “raise”, [1022, 10]],
                     [:args_add_block,
-                     [[:vcall, [:@ident, “b”, [1022, 16]]]],
+                     [[:var_ref, [:@ident, “b”, [1022, 16]]]],
                      false]]],
                   [:else, [[:var_ref, [:@kw, “true”, [1024, 10]]]]]]]],
                nil,
@@ -3876,7 +3876,7 @@
                      [:@int, “0”, [1033, 15]]],
                     :“&&“,
                     [:binary,
-                     [:vcall, [:@ident, “b”, [1033, 20]]],
+                     [:var_ref, [:@ident, “b”, [1033, 20]]],
                      :==,
                      [:hash, nil]]]],
                   nil]]],
@@ -3946,7 +3946,7 @@
                      [:@int, “0”, [1042, 15]]],
                     :“&&“,
                     [:binary,
-                     [:vcall, [:@ident, “b”, [1042, 20]]],
+                     [:var_ref, [:@ident, “b”, [1042, 20]]],
                      :==,
                      [:hash,
                       [:assoclist_from_args,
@@ -5206,7 +5206,7 @@
                      [[:assoc_new,
                        [:@label, “c:“, [1352, 22]],
                        [:@int, “0”, [1352, 25]]]]]],
-                   [:vcall, [:@ident, “r”, [1352, 29]]]],
+                   [:var_ref, [:@ident, “r”, [1352, 29]]]],
                   false]]],
                [:binary,
                 [:call,
@@ -5299,7 +5299,7 @@
                       [:assoc_new,
                        [:@label, “c:“, [1367, 34]],
                        [:@int, “0”, [1367, 37]]]]]],
-                   [:vcall, [:@ident, “r”, [1367, 41]]]],
+                   [:var_ref, [:@ident, “r”, [1367, 41]]]],
                   false]]],
                [:binary,
                 [:call,
@@ -5931,7 +5931,7 @@
              [:in,
               [:hshptn, nil, [], [:var_field, [:@ident, “r”, [1533, 11]]]],
               [[:binary,
-                [:vcall, [:@ident, “r”, [1534, 8]]],
+                [:var_ref, [:@ident, “r”, [1534, 8]]],
                 :==,
                 [:hash,
                  [:assoclist_from_args,