Feature #1125: [*x] (array consisting only of a splat) does not necessarily return a new array

• Status: Closed
• Assignee: nobu (Nobuyoshi Nakada)
• Target version: 2.0.0

[mernen (Daniel Luz)]

=begin
For [*x], these are basically the possible outcomes:

1. if x is an Array, returns it unmodified.
2. elsif x responds to to_ary (to_a on 1.9.1), invokes that method and returns its result unmodified.
3. 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

[marcandre (Marc-Andre Lafortune)]

• Category set to core
• Status changed from Open to Assigned
• Assignee set to matz (Yukihiro Matsumoto)

=begin

=end

[mame (Yusuke Endoh)]

=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 [e-mail]
=end

[mame (Yusuke Endoh)]

• Target version set to 2.0.0

=begin

=end

[mame (Yusuke Endoh)]

• 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 [e-mail]

[nobu (Nobuyoshi Nakada)]

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

[mame (Yusuke Endoh)]

• Assignee changed from mame (Yusuke Endoh) to nobu (Nobuyoshi Nakada)

Hello,

2012/2/14 Nobuyoshi Nakada [e-mail]:

Isn't the flag of splatarray for this purpose?

I don't know. Please go ahead. You have control!

--
Yusuke Endoh [e-mail]

[nobu (Nobuyoshi Nakada)]

• 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]