Skip to content

Adds Python3.8 support #24

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 12 commits into
base: master
Choose a base branch
from
1 change: 1 addition & 0 deletions .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ python:
- "3.5"
- "3.6"
- "3.7"
- "3.8"
- "pypy"
- "pypy3"

Expand Down
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
[![Pypi Entry](https://badge.fury.io/py/goto-statement.svg)](https://pypi.python.org/pypi/goto-statement)

A function decorator to use `goto` in Python.
Tested on Python 2.6 through 3.7 and PyPy.
Tested on Python 2.6 through 3.8 and PyPy.

[![](https://imgs.xkcd.com/comics/goto.png)](https://xkcd.com/292/)

Expand Down
77 changes: 62 additions & 15 deletions goto.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@ def __init__(self):
self.have_argument = dis.HAVE_ARGUMENT
self.jump_unit = 1

self.has_loop_blocks = 'SETUP_LOOP' in dis.opmap
Copy link
Owner

Choose a reason for hiding this comment

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

I'm wondering if we should rather check for 'SETUP_LOOP' in dis.opmap in place than aliasing the result of this check here? It doesn't seem to save much writing, neither does it seem to improve performance.

Copy link
Author

Choose a reason for hiding this comment

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

I think it's cleaner, and it's far from a sole occurrence - see the later PRs which add more such checks, some (I think) used more than once.


@property
def argument_bits(self):
return self.argument.size * 8
Expand All @@ -43,20 +45,23 @@ def argument_bits(self):


def _make_code(code, codestring):
args = [
code.co_argcount, code.co_nlocals, code.co_stacksize,
code.co_flags, codestring, code.co_consts,
code.co_names, code.co_varnames, code.co_filename,
code.co_name, code.co_firstlineno, code.co_lnotab,
code.co_freevars, code.co_cellvars
]

try:
args.insert(1, code.co_kwonlyargcount) # PY3
except AttributeError:
pass
return code.replace(co_code=codestring) # new in 3.8+
except:
Copy link
Owner

Choose a reason for hiding this comment

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

Please no bare expects, this should be expect AttributeError.

Copy link
Author

Choose a reason for hiding this comment

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

True, I'll fix

args = [
code.co_argcount, code.co_nlocals, code.co_stacksize,
code.co_flags, codestring, code.co_consts,
code.co_names, code.co_varnames, code.co_filename,
code.co_name, code.co_firstlineno, code.co_lnotab,
code.co_freevars, code.co_cellvars
]

return types.CodeType(*args)
try:
args.insert(1, code.co_kwonlyargcount) # PY3
except AttributeError:
pass

return types.CodeType(*args)


def _parse_instructions(code):
Expand Down Expand Up @@ -86,6 +91,28 @@ def _parse_instructions(code):
extended_arg_offset = None
yield (dis.opname[opcode], oparg, offset)

def _get_instruction_size(opname, oparg=0):
size = 1

extended_arg = oparg >> _BYTECODE.argument_bits
if extended_arg != 0:
size += _get_instruction_size('EXTENDED_ARG', extended_arg)
oparg &= (1 << _BYTECODE.argument_bits) - 1

opcode = dis.opmap[opname]
if opcode >= _BYTECODE.have_argument:
size += _BYTECODE.argument.size

return size

def _get_instructions_size(ops):
Copy link
Owner

Choose a reason for hiding this comment

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

It seems you did a mistake during merge. This function, and the one above are now defined twice, see below.

Copy link
Author

Choose a reason for hiding this comment

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

Yeah, merge failure. I see you added flake8 which catches this, I'll fix it (in the other PR).

size = 0
for op in ops:
if isinstance(op, str):
size += _get_instruction_size(op)
else:
size += _get_instruction_size(*op)
return size

def _get_instruction_size(opname, oparg=0):
size = 1
Expand Down Expand Up @@ -128,6 +155,13 @@ def _write_instruction(buf, pos, opname, oparg=0):

return pos

def _write_instructions(buf, pos, ops):
for op in ops:
if isinstance(op, str):
pos = _write_instruction(buf, pos, op)
else:
pos = _write_instruction(buf, pos, *op)
return pos

def _write_instructions(buf, pos, ops):
for op in ops:
Expand All @@ -144,6 +178,7 @@ def _find_labels_and_gotos(code):

block_stack = []
block_counter = 0
block_exits = []

opname1 = oparg1 = offset1 = None
opname2 = oparg2 = offset2 = None
Expand All @@ -170,9 +205,16 @@ def _find_labels_and_gotos(code):
'SETUP_EXCEPT', 'SETUP_FINALLY',
'SETUP_WITH', 'SETUP_ASYNC_WITH'):
block_counter += 1
block_stack.append(block_counter)
block_stack.append((opname1, block_counter))
elif not _BYTECODE.has_loop_blocks and opname1 == 'FOR_ITER':
block_counter += 1
block_stack.append((opname1, block_counter))
block_exits.append(offset1 + oparg1)
elif opname1 == 'POP_BLOCK' and block_stack:
block_stack.pop()
elif block_exits and offset1 == block_exits[-1] and block_stack:
block_stack.pop()
block_exits.pop()

opname1, oparg1, offset1 = opname2, oparg2, offset2
opname2, oparg2, offset2 = opname3, oparg3, offset3
Expand Down Expand Up @@ -206,8 +248,13 @@ def _patch_code(code):
raise SyntaxError('Jump into different block')

ops = []
for i in range(len(origin_stack) - target_depth):
ops.append('POP_BLOCK')

for block, _ in origin_stack[target_depth:]:
if block == 'FOR_ITER':
ops.append('POP_TOP')
else:
ops.append('POP_BLOCK')

ops.append(('JUMP_ABSOLUTE', target // _BYTECODE.jump_unit))

if pos + _get_instructions_size(ops) > end:
Expand Down
14 changes: 12 additions & 2 deletions test_goto.py
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,18 @@ def func():
assert func() == 0


def test_jump_out_of_loop_and_survive():
Copy link
Owner

Choose a reason for hiding this comment

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

I might (likely) miss something here, but how is this new test related to adding support for Python 3.8?

@with_goto
def func():
for i in range(10):
for j in range(10):
goto .end
label .end
return (i, j)

assert func() == (9, 0)


def test_jump_into_loop():
def func():
for i in range(10):
Expand Down Expand Up @@ -118,7 +130,6 @@ def func():

assert func() == (0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0)


def test_jump_across_loops():
def func():
for i in range(10):
Expand All @@ -145,7 +156,6 @@ def func():

assert func() is None


def test_jump_into_try_block():
def func():
try:
Expand Down
2 changes: 1 addition & 1 deletion tox.ini
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
[tox]
envlist = py26,py27,py32,py33,py34,py35,py36,py37,pypy,pypy3,flake8
envlist = py26,py27,py32,py33,py34,py35,py36,py37,py38,pypy,pypy3,flake8
skip_missing_interpreters = true

[testenv]
Expand Down