Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(267939)

#19722: Expose stack effect calculator to Python

Can't Edit
Can't Publish+Mail
Start Review
Created:
5 years, 9 months ago by larry
Modified:
5 years, 9 months ago
Reviewers:
pitrou
CC:
brett.cannon, Nick Coghlan, larry, devnull_psf.upfronthosting.co.za, eric.snow
Visibility:
Public.

Patch Set 1 #

Total comments: 15

Patch Set 2 #

Patch Set 3 #

Unified diffs Side-by-side diffs Delta from patch set Stats Patch
Doc/library/dis.rst View 1 2 1 chunk +7 lines, -0 lines 0 comments Download
Include/compile.h View 1 2 1 chunk +3 lines, -0 lines 0 comments Download
Lib/opcode.py View 1 2 1 chunk +13 lines, -0 lines 0 comments Download
Lib/test/test__opcode.py View 1 2 1 chunk +22 lines, -0 lines 0 comments Download
Modules/_opcode.c View 1 2 1 chunk +116 lines, -0 lines 0 comments Download
Python/compile.c View 1 2 4 chunks +12 lines, -8 lines 0 comments Download
setup.py View 1 2 1 chunk +2 lines, -0 lines 0 comments Download

Messages

Total messages: 2
AntoinePitrou
I'm a bit mild on a separate module just for this... isn't there an existing ...
5 years, 9 months ago #1
larry
5 years, 9 months ago #2
Honestly, I looked around--I didn't want to go through the
hassle of making a new module, just for this.  But there was
simply no good place.  If you know of one please suggest it!

http://bugs.python.org/review/19722/diff/10050/Doc/library/dis.rst
File Doc/library/dis.rst (right):

http://bugs.python.org/review/19722/diff/10050/Doc/library/dis.rst#newcode223
Doc/library/dis.rst:223: Compute the stack effect of *opcode* with argument
*oparg*.
On 2013/11/22 21:02:23, AntoinePitrou wrote:
> Lacks a versionadded marker.

Done.

http://bugs.python.org/review/19722/diff/10050/Include/compile.h
File Include/compile.h (right):

http://bugs.python.org/review/19722/diff/10050/Include/compile.h#newcode58
Include/compile.h:58: PyAPI_FUNC(int) PyCompile_OpcodeStackEffect(int opcode,
int oparg);
On 2013/11/22 21:02:23, AntoinePitrou wrote:
> It is worthwhile making it a public API? Otherwise, you should prefix those
two
> with an underscore.

If it's interesting enough to expose in Python, I figure
it's interesting enough to expose in C.  I can add the
underscore if someone feels strongly about it.

http://bugs.python.org/review/19722/diff/10050/Lib/opcode.py
File Lib/opcode.py (right):

http://bugs.python.org/review/19722/diff/10050/Lib/opcode.py#newcode16
Lib/opcode.py:16: # so failing to import it is harmless.
On 2013/11/22 21:02:23, AntoinePitrou wrote:
> If it's verse, perhaps you could add some rhymes :-)

Done.

http://bugs.python.org/review/19722/diff/10050/Lib/test/test__opcode.py
File Lib/test/test__opcode.py (right):

http://bugs.python.org/review/19722/diff/10050/Lib/test/test__opcode.py#newco...
Lib/test/test__opcode.py:11:
self.assertEqual(_opcode.stack_effect(dis.opmap['BUILD_SLICE']), -1)
On 2013/11/22 21:02:23, AntoinePitrou wrote:
> Is it a reasonable return value? BUILD_SLICE's stack effect is undefined if
you
> don't specific an oparg.

stack_effect() has a default oparg of 0.  I figured that was
nicer than always requiring an oparg.

http://bugs.python.org/review/19722/diff/10050/Modules/_opcode.c
File Modules/_opcode.c (right):

http://bugs.python.org/review/19722/diff/10050/Modules/_opcode.c#newcode10
Modules/_opcode.c:10: opcode : int
On 2013/11/22 21:02:23, AntoinePitrou wrote:
> Should there be a space before colons? I'd have thought no.

Done.

http://bugs.python.org/review/19722/diff/10050/Modules/_opcode.c#newcode14
Modules/_opcode.c:14: Computes the stack effect of the opcode.
On 2013/11/22 21:02:23, AntoinePitrou wrote:
> s/Computes/Compute/

Done.

http://bugs.python.org/review/19722/diff/10050/Modules/_opcode.c#newcode75
Modules/_opcode.c:75: "_opcode",
On 2013/11/22 21:02:23, AntoinePitrou wrote:
> Bad indentation in this struct.

Done.
Sign in to reply to this message.

RSS Feeds Recent Issues | This issue
This is Rietveld 894c83f36cb7+