◐ Shell
reader mode source ↗
Skip to content

bpo-29469: Move constant folding at AST level#2858

Merged
methane merged 20 commits into
python:masterfrom
methane:ast-constant-fold
Dec 14, 2017
Merged

bpo-29469: Move constant folding at AST level#2858
methane merged 20 commits into
python:masterfrom
methane:ast-constant-fold

Conversation

@methane

@methane methane commented Jul 25, 2017 •
edited
Loading

Copy link
Copy Markdown
Member

@methane methane changed the title bpo-29469: Move constant folding at AST level Jul 25, 2017
@methane

methane commented Jul 25, 2017

Copy link
Copy Markdown
Member Author

I'll check all comments in https://bugs.python.org/review/29469/
Please don't review for a while.

@methane methane force-pushed the ast-constant-fold branch from 93677ba to 86c73de Compare July 26, 2017 02:27
@methane methane changed the title bpo-29469: Move constant folding at AST level (wip, don't review) Jul 26, 2017
@rhettinger

rhettinger commented Jul 26, 2017

Copy link
Copy Markdown
Contributor

Overall, this looks great. The code is remarkably clean.

@methane methane force-pushed the ast-constant-fold branch from a776555 to b0b9db9 Compare July 26, 2017 11:49
@methane methane force-pushed the ast-constant-fold branch from b0b9db9 to a603d5b Compare July 26, 2017 16:35
@methane

methane commented Sep 17, 2017

Copy link
Copy Markdown
Member Author

Python/importlib.h is conflicted very often. So I'll fix it right before merge.

@serhiy-storchaka

serhiy-storchaka commented Oct 5, 2017

Copy link
Copy Markdown
Member

See my comment on the tracker: https://bugs.python.org/issue29469#msg297596

@methane methane requested a review from a team as a code owner October 6, 2017 11:59
@serhiy-storchaka serhiy-storchaka removed the request for review from a team October 7, 2017 19:51
@serhiy-storchaka

serhiy-storchaka commented Nov 20, 2017

Copy link
Copy Markdown
Member

Could you please remove asdl_ct.py and simplify ast_opt.c by getting rid of useless functions like astfold_boolop? The AST types are rarely changed, and I think it would be not hard to update the cleaner code. If sometimes we will need the general code generator we could restore it from your patches.

@methane methane requested a review from a team December 12, 2017 13:22
serhiy-storchaka
serhiy-storchaka reviewed Dec 12, 2017
View reviewed changes

@serhiy-storchaka serhiy-storchaka left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Hide comment

It may be worth to document this change in What's New. Even without additional optimizations it can optimize more than the peephole optimizer.

1 hidden conversation Load more…
serhiy-storchaka
serhiy-storchaka approved these changes Dec 13, 2017
View reviewed changes

@serhiy-storchaka serhiy-storchaka left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Hide comment

LGTM if fix a one simple error.

@serhiy-storchaka

serhiy-storchaka commented Dec 13, 2017 •
edited
Loading

Copy link
Copy Markdown
Member

Generated files not up to date.

@methane methane merged commit 7ea143a into python:master Dec 14, 2017
@methane methane deleted the ast-constant-fold branch December 14, 2017 07:47
@serhiy-storchaka

serhiy-storchaka commented Dec 14, 2017

Copy link
Copy Markdown
Member

Great! Many thanks @methane for your work!

Now I'll try to implement additional optimizations.

@s-cork s-cork mentioned this pull request Jul 24, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@serhiy-storchaka serhiy-storchaka serhiy-storchaka approved these changes

@vstinner vstinner

+1 more reviewer

@adrian17 adrian17 adrian17 left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@methane @rhettinger @serhiy-storchaka @adrian17 @the-knights-who-say-ni @bedevere-bot