https://redmine.ruby-lang.org/
https://redmine.ruby-lang.org/favicon.ico?1711330511
2014-12-15T19:18:41Z
Ruby Issue Tracking System
Ruby master - Bug #10601: Unexpected behaviour changes for Struct class
https://redmine.ruby-lang.org/issues/10601?journal_id=50414
2014-12-15T19:18:41Z
normalperson (Eric Wong)
normalperson@yhbt.net
<ul></ul><p>I was preparing to revert and refix for 2.3, but<br>
then I see nobu working on r48849, r48850, r48851...<br>
I guess nobu is fixing this problem?</p>
Ruby master - Bug #10601: Unexpected behaviour changes for Struct class
https://redmine.ruby-lang.org/issues/10601?journal_id=50415
2014-12-15T19:20:49Z
nobu (Nobuyoshi Nakada)
nobu@ruby-lang.org
<ul></ul><p>What do you think about this?</p>
<pre><code class="diff syntaxhl" data-language="diff"> iseq.c: struct accessors
* iseq.c (rb_method_for_self_aref, rb_method_for_self_aset): call
accessor functions directly, not to be affected by [] and []=
methods. <a href="/issues/10601">[ruby-core:66846]</a> [Bug #10601]
* struct.c (define_aref_method, define_aset_method): ditto.
* vm_insnhelper.c (rb_vm_opt_struct_aref, rb_vm_opt_struct_aset):
direct accessors of Struct.
<span class="err">
</span><span class="gh">diff --git a/iseq.c b/iseq.c
index de30490..8d477c9 100644
</span><span class="gd">--- a/iseq.c
</span><span class="gi">+++ b/iseq.c
</span><span class="p">@@ -565,14 +565,14 @@</span> caller_location(VALUE *path)
}
VALUE
<span class="gd">-rb_method_for_self_aref(VALUE name, VALUE arg)
</span><span class="gi">+rb_method_for_self_aref(VALUE name, VALUE arg, rb_insn_func_t func)
</span> {
<span class="gi">+ rb_control_frame_t *FUNC_FASTCALL(rb_vm_struct_aref_c)(rb_thread_t *, rb_control_frame_t *);
</span> VALUE iseqval = iseq_alloc(rb_cISeq);
rb_iseq_t *iseq;
VALUE path, lineno = caller_location(&path);
VALUE parent = 0;
VALUE misc, locals, params, exception, body, send_arg;
<span class="gd">- int flag = VM_CALL_FCALL | VM_CALL_ARGS_SIMPLE;
</span>
GetISeqPtr(iseqval, iseq);
iseq->self = iseqval;
<span class="p">@@ -589,18 +589,10 @@</span> rb_method_for_self_aref(VALUE name, VALUE arg)
#define ADD(a) rb_ary_push(body, rb_obj_hide(a))
/* def name; self[arg]; end */
ADD(lineno);
<span class="gd">- ADD(rb_ary_new3(1, S(putself)));
</span> ADD(rb_ary_new3(2, S(putobject), arg));
<span class="gd">- /* {:mid=>:[], :flag=>264, :blockptr=>nil, :orig_argc=>1} */
- send_arg = rb_hash_new();
- rb_hash_aset(send_arg, S(mid), ID2SYM(idAREF));
- rb_hash_aset(send_arg, S(flag), INT2FIX(flag));
- rb_hash_aset(send_arg, S(blockptr), Qnil);
- rb_hash_aset(send_arg, S(orig_argc), INT2FIX(1));
-
- /* we do not want opt_aref for struct */
- ADD(rb_ary_new3(2, S(opt_send_without_block), send_arg));
</span><span class="gi">+ send_arg = rb_ary_new3(2, S(opt_call_c_function), (VALUE)func);
+ ADD(send_arg);
</span> ADD(rb_ary_new3(1, S(leave)));
#undef S
#undef ADD
<span class="p">@@ -609,19 +601,19 @@</span> rb_method_for_self_aref(VALUE name, VALUE arg)
cleanup_iseq_build(iseq);
rb_ary_clear(body);
<span class="gi">+ rb_ary_clear(send_arg);
</span>
return iseqval;
}
VALUE
<span class="gd">-rb_method_for_self_aset(VALUE name, VALUE arg)
</span><span class="gi">+rb_method_for_self_aset(VALUE name, VALUE arg, rb_insn_func_t func)
</span> {
VALUE iseqval = iseq_alloc(rb_cISeq);
rb_iseq_t *iseq;
VALUE path, lineno = caller_location(&path);
VALUE parent = 0;
VALUE misc, locals, params, exception, body, send_arg;
<span class="gd">- int flag = VM_CALL_FCALL | VM_CALL_ARGS_SIMPLE;
</span>
GetISeqPtr(iseqval, iseq);
iseq->self = iseqval;
<span class="p">@@ -637,26 +629,16 @@</span> rb_method_for_self_aset(VALUE name, VALUE arg)
locals = rb_obj_hide(rb_ary_new3(1, S(val)));
params = rb_hash_new();
exception = rb_ary_tmp_new(0); /* empty */
<span class="gd">- body = rb_ary_tmp_new(9);
</span><span class="gi">+ body = rb_ary_tmp_new(6);
</span>
rb_hash_aset(params, S(lead_num), INT2FIX(1));
ADD(lineno);
<span class="gd">- ADD(rb_ary_new3(1, S(putnil)));
- ADD(rb_ary_new3(1, S(putself)));
- ADD(rb_ary_new3(2, S(putobject), arg));
</span> ADD(rb_ary_new3(3, S(getlocal), INT2FIX(2), INT2FIX(0)));
<span class="gd">- ADD(rb_ary_new3(2, S(setn), INT2FIX(3)));
-
- /* {:mid=>:[]=, :flag=>264, :blockptr=>nil, :orig_argc=>2} */
- send_arg = rb_hash_new();
- rb_hash_aset(send_arg, S(mid), ID2SYM(idASET));
- rb_hash_aset(send_arg, S(flag), INT2FIX(flag));
- rb_hash_aset(send_arg, S(blockptr), Qnil);
- rb_hash_aset(send_arg, S(orig_argc), INT2FIX(2));
</span><span class="gi">+ ADD(rb_ary_new3(2, S(putobject), arg));
</span>
<span class="gd">- /* we do not want opt_aset for struct */
- ADD(rb_ary_new3(2, S(opt_send_without_block), send_arg));
</span><span class="gi">+ send_arg = rb_ary_new3(2, S(opt_call_c_function), (VALUE)func);
+ ADD(send_arg);
</span>
ADD(rb_ary_new3(1, S(pop)));
ADD(rb_ary_new3(1, S(leave)));
<span class="p">@@ -667,6 +649,7 @@</span> rb_method_for_self_aset(VALUE name, VALUE arg)
cleanup_iseq_build(iseq);
rb_ary_clear(body);
<span class="gi">+ rb_ary_clear(send_arg);
</span>
return iseqval;
}
<span class="gh">diff --git a/struct.c b/struct.c
index 080c0f3..c123318 100644
</span><span class="gd">--- a/struct.c
</span><span class="gi">+++ b/struct.c
</span><span class="p">@@ -13,8 +13,8 @@</span>
#include "vm_core.h"
#include "method.h"
<span class="gd">-VALUE rb_method_for_self_aref(VALUE name, VALUE arg);
-VALUE rb_method_for_self_aset(VALUE name, VALUE arg);
</span><span class="gi">+VALUE rb_method_for_self_aref(VALUE name, VALUE arg, rb_insn_func_t func);
+VALUE rb_method_for_self_aset(VALUE name, VALUE arg, rb_insn_func_t func);
</span>
VALUE rb_cStruct;
static ID id_members;
<span class="p">@@ -175,7 +175,8 @@</span> new_struct(VALUE name, VALUE super)
static void
define_aref_method(VALUE nstr, VALUE name, VALUE off)
{
<span class="gd">- VALUE iseqval = rb_method_for_self_aref(name, off);
</span><span class="gi">+ rb_control_frame_t *FUNC_FASTCALL(rb_vm_opt_struct_aref)(rb_thread_t *, rb_control_frame_t *);
+ VALUE iseqval = rb_method_for_self_aref(name, off, rb_vm_opt_struct_aref);
</span> rb_iseq_t *iseq = DATA_PTR(iseqval);
rb_add_method(nstr, SYM2ID(name), VM_METHOD_TYPE_ISEQ, iseq, NOEX_PUBLIC);
<span class="p">@@ -185,7 +186,8 @@</span> define_aref_method(VALUE nstr, VALUE name, VALUE off)
static void
define_aset_method(VALUE nstr, VALUE name, VALUE off)
{
<span class="gd">- VALUE iseqval = rb_method_for_self_aset(name, off);
</span><span class="gi">+ rb_control_frame_t *FUNC_FASTCALL(rb_vm_opt_struct_aset)(rb_thread_t *, rb_control_frame_t *);
+ VALUE iseqval = rb_method_for_self_aset(name, off, rb_vm_opt_struct_aset);
</span> rb_iseq_t *iseq = DATA_PTR(iseqval);
rb_add_method(nstr, SYM2ID(name), VM_METHOD_TYPE_ISEQ, iseq, NOEX_PUBLIC);
<span class="gh">diff --git a/test/ruby/test_struct.rb b/test/ruby/test_struct.rb
index 8307d79..df859f7 100644
</span><span class="gd">--- a/test/ruby/test_struct.rb
</span><span class="gi">+++ b/test/ruby/test_struct.rb
</span><span class="p">@@ -195,6 +195,39 @@</span> module TestStruct
assert_equal(:foo, o[25])
end
<span class="gi">+ def test_overridden_aset
+ bug10601 = '<a href="/issues/10601">[ruby-core:66846]</a> [Bug #10601]: should not be affected by []= method'
+
+ struct = Class.new(Struct.new(*(:a..:z), :result)) do
+ def []=(*args)
+ raise args.inspect
+ end
+ end
+
+ obj = struct.new
+ assert_nothing_raised(RuntimeError, bug10601) do
+ obj.result = 42
+ end
+ assert_equal(42, obj.result, bug10601)
+ end
+
+ def test_overridden_aref
+ bug10601 = '<a href="/issues/10601">[ruby-core:66846]</a> [Bug #10601]: should not be affected by [] method'
+
+ struct = Class.new(Struct.new(*(:a..:z), :result)) do
+ def [](*args)
+ raise args.inspect
+ end
+ end
+
+ obj = struct.new
+ obj.result = 42
+ result = assert_nothing_raised(RuntimeError, bug10601) do
+ break obj.result
+ end
+ assert_equal(42, result, bug10601)
+ end
+
</span> def test_equal
klass1 = @Struct.new(:a)
klass2 = @Struct.new(:a, :b)
<span class="gh">diff --git a/vm_insnhelper.c b/vm_insnhelper.c
index edb0e06..d5314c7 100644
</span><span class="gd">--- a/vm_insnhelper.c
</span><span class="gi">+++ b/vm_insnhelper.c
</span><span class="p">@@ -2107,3 +2107,16 @@</span> vm_once_clear(VALUE data)
return Qnil;
}
<span class="gi">+rb_control_frame_t *
+FUNC_FASTCALL(rb_vm_opt_struct_aref)(rb_thread_t *th, rb_control_frame_t *reg_cfp)
+{
+ TOPN(0) = rb_struct_aref(GET_SELF(), TOPN(0));
+ return reg_cfp;
+}
+
+rb_control_frame_t *
+FUNC_FASTCALL(rb_vm_opt_struct_aset)(rb_thread_t *th, rb_control_frame_t *reg_cfp)
+{
+ rb_struct_aset(GET_SELF(), TOPN(0), TOPN(1));
+ return reg_cfp;
+}
</span></code></pre>
Ruby master - Bug #10601: Unexpected behaviour changes for Struct class
https://redmine.ruby-lang.org/issues/10601?journal_id=50416
2014-12-15T19:48:41Z
normalperson (Eric Wong)
normalperson@yhbt.net
<ul></ul><p>nobu: thanks, diff looks good and passes tests on my 32-bit x86 system</p>
Ruby master - Bug #10601: Unexpected behaviour changes for Struct class
https://redmine.ruby-lang.org/issues/10601?journal_id=50424
2014-12-16T06:18:44Z
nobu (Nobuyoshi Nakada)
nobu@ruby-lang.org
<ul><li><strong>Status</strong> changed from <i>Open</i> to <i>Closed</i></li><li><strong>% Done</strong> changed from <i>0</i> to <i>100</i></li></ul><p>Applied in changeset r48864.</p>
<hr>
<p>iseq.c: struct accessors</p>
<ul>
<li>iseq.c (rb_method_for_self_aref, rb_method_for_self_aset): call<br>
accessor functions directly, not to be affected by [] and []=<br>
methods. <a href="/issues/10601">[ruby-core:66846]</a> [Bug <a class="issue tracker-1 status-5 priority-4 priority-default closed" title="Bug: Unexpected behaviour changes for Struct class (Closed)" href="https://redmine.ruby-lang.org/issues/10601">#10601</a>]</li>
<li>struct.c (define_aref_method, define_aset_method): ditto.</li>
<li>vm_insnhelper.c (rb_vm_opt_struct_aref, rb_vm_opt_struct_aset):<br>
direct accessors of Struct.</li>
</ul>
Ruby master - Bug #10601: Unexpected behaviour changes for Struct class
https://redmine.ruby-lang.org/issues/10601?journal_id=51116
2015-01-20T02:47:52Z
usa (Usaku NAKAMURA)
usa@garbagecollect.jp
<ul><li><strong>Backport</strong> changed from <i>2.0.0: UNKNOWN, 2.1: UNKNOWN</i> to <i>2.0.0: DONTNEED, 2.1: DONTNEED</i></li></ul>