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

Fix is-lazy propagation bug in List::Reifier #2493

Closed
wants to merge 3 commits into from

Conversation

jstuder-gh
Copy link
Contributor

Triangle reduce was not recognizing Lists containing lazy Slips as lazy.
This can be tracked back to the is-lazy method on List::Reifier (and was
ultimately being propagated up to the triangle reduce routine). is-lazy
in List::Reifier was not considering the possibility of lazy Slips. Fix
this by having the method check any $!future elements it may have for
laziness.

See Github Issue #2122

Triangle reduce was not recognizing Lists containing lazy Slips as lazy.
This can be tracked back to the is-lazy method on List::Reifier (and was
ultimately being propagated up to the triangle reduce routine). is-lazy
in List::Reifier was not considering the possibility of lazy Slips. Fix
this by having the method check any $!future elements it may have for
laziness.

See [Github Issue rakudo#2122](rakudo#2122)
Make is-lazy only check the is-lazy values of Slips in the $!future.
The way it was before, any lazy element following a Slip would cause the
result to be lazy, which is not quite right.

    my @A = [\+] (0, |(1, 2), (3, 4 ... *));
    # RESULT: @A would be lazy, but shouldn't be
@jstuder-gh
Copy link
Contributor Author

Pushed up one more commit. The previous one was a bit too overzealous in declaring the resulting List lazy. From the commit message:

Make is-lazy only check the is-lazy values of Slips in the $!future.
The way it was before, any lazy element following a Slip would cause the
result to be lazy, which is not quite right.

my @a = [\+] (0, |(1, 2), (3, 4 ... *));
# RESULT: @a would be lazy, but shouldn't be

lizmat added a commit that referenced this pull request Mar 2, 2019
To fix at least laziness check on [\+]  Jeremy Studer++ for PR
@lizmat
Copy link
Contributor

lizmat commented Mar 2, 2019

Thank you for your work. Since there was no reply to my comment and we're 3 months later now, I decided to take the idea and re-imagine it with 8f424c9.

@lizmat lizmat closed this Mar 2, 2019
Kaiepi pushed a commit to Kaiepi/rakudo that referenced this pull request Mar 7, 2019
To fix at least laziness check on [\+]  Jeremy Studer++ for PR
Kaiepi pushed a commit to Kaiepi/rakudo that referenced this pull request Mar 10, 2019
To fix at least laziness check on [\+]  Jeremy Studer++ for PR
lizmat added a commit that referenced this pull request Jun 1, 2019
This reverts commit 8f424c9.

Not seeing a way to fix this atm, so reverting and re-opening the
pull request on which this was based (as well as the issue that
caused this).
@lizmat
Copy link
Contributor

lizmat commented Jun 1, 2019

Reverted my version of the PR as it caused a regression: #2920 .

@lizmat lizmat reopened this Jun 1, 2019
Kaiepi pushed a commit to Kaiepi/rakudo that referenced this pull request Jun 5, 2019
This reverts commit 8f424c9.

Not seeing a way to fix this atm, so reverting and re-opening the
pull request on which this was based (as well as the issue that
caused this).
@JJ
Copy link
Collaborator

JJ commented May 4, 2020

Can you please fix the conflict and continue here?

@lizmat
Copy link
Contributor

lizmat commented Jan 5, 2022

Closing this PR as my re-imagining didn't work out, and this PR didn't get its conflicts solved.

@lizmat lizmat closed this Jan 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants