Project

General

Profile

Actions

Bug #7768

closed

Inherited Array class missing

Added by england (Roman Ivanilov) almost 12 years ago. Updated about 5 years ago.

Status:
Closed
Target version:
-
ruby -v:
1.9
Backport:
[ruby-core:51792]

Description

Hello. I apologize if I missed something.
I found strange behavior in ruby 1.9:

 class Custom < Array; end
 Custom.new(0){|i| i + 1}.uniq.class # => Array
 Custom.new(2){|i| i + 1}.uniq.class # => Custom

while in 1.8 it works just as I expected.

 class Custom < Array; end
 Custom.new(0){|i| i + 1}.uniq.class # => Custom
 Custom.new(2){|i| i + 1}.uniq.class # => Custom
  • it is actual not only for the uniq method.
  • tested with ree-1.8.7-2010.02, ruby-1.9.2-p290, ruby-1.9.3-p375, ruby-1.9.3-p125

Any bug here?


Files

array-uniq-subclass-instance.patch (2.15 KB) array-uniq-subclass-instance.patch jeremyevans0 (Jeremy Evans), 07/15/2019 09:07 PM

Related issues 2 (0 open2 closed)

Related to Ruby master - Bug #7625: Arrayを継承したオブジェクトのcompactがArrayを返すClosedmatz (Yukihiro Matsumoto)12/26/2012Actions
Related to Ruby master - Bug #4136: Enumerable#reject should not inherit the receiver's instance variablesClosed12/09/2010Actions

Updated by Anonymous almost 12 years ago

  • Target version set to 2.0.0
  • Assignee set to Anonymous

=begin
Looks like a regression introduced in r26987
=end

Actions #2

Updated by Anonymous almost 12 years ago

  • Status changed from Open to Closed
  • % Done changed from 0 to 100

This issue was solved with changeset r39004.
Roman, thank you for reporting this issue.
Your contribution to Ruby is greatly appreciated.
May Ruby be with you.


  • array.c (rb_ary_dup): make returned array the same class as the original
    array [Bug #7768] [ruby-core:51792]
  • test/ruby/test_array.rb (class TestArray): add test

Updated by mrkn (Kenta Murata) almost 12 years ago

#7625 is related to this issue.

Updated by Anonymous over 11 years ago

=begin
Summary of my discussion with mame in #ruby-core:

[00:33:08] r30148
[00:34:16] some people complains to your r39004
[00:34:43] because it might be an intentional change by matz at r30148
[00:34:53] http://svn.ruby-lang.org/cgi-bin/viewvc.cgi?revision=30148&view=revision
[00:35:09] * array.c (rb_ary_dup): should copy contents only. no instance
[00:35:09] variable, no class would be copied.
[00:36:04] The point is that don't change behavior without matz's accept
[00:36:09] however, matz is now saying the opposite opinion against himself
[00:36:38] sorry for changing behaviour
[00:36:44] he now prefers the old 1.9.3 behavior
[00:36:59] the bug looked like a regression, but i fixed the wrong place
[00:37:25] oops, 1.9.3 behavior -> 1.9.2 behavior
[00:38:53] indeed rb_ary_dup impacts too extensively a little
[00:39:10] then how do you fix?
[00:39:36] so this was the bug http://bugs.ruby-lang.org/issues/7768
[00:40:25] fix only Array#uniq ?
[00:40:35] mame1: that would be the least intrusive way
[00:41:16] mame1: do you want me to do it?
[00:42:22] could you create a patch for the way?
[00:42:36] sure, one moment
[00:43:25] just confirm: you didn't intend to change the behavior of other methods than Array#uniq, right?
[00:43:53] then, it looks good to me to choose "the least intrusive way"
[00:44:26] mame1: i thought that perhaps there were other methods that would behave inconsistently in some cases like #uniq
[00:44:47] but i will give you a patch that undoes it
[00:45:19] honestly i agree with you to some extent
[00:45:33] but the timing is bad a little
[00:45:39] yes, i agree
[00:45:41] sorry once again
[00:46:07] something to discuss after 2.0 is released maybe?
[00:46:18] no prob, thank you for your cooperation
[00:46:35] no plan :-)
[00:47:12] ah, it may be difficult to change 2.0.0 after 2.0.0-p0 is released
[00:47:38] for next minor, i mean
[00:47:41] 2.0.1 or 2.1.0 will include the change, which matz will decide
[00:47:49] yes
[00:48:29] mame1: https://gist.github.com/charliesome/adeff49e900f6b8a75fc
[00:48:55] the test i added in r39004 still passes
[00:49:11] so i think this is ok if you're happy to change Array#uniq's behaviour
[00:51:31] charliesome: it is really a regression, isn't it?
[00:52:04] mame1: i think so - see http://bugs.ruby-lang.org/projects/ruby-trunk/repository/revisions/26987/diff/array.c
[00:52:19] it looks like an attempt at an optimization
[00:52:51] yeah, i've seen the diff, but i've not checked the other region of code
[00:53:15] mame1: maybe the safest option is to revert r39004 completely?
[00:53:38] it returns a new object whose class is the same of the argument, say, when its length is >= 2?
[00:53:45] mame1: yes
[00:54:22] hmm, thanks. let me consider for a minute :-)
[00:54:28] no problem
[01:15:14] charliesome: sorry for let you wait
[01:15:25] i decided that i'll release rc2 with the current behavior
[01:15:45] mame1: current as in trunk or current 1.9.3?
[01:16:00] because matz again prefered not only Array#uniq but also other Array methods to return the original class rather than Array
[01:16:04] current trunk
[01:16:35] iow your r39004 is accepted :-)
=end

Updated by shugo (Shugo Maeda) over 11 years ago

  • Status changed from Closed to Open
  • Assignee changed from Anonymous to mame (Yusuke Endoh)

I believe r39004 should be reverted.

Matz said "If a method is originally defined in Enumerable, i.e. its return value (Array) is a collection of values from enumerable." at http://bugs.ruby-lang.org/issues/4136#note-7.
However, Array#sort returns an instance of a subclass of Array, by r39004.

$ ./ruby -ve 'class Foo < Array; end; p Foo[2,1,3].sort.class'
ruby 2.0.0dev (2013-02-08 trunk 39154) [i686-linux]
Foo

I'm not sure Matz is right. What should Array#uniq return if Enumerable#uniq is added in the future?

Anyway, there is no enough time to discuss details, so r39004 should be reverted.
Haste makes waste.
If this issue is regarded as a bug, not as a spec change, it can be fixed after the release of 2.0.0.

Updated by usa (Usaku NAKAMURA) over 11 years ago

  • Status changed from Open to Assigned

Stated as the maintainer of 1.9.3, +1 to shugo.

Updated by mame (Yusuke Endoh) over 11 years ago

@shugo (Shugo Maeda)
Okay, I agree with reverting r39004. Sorry for my poor decision.

@charliesome
Sorry for confusing you. Please commit it again after I create ruby_2_0_0 branch.

--
Yusuke Endoh

Actions #8

Updated by usa (Usaku NAKAMURA) over 11 years ago

  • Status changed from Assigned to Closed

This issue was solved with changeset r39157.
Roman, thank you for reporting this issue.
Your contribution to Ruby is greatly appreciated.
May Ruby be with you.


  • array.c (rb_ary_dup): reverted r39004. see [Bug #7768], and the
    release manager finailly decided to revert it.

Updated by usa (Usaku NAKAMURA) over 11 years ago

  • Status changed from Closed to Assigned
  • Target version changed from 2.0.0 to 2.6

Updated by mame (Yusuke Endoh) over 11 years ago

  • Assignee changed from mame (Yusuke Endoh) to Anonymous

@usa (Usaku NAKAMURA)
Thank you!

@charliesome
I think that it is a good idea to fix only Array#uniq first.
Then, if you want to change other Array methods, please ask matz.

--
Yusuke Endoh

Updated by blazeeboy (Emad Elsaid) about 9 years ago

Charlie Somerville wrote:

=begin
Looks like a regression introduced in r26987
=end

This Bug stil exists in my current ruby version

ruby 2.2.0p0 (2014-12-25 revision 49005) [x86_64-darwin14]

and had to make this workaround in my custom class

class Custom < Array
def uniq
return self if empty?
super
end
end

Updated by jeremyevans0 (Jeremy Evans) over 5 years ago

mame (Yusuke Endoh) wrote:

I think that it is a good idea to fix only Array#uniq first.

Looks like Array#uniq was never fixed. It still returns Array instance if length <= 1 and subclass instance otherwise. Attached is a patch that fixes this issue.

Updated by ko1 (Koichi Sasada) over 5 years ago

  • Description updated (diff)
  • Assignee changed from Anonymous to matz (Yukihiro Matsumoto)

Updated by matz (Yukihiro Matsumoto) about 5 years ago

Array#uniq should be fixed.

Matz.

Actions

Also available in: Atom PDF

Like0
Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0