Skip to content

Commit

Permalink
Classes made from Struct should have default new singleton method.
Browse files Browse the repository at this point in the history
[Bug #16465] [Bug #16801]
[Fix GH-2795] [Fix GH-2944] [Fix GH-3045] [Fix GH-3093]

Note: Backporting shouldn't modify object.h and instead can use
struct_new_kw which is basically a duplicate implementation of
rb_class_new_instance_pass_kw

Co-authored-by: Yusuke Endoh <mame@ruby-lang.org>
Co-authored-by: John Hawthorn <john@hawthorn.email>
Co-authored-by: Adam Hess <HParker@github.com>
Co-authored-by: Jose Cortinas <jacortinas@gmail.com>
Co-authored-by: Jean Boussier <jean.boussier@gmail.com>
  • Loading branch information
6 people committed May 8, 2020
1 parent da345ad commit adf709a
Show file tree
Hide file tree
Showing 4 changed files with 24 additions and 22 deletions.
1 change: 1 addition & 0 deletions include/ruby/3/intern/object.h
Expand Up @@ -30,6 +30,7 @@ RUBY3_SYMBOL_EXPORT_BEGIN()
((obj) != (orig) && (rb_obj_init_copy((obj), (orig)), 1))
#define OBJ_INIT_COPY(obj, orig) RB_OBJ_INIT_COPY(obj, orig)

VALUE rb_class_new_instance_pass_kw(int, const VALUE *, VALUE);
VALUE rb_class_new_instance(int, const VALUE*, VALUE);
VALUE rb_class_new_instance_kw(int, const VALUE*, VALUE, int);

Expand Down
6 changes: 3 additions & 3 deletions object.c
Expand Up @@ -2203,8 +2203,8 @@ rb_class_allocate_instance(VALUE klass)
*
*/

static VALUE
rb_class_s_new(int argc, const VALUE *argv, VALUE klass)
VALUE
rb_class_new_instance_pass_kw(int argc, const VALUE *argv, VALUE klass)
{
VALUE obj;

Expand Down Expand Up @@ -4726,7 +4726,7 @@ InitVM_Object(void)
rb_define_method(rb_cModule, "singleton_class?", rb_mod_singleton_p, 0);

rb_define_method(rb_cClass, "allocate", rb_class_alloc_m, 0);
rb_define_method(rb_cClass, "new", rb_class_s_new, -1);
rb_define_method(rb_cClass, "new", rb_class_new_instance_pass_kw, -1);
rb_define_method(rb_cClass, "initialize", rb_class_initialize, -1);
rb_define_method(rb_cClass, "superclass", rb_class_superclass, 0);
rb_define_alloc_func(rb_cClass, rb_class_s_alloc);
Expand Down
21 changes: 6 additions & 15 deletions struct.c
Expand Up @@ -317,24 +317,15 @@ rb_struct_s_inspect(VALUE klass)
}

static VALUE
struct_new_kw(int argc, const VALUE *argv, VALUE klass)
{
return rb_class_new_instance_kw(argc, argv, klass, RB_PASS_CALLED_KEYWORDS);
}

static VALUE
setup_struct(VALUE nstr, VALUE members, int keyword_init)
setup_struct(VALUE nstr, VALUE members)
{
long i, len;
VALUE (*new_func)(int, const VALUE *, VALUE) = rb_class_new_instance;

if (keyword_init) new_func = struct_new_kw;

members = struct_set_members(nstr, members);

rb_define_alloc_func(nstr, struct_alloc);
rb_define_singleton_method(nstr, "new", new_func, -1);
rb_define_singleton_method(nstr, "[]", new_func, -1);
rb_define_singleton_method(nstr, "new", rb_class_new_instance_pass_kw, -1);
rb_define_singleton_method(nstr, "[]", rb_class_new_instance_pass_kw, -1);
rb_define_singleton_method(nstr, "members", rb_struct_s_members_m, 0);
rb_define_singleton_method(nstr, "inspect", rb_struct_s_inspect, 0);
len = RARRAY_LEN(members);
Expand Down Expand Up @@ -449,7 +440,7 @@ rb_struct_define(const char *name, ...)

if (!name) st = anonymous_struct(rb_cStruct);
else st = new_struct(rb_str_new2(name), rb_cStruct);
return setup_struct(st, ary, 0);
return setup_struct(st, ary);
}

VALUE
Expand All @@ -462,7 +453,7 @@ rb_struct_define_under(VALUE outer, const char *name, ...)
ary = struct_make_members_list(ar);
va_end(ar);

return setup_struct(rb_define_class_under(outer, name, rb_cStruct), ary, 0);
return setup_struct(rb_define_class_under(outer, name, rb_cStruct), ary);
}

/*
Expand Down Expand Up @@ -581,7 +572,7 @@ rb_struct_s_def(int argc, VALUE *argv, VALUE klass)
else {
st = new_struct(name, klass);
}
setup_struct(st, rest, (int)keyword_init);
setup_struct(st, rest);
rb_ivar_set(st, id_keyword_init, keyword_init);
if (rb_block_given_p()) {
rb_mod_module_eval(0, 0, st);
Expand Down
18 changes: 14 additions & 4 deletions test/ruby/test_struct.rb
Expand Up @@ -118,10 +118,9 @@ def test_struct_new_with_keyword_init
assert_equal "#{@Struct}::KeywordInitFalse", @Struct::KeywordInitFalse.inspect
assert_equal "#{@Struct}::KeywordInitTrue(keyword_init: true)", @Struct::KeywordInitTrue.inspect
# eval is needed to prevent the warning duplication filter
k = eval("Class.new(@Struct::KeywordInitFalse) {def initialize(**) end}")
assert_raise(ArgumentError) { k.new(a: 1, b: 2) }
k = Class.new(@Struct::KeywordInitTrue) {def initialize(**) end}
assert_warn('') {k.new(a: 1, b: 2)}
k = Class.new(@Struct::KeywordInitTrue) {def initialize(b, options); super(a: options, b: b); end}
o = assert_warn('') { k.new(42, {foo: 1, bar: 2}) }
assert_equal(1, o.a[:foo])

@Struct.instance_eval do
remove_const(:KeywordInitTrue)
Expand Down Expand Up @@ -150,6 +149,17 @@ def initialize(a, b)
assert_equal 3, klass.new(1,2).total
end

def test_initialize_with_kw
klass = @Struct.new(:foo, :options) do
def initialize(foo, **options)
super(foo, options)
end
end
assert_equal({}, klass.new(42, **Hash.new).options)
x = assert_warn('') { klass.new(1, bar: 2) }
assert_equal 2, x.options[:bar]
end

def test_each
klass = @Struct.new(:a, :b)
o = klass.new(1, 2)
Expand Down

0 comments on commit adf709a

Please sign in to comment.