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

Issue 1397: Reifiy pending elements in Seq prior to rotor call #1400

Merged
merged 1 commit into from Jan 14, 2018

Conversation

jstuder-gh
Copy link
Contributor

There are instances where a Seq is newly created and its elements have
not been iterated/cached yet. Calling a method like rotor that
anticipates such elements will fail. Add a method in Seq that intercepts
the Seq and reifies any pending elements prior to dispatching it to the
Any rotor method for processing.

Addresses Issue 1397

@jstuder-gh
Copy link
Contributor Author

Passes existing spectest. Will need to write additional tests if accepted.

@zoffixznet
Copy link
Contributor

I don't think we need to cache the Seq. The crash in the ticket is because Rotor iterator violates Iterator protocol and pulls one too many times.

Looks like the previous fix for the same crash did not address the 2 => -1 combination, which evidently uses a different path.

Something similar needs to be applied to around this area

I wrote a test to cover the bug you can use.

Rakudo::Iterator::Rotor was calling .pull-one too many times causing
error. This type of error has been encountered before, on another
code-path involving Rakudo::Iterator::Batch. zoffix++ for pointing
this out.

Addresses [Issue 1397](rakudo#1397)

See commit for more details:
    bcd902a
@jstuder-gh
Copy link
Contributor Author

Caching did resolve the error in this instance, but your way is better and resolves the underlying issue.

I've updated the branch with the new fix.

@zoffixznet zoffixznet merged commit 1ed8f5d into rakudo:master Jan 14, 2018
@zoffixznet
Copy link
Contributor

👍 Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants