Fixes #274

[x0ret]

Fixes #274

It passed all the test cases, Please let me know if there should be changes.

Thanks.

[rocky]

In 3.7.3 when I compile test/simple_source/bug30/00_chained-compare.py and then verify that I get

$ pyenv local 3.7.3
$ ./add-test.py --run simple_source/bug30/00_chained-compare.py
$ make check-bytecode-3.7
python test_pythonlib.py --bytecode-3.7-run --verify-run
Wed Jul 10 15:53:53 2019
Source directory:  bytecode_3.7_run
Output directory:  /tmp/py-dis-4bstz6st/bytecode_3.7_run
Instruction context:
   
   5      50  LOAD_CONST               1
             52  BUILD_LIST_1          1 
             54  STORE_NAME               r
           56_0  COME_FROM            48  '48'
->         56_1  COME_FROM            42  '42'


# file bytecode_3.7_run/00_chained-compare.pyc
# --- This code section failed: ---

   3       0  BUILD_LIST_0          0 
           2  BUILD_LIST_0          0 
           4  BUILD_LIST_0          0 
           6  ROT_THREE        
           8  ROT_TWO          
          10  STORE_NAME               r
          12  STORE_NAME               w
          14  STORE_NAME               e

   4      16  BUILD_LIST_0          0 
          18  LOAD_NAME                r
          20  DUP_TOP          
          22  ROT_THREE        
          24  COMPARE_OP               ==
          26  POP_JUMP_IF_FALSE    46  'to 46'
          28  LOAD_NAME                w
          30  DUP_TOP          
          32  ROT_THREE        
          34  COMPARE_OP               ==
          36  POP_JUMP_IF_FALSE    46  'to 46'
          38  LOAD_NAME                e
          40  COMPARE_OP               ==
          42  POP_JUMP_IF_FALSE    56  'to 56'
          44  JUMP_FORWARD         50  'to 50'
        46_0  COME_FROM            26  '26'
          46  POP_TOP          
          48  JUMP_FORWARD         56  'to 56'
        50_0  COME_FROM            44  '44'

   5      50  LOAD_CONST               1
          52  BUILD_LIST_1          1 
          54  STORE_NAME               r
        56_0  COME_FROM            48  '48'
        56_1  COME_FROM            42  '42'

   6      56  LOAD_NAME                r
          58  LOAD_CONST               1
          60  BUILD_LIST_1          1 
          62  COMPARE_OP               ==
          64  POP_JUMP_IF_TRUE     70  'to 70'
          66  LOAD_ASSERT              AssertionError
          68  RAISE_VARARGS_1       1  ''
        70_0  COME_FROM            64  '64'

Parse error at or near `COME_FROM' instruction at offset 56_1

bytecode_3.7_run/00_chained-compare.pyc -- 

The earliest I can look into this to narrow what's going on and how to fix is this weekend.

[x0ret]

Sorry about that i always forget about verify-run. I'm handling this, thanks.

[x0ret]

For now we are passing make check-bytecode-3.7 with this change, however i'm not sure it's the best choice and i'd be grateful if you comment on this in your free time.

Anyway having come_froms in _ifstmt_jump seems to force nested if (parent ifstmt's) statement to be pass stmt which we are trying to avoid in this PR.

Thanks

[rocky]

when you run run under python 3.7.3:

 ./test_pyenvlib.py --3.7.3 --verify --max 3000

before and after the change what do you get?

[x0ret]

I get multiple parse error. I'll take a look into this more carefully. Thanks.

[x0ret]

@rocky, i failed to pass 3.7 library tests even without this changes and eventually i recall the #235 issue which i failed to complete.
So i think we can't count on these tests unless we fixed the 3.7 library full decompilation.

What do you think?

[rocky]

So i think we can't count on these tests unless we fixed the 3.7 library full decompilation.

What do you think?

Yeah, I was aware of incompletness starting at around 3.5 or 3.6 or so. So I was interested, overall, whether the count got larger or smaller. That is, whether by doing this we have just traded off some bugs for some other bugs. That is in fact how in the past I have decided whether I should put in a change like this when there is no clear explanation from first principles as to why this change is better.

For example, in a recent change I think you proposed, I could see from the grammar that a COME_FROM token at the end of a grammar rule was clearly wrong, and I gave a reason why I thought that was the case.

If we could show why the existing rules that get removed are wrong, and how the new rules are better, then of course that is a stronger case than the statistical one I was asking about. I am not saying that is not the situation here. I don't know, and haven't done the work to verify.

In the broader sense though as I have written elsewhere, I think better would be to attack what is really the main problem head on. It is going to be a lot of work, and it is hard in that sense. But if we don't continue, we'll never finish it.

[rocky]

I am optimistic that we'll find a better way to handle this. Closing this since it fixes some things and possibly breaks others.