Skip to content

gh-87092: move CFG related code from compile.c to flowgraph.c - #103021

Merged
iritkatriel merged 15 commits into
python:mainfrom
iritkatriel:cfg.c
Mar 31, 2023
Merged

gh-87092: move CFG related code from compile.c to flowgraph.c#103021
iritkatriel merged 15 commits into
python:mainfrom
iritkatriel:cfg.c

Conversation

@iritkatriel

@iritkatrieliritkatriel commented Mar 24, 2023•
edited by bedevere-bot
Loading

Copy link
Copy Markdown
Member

@bedevere-botbedevere-bot mentioned this pull request Mar 24, 2023
Closed
@iritkatrieliritkatriel added skip news interpreter-core (Objects, Python, Grammar, and Parser dirs) labels Mar 24, 2023
@iritkatriel
iritkatriel requested a review from a team as a code ownerMarch 25, 2023 18:48
@iritkatrieliritkatriel added the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Mar 25, 2023
@bedevere-bot

bedevere-bot commented Mar 25, 2023

Copy link
Copy Markdown

🤖 New build scheduled with the buildbot fleet by @iritkatriel for commit abaeeac 🤖

If you want to schedule another build, you need to add the 🔨 test-with-buildbots label again.

@bedevere-botbedevere-bot removed the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Mar 25, 2023
@iritkatrieliritkatriel changed the title gh-87092: move CFG related code from compile.c to cfg.cgh-87092: move CFG related code from compile.c to cfg_opt.cMar 27, 2023
@iritkatrieliritkatriel changed the title gh-87092: move CFG related code from compile.c to cfg_opt.cgh-87092: move CFG related code from compile.c to flowgraph.cMar 29, 2023
markshannon
markshannon requested changes Mar 31, 2023
View reviewed changes

@markshannonmarkshannon 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.

A bit of duplicated code, otherwise looks fine.

I don't think _PyOpcode_RelativeJump is needed any more, as all jumps are relative.

Comment threadInclude/internal/pycore_compile.h Outdated
Comment threadInclude/internal/pycore_opcode_utils.h
Comment threadInclude/internal/pycore_opcode_utils.h Outdated
@bedevere-bot

bedevere-bot commented Mar 31, 2023

Copy link
Copy Markdown

When you're done making the requested changes, leave the comment: I have made the requested changes; please review again.

And if you don't make the requested changes, you will be put in the comfy chair!

@iritkatriel

iritkatriel commented Mar 31, 2023

Copy link
Copy Markdown
MemberAuthor

I don't think _PyOpcode_RelativeJump is needed any more, as all jumps are relative.

I agree, but I'd like to do it in a separate PR because I would need to remove usages and the diff here is large enough.

@iritkatriel

iritkatriel commented Mar 31, 2023

Copy link
Copy Markdown
MemberAuthor

I have made the requested changes; please review again.

@bedevere-bot

bedevere-bot commented Mar 31, 2023

Copy link
Copy Markdown

Thanks for making the requested changes!

@markshannon: please review the changes made to this pull request.

@iritkatriel
iritkatriel merged commit 80163e1 into python:mainMar 31, 2023
@iritkatriel
iritkatriel deleted the cfg.c branch April 3, 2023 17:41
warsaw pushed a commit to warsaw/cpython that referenced this pull request Apr 11, 2023
@iritkatriel@warsaw
pythongh-87092: move CFG related code from compile.c to flowgraph.c (p…
68bbf79
…ython#103021)
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@markshannonmarkshannonmarkshannon approved these changes

Assignees

No one assigned

Labels

interpreter-core(Objects, Python, Grammar, and Parser dirs)skip news

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@iritkatriel@bedevere-bot@markshannon