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

Less dishonest COME_FROMs in 3.6+ code #237

Merged
merged 4 commits into from
May 14, 2019
Merged

Less dishonest COME_FROMs in 3.6+ code #237

merged 4 commits into from
May 14, 2019

Conversation

rocky
Copy link
Owner

@rocky rocky commented May 14, 2019

Addresses all of problems seen in 3.7 datetime.py

See also Issue #235

@rocky rocky requested a review from x0ret May 14, 2019 03:05
Copy link
Owner Author

@rocky rocky left a comment

Choose a reason for hiding this comment

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

This change is causing many some failures to the 3.7.3 library. But I wonder if still shouldn't go forward and fix up the grammar more. What do you think?

Addresses all of the problems seen in 3.7 datetime.py.

However we limit COME_FROMs only to forward jumps, not back (which in the case of Python code right now means looping) jumps.
@rocky rocky force-pushed the for-iter-come-from branch from bea9a6a to 44e1288 Compare May 14, 2019 03:17
@rocky rocky changed the title Honest COME_FROMs can get added before FOR_ITER Less dishonest COME_FROMs in 3.6+ code May 14, 2019
@x0ret
Copy link
Collaborator

x0ret commented May 14, 2019

Thank you, i think we can fix related issues in a reasonable time.
however i started reading python-control-flow. Maybe we can have a schedule to embed into this project.

I will check again the library and continue.

@@ -33,6 +33,10 @@ def p_36misc(self, args):
"""
sstmt ::= sstmt RETURN_LAST

cf_for_iter ::= _come_froms FOR_ITER
Copy link
Owner Author

Choose a reason for hiding this comment

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

Oops - this isn't needed (or used) anymore. The decision (for now) is not to add COME_FROMS to backward (in current Python code generation that means looping) jumps.

@rocky
Copy link
Owner Author

rocky commented May 14, 2019

I have thought a lot about using python-control-flow. It would require a very serious effort to get code as good as we have now - Python is complicated in its control flow. However it would revolutionize this project and put it a much better place for Python 3.7, 3.8 and beyond. And we could get rid of a lot of hacky code and things would be more intelligible.

A strategy for rolling this in is to do it in a branch and just try one of the more recent Pythons on it. After that works expand to other Pythons.

Another approach is to create an entirely new project that just has code for one Python that uses this. And after that works well expand it.

Your thoughts?

@x0ret
Copy link
Collaborator

x0ret commented May 14, 2019

A strategy for rolling this in is to do it in a branch and just try one of the more recent Pythons on it. After that works expand to other Pythons.

To be honest, I do prefer to have this progress in this project in the future : ) , since you do a lot of work on it. However i think isolating was always a good option for me. Maybe we can start new project for as you said (python 3.8?) and see the progress to make final decision.

I'm trying to adjust myself to python-control-flow as soon as possible.

I really appreciate your efforts in this project. This is so promising.

@x0ret
Copy link
Collaborator

x0ret commented May 14, 2019

@rocky, i checked half of 3.7 lib issues, and it seems most of them are related to chained comparison, So do we continue in this PR?

EDIT: Ok, i get it;

@rocky
Copy link
Owner Author

rocky commented May 14, 2019

@x0ret I am going to stop working on this for today. I have too much other work. If you could get this to pass the test, I think we could have most of the 3.7 issues behind us.

After that, maybe a release. And then focus more on control flow better way.

@rocky
Copy link
Owner Author

rocky commented May 14, 2019

Thanks! Let's do it and move on..

@rocky rocky merged commit 2614093 into master May 14, 2019
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