Repository navigation
PyCompile_OpcodeStackEffect() and dis.stack_effect() are not particularly useful #76636
Description
| BPO | 32455 |
|---|---|
| Nosy | @ncoghlan, @vstinner, @larryhastings, @benjaminp, @ericsnowcurrently, @serhiy-storchaka, @godaygo |
| PRs | |
| Dependencies |
Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.
Show more details
GitHub fields:
assignee = 'https://github.com/serhiy-storchaka'
closed_at = <Date 2018-09-18.07:00:26.286>
created_at = <Date 2017-12-30.14:05:46.138>
labels = ['interpreter-core', '3.8', 'type-feature', 'library']
title = 'PyCompile_OpcodeStackEffect() and dis.stack_effect() are not particularly useful'
updated_at = <Date 2018-09-18.07:00:26.285>
user = 'https://github.com/serhiy-storchaka'bugs.python.org fields:
activity = <Date 2018-09-18.07:00:26.285>
actor = 'serhiy.storchaka'
assignee = 'serhiy.storchaka'
closed = True
closed_date = <Date 2018-09-18.07:00:26.286>
closer = 'serhiy.storchaka'
components = ['Interpreter Core', 'Library (Lib)']
creation = <Date 2017-12-30.14:05:46.138>
creator = 'serhiy.storchaka'
dependencies = ['24340']
files = []
hgrepos = []
issue_num = 32455
keywords = ['patch']
message_count = 10.0
messages = ['309232', '309427', '309448', '309450', '309452', '309453', '315786', '315812', '315814', '325619']
nosy_count = 7.0
nosy_names = ['ncoghlan', 'vstinner', 'larry', 'benjamin.peterson', 'eric.snow', 'serhiy.storchaka', 'godaygo']
pr_nums = ['6610']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue32455'
versions = ['Python 3.8']
Activity
serhiy-storchaka commentedon Dec 30, 2017
The information provided by PyCompile_OpcodeStackEffect() and dis.stack_effect() (added in bpo-19722) is not enough for practical use.
Some opcodes (like JUMP_IF_TRUE_OR_POP or FOR_ITER) have different stack effect when when execution is continued from the following opcode or jumped to the address specified by oparg. Thus there are two different values for stack effect of these opcodes.
Some opcodes (RETURN_VALUE, RAISE_VARARGS) stops execution. Their stack effect doesn't have meaning, and the following opcodes should be ignored.
Some opcodes (JUMP_ABSOLUTE, JUMP_RELATIVE) always jump. The opcodes following them should be ignored.
Some opcodes (like SETUP_FINALLY or SETUP_WITH) need additional space on the stack for raised exceptions. Their stack effect is virtual. Its value can't be used for calculating the actual position on the stack. This isn't documented.
The possible solution is to add a boolean flag for distinguishing the stack effects in case of consequent execution and jumping. Return a special sentinel or raise an exception if the opcode doesn't pass execution in this direction.
serhiy-storchaka commentedon Jan 3, 2018
After resolving bpo-24340 I'm going to add PyCompile_OpcodeStackEffect() that takes an additional integer argument, and add an optional tristate argument to dis.stack_effect(). If it is True, return the effect when jump, if it is false, return the effect when not jump, when omitted or None, return the maximal effect.
benjaminp commentedon Jan 3, 2018
What was the rationale for exposing these interfaces in the first place? If they're not useful, I'd rather deprecate and remove them.
larryhastings commentedon Jan 4, 2018
The rationale: without this information, it is impossible for anybody else to write a bytecode compiler / assembler, because when you create a code object you have to specify its stack depth. I used this information for my "maynard" bytecode assembler / disassembler.
That said, I'm not sure who needs these super-fancy versions Serhiy is proposing. To calculate stack depth, all you really need is the *maximum* stack depth per instruction. You might slightly over-allocate but it shouldn't really be much of a problem.
Serhiy: what's your use case for all these complicated new features?
serhiy-storchaka commentedon Jan 4, 2018
After adding new opcodes or changing existing opcodes you would need to change the code only in one place.
larryhastings commentedon Jan 4, 2018
Simplify whose code, the caller or the callee? It seems like it's simplifying the life of the callee (PyCompile_OpcodeStackEffectEx) at the expense of pushing complexity into the caller.
What complicated code? Compilers simply call PyCompile_OpcodeStackEffect(), passing in an opcode + oparg pair, and it simply returns the maximum stack effect. No crazy "is it a jump" logic needed. Again: this may mean overallocating the stack. But CPython has behaved like this for twenty years and I'm not convinced it's a problem that needs solving.
Victor, do you want to use Serhiy's proposed API in your library?
serhiy-storchaka commentedon Apr 26, 2018
In 3.7 the stack effect is calculated more accurately by the compiler. PR 6610 exposes this feature to users. It add jump parameter to dis.stack_effect() and to PyCompile_OpcodeStackEffect(). By default the maximal value is returned. Passing jump=True for a non-jumping code or passing jump=False to RETURN_VALUE or JUMP_ABSOLUTE don't raise an exception and don't return a special value. It is up to the caller to pass reliable arguments.
This will make the workaround for stack_effect() in the bytecode project [1] not needed in 3.8.
[1] https://github.com/vstinner/bytecode/pull/29
godaygo commentedon Apr 26, 2018
Sorry if this doesn't fit this issue and needs a separate one.
Since Python switched to 2 byte wordcode, all opcodes which do not imply an argument, technically have it - augmented with 0. So it is convenient to iterate over bytecode like:
op, arg = instruction.
But there is a check in stack_effect that the second argument for this opcodes must be None.
file::_opcode.c
So you need to perform a somewhat _redundant_ check before calling:
Maybe it's normal to relax this condition - be None or 0 for opcode < opcode.HAVE_ARGUMENT?
serhiy-storchaka commentedon Apr 26, 2018
I concur with Kirill and was going to propose this change in a separate issue.
serhiy-storchaka commentedon Sep 18, 2018
New changeset 7bdf282 by Serhiy Storchaka in branch 'master':
bpo-32455: Add jump parameter to dis.stack_effect(). (GH-6610)
7bdf282