bpo-41463: Generate information about jumps from 'opcode.py' rather than duplicating it in 'compile.c' by markshannon · Pull Request #21714 · python/cpython
The motivation for this PR is to avoid errors when modifying compile.c.
Setting the i_jabs and i_jrel fields of every instruction is error prone and unnecessary.
| int i_lineno; | ||
| }; | ||
|
|
||
| static int |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| static int | |
| static inline int |
| return (_PyOpcode_RelativeJump[opcode>>5]>>(opcode&31))&1; | ||
| } | ||
|
|
||
| static int |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| static int | |
| static inline int |
| is_relative_jump(struct instr *i) | ||
| { | ||
| int opcode = i->i_opcode; | ||
| return (_PyOpcode_RelativeJump[opcode>>5]>>(opcode&31))&1; |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you define 5 and 31 as globals with a name (using #define or with static variables). That way this will read a bit better
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Look great, I left some minor comments
| int oparg = inst->i_oparg; | ||
| int nextop = i+1 < bb->b_iused ? bb->b_instr[i+1].i_opcode : 0; | ||
| if (inst->i_jabs || inst->i_jrel) { | ||
| if (is_jump(inst)) { |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great!
| static inline int | ||
| is_relative_jump(struct instr *i) | ||
| { | ||
| int opcode = i->i_opcode; |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Apologies for insisting a bit but it still took me a minute to parse the expression to follow how the function is determining if is a relative jump. I would suggest to maybe add a comment here briefly explaining the process as someone with less context will certainly struggle
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
I left a comment, feel free to address it before landing
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perfect!
shihai1991 pushed a commit to shihai1991/cpython that referenced this pull request
…han duplicating it in 'compile.c' (pythonGH-21714) Generate information about jumps from 'opcode.py' rather than duplicate it in 'compile.c'
xzy3 pushed a commit to xzy3/cpython that referenced this pull request
…han duplicating it in 'compile.c' (pythonGH-21714) Generate information about jumps from 'opcode.py' rather than duplicate it in 'compile.c'