Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Should Enumerator#initialize check frozen status? #368

Closed
wants to merge 1 commit into from

Conversation

kachick
Copy link
Member

@kachick kachick commented Jul 28, 2013

ruby -v #=> ruby 2.0.0p247 (2013-06-27 revision 41674) [i686-linux]

Frozen Enumerator can modify positions.

enum = [1, 2].to_enum.freeze
enum.next #=> 1
enum.rewind
enum.next #=> 1

But it can't replace receiver-object/block on following methods.

  • Enumerator#initialize_copy
  • Enumerator::Lazy#initialize_copy
  • Enumerator::Lazy#initialize
enum = [1, 2].to_enum.freeze
enum.send :initialize_copy, [1, 2]   #=> RuntimeError: can't modify frozen Enumerator

lazy = [1, 2].to_enum.lazy.freeze
lazy.send :initialize_copy, [1, 2]   #=> RuntimeError: can't modify frozen Enumerator::Lazy
lazy.send :initialize, [1, 2], &->{} #=> RuntimeError: can't modify frozen Enumerator::Lazy

Except Enumerator#initialize.

enum = [1, 2].to_enum.freeze
enum.send :initialize, [3, 4] #=> warning: Enumerator.new without a block is deprecated; use Object#to_enum
                              #   <Enumerator: [3, 4]:each>
enum.send :initialize, &->{}  #=> #<Enumerator: #<Enumerator::Generator:0xb7fa7258>:each>

Is this a spec?

* enumerator.c (enumerator_initialize): check frozen status
  after checking arity

* test/ruby/test_enumerator.rb: test for above

* test/ruby/test_lazy_enumerator.rb: ensure current behavior
@knu
Copy link
Member

knu commented Jul 29, 2013

The reason #initialize_copy raises an error on a frozen object is just because of its common behavior (see rb_obj_init_copy()) and it is not because Enumerator explicitly checks the frozenness. Plus, using a private method #initialize to modify an enumerator object is an unsupported operation and kind of abuse.

To wrap up, my take is that Enumerator doesn't have the sense of frozenness, so there is no spec at the moment.

Given that, I'm not sure we would want to decently implement the frozenness in Enumerator because a frozen enumerator, if at all, would have no use. Even #peek could change the state of the object, so all you could do with it is call #size or #inspect. There should be something better to do than pass an unusable object around.

@kachick
Copy link
Member Author

kachick commented Jul 29, 2013

Thank you for sharing that with me!
I quite agree "#initialize to modify an enumerator object is an unsupported operation and kind of abuse".

I felt strange Enumerator#initialize. This is because Regexp#initialize explicitly checking frozen and I don't know when using frozen Regexp.
I guessed every #initialize required checking frozen.

But I'm sure frozen Enumerator has no meanings and the implementation is needless.
Thank you :)

@kachick kachick closed this Jul 29, 2013
@kachick kachick deleted the enum-init-check_frozen branch July 29, 2013 07:34
@knu
Copy link
Member

knu commented Jul 29, 2013

Actually, I didn't mean to reject this. What I wrote above was just an excuse for not eagerly supporting frozenness in Enumerator.
I think it makes much sense to prevent a frozen enumerator from being modified with #initialize, so I'm going to merge this.
Thanks for your report!

knu added a commit that referenced this pull request Jul 29, 2013
* enumerator.c (enumerator_init): Add a frozenness check to
  prevent a frozen Enumerator object from being reinitialized with
  a different enumerable object.  This is the least we should do,
  and more fixes will follow. [Fixes GH-368] Patch by Kenichi
  Kamiya.

git-svn-id: svn+ssh://ci.ruby-lang.org/ruby/trunk@42233 b2dd03c8-39d4-4d8f-98ff-823fe69b080e
@kachick
Copy link
Member Author

kachick commented Jul 29, 2013

I'm happy with the results :)
Thank you!

kachick added a commit to rubinius/rubinius that referenced this pull request Aug 14, 2013
tenderlove pushed a commit to tenderlove/ruby that referenced this pull request Jan 24, 2014
* enumerator.c (enumerator_init): Add a frozenness check to
  prevent a frozen Enumerator object from being reinitialized with
  a different enumerable object.  This is the least we should do,
  and more fixes will follow. [Fixes rubyGH-368] Patch by Kenichi
  Kamiya.

git-svn-id: svn+ssh://ci.ruby-lang.org/ruby/trunk@42233 b2dd03c8-39d4-4d8f-98ff-823fe69b080e
kddnewton pushed a commit to kddnewton/ruby that referenced this pull request Aug 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
2 participants