Remove circular parameter syntax error

https://bugs.ruby-lang.org/issues/20478

parse.y

@@ -539,8 +539,6 @@ struct parser_params {
         int end_col;
     } delayed;
 
-    ID cur_arg;
-
     rb_ast_t *ast;
     int node_id;
 
@@ -1305,7 +1303,6 @@ struct RNode_DEF_TEMP {
     ID nd_mid;
 
     struct {
-        ID cur_arg;
         int max_numparam;
         NODE *numparam_save;
         struct lex_context ctxt;
@@ -1671,7 +1668,6 @@ restore_defun(struct parser_params *p, rb_node_def_temp_t *temp)
 {
     /* See: def_name action */
     struct lex_context ctxt = temp->save.ctxt;
-    p->cur_arg = temp->save.cur_arg;
     p->ctxt.in_def = ctxt.in_def;
     p->ctxt.shareable_constant_value = ctxt.shareable_constant_value;
     p->ctxt.in_rescue = ctxt.in_rescue;
@@ -2900,7 +2896,6 @@ rb_parser_ary_free(rb_parser_t *p, rb_parser_ary_t *ary)
  */
 %rule f_opt(value) <node_opt_arg>: f_arg_asgn f_eq value
                                     {
-                                        p->cur_arg = 0;
                                         p->ctxt.in_argdef = 1;
                                         $$ = NEW_OPT_ARG(assignable(p, $1, $3, &@$), &@$);
                                     /*% ripper: [$:$, $:3] %*/
@@ -3378,7 +3373,6 @@ def_name	: fname
                         ID fname = $1;
                         numparam_name(p, fname);
                         local_push(p, 0);
-                        p->cur_arg = 0;
                         p->ctxt.in_def = 1;
                         p->ctxt.in_rescue = before_rescue;
                         $$ = $1;
@@ -5113,7 +5107,6 @@ opt_block_param	: none
 
 block_param_def	: '|' opt_bv_decl '|'
                     {
-                        p->cur_arg = 0;
                         p->max_numparam = ORDINAL_PARAM;
                         p->ctxt.in_argdef = 0;
                         $$ = 0;
@@ -5121,7 +5114,6 @@ block_param_def	: '|' opt_bv_decl '|'
                     }
                 | '|' block_param opt_bv_decl '|'
                     {
-                        p->cur_arg = 0;
                         p->max_numparam = ORDINAL_PARAM;
                         p->ctxt.in_argdef = 0;
                         $$ = $2;
@@ -6560,14 +6552,12 @@ f_arg_asgn	: f_norm_arg
                     {
                         ID id = $1;
                         arg_var(p, id);
-                        p->cur_arg = id;
                         $$ = $1;
                     }
                 ;
 
 f_arg_item	: f_arg_asgn
                     {
-                        p->cur_arg = 0;
                         $$ = NEW_ARGS_AUX($1, 1, &NULL_LOC);
                     /*% ripper: $:1 %*/
                     }
@@ -6606,7 +6596,6 @@ f_arg		: f_arg_item
 f_label 	: tLABEL
                     {
                         arg_var(p, formal_argument(p, $1));
-                        p->cur_arg = $1;
                         p->max_numparam = ORDINAL_PARAM;
                         p->ctxt.in_argdef = 0;
                         $$ = $1;
@@ -6616,14 +6605,12 @@ f_label 	: tLABEL
 
 f_kw		: f_label arg_value
                     {
-                        p->cur_arg = 0;
                         p->ctxt.in_argdef = 1;
                         $$ = new_kw_arg(p, assignable(p, $1, $2, &@$), &@$);
                     /*% ripper: [$:$, $:2] %*/
                     }
                 | f_label
                     {
-                        p->cur_arg = 0;
                         p->ctxt.in_argdef = 1;
                         $$ = new_kw_arg(p, assignable(p, $1, NODE_SPECIAL_REQUIRED_KEYWORD, &@$), &@$);
                     /*% ripper: [$:$, 0] %*/
@@ -12500,7 +12487,6 @@ static rb_node_def_temp_t *
 rb_node_def_temp_new(struct parser_params *p, const YYLTYPE *loc)
 {
     rb_node_def_temp_t *n = NODE_NEWNODE((enum node_type)NODE_DEF_TEMP, rb_node_def_temp_t, loc);
-    n->save.cur_arg = p->cur_arg;
     n->save.numparam_save = 0;
     n->save.max_numparam = 0;
     n->save.ctxt = p->ctxt;
@@ -13031,19 +13017,11 @@ gettable(struct parser_params *p, ID id, const YYLTYPE *loc)
       case ID_LOCAL:
         if (dyna_in_block(p) && dvar_defined_ref(p, id, &vidp)) {
             if (NUMPARAM_ID_P(id) && (numparam_nested_p(p) || it_used_p(p))) return 0;
-            if (id == p->cur_arg) {
-                compile_error(p, "circular argument reference - %"PRIsWARN, rb_id2str(id));
-                return 0;
-            }
             if (vidp) *vidp |= LVAR_USED;
             node = NEW_DVAR(id, loc);
             return node;
         }
         if (local_id_ref(p, id, &vidp)) {
-            if (id == p->cur_arg) {
-                compile_error(p, "circular argument reference - %"PRIsWARN, rb_id2str(id));
-                return 0;
-            }
             if (vidp) *vidp |= LVAR_USED;
             node = NEW_LVAR(id, loc);
             return node;

prism/prism.c

@@ -14394,7 +14394,7 @@ parse_parameters(
                     context_push(parser, PM_CONTEXT_DEFAULT_PARAMS);
 
                     pm_constant_id_t name_id = pm_parser_constant_id_token(parser, &name);
-                    uint32_t reads = pm_locals_reads(&parser->current_scope->locals, name_id);
+                    uint32_t reads = parser->version == PM_OPTIONS_VERSION_CRUBY_3_3 ? pm_locals_reads(&parser->current_scope->locals, name_id) : 0;
 
                     pm_node_t *value = parse_value_expression(parser, binding_power, false, PM_ERR_PARAMETER_NO_DEFAULT);
                     pm_optional_parameter_node_t *param = pm_optional_parameter_node_create(parser, &name, &operator, value);
@@ -14407,7 +14407,7 @@ parse_parameters(
                     // If the value of the parameter increased the number of
                     // reads of that parameter, then we need to warn that we
                     // have a circular definition.
-                    if (pm_locals_reads(&parser->current_scope->locals, name_id) != reads) {
+                    if ((parser->version == PM_OPTIONS_VERSION_CRUBY_3_3) && (pm_locals_reads(&parser->current_scope->locals, name_id) != reads)) {
                         PM_PARSER_ERR_TOKEN_FORMAT_CONTENT(parser, name, PM_ERR_PARAMETER_CIRCULAR);
                     }
 
@@ -14486,10 +14486,10 @@ parse_parameters(
                             context_push(parser, PM_CONTEXT_DEFAULT_PARAMS);
 
                             pm_constant_id_t name_id = pm_parser_constant_id_token(parser, &local);
-                            uint32_t reads = pm_locals_reads(&parser->current_scope->locals, name_id);
+                            uint32_t reads = parser->version == PM_OPTIONS_VERSION_CRUBY_3_3 ? pm_locals_reads(&parser->current_scope->locals, name_id) : 0;
                             pm_node_t *value = parse_value_expression(parser, binding_power, false, PM_ERR_PARAMETER_NO_DEFAULT_KW);
 
-                            if (pm_locals_reads(&parser->current_scope->locals, name_id) != reads) {
+                            if (parser->version == PM_OPTIONS_VERSION_CRUBY_3_3 && (pm_locals_reads(&parser->current_scope->locals, name_id) != reads)) {
                                 PM_PARSER_ERR_TOKEN_FORMAT_CONTENT(parser, local, PM_ERR_PARAMETER_CIRCULAR);
                             }
 

spec/ruby/language/block_spec.rb

@@ -960,24 +960,27 @@ describe "Post-args" do
     end
 
     describe "with a circular argument reference" do
-      it "raises a SyntaxError if using an existing local with the same name as the argument" do
+      ruby_version_is ""..."3.4" do
-        a = 1
+        it "raises a SyntaxError if using the argument in its default value" do
-        -> {
+          a = 1
-          @proc = eval "proc { |a=a| a }"
+          -> {
-        }.should raise_error(SyntaxError)
+            eval "proc { |a=a| a }"
+          }.should raise_error(SyntaxError)
+        end
       end
 
-      it "raises a SyntaxError if there is an existing method with the same name as the argument" do
+      ruby_version_is "3.4" do
-        def a; 1; end
+        it "is nil if using the argument in its default value" do
-        -> {
+          -> {
-          @proc = eval "proc { |a=a| a }"
+            eval "proc { |a=a| a }.call"
-        }.should raise_error(SyntaxError)
+          }.call.should == nil
+        end
       end
+    end
 
-      it "calls an existing method with the same name as the argument if explicitly using ()" do
+    it "calls an existing method with the same name as the argument if explicitly using ()" do
-        def a; 1; end
+      def a; 1; end
-        proc { |a=a()| a }.call.should == 1
+      proc { |a=a()| a }.call.should == 1
-      end
     end
   end
 

spec/ruby/language/def_spec.rb

@@ -197,15 +197,25 @@ describe "An instance method with a default argument" do
     foo(2,3,3).should == [2,3,[3]]
   end
 
-  it "raises a SyntaxError when there is an existing method with the same name as the local variable" do
+  ruby_version_is ""..."3.4" do
-    def bar
+    it "raises a SyntaxError if using the argument in its default value" do
-      1
+      -> {
+        eval "def foo(bar = bar)
+          bar
+        end"
+      }.should raise_error(SyntaxError)
+    end
+  end
+
+  ruby_version_is "3.4" do
+    it "is nil if using the argument in its default value" do
+      -> {
+        eval "def foo(bar = bar)
+          bar
+        end
+        foo"
+      }.call.should == nil
     end
-    -> {
-      eval "def foo(bar = bar)
-        bar
-      end"
-    }.should raise_error(SyntaxError)
   end
 
   it "calls a method with the same name as the local when explicitly using ()" do

spec/ruby/language/lambda_spec.rb

@@ -263,18 +263,21 @@ describe "A lambda literal -> () { }" do
     end
 
     describe "with circular optional argument reference" do
-      it "raises a SyntaxError if using an existing local with the same name as the argument" do
+      ruby_version_is ""..."3.4" do
-        a = 1
+        it "raises a SyntaxError if using the argument in its default value" do
-        -> {
+          a = 1
-          @proc = eval "-> (a=a) { a }"
+          -> {
-        }.should raise_error(SyntaxError)
+            eval "-> (a=a) { a }"
+          }.should raise_error(SyntaxError)
+        end
       end
 
-      it "raises a SyntaxError if there is an existing method with the same name as the argument" do
+      ruby_version_is "3.4" do
-        def a; 1; end
+        it "is nil if using the argument in its default value" do
-        -> {
+          -> {
-          @proc = eval "-> (a=a) { a }"
+            eval "-> (a=a) { a }.call"
-        }.should raise_error(SyntaxError)
+          }.call.should == nil
+        end
       end
 
       it "calls an existing method with the same name as the argument if explicitly using ()" do

test/.excludes-prism/TestSyntax.rb

@@ -1,2 +0,0 @@
-exclude(:test_optional_self_reference, "https://bugs.ruby-lang.org/issues/20478")
-exclude(:test_keyword_self_reference, "https://bugs.ruby-lang.org/issues/20478")

test/prism/errors_test.rb

@@ -1990,12 +1990,18 @@ module Prism
         proc { |foo: foo| }
       RUBY
 
-      assert_errors expression(source), source, [
+      assert_errors(
-        ["circular argument reference - bar", 8..11],
+        expression(source),
-        ["circular argument reference - bar", 32..35],
+        source,
-        ["circular argument reference - foo", 55..58],
+        [
-        ["circular argument reference - foo", 76..79]
+          ["circular argument reference - bar", 8..11],
-      ]
+          ["circular argument reference - bar", 32..35],
+          ["circular argument reference - foo", 55..58],
+          ["circular argument reference - foo", 76..79]
+        ],
+        check_valid_syntax: false,
+        version: "3.3.0"
+      )
 
       refute_error_messages("def foo(bar: bar = 1); end")
     end
@@ -2244,10 +2250,10 @@ module Prism
 
     private
 
-    def assert_errors(expected, source, errors, check_valid_syntax: true)
+    def assert_errors(expected, source, errors, check_valid_syntax: true, **options)
       refute_valid_syntax(source) if check_valid_syntax
 
-      result = Prism.parse(source)
+      result = Prism.parse(source, **options)
       node = result.value.statements.body.last
 
       assert_equal_nodes(expected, node, compare_location: false)

test/ruby/test_syntax.rb

@@ -392,12 +392,11 @@ class TestSyntax < Test::Unit::TestCase
   end
 
   def test_keyword_self_reference
-    message = /circular argument reference - var/
+    assert_valid_syntax("def foo(var: defined?(var)) var end")
-    assert_syntax_error("def foo(var: defined?(var)) var end", message)
+    assert_valid_syntax("def foo(var: var) var end")
-    assert_syntax_error("def foo(var: var) var end", message)
+    assert_valid_syntax("def foo(var: bar(var)) var end")
-    assert_syntax_error("def foo(var: bar(var)) var end", message)
+    assert_valid_syntax("def foo(var: bar {var}) var end")
-    assert_syntax_error("def foo(var: bar {var}) var end", message)
+    assert_valid_syntax("def foo(var: (1 in ^var)); end")
-    assert_syntax_error("def foo(var: (1 in ^var)); end", message)
 
     o = Object.new
     assert_warn("") do
@@ -423,6 +422,9 @@ class TestSyntax < Test::Unit::TestCase
     assert_warn("") do
       o.instance_eval("proc {|var: 1| var}")
     end
+
+    o = Object.new
+    assert_nil(o.instance_eval("def foo(bar: bar) = bar; foo"))
   end
 
   def test_keyword_invalid_name
@@ -456,14 +458,13 @@ class TestSyntax < Test::Unit::TestCase
   end
 
   def test_optional_self_reference
-    message = /circular argument reference - var/
+    assert_valid_syntax("def foo(var = defined?(var)) var end")
-    assert_syntax_error("def foo(var = defined?(var)) var end", message)
+    assert_valid_syntax("def foo(var = var) var end")
-    assert_syntax_error("def foo(var = var) var end", message)
+    assert_valid_syntax("def foo(var = bar(var)) var end")
-    assert_syntax_error("def foo(var = bar(var)) var end", message)
+    assert_valid_syntax("def foo(var = bar {var}) var end")
-    assert_syntax_error("def foo(var = bar {var}) var end", message)
+    assert_valid_syntax("def foo(var = (def bar;end; var)) var end")
-    assert_syntax_error("def foo(var = (def bar;end; var)) var end", message)
+    assert_valid_syntax("def foo(var = (def self.bar;end; var)) var end")
-    assert_syntax_error("def foo(var = (def self.bar;end; var)) var end", message)
+    assert_valid_syntax("def foo(var = (1 in ^var)); end")
-    assert_syntax_error("def foo(var = (1 in ^var)); end", message)
 
     o = Object.new
     assert_warn("") do
@@ -489,6 +490,9 @@ class TestSyntax < Test::Unit::TestCase
     assert_warn("") do
       o.instance_eval("proc {|var = 1| var}")
     end
+
+    o = Object.new
+    assert_nil(o.instance_eval("def foo(bar: bar) = bar; foo"))
   end
 
   def test_warn_grouped_expression