Add a spec for Enumerator#size of zero - #1394
Open
edgibbs wants to merge 1 commit into
Open
Conversation
`Enumerator#size` returns the size of the enumerator, or nil when the size
cannot be determined without iterating. 'core/enumerator/size_spec.rb' pins an
Integer size (100), nil, and a Proc, but never zero.
The case cannot fail on CRuby, where 0 is truthy and INT2FIX(0) is a nonzero
VALUE distinct from Qnil. It can fail on implementations hosted in a language
where 0 is falsy: Opal returns nil for Enumerator.new(0) {}.size, because
`opal/corelib/enumerator.rb` assigns the size with `arguments[0] || nil` and JS
|| collapses a size of 0 to nil. The neighbouring Enumerator.new(100) example
passes there, so nothing in the suite currently detects this.
Matchers follow the existing examples in the file.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
👋🏾 Found this when doing some work on
garnetjs. After doing some digging verified it was a potential issue and impacted Opal in the same way.Enumerator#sizereturns the size of the enumerator, or nil when the size cannot be determined without iterating. 'core/enumerator/size_spec.rb' pins an Integer size (100), nil, and a Proc, but never zero.The case cannot fail on CRuby, where 0 is truthy and INT2FIX(0) is a nonzero VALUE distinct from Qnil. It can fail on implementations hosted in a language where 0 is falsy: Opal returns nil for Enumerator.new(0) {}.size, because
opal/corelib/enumerator.rbassigns the size witharguments[0] || niland JS || collapses a size of 0 to nil. The neighbouring Enumerator.new(100) example passes there, so nothing in the suite currently detects this.Matchers follow the existing examples in the file.
Reproduction steps
Reproducing the Opal failure (requires Node, since Opal compiles to JS):
The existing
Enumerator.new(100)example passes on Opal. Only the new case will fail.Cause —
opal/corelib/enumerator.rb:33, insideEnumerator#initialize:That's inline JavaScript. 0 || nil evaluates to nil because 0 is falsy in JS, so a known size of zero becomes "size unknown." Nothing downstream can recover it —
#size(line 56) just returns@size: