Bug #7768

Inherited Array class missing

Added by Roman Ivanilov about 1 year ago. Updated about 1 year ago.

[ruby-core:51792]
Status:Assigned
Priority:Normal
Assignee:Charlie Somerville
Category:-
Target version:next minor
ruby -v:1.9 Backport:

Description

=begin
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?

=end


Related issues

Related to ruby-trunk - Bug #7625: Arrayを継承したオブジェクトのcompactがArrayを返す Closed 12/26/2012
Related to ruby-trunk - Bug #4136: Enumerable#reject should not inherit the receiver's insta... Closed 12/09/2010

Associated revisions

Revision 39004
Added by Charlie Somerville about 1 year ago

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

Revision 39157
Added by Usaku NAKAMURA about 1 year ago

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

History

#1 Updated by Charlie Somerville about 1 year ago

  • Assignee set to Charlie Somerville
  • Target version set to 2.0.0

=begin
Looks like a regression introduced in r26987
=end

#2 Updated by Charlie Somerville about 1 year 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 (rbarydup): make returned array the same class as the original array [Bug #7768]
  • test/ruby/test_array.rb (class TestArray): add test

#3 Updated by Kenta Murata about 1 year ago

#7625 is related to this issue.

#4 Updated by Charlie Somerville about 1 year 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 (rbarydup): 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 rbarydup 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

#5 Updated by Shugo Maeda about 1 year ago

  • Status changed from Closed to Open
  • Assignee changed from Charlie Somerville to 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.

#6 Updated by Usaku NAKAMURA about 1 year ago

  • Status changed from Open to Assigned

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

#7 Updated by Yusuke Endoh about 1 year ago

@shugo
Okay, I agree with reverting r39004. Sorry for my poor decision.

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

Yusuke Endoh mame@tsg.ne.jp

#8 Updated by Usaku NAKAMURA about 1 year 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 (rbarydup): reverted r39004. see [Bug #7768], and the release manager finailly decided to revert it.

#9 Updated by Usaku NAKAMURA about 1 year ago

  • Status changed from Closed to Assigned
  • Target version changed from 2.0.0 to next minor

#10 Updated by Yusuke Endoh about 1 year ago

  • Assignee changed from Yusuke Endoh to Charlie Somerville

@usa
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 mame@tsg.ne.jp

Also available in: Atom PDF