Feature #1125
closed[*x] (array consisting only of a splat) does not necessarily return a new array
Description
=begin
For [*x], these are basically the possible outcomes:
- if x is an Array, returns it unmodified.
- elsif x responds to to_ary (to_a on 1.9.1), invokes that method and returns its result unmodified.
- else, returns a new array with x as its only element.
Cases #1 and #2 IMO violate the POLS, as I expected that an array literal would always return a new array. The practical consequence here is that I expected I'd be free to modify it without side effects. (For comparison, "#{x}" always returns a new string)
Simple test case:
x = [1, 2, 3]
[*x] << 4
p x # => [1, 2, 3, 4]
Thus, I propose ensuring these two cases always return new arrays.
One possible solution would be dup'ing the resulting array (I guess that'd have a rather low cost; the third case would result in an unnecessary dup, but at least it's just a single-item array). Another one would be to dumb down the interpreter, making it create a zero-length array and then concat the result of the splat to it.
=end
Updated by marcandre (Marc-Andre Lafortune) about 15 years ago
- Category set to core
- Status changed from Open to Assigned
- Assignee set to matz (Yukihiro Matsumoto)
=begin
=end
Updated by mame (Yusuke Endoh) over 14 years ago
=begin
Hi,
I expected that an array literal would always return a new array.
Agreed, but "bug" is too strong a word for this.
I move this ticket to 1.9.x feature.
--
Yusuke Endoh mame@tsg.ne.jp
=end
Updated by mame (Yusuke Endoh) over 12 years ago
- Assignee changed from matz (Yukihiro Matsumoto) to mame (Yusuke Endoh)
Hello,
I'll commit the following patch unless there is objection:
diff --git a/compile.c b/compile.c
index 32bda52..9ea2a3c 100644
--- a/compile.c
+++ b/compile.c
@@ -4644,6 +4644,8 @@ iseq_compile_each(rb_iseq_t *iseq, LINK_ANCHOR *ret, NODE * node, int poped)
case NODE_SPLAT:{
COMPILE(ret, "splat", node->nd_head);
ADD_INSN1(ret, nd_line(node), splatarray, Qfalse);
-
ADD_INSN1(ret, nd_line(node), newarray, INT2FIX(0));
-
ADD_INSN(ret, nd_line(node), concatarray);
if (poped) {
ADD_INSN(ret, nd_line(node), pop);
--
Yusuke Endoh mame@tsg.ne.jp
Updated by nobu (Nobuyoshi Nakada) over 12 years ago
Hi,
(12/02/13 23:27), Yusuke Endoh wrote:
I'll commit the following patch unless there is objection:
Isn't the flag of splatarray for this purpose?
diff --git i/compile.c w/compile.c
index 32bda52..c8aced3 100644
--- i/compile.c
+++ w/compile.c
@@ -4643,7 +4643,7 @@ iseq_compile_each(rb_iseq_t *iseq, LINK_ANCHOR *ret, NODE * node, int poped)
}
case NODE_SPLAT:{
COMPILE(ret, "splat", node->nd_head);
- ADD_INSN1(ret, nd_line(node), splatarray, Qfalse);
-
ADD_INSN1(ret, nd_line(node), splatarray, Qtrue);
if (poped) {
ADD_INSN(ret, nd_line(node), pop);
diff --git i/insns.def w/insns.def
index 8ec05de..59a98c0 100644
--- i/insns.def
+++ w/insns.def
@@ -533,6 +533,9 @@ splatarray
if (NIL_P(tmp)) {
tmp = rb_ary_new3(1, ary);
} -
else if (RTEST(flag)) {
-
tmp = rb_ary_dup(tmp);
-
}
obj = tmp;
}
diff --git i/test/ruby/test_basicinstructions.rb w/test/ruby/test_basicinstructions.rb
index ff14e4a..fdcd1b0 100644
--- i/test/ruby/test_basicinstructions.rb
+++ w/test/ruby/test_basicinstructions.rb
@@ -665,12 +665,15 @@ class TestBasicInstructions < Test::Unit::TestCase
a = []
assert_equal [], [*a]
assert_equal [1], [1, *a]
-
assert_not_same a, [*a]
a = [2]
assert_equal [2], [*a]
assert_equal [1, 2], [1, *a] -
assert_not_same a, [*a]
a = [2, 3]
assert_equal [2, 3], [*a]
assert_equal [1, 2, 3], [1, *a] -
assert_not_same a, [*a]
a = nil
assert_equal [], [*a]
--
Nobu Nakada
Updated by mame (Yusuke Endoh) over 12 years ago
- Assignee changed from mame (Yusuke Endoh) to nobu (Nobuyoshi Nakada)
Hello,
2012/2/14 Nobuyoshi Nakada nobu@ruby-lang.org:
Isn't the flag of splatarray for this purpose?
I don't know. Please go ahead. You have control!
--
Yusuke Endoh mame@tsg.ne.jp
Updated by nobu (Nobuyoshi Nakada) over 12 years ago
- Status changed from Assigned to Closed
- % Done changed from 0 to 100
This issue was solved with changeset r34633.
Daniel, thank you for reporting this issue.
Your contribution to Ruby is greatly appreciated.
May Ruby be with you.
- insns.def (splatarray): make new array if flag is set.
- compile.c (iseq_compile_each): make new array with
splat. [ruby-core:21901][Feature #1125]