parse.y: fix var_field

* parse.y (mlhs_node): dispatch var_field as well as lhs.

* parse.y (lhs, var_hs): dispatch var_field before assignable
  check so it is inside assign_error at a wrong assignment, as
  well as backref_assign_error.

* parse.y (var_field_gen): wrap var_field value with the variable
  ID.

* parse.y (assignable_gen, const_decl_gen): return the callback
  result on a syntax error.

git-svn-id: svn+ssh://ci.ruby-lang.org/ruby/trunk@60116 b2dd03c8-39d4-4d8f-98ff-823fe69b080e

2 files changed, 57 insertions, 38 deletions

diff --git a/parse.y b/parse.y
index 1782bac173..586e5a78ce 100644
--- a/parse.y
+++ b/parse.y
@@ -548,7 +548,8 @@ static VALUE new_xstring_gen(struct parser_params *, VALUE);
 static VALUE const_decl_gen(struct parser_params *parser, VALUE path);
 #define const_decl(path) const_decl_gen(parser, path)
 
-#define var_field(n) dispatch1(var_field, (n))
+static VALUE var_field_gen(struct parser_params *parser, VALUE a);
+#define var_field(a) var_field_gen(parser, (a))
 static VALUE assign_error_gen(struct parser_params *parser, VALUE a);
 #define assign_error(a) assign_error_gen(parser, (a))
 #define backref_assign_error(n, a) assign_error(a)
@@ -1711,11 +1712,11 @@ mlhs_post	: mlhs_item
 
 mlhs_node	: user_variable
 		    {
-			$$ = assignable($1, 0);
+			$$ = assignable(var_field($1), 0);
 		    }
 		| keyword_variable
 		    {
-			$$ = assignable($1, 0);
+			$$ = assignable(var_field($1), 0);
 		    }
 		| primary_value '[' opt_call_args rbracket
 		    {
@@ -1766,20 +1767,18 @@ mlhs_node	: user_variable
 
 lhs		: user_variable
 		    {
-			$$ = assignable($1, 0);
+			$$ = assignable(var_field($1), 0);
 		    /*%%%*/
 			if (!$$) $$ = NEW_BEGIN(0);
 		    /*%
-			$$ = dispatch1(var_field, $$);
 		    %*/
 		    }
 		| keyword_variable
 		    {
-			$$ = assignable($1, 0);
+			$$ = assignable(var_field($1), 0);
 		    /*%%%*/
 			if (!$$) $$ = NEW_BEGIN(0);
 		    /*%
-			$$ = dispatch1(var_field, $$);
 		    %*/
 		    }
 		| primary_value '[' opt_call_args rbracket
@@ -4142,19 +4141,11 @@ var_ref		: user_variable
 
 var_lhs		: user_variable
 		    {
-			$$ = assignable($1, 0);
-		    /*%%%*/
-		    /*%
-			$$ = dispatch1(var_field, $$);
-		    %*/
+			$$ = assignable(var_field($1), 0);
 		    }
 		| keyword_variable
 		    {
-			$$ = assignable($1, 0);
-		    /*%%%*/
-		    /*%
-			$$ = dispatch1(var_field, $$);
-		    %*/
+			$$ = assignable(var_field($1), 0);
 		    }
 		;
 
@@ -9258,8 +9249,8 @@ assignable_gen(struct parser_params *parser, ID id, NODE *val)
 {
 #ifdef RIPPER
     ID id = get_id(lhs);
-# define assignable_result(x) get_value(lhs)
-# define parser_yyerror(parser, x) assign_error_gen(parser, lhs)
+# define assignable_result(x) (lhs)
+# define parser_yyerror(parser, x) (lhs = assign_error_gen(parser, lhs))
 #else
 # define assignable_result(x) (x)
 #endif
@@ -10296,7 +10287,8 @@ static VALUE
 const_decl_gen(struct parser_params *parser, VALUE path)
 {
     if (in_def || in_single) {
-	assign_error(path);
+	path = dispatch1(assign_error, path);
+	ripper_error();
     }
     return path;
 }
@@ -10308,6 +10300,12 @@ assign_error_gen(struct parser_params *parser, VALUE a)
     ripper_error();
     return a;
 }
+
+static VALUE
+var_field_gen(struct parser_params *parser, VALUE a)
+{
+    return ripper_new_yylval(get_id(a), dispatch1(var_field, a), 0);
+}
 #endif
 
 static void
diff --git a/test/ripper/test_parser_events.rb b/test/ripper/test_parser_events.rb
index ccebe2dbb7..cf118a49c2 100644
--- a/test/ripper/test_parser_events.rb
+++ b/test/ripper/test_parser_events.rb
@@ -194,7 +194,10 @@ class TestRipper::ParserEvents < Test::Unit::TestCase
   end
 
   def test_assign_error
-    # for test_coverage
+    thru_assign_error = false
+    result = parse('self = 1', :on_assign_error) {thru_assign_error = true}
+    assert_equal true, thru_assign_error
+    assert_equal '[assign(assign_error(var_field(self)),1)]', result
   end
 
   def test_assign_error_backref
@@ -208,29 +211,47 @@ class TestRipper::ParserEvents < Test::Unit::TestCase
     result =
       parse('$`, _ = 1', :on_assign_error) {thru_assign_error = true}
     assert_equal true, thru_assign_error
-    assert_equal '[massign([assign_error(var_field($`)),_],1)]', result
+    assert_equal '[massign([assign_error(var_field($`)),var_field(_)],1)]', result
   end
 
   def test_assign_error_const_qualified
     thru_assign_error = false
-    parse('self::X = 1', :on_assign_error) {thru_assign_error = true}
+    result =
+      parse('self::X = 1', :on_assign_error) {thru_assign_error = true}
     assert_equal false, thru_assign_error
-    parse("def m\n self::X = 1\nend", :on_assign_error) {thru_assign_error = true}
+    assert_equal "[assign(const_path_field(ref(self),X),1)]", result
+
+    thru_assign_error = false
+    result =
+      parse("def m\n self::X = 1\nend", :on_assign_error) {thru_assign_error = true}
     assert_equal true, thru_assign_error
+    assert_include result, "assign_error(const_path_field(ref(self),X))"
+
     thru_assign_error = false
-    parse("def m\n self::X, a = 1, 2\nend", :on_assign_error) {thru_assign_error = true}
+    result =
+      parse("def m\n self::X, a = 1, 2\nend", :on_assign_error) {thru_assign_error = true}
     assert_equal true, thru_assign_error
+    assert_include result, "assign_error(const_path_field(ref(self),X))"
   end
 
   def test_assign_error_const
     thru_assign_error = false
-    parse('X = 1', :on_assign_error) {thru_assign_error = true}
+    result = parse('X = 1', :on_assign_error) {thru_assign_error = true}
+    assert_equal false, thru_assign_error
+    assert_equal "[assign(var_field(X),1)]", result
+
+    thru_assign_error = false
+    result = parse('X, a = 1, 2', :on_assign_error) {thru_assign_error = true}
     assert_equal false, thru_assign_error
-    parse("def m\n X = 1\nend", :on_assign_error) {thru_assign_error = true}
+    assert_include result, "massign([var_field(X),var_field(a)],"
+
+    result = parse("def m\n X = 1\nend", :on_assign_error) {thru_assign_error = true}
     assert_equal true, thru_assign_error
+    assert_include result, "assign_error(var_field(X))"
     thru_assign_error = false
-    parse("def m\n X, a = 1, 2\nend", :on_assign_error) {thru_assign_error = true}
+    result = parse("def m\n X, a = 1, 2\nend", :on_assign_error) {thru_assign_error = true}
     assert_equal true, thru_assign_error
+    assert_include result, "assign_error(var_field(X))"
   end
 
   def test_assign_error_const_toplevel
@@ -475,46 +496,46 @@ class TestRipper::ParserEvents < Test::Unit::TestCase
     thru_mlhs_add_star = false
     tree = parse("a, *b = 1, 2", :on_mlhs_add_star) {thru_mlhs_add_star = true}
     assert_equal true, thru_mlhs_add_star
-    assert_include(tree, "massign([a,*b]")
+    assert_include(tree, "massign([var_field(a),*var_field(b)]")
     thru_mlhs_add_star = false
     tree = parse("a, *b, c = 1, 2", :on_mlhs_add_star) {thru_mlhs_add_star = true}
     assert_equal true, thru_mlhs_add_star
-    assert_include(tree, "massign([a,*b,c]", bug2232)
+    assert_include(tree, "massign([var_field(a),*var_field(b),var_field(c)]", bug2232)
     thru_mlhs_add_star = false
     tree = parse("a, *, c = 1, 2", :on_mlhs_add_star) {thru_mlhs_add_star = true}
     assert_equal true, thru_mlhs_add_star
-    assert_include(tree, "massign([a,*,c]", bug4364)
+    assert_include(tree, "massign([var_field(a),*,var_field(c)]", bug4364)
     thru_mlhs_add_star = false
     tree = parse("*b, c = 1, 2", :on_mlhs_add_star) {thru_mlhs_add_star = true}
     assert_equal true, thru_mlhs_add_star
-    assert_include(tree, "massign([*b,c]", bug4364)
+    assert_include(tree, "massign([*var_field(b),var_field(c)]", bug4364)
     thru_mlhs_add_star = false
     tree = parse("*, c = 1, 2", :on_mlhs_add_star) {thru_mlhs_add_star = true}
     assert_equal true, thru_mlhs_add_star
-    assert_include(tree, "massign([*,c],", bug4364)
+    assert_include(tree, "massign([*,var_field(c)],", bug4364)
   end
 
   def test_mlhs_add_post
     thru_mlhs_add_post = false
     tree = parse("a, *b = 1, 2", :on_mlhs_add_post) {thru_mlhs_add_post = true}
     assert_equal false, thru_mlhs_add_post
-    assert_include(tree, "massign([a,*b],")
+    assert_include(tree, "massign([var_field(a),*var_field(b)],")
     thru_massign_add_post = false
     tree = parse("a, *b, c = 1, 2", :on_mlhs_add_post) {thru_mlhs_add_post = true}
     assert_equal true, thru_mlhs_add_post
-    assert_include(tree, "massign([a,*b,c],")
+    assert_include(tree, "massign([var_field(a),*var_field(b),var_field(c)],")
     thru_mlhs_add_post = false
     tree = parse("a, *, c = 1, 2", :on_mlhs_add_post) {thru_mlhs_add_post = true}
     assert_equal true, thru_mlhs_add_post
-    assert_include(tree, "massign([a,*,c],")
+    assert_include(tree, "massign([var_field(a),*,var_field(c)],")
     thru_mlhs_add_post = false
     tree = parse("*b, c = 1, 2", :on_mlhs_add_post) {thru_mlhs_add_post = true}
     assert_equal true, thru_mlhs_add_post
-    assert_include(tree, "massign([*b,c],")
+    assert_include(tree, "massign([*var_field(b),var_field(c)],")
     thru_mlhs_add_post = false
     tree = parse("*, c = 1, 2", :on_mlhs_add_post) {thru_mlhs_add_post = true}
     assert_equal true, thru_mlhs_add_post
-    assert_include(tree, "massign([*,c],")
+    assert_include(tree, "massign([*,var_field(c)],")
   end
 
   def test_mlhs_new