Project

General

Profile

Actions

Feature #1125

closed

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

Added by mernen (Daniel Luz) over 15 years ago. Updated over 12 years ago.

Status:
Closed
Target version:
[ruby-core:21901]

Description

=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


Related issues 1 (0 open1 closed)

Has duplicate Backport193 - Backport #5124: foo = [*bar] implies foo.equal?(bar)Closednaruse (Yui NARUSE)07/31/2011Actions
Actions #1

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

Actions #2

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
=end

Actions #3

Updated by mame (Yusuke Endoh) over 14 years ago

  • Target version set to 2.0.0

=begin

=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

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 :

Isn't the flag of splatarray for this purpose?

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

--
Yusuke Endoh

Actions #7

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

Also available in: Atom PDF

Like0
Like0Like0Like0Like0Like0Like0Like0