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

cp0: combine dropping a binding and begin rotation #796

Merged
merged 1 commit into from
Jan 31, 2024

Conversation

mflatt
Copy link
Contributor

@mflatt mflatt commented Jan 24, 2024

A follow-up to c081296, this commit adjusts the cp0 change to prefer an existing case instead of the new one. This order still passes the new test, it passes old ones with a small adjustment, and it passes Racket tests that are similar to "cp0.ms" tests.

@mflatt mflatt marked this pull request as draft January 24, 2024 21:37
@mflatt
Copy link
Contributor Author

mflatt commented Jan 24, 2024

Er... looking one more time, the new example suggests that the updated cp0 order is worse, not better! I'll take this from the top on the Racket test-suite side and figure out where I got confused.

A follow-up to c081296, this commit adjusts the cp0 change to avoid
skipping the variable-dropping rewrite when the `begin` rotation
applies. This combination passes the new test, passes old tests with
small adjustments, and allows Racket to pass some tests that are
similar to "cp0.ms" tests.

Meanwhile, c081296 should have noted the PR (cisco#789) it squashes and
some author information that was lost in the squash:
Co-authored-by: R. Kent Dybvig <[email protected]>
@mflatt mflatt marked this pull request as ready for review January 25, 2024 01:58
@mflatt
Copy link
Contributor Author

mflatt commented Jan 25, 2024

Trying again, this time with @owaddell's repair, a test that reflects the original issue for Racket, and a corrected version of the broken test that showed why my first attempt was bad.

@mflatt mflatt changed the title cp0: prefer dropping a binding to begin rotation cp0: combine dropping a binding and begin rotation Jan 25, 2024
@mflatt mflatt merged commit 822d815 into cisco:main Jan 31, 2024
13 checks passed
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.

2 participants