Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Code after a tree-transformation phase has the wrong indent. #200

Closed
boeddeker opened this issue Dec 31, 2018 · 6 comments
Closed

Code after a tree-transformation phase has the wrong indent. #200

boeddeker opened this issue Dec 31, 2018 · 6 comments

Comments

@boeddeker
Copy link

Description

Code after a "tree-transformation phase" (see #183) has the wrong indent.

How to Reproduce

Toy example code:

def obj(b):
    if b == 1:
        a = 1
    elif b == 2:
        a = 2
    else:
        a = 4
        
    return a

import uncompyle6, io, sys, IPython

print('sys.version_info', sys.version_info)
print('uncompyle6.version.VERSION', uncompyle6.version.VERSION)

version = float('{}.{}'.format(sys.version_info.major, sys.version_info.minor))
s = io.StringIO()
_ = uncompyle6.deparse_code(version, obj.__code__, out=s)
code = s.getvalue()
IPython.display.display(IPython.display.Code(code, language='python'))

Output from my anaconda 3.6:

sys.version_info sys.version_info(major=3, minor=6, micro=3, releaselevel='final', serial=0)
uncompyle6.version.VERSION 3.2.5
if b == 1:
    a = 1
else:
    if b == 2:
        a = 2
    else:
        a = 4
    return a

Output from my python 3.7:

sys.version_info sys.version_info(major=3, minor=7, micro=0, releaselevel='final', serial=0)
uncompyle6.version.VERSION 3.2.5
if b == 1:
    a = 1
else:
    if b == 2:
        a = 2
    else:
        a = 4
    return a

Expected behavior

The return in the last line should not have an indent.

Environment

Uncompyle6: 3.2.5
Python: 3.6.3 and 3.7.0
OS: Ubuntu 16.04
@rocky rocky closed this as completed in d443295 Dec 31, 2018
@rocky
Copy link
Owner

rocky commented Dec 31, 2018 •
edited

Tree transformation hasn''t been done yet, so this is not related to that, except that it might be simplified by that transformation were it to exist.

A more elegant way to handle this would be via the work in https://github.com/rocky/python-control-flow

Want to try integrating that into this project?

@boeddeker
Copy link
Author

You are right. I was only searching for a name for such code (maybe switch is a better name)

Thanks for the fast bugfix, but the problem is not completely solved (see the bottom example and output).

I think fixing bugs in uncompyle6 is more important than making the code beautiful.

Example:

def obj(b):
    if b == 1:
        b = 5
    elif b == 2:
        pass
    else:
        b = 5
    return b

import uncompyle6, io, sys, IPython

print('sys.version_info', sys.version_info)
print('uncompyle6.version.VERSION', uncompyle6.version.VERSION)

version = float('{}.{}'.format(sys.version_info.major, sys.version_info.minor))
s = io.StringIO()
_ = uncompyle6.deparse_code(version, obj.__code__, out=s)
code = s.getvalue()
IPython.display.display(IPython.display.Code(code, language='python'))

Output: (Return indent is again wrong)

sys.version_info sys.version_info(major=3, minor=6, micro=3, releaselevel='final', serial=0)
uncompyle6.version.VERSION 3.2.5
if b == 1:
    b = 5
else:
    if b == 2:
        pass
    else:
        b = 5
    return b

@rocky
Copy link
Owner

rocky commented Jan 4, 2019

From current git sources, I am not seeing the results you report. A possible reason is that you are not using the current git sources.

Compare with the below and especially look for

L.  5:  14-32  ifelsestmt ::= testexpr c_stmts_opt jump_forward_else else_suite _come_froms (20)
Reduce ifelsestmt invalid by check

$ ./bin/uncompyle6 -g /tmp/bug-3.7.pyc 
# uncompyle6 version 3.2.5
# Python bytecode 3.7 (3394)
# Decompiled from: Python 3.7.2 (default, Dec 30 2018, 11:53:09) 
# [GCC 8.2.0]
# Embedded file name: exec
# Size of source mod 2**32: 296 bytes
               expr ::= LOAD_CONST (1)
               ret_expr ::= expr (1)
               assert_expr ::= expr (1)
         2     expr ::= LOAD_CONST (2)
         2     pos_arg ::= expr (2)
               mkfunc ::= LOAD_CONST LOAD_CONST MAKE_FUNCTION_0 (3)
         6     store ::= STORE_NAME (4)
               function_def ::= mkfunc store (4)
               stmt ::= function_def (4)
               sstmt ::= stmt (4)
               stmts ::= sstmt (4)
               START ::= |- stmts (4)
        10     expr ::= LOAD_CONST (6)
        10     ret_expr ::= expr (6)
        10     assert_expr ::= expr (6)
        10-12  get_iter ::= expr GET_ITER (7)
        10     expr ::= get_iter (7)
        10     ret_expr ::= expr (7)
        10     assert_expr ::= expr (7)
        12-14  for_iter ::= GET_ITER FOR_ITER (8)
        16     store ::= STORE_NAME (9)
L. 13:  18     expr ::= LOAD_NAME (10)
L. 13:  18     ret_expr ::= expr (10)
L. 13:  18     assert_expr ::= expr (10)
        20     expr ::= LOAD_NAME (11)
        20     pos_arg ::= expr (11)
L. 13:  18-22  call ::= expr pos_arg CALL_FUNCTION_1 (12)
L. 13:  18     expr ::= call (12)
L. 13:  18     ret_expr ::= expr (12)
L. 13:  18     assert_expr ::= expr (12)
        24     store ::= STORE_NAME (13)
L. 13:  18-24  assign ::= expr store (13)
L. 13:  18     stmt ::= assign (13)
L. 13:  18     _stmts ::= stmt (13)
L. 13:  18     l_stmts ::= _stmts (13)
L. 13:  18     for_block ::= l_stmts (13)
L. 13:  18     l_stmts_opt ::= l_stmts (13)
L. 14:  26     expr ::= LOAD_NAME (14)
L. 14:  26     ret_expr ::= expr (14)
L. 14:  26     assert_expr ::= expr (14)
        28     expr ::= LOAD_CONST (15)
        28     pos_arg ::= expr (15)
L. 14:  26-30  compare_single ::= expr expr COMPARE_OP (16)
L. 14:  26     compare ::= compare_single (16)
L. 14:  26     expr ::= compare (16)
L. 14:  26     ret_expr ::= expr (16)
L. 14:  26     assert_expr ::= expr (16)
        32     jmp_true ::= POP_JUMP_IF_TRUE (17)
L. 14:  26-32  testtrue ::= expr jmp_true (17)
L. 14:  26     testexpr ::= testtrue (17)
L. 14:  26-32  iflaststmtl ::= testexpr \e_c_stmts_opt (17)
L. 14:  26-32  ifstmt ::= testexpr \e__ifstmts_jump (17)
L. 14:  26     lastl_stmt ::= iflaststmtl (17)
L. 14:  26     stmt ::= ifstmt (17)
L. 13:  18-32  l_stmts ::= _stmts lastl_stmt (17)
L. 13:  18-32  _stmts ::= _stmts stmt (17)
L. 13:  18     for_block ::= l_stmts (17)
L. 13:  18     l_stmts_opt ::= l_stmts (17)
L. 13:  18     l_stmts ::= _stmts (17)
        34     expr ::= LOAD_GLOBAL (18)
L. 14:  26-34  assert_expr_or ::= assert_expr jmp_true expr (18)
        34     ret_expr ::= expr (18)
        34     assert_expr ::= expr (18)
L. 14:  26     assert_expr ::= assert_expr_or (18)
        36     expr ::= LOAD_CONST (19)
        36     expr ::= LOAD_CONST (19)
        36     pos_arg ::= expr (19)
        36     pos_arg ::= expr (19)
        34-38  call ::= expr pos_arg CALL_FUNCTION_1 (20)
        34     expr ::= call (20)
L. 14:  26-38  assert_expr_or ::= assert_expr jmp_true expr (20)
        34     ret_expr ::= expr (20)
        34     assert_expr ::= expr (20)
L. 14:  26     assert_expr ::= assert_expr_or (20)
        34-40  raise_stmt1 ::= expr RAISE_VARARGS_1 (21)
        34     stmt ::= raise_stmt1 (21)
        34     _stmts ::= stmt (21)
L. 13:  18-40  _stmts ::= _stmts stmt (21)
        34     c_stmts ::= _stmts (21)
L. 13:  18     l_stmts ::= _stmts (21)
        34     c_stmts_opt ::= c_stmts (21)
L. 13:  18     for_block ::= l_stmts (21)
L. 13:  18     l_stmts_opt ::= l_stmts (21)
L. 14:  26-40  iflaststmtl ::= testexpr c_stmts_opt (21)
        34     _ifstmts_jump ::= c_stmts_opt (21)
L. 14:  26     lastl_stmt ::= iflaststmtl (21)
L. 14:  26-40  ifstmt ::= testexpr _ifstmts_jump (21)
L. 13:  18-40  l_stmts ::= _stmts lastl_stmt (21)
L. 14:  26     stmt ::= ifstmt (21)
        34-42  _ifstmts_jump ::= c_stmts_opt COME_FROM (22)
        42-42  _come_froms ::= \e__come_froms COME_FROM (22)
L. 14:  26-42  ifstmt ::= testexpr _ifstmts_jump (22)
L. 14:  26     stmt ::= ifstmt (22)
L. 13:  18-42  _stmts ::= _stmts stmt (22)
L. 13:  18     l_stmts ::= _stmts (22)
L. 13:  18     for_block ::= l_stmts (22)
L. 13:  18     l_stmts_opt ::= l_stmts (22)
L. 15:  42     expr ::= LOAD_NAME (23)
L. 15:  42     ret_expr ::= expr (23)
L. 15:  42     assert_expr ::= expr (23)
        44     expr ::= LOAD_NAME (24)
        44     pos_arg ::= expr (24)
L. 15:  42-46  compare_single ::= expr expr COMPARE_OP (25)
L. 15:  42     compare ::= compare_single (25)
L. 15:  42     expr ::= compare (25)
L. 15:  42     ret_expr ::= expr (25)
L. 15:  42     assert_expr ::= expr (25)
        48     jmp_true ::= POP_JUMP_IF_TRUE (26)
L. 15:  42-48  testtrue ::= expr jmp_true (26)
L. 15:  42     testexpr ::= testtrue (26)
L. 15:  42-48  iflaststmtl ::= testexpr \e_c_stmts_opt (26)
L. 15:  42-48  ifstmt ::= testexpr \e__ifstmts_jump (26)
L. 15:  42     lastl_stmt ::= iflaststmtl (26)
L. 15:  42     stmt ::= ifstmt (26)
L. 13:  18-48  l_stmts ::= _stmts lastl_stmt (26)
L. 13:  18-48  _stmts ::= _stmts stmt (26)
L. 13:  18     for_block ::= l_stmts (26)
L. 13:  18     l_stmts_opt ::= l_stmts (26)
L. 13:  18     l_stmts ::= _stmts (26)
        50     expr ::= LOAD_ASSERT (27)
L. 15:  42-50  assert_expr_or ::= assert_expr jmp_true expr (27)
        50     ret_expr ::= expr (27)
        50     assert_expr ::= expr (27)
L. 15:  42     assert_expr ::= assert_expr_or (27)
        50-52  raise_stmt1 ::= expr RAISE_VARARGS_1 (28)
        50     stmt ::= raise_stmt1 (28)
        50     _stmts ::= stmt (28)
L. 13:  18-52  _stmts ::= _stmts stmt (28)
        50     c_stmts ::= _stmts (28)
L. 13:  18     l_stmts ::= _stmts (28)
        50     c_stmts_opt ::= c_stmts (28)
L. 13:  18     for_block ::= l_stmts (28)
L. 13:  18     l_stmts_opt ::= l_stmts (28)
L. 15:  42-52  iflaststmtl ::= testexpr c_stmts_opt (28)
        50     _ifstmts_jump ::= c_stmts_opt (28)
L. 15:  42     lastl_stmt ::= iflaststmtl (28)
L. 15:  42-52  ifstmt ::= testexpr _ifstmts_jump (28)
L. 13:  18-52  l_stmts ::= _stmts lastl_stmt (28)
L. 15:  42     stmt ::= ifstmt (28)
L. 15:  42-54  iflaststmtl ::= testexpr c_stmts_opt JUMP_BACK (29)
L. 13:  18-54  for_block ::= l_stmts_opt \e_come_from_loops JUMP_BACK (29)
L. 13:  18-54  for_block ::= l_stmts_opt \e__come_froms JUMP_BACK (29)
L. 15:  42     lastl_stmt ::= iflaststmtl (29)
L. 13:  18-54  l_stmts ::= _stmts lastl_stmt (29)
L. 13:  18     for_block ::= l_stmts (29)
L. 13:  18     l_stmts_opt ::= l_stmts (29)
L. 15:  42-56  iflaststmtl ::= testexpr c_stmts_opt JUMP_BACK POP_BLOCK (30)
L. 15:  42     lastl_stmt ::= iflaststmtl (30)
L. 13:  18-56  l_stmts ::= _stmts lastl_stmt (30)
L. 13:  18     for_block ::= l_stmts (30)
L. 13:  18     l_stmts_opt ::= l_stmts (30)
               for ::= SETUP_LOOP expr for_iter store for_block POP_BLOCK COME_FROM_LOOP (31)
               come_from_loops ::= \e_come_from_loops COME_FROM_LOOP (31)
               stmt ::= for (31)
               sstmt ::= stmt (31)
               stmts ::= stmts sstmt (31)
               START ::= |- stmts (31)
               expr ::= LOAD_FAST (1)
               ret_expr ::= expr (1)
               assert_expr ::= expr (1)
         2     expr ::= LOAD_CONST (2)
         2     pos_arg ::= expr (2)
               compare_single ::= expr expr COMPARE_OP (3)
               compare ::= compare_single (3)
               expr ::= compare (3)
               ret_expr ::= expr (3)
               assert_expr ::= expr (3)
         6     jmp_false ::= POP_JUMP_IF_FALSE (4)
               testfalse ::= expr jmp_false (4)
               testexpr ::= testfalse (4)
               ifstmt ::= testexpr \e__ifstmts_jump (4)
               stmt ::= ifstmt (4)
               sstmt ::= stmt (4)
               stmts ::= sstmt (4)
               START ::= |- stmts (4)
L.  4:   8     expr ::= LOAD_CONST (5)
L.  4:   8     expr ::= LOAD_CONST (5)
               and ::= expr jmp_false expr (5)
               assert_expr_and ::= assert_expr jmp_false expr (5)
L.  4:   8     ret_expr ::= expr (5)
L.  4:   8     assert_expr ::= expr (5)
               expr ::= and (5)
               assert_expr ::= assert_expr_and (5)
               ret_expr ::= expr (5)
               assert_expr ::= expr (5)
        10     store ::= STORE_FAST (6)
L.  4:   8-10  assign ::= expr store (6)
               assign ::= expr store (6)
L.  4:   8     stmt ::= assign (6)
               stmt ::= assign (6)
L.  4:   8     _stmts ::= stmt (6)
L.  4:   8     sstmt ::= stmt (6)
               sstmt ::= stmt (6)
L.  4:   8     c_stmts ::= _stmts (6)
               stmts ::= stmts sstmt (6)
               stmts ::= sstmt (6)
L.  4:   8     c_stmts_opt ::= c_stmts (6)
               START ::= |- stmts (6)
L.  4:   8     _ifstmts_jump ::= c_stmts_opt (6)
               ifstmt ::= testexpr _ifstmts_jump (6)
               stmt ::= ifstmt (6)
L.  4:   8-12  _ifstmts_jump ::= c_stmts_opt JUMP_FORWARD \e__come_froms (7)
               ifstmt ::= testexpr _ifstmts_jump (7)
               stmt ::= ifstmt (7)
               sstmt ::= stmt (7)
               stmts ::= sstmt (7)
               START ::= |- stmts (7)
        12-14  jump_forward_else ::= JUMP_FORWARD ELSE (8)
L.  5:  14     expr ::= LOAD_FAST (9)
L.  5:  14     ret_expr ::= expr (9)
L.  5:  14     assert_expr ::= expr (9)
        16     expr ::= LOAD_CONST (10)
        16     pos_arg ::= expr (10)
L.  5:  14-18  compare_single ::= expr expr COMPARE_OP (11)
L.  5:  14     compare ::= compare_single (11)
L.  5:  14     expr ::= compare (11)
L.  5:  14     ret_expr ::= expr (11)
L.  5:  14     assert_expr ::= expr (11)
        20     jmp_false ::= POP_JUMP_IF_FALSE (12)
L.  5:  14-20  testfalse ::= expr jmp_false (12)
L.  5:  14     testexpr ::= testfalse (12)
L.  5:  14-20  iflaststmtl ::= testexpr \e_c_stmts_opt (12)
L.  5:  14-20  ifstmt ::= testexpr \e__ifstmts_jump (12)
L.  5:  14     lastl_stmt ::= iflaststmtl (12)
L.  5:  14     stmt ::= ifstmt (12)
L.  5:  14     _stmts ::= stmt (12)
L.  5:  14     suite_stmts ::= _stmts (12)
L.  5:  14     else_suite ::= suite_stmts (12)
               ifelsestmt ::= testexpr c_stmts_opt jump_forward_else else_suite \e__come_froms (12)
               stmt ::= ifelsestmt (12)
               sstmt ::= stmt (12)
               stmts ::= sstmt (12)
               START ::= |- stmts (12)
L.  6:  22     expr ::= LOAD_CONST (13)
L.  6:  22     expr ::= LOAD_CONST (13)
L.  5:  14-22  and ::= expr jmp_false expr (13)
L.  5:  14-22  assert_expr_and ::= assert_expr jmp_false expr (13)
L.  6:  22     ret_expr ::= expr (13)
L.  6:  22     assert_expr ::= expr (13)
L.  5:  14     expr ::= and (13)
L.  5:  14     assert_expr ::= assert_expr_and (13)
L.  5:  14     ret_expr ::= expr (13)
L.  5:  14     assert_expr ::= expr (13)
        24     store ::= STORE_FAST (14)
L.  6:  22-24  assign ::= expr store (14)
L.  5:  14-24  assign ::= expr store (14)
L.  6:  22     stmt ::= assign (14)
L.  5:  14     stmt ::= assign (14)
L.  6:  22     _stmts ::= stmt (14)
L.  5:  14-24  _stmts ::= _stmts stmt (14)
L.  6:  22     sstmt ::= stmt (14)
L.  5:  14     _stmts ::= stmt (14)
L.  6:  22     c_stmts ::= _stmts (14)
L.  5:  14     suite_stmts ::= _stmts (14)
               stmts ::= stmts sstmt (14)
L.  6:  22     c_stmts_opt ::= c_stmts (14)
L.  5:  14     else_suite ::= suite_stmts (14)
               START ::= |- stmts (14)
L.  5:  14-24  iflaststmtl ::= testexpr c_stmts_opt (14)
L.  6:  22     _ifstmts_jump ::= c_stmts_opt (14)
               ifelsestmt ::= testexpr c_stmts_opt jump_forward_else else_suite \e__come_froms (14)
L.  5:  14     lastl_stmt ::= iflaststmtl (14)
L.  5:  14-24  ifstmt ::= testexpr _ifstmts_jump (14)
               stmt ::= ifelsestmt (14)
L.  5:  14     stmt ::= ifstmt (14)
               sstmt ::= stmt (14)
               stmts ::= sstmt (14)
L.  6:  22-26  _ifstmts_jump ::= c_stmts_opt JUMP_FORWARD \e__come_froms (15)
L.  5:  14-26  ifstmt ::= testexpr _ifstmts_jump (15)
L.  5:  14     stmt ::= ifstmt (15)
L.  5:  14     _stmts ::= stmt (15)
L.  5:  14     suite_stmts ::= _stmts (15)
L.  5:  14     else_suite ::= suite_stmts (15)
               ifelsestmt ::= testexpr c_stmts_opt jump_forward_else else_suite \e__come_froms (15)
               stmt ::= ifelsestmt (15)
               sstmt ::= stmt (15)
               stmts ::= sstmt (15)
               START ::= |- stmts (15)
        26-28  jump_forward_else ::= JUMP_FORWARD ELSE (16)
L.  8:  28     expr ::= LOAD_CONST (17)
L.  8:  28     ret_expr ::= expr (17)
L.  8:  28     assert_expr ::= expr (17)
        30     store ::= STORE_FAST (18)
L.  8:  28-30  assign ::= expr store (18)
L.  8:  28     stmt ::= assign (18)
L.  8:  28     _stmts ::= stmt (18)
L.  8:  28     suite_stmts ::= _stmts (18)
L.  8:  28     else_suite ::= suite_stmts (18)
L.  5:  14-30  ifelsestmt ::= testexpr c_stmts_opt jump_forward_else else_suite \e__come_froms (18)
L.  5:  14     stmt ::= ifelsestmt (18)
L.  5:  14     _stmts ::= stmt (18)
L.  5:  14     suite_stmts ::= _stmts (18)
L.  5:  14     else_suite ::= suite_stmts (18)
               ifelsestmt ::= testexpr c_stmts_opt jump_forward_else else_suite \e__come_froms (18)
               stmt ::= ifelsestmt (18)
               sstmt ::= stmt (18)
               stmts ::= sstmt (18)
               START ::= |- stmts (18)
        32-32  _come_froms ::= \e__come_froms COME_FROM (19)
L.  5:  14-32  ifelsestmt ::= testexpr c_stmts_opt jump_forward_else else_suite _come_froms (19)
               ifelsestmt ::= testexpr c_stmts_opt jump_forward_else else_suite _come_froms (19)
L.  5:  14     stmt ::= ifelsestmt (19)
               stmt ::= ifelsestmt (19)
L.  5:  14     _stmts ::= stmt (19)
               sstmt ::= stmt (19)
L.  5:  14     suite_stmts ::= _stmts (19)
               stmts ::= sstmt (19)
L.  5:  14     else_suite ::= suite_stmts (19)
               START ::= |- stmts (19)
               ifelsestmt ::= testexpr c_stmts_opt jump_forward_else else_suite \e__come_froms (19)
        32-32  _come_froms ::= _come_froms COME_FROM (20)
        32-32  _come_froms ::= \e__come_froms COME_FROM (20)
L.  5:  14-32  ifelsestmt ::= testexpr c_stmts_opt jump_forward_else else_suite _come_froms (20)
Reduce ifelsestmt invalid by check
               ifelsestmt ::= testexpr c_stmts_opt jump_forward_else else_suite _come_froms (20)
               stmt ::= ifelsestmt (20)
               sstmt ::= stmt (20)
               stmts ::= sstmt (20)
               START ::= |- stmts (20)
L. 10:  32     expr ::= LOAD_FAST (21)
L. 10:  32     ret_expr ::= expr (21)
L. 10:  32     assert_expr ::= expr (21)
L. 10:  32-34  return ::= ret_expr RETURN_VALUE (22)
L. 10:  32     stmt ::= return (22)
L. 10:  32     sstmt ::= stmt (22)
               stmts ::= stmts sstmt (22)
               START ::= |- stmts (22)
               sstmt ::= return RETURN_LAST (23)
               sstmt ::= sstmt RETURN_LAST (23)
               stmts ::= stmts sstmt (23)
               START ::= |- stmts (23)


def testit(b):
    if b == 1:
        a = 1
    else:
        if b == 2:
            a = 2
        else:
            a = 4
    return a


for x in (1, 2, 4):
    x = testit(x)
    if not x is not None:
        raise AssertionError('Should have returned a value, not None')
    if not x == x:
        raise AssertionError
# okay decompiling /tmp/bug-3.7.pyc
$

@boeddeker
Copy link
Author

Sorry, I forgot to mention, that I used a new example.
The initial example is solved.
When I now use only pass in one elif block, it fails again with the initial bug.

def testit(b):
    if b == 1:
        b = 5
    elif b == 2:
        pass  # <----- only a pass in this branch
    else:
        b = 5
    return b

@rocky
Copy link
Owner

rocky commented Jan 4, 2019

Hey, now that I've shown you two ways that this might be handled, why don't you try to see how to extend the code to handle?

@boeddeker
Copy link
Author

ok, I will try to solve this.

@jpivarski jpivarski mentioned this issue Mar 22, 2019
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@rocky @boeddeker