From 982443e6e373f5a3ac22ee495909cb9adffcd08d Mon Sep 17 00:00:00 2001 From: Yusuke Endoh Date: Fri, 18 Dec 2020 14:12:42 +0900 Subject: [PATCH] Revert "Better cooperation between public/protected/private with attr* and alias_method" This reverts commit 81739ad4fdfcc86a769056fec352f27c686fba1b. --- NEWS.md | 10 --- object.c | 61 ++++++------------- process.c | 3 +- spec/ruby/core/main/fixtures/classes.rb | 8 --- spec/ruby/core/main/private_spec.rb | 31 ++-------- spec/ruby/core/main/public_spec.rb | 27 +------- spec/ruby/core/module/alias_method_spec.rb | 15 +---- spec/ruby/core/module/attr_accessor_spec.rb | 16 ----- spec/ruby/core/module/attr_reader_spec.rb | 16 ----- spec/ruby/core/module/attr_spec.rb | 20 ------ spec/ruby/core/module/attr_writer_spec.rb | 16 ----- .../ruby/core/module/shared/set_visibility.rb | 34 ----------- vm_method.c | 40 +++--------- 13 files changed, 42 insertions(+), 255 deletions(-) diff --git a/NEWS.md b/NEWS.md index 0d794e6ed7..1f7b02eb27 100644 --- a/NEWS.md +++ b/NEWS.md @@ -249,16 +249,6 @@ Outstanding ones only. p C.ancestors #=> [C, M1, M2, Object, Kernel, BasicObject] ``` - * Module#public, Module#protected and Module#private methods now accept single - array argument with a list of method names. [[Feature #17314]] - - * Module#attr_accessor, Module#attr_reader, Module#attr_writer and Module#attr - methods now return array of defined methods names as symbols. - [[Feature #17314]] - - * Module#alias_method now returns the defined alias as symbol. - [[Feature #17314]] - * Mutex * `Mutex` is now acquired per-`Fiber` instead of per-`Thread`. This change diff --git a/object.c b/object.c index 4fe6b02e03..552b10ba49 100644 --- a/object.c +++ b/object.c @@ -2255,42 +2255,37 @@ id_for_attr(VALUE obj, VALUE name) /* * call-seq: - * attr_reader(symbol, ...) -> array - * attr(symbol, ...) -> array - * attr_reader(string, ...) -> array - * attr(string, ...) -> array + * attr_reader(symbol, ...) -> nil + * attr(symbol, ...) -> nil + * attr_reader(string, ...) -> nil + * attr(string, ...) -> nil * * Creates instance variables and corresponding methods that return the * value of each instance variable. Equivalent to calling * ``attr:name'' on each name in turn. * String arguments are converted to symbols. - * Returns an array of defined methods names as symbols. */ static VALUE rb_mod_attr_reader(int argc, VALUE *argv, VALUE klass) { int i; - VALUE names = rb_ary_new2(argc); for (i=0; i array - * attr(name, true) -> array - * attr(name, false) -> array + * attr(name, ...) -> nil + * attr(name, true) -> nil + * attr(name, false) -> nil * * The first form is equivalent to #attr_reader. * The second form is equivalent to attr_accessor(name) but deprecated. * The last form is equivalent to attr_reader(name) but deprecated. - * Returns an array of defined methods names as symbols. *-- * \private * \todo can be static? @@ -2300,57 +2295,47 @@ VALUE rb_mod_attr(int argc, VALUE *argv, VALUE klass) { if (argc == 2 && (argv[1] == Qtrue || argv[1] == Qfalse)) { - ID id = id_for_attr(klass, argv[0]); - VALUE names = rb_ary_new(); - rb_warning("optional boolean argument is obsoleted"); - rb_attr(klass, id, 1, RTEST(argv[1]), TRUE); - rb_ary_push(names, ID2SYM(id)); - if (argv[1] == Qtrue) rb_ary_push(names, rb_str_intern(rb_sprintf("%"PRIsVALUE"=", ID2SYM(id)))); - return names; + rb_attr(klass, id_for_attr(klass, argv[0]), 1, RTEST(argv[1]), TRUE); + return Qnil; } return rb_mod_attr_reader(argc, argv, klass); } /* * call-seq: - * attr_writer(symbol, ...) -> array - * attr_writer(string, ...) -> array + * attr_writer(symbol, ...) -> nil + * attr_writer(string, ...) -> nil * * Creates an accessor method to allow assignment to the attribute * symbol.id2name. * String arguments are converted to symbols. - * Returns an array of defined methods names as symbols. */ static VALUE rb_mod_attr_writer(int argc, VALUE *argv, VALUE klass) { int i; - VALUE names = rb_ary_new2(argc); for (i=0; i do - eval "private :main_public_method, :main_undefined_method", TOPLEVEL_BINDING + eval "private :main_undefined_method", TOPLEVEL_BINDING end.should raise_error(NameError) end end diff --git a/spec/ruby/core/main/public_spec.rb b/spec/ruby/core/main/public_spec.rb index bfc27a9e80..afe25c705a 100644 --- a/spec/ruby/core/main/public_spec.rb +++ b/spec/ruby/core/main/public_spec.rb @@ -4,32 +4,11 @@ require_relative 'fixtures/classes' describe "main#public" do after :each do Object.send(:private, :main_private_method) - Object.send(:private, :main_private_method2) end - context "when single argument is passed and it is not an array" do - it "sets the visibility of the given methods to public" do - eval "public :main_private_method", TOPLEVEL_BINDING - Object.should_not have_private_method(:main_private_method) - end - end - - context "when multiple arguments are passed" do - it "sets the visibility of the given methods to public" do - eval "public :main_private_method, :main_private_method2", TOPLEVEL_BINDING - Object.should_not have_private_method(:main_private_method) - Object.should_not have_private_method(:main_private_method2) - end - end - - ruby_version_is "3.0" do - context "when single argument is passed and is an array" do - it "sets the visibility of the given methods to public" do - eval "public [:main_private_method, :main_private_method2]", TOPLEVEL_BINDING - Object.should_not have_private_method(:main_private_method) - Object.should_not have_private_method(:main_private_method2) - end - end + it "sets the visibility of the given method to public" do + eval "public :main_private_method", TOPLEVEL_BINDING + Object.should_not have_private_method(:main_private_method) end it "returns Object" do diff --git a/spec/ruby/core/module/alias_method_spec.rb b/spec/ruby/core/module/alias_method_spec.rb index 5d3d0c23d9..742e289a3f 100644 --- a/spec/ruby/core/module/alias_method_spec.rb +++ b/spec/ruby/core/module/alias_method_spec.rb @@ -85,19 +85,8 @@ describe "Module#alias_method" do Module.should have_public_instance_method(:alias_method, false) end - describe "returned value" do - ruby_version_is ""..."3.0" do - it "returns self" do - @class.send(:alias_method, :checking_return_value, :public_one).should equal(@class) - end - end - - ruby_version_is "3.0" do - it "returns symbol of the defined method name" do - @class.send(:alias_method, :checking_return_value, :public_one).should equal(:checking_return_value) - @class.send(:alias_method, 'checking_return_value', :public_one).should equal(:checking_return_value) - end - end + it "returns self" do + @class.send(:alias_method, :checking_return_value, :public_one).should equal(@class) end it "works in module" do diff --git a/spec/ruby/core/module/attr_accessor_spec.rb b/spec/ruby/core/module/attr_accessor_spec.rb index 665a9346bd..6a749341be 100644 --- a/spec/ruby/core/module/attr_accessor_spec.rb +++ b/spec/ruby/core/module/attr_accessor_spec.rb @@ -67,22 +67,6 @@ describe "Module#attr_accessor" do Module.should have_public_instance_method(:attr_accessor, false) end - ruby_version_is ""..."3.0" do - it "returns nil" do - Class.new do - (attr_accessor :foo, 'bar').should == nil - end - end - end - - ruby_version_is "3.0" do - it "returns an array of defined methods names as symbols" do - Class.new do - (attr_accessor :foo, 'bar').should == [:foo, :foo=, :bar, :bar=] - end - end - end - describe "on immediates" do before :each do class Fixnum diff --git a/spec/ruby/core/module/attr_reader_spec.rb b/spec/ruby/core/module/attr_reader_spec.rb index e16b7ba2e3..238e3db9ea 100644 --- a/spec/ruby/core/module/attr_reader_spec.rb +++ b/spec/ruby/core/module/attr_reader_spec.rb @@ -61,20 +61,4 @@ describe "Module#attr_reader" do it "is a public method" do Module.should have_public_instance_method(:attr_reader, false) end - - ruby_version_is ""..."3.0" do - it "returns nil" do - Class.new do - (attr_reader :foo, 'bar').should == nil - end - end - end - - ruby_version_is "3.0" do - it "returns an array of defined methods names as symbols" do - Class.new do - (attr_reader :foo, 'bar').should == [:foo, :bar] - end - end - end end diff --git a/spec/ruby/core/module/attr_spec.rb b/spec/ruby/core/module/attr_spec.rb index 060d072f27..8b91e77658 100644 --- a/spec/ruby/core/module/attr_spec.rb +++ b/spec/ruby/core/module/attr_spec.rb @@ -145,24 +145,4 @@ describe "Module#attr" do it "is a public method" do Module.should have_public_instance_method(:attr, false) end - - ruby_version_is ""..."3.0" do - it "returns nil" do - Class.new do - (attr :foo, 'bar').should == nil - (attr :baz, false).should == nil - (attr :qux, true).should == nil - end - end - end - - ruby_version_is "3.0" do - it "returns an array of defined methods names as symbols" do - Class.new do - (attr :foo, 'bar').should == [:foo, :bar] - (attr :baz, false).should == [:baz] - (attr :qux, true).should == [:qux, :qux=] - end - end - end end diff --git a/spec/ruby/core/module/attr_writer_spec.rb b/spec/ruby/core/module/attr_writer_spec.rb index 59d8b7bf53..e4b193a9d8 100644 --- a/spec/ruby/core/module/attr_writer_spec.rb +++ b/spec/ruby/core/module/attr_writer_spec.rb @@ -61,20 +61,4 @@ describe "Module#attr_writer" do it "is a public method" do Module.should have_public_instance_method(:attr_writer, false) end - - ruby_version_is ""..."3.0" do - it "returns nil" do - Class.new do - (attr_writer :foo, 'bar').should == nil - end - end - end - - ruby_version_is "3.0" do - it "returns an array of defined methods names as symbols" do - Class.new do - (attr_writer :foo, 'bar').should == [:foo=, :bar=] - end - end - end end diff --git a/spec/ruby/core/module/shared/set_visibility.rb b/spec/ruby/core/module/shared/set_visibility.rb index 9f31e230ca..a04b1a54a0 100644 --- a/spec/ruby/core/module/shared/set_visibility.rb +++ b/spec/ruby/core/module/shared/set_visibility.rb @@ -6,40 +6,6 @@ describe :set_visibility, shared: true do end describe "with argument" do - describe "one or more arguments" do - it "sets visibility of given method names" do - visibility = @method - old_visibility = [:protected, :private].find {|vis| vis != visibility } - - mod = Module.new { - send old_visibility - def test1() end - def test2() end - send visibility, :test1, :test2 - } - mod.should send(:"have_#{visibility}_instance_method", :test1, false) - mod.should send(:"have_#{visibility}_instance_method", :test2, false) - end - end - - ruby_version_is "3.0" do - describe "array as a single argument" do - it "sets visibility of given method names" do - visibility = @method - old_visibility = [:protected, :private].find {|vis| vis != visibility } - - mod = Module.new { - send old_visibility - def test1() end - def test2() end - send visibility, [:test1, :test2] - } - mod.should send(:"have_#{visibility}_instance_method", :test1, false) - mod.should send(:"have_#{visibility}_instance_method", :test2, false) - end - end - end - it "does not clone method from the ancestor when setting to the same visibility in a child" do visibility = @method parent = Module.new { diff --git a/vm_method.c b/vm_method.c index e2c5bfb064..e526ee0130 100644 --- a/vm_method.c +++ b/vm_method.c @@ -1941,13 +1941,13 @@ rb_alias(VALUE klass, ID alias_name, ID original_name) /* * call-seq: - * alias_method(new_name, old_name) -> symbol + * alias_method(new_name, old_name) -> self * * Makes new_name a new copy of the method old_name. This can * be used to retain access to methods that are overridden. * * module Mod - * alias_method :orig_exit, :exit #=> :orig_exit + * alias_method :orig_exit, :exit * def exit(code=0) * puts "Exiting with code #{code}" * orig_exit(code) @@ -1968,19 +1968,8 @@ rb_mod_alias_method(VALUE mod, VALUE newname, VALUE oldname) if (!oldid) { rb_print_undef_str(mod, oldname); } - VALUE id = rb_to_id(newname); - rb_alias(mod, id, oldid); - return ID2SYM(id); -} - -static void -check_and_export_method(VALUE self, VALUE name, rb_method_visibility_t visi) -{ - ID id = rb_check_id(&name); - if (!id) { - rb_print_undef_str(self, name); - } - rb_export_method(self, id, visi); + rb_alias(mod, rb_to_id(newname), oldid); + return mod; } static void @@ -1995,19 +1984,13 @@ set_method_visibility(VALUE self, int argc, const VALUE *argv, rb_method_visibil return; } - - VALUE v; - - if (argc == 1 && (v = rb_check_array_type(argv[0])) != Qnil) { - long j; - - for (j = 0; j < RARRAY_LEN(v); j++) { - check_and_export_method(self, RARRAY_AREF(v, j), visi); + for (i = 0; i < argc; i++) { + VALUE v = argv[i]; + ID id = rb_check_id(&v); + if (!id) { + rb_print_undef_str(self, v); } - } else { - for (i = 0; i < argc; i++) { - check_and_export_method(self, argv[i], visi); - } + rb_export_method(self, id, visi); } } @@ -2029,7 +2012,6 @@ set_visibility(int argc, const VALUE *argv, VALUE module, rb_method_visibility_t * public -> self * public(symbol, ...) -> self * public(string, ...) -> self - * public(array) -> self * * With no arguments, sets the default visibility for subsequently * defined methods to public. With arguments, sets the named methods to @@ -2048,7 +2030,6 @@ rb_mod_public(int argc, VALUE *argv, VALUE module) * protected -> self * protected(symbol, ...) -> self * protected(string, ...) -> self - * protected(array) -> self * * With no arguments, sets the default visibility for subsequently * defined methods to protected. With arguments, sets the named methods @@ -2076,7 +2057,6 @@ rb_mod_protected(int argc, VALUE *argv, VALUE module) * private -> self * private(symbol, ...) -> self * private(string, ...) -> self - * private(array) -> self * * With no arguments, sets the default visibility for subsequently * defined methods to private. With arguments, sets the named methods