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

Update use of private interpreter variables to have underscore suffix #15849

Conversation

pcardune
Copy link
Contributor

A bunch of private interpreter APIs got underscores added to the end of them. This PR updates our code to reference the new names.

Copy link
Contributor

@joshlory joshlory left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change LGTM. Will you be around the day after the merge in case there are any issues? 😛

@pcardune pcardune force-pushed the pcardune-more-codegen-stuff branch from b7fe401 to 2995b7b Compare June 14, 2017 18:34
@pcardune pcardune force-pushed the pcardune-more-interpreter-upgrades branch from ee20bfc to 1e6274c Compare June 14, 2017 18:38
@pcardune
Copy link
Contributor Author

yes, but I'll probably wait until Monday to merge it (and others) anyway.

@pcardune
Copy link
Contributor Author

Ok, this change is not good. While all of our tests pass, the interpreter test suite indicates that the most recent merges from Neil's branch broke continue statements completely. Glad I implemented that test suite!

The break was fixed 19 commits later, so I'm going to try to merge in those 19 commits and hope no other private APIs were changed. We'll see.

This is needed to get a9ae586 (Proper 'this' boxing in non-strict mode.) from
the interpreter to work
@pcardune
Copy link
Contributor Author

ok, now this should be ready to go for reals.

@pcardune pcardune changed the base branch from pcardune-more-codegen-stuff to pcardune-blessed-interpreter-upgrades June 14, 2017 22:52
Copy link
Contributor

@islemaster islemaster left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@pcardune pcardune merged commit cdc1b04 into pcardune-blessed-interpreter-upgrades Jun 15, 2017
@pcardune pcardune deleted the pcardune-more-interpreter-upgrades branch June 15, 2017 18:45
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

3 participants