FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

bpo-46344: Fix trace bug in else of try and try-star blocks by iritkatriel · Pull Request #30544 · python/cpython · GitHub

  1. python
  2. cpython

Repository navigation

bpo-46344: Fix trace bug in else of try and try-star blocks #30544

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

Merged
iritkatriel merged 8 commits into from
Jan 13, 2022
Conversation 21 Commits 8 Checks 0 Files changed
Merged

bpo-46344: Fix trace bug in else of try and try-star blocks #30544

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
503e79e
bpo-46344: Fix trace bug in else of try and try star blocks
iritkatriel Jan 11, 2022
87c25b1
add non-const predicate to test
iritkatriel Jan 11, 2022
46cf04a
fix test_dis
iritkatriel Jan 11, 2022
beb64bc
put back the JUMP_FORWARDS and add UNSET_LOC to fix the trace
iritkatriel Jan 12, 2022
7ff6500
Revert "fix test_dis"
iritkatriel Jan 12, 2022
43aefd2
Revert "Revert "fix test_dis""
iritkatriel Jan 12, 2022
aac20fa
Revert "put back the JUMP_FORWARDS and add UNSET_LOC to fix the trace"
iritkatriel Jan 12, 2022
d95af87
remove else from condition in test
iritkatriel Jan 13, 2022
File filter

Filter by extension

Filter by extension .c  (1) .py  (2) All 2 file types selected
Viewed files
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Unified
Split
Hide whitespace
Diff view
Unified
Split
Hide whitespace
56 changes: 32 additions & 24 deletions Lib/test/test_dis.py
Show comments
View file
Open in desktop
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Original file line number Diff line number Diff line change
Expand Up @@ -1182,7 +1182,7 @@ def _prepare_test_cases():
Instruction(opname='CALL_NO_KW', opcode=169, arg=1, argval=1, argrepr='', offset=130, starts_line=None, is_jump_target=False, positions=None),
Instruction(opname='POP_TOP', opcode=1, arg=None, argval=None, argrepr='', offset=132, starts_line=None, is_jump_target=False, positions=None),
Instruction(opname='POP_EXCEPT', opcode=89, arg=None, argval=None, argrepr='', offset=134, starts_line=None, is_jump_target=False, positions=None),
Instruction(opname='JUMP_FORWARD', opcode=110, arg=34, argval=206, argrepr='to 206', offset=136, starts_line=None, is_jump_target=False, positions=None),
Instruction(opname='JUMP_FORWARD', opcode=110, arg=35, argval=208, argrepr='to 208', offset=136, starts_line=None, is_jump_target=False, positions=None),
Instruction(opname='RERAISE', opcode=119, arg=0, argval=0, argrepr='', offset=138, starts_line=22, is_jump_target=True, positions=None),
Instruction(opname='COPY', opcode=120, arg=3, argval=3, argrepr='', offset=140, starts_line=None, is_jump_target=False, positions=None),
Instruction(opname='POP_EXCEPT', opcode=89, arg=None, argval=None, argrepr='', offset=142, starts_line=None, is_jump_target=False, positions=None),
Expand All @@ -1199,7 +1199,7 @@ def _prepare_test_cases():
Instruction(opname='DUP_TOP', opcode=4, arg=None, argval=None, argrepr='', offset=164, starts_line=None, is_jump_target=False, positions=None),
Instruction(opname='CALL_NO_KW', opcode=169, arg=3, argval=3, argrepr='', offset=166, starts_line=None, is_jump_target=False, positions=None),
Instruction(opname='POP_TOP', opcode=1, arg=None, argval=None, argrepr='', offset=168, starts_line=None, is_jump_target=False, positions=None),
Instruction(opname='JUMP_FORWARD', opcode=110, arg=11, argval=194, argrepr='to 194', offset=170, starts_line=None, is_jump_target=False, positions=None),
Instruction(opname='JUMP_FORWARD', opcode=110, arg=25, argval=222, argrepr='to 222', offset=170, starts_line=None, is_jump_target=False, positions=None),
Instruction(opname='PUSH_EXC_INFO', opcode=35, arg=None, argval=None, argrepr='', offset=172, starts_line=None, is_jump_target=False, positions=None),
Instruction(opname='WITH_EXCEPT_START', opcode=49, arg=None, argval=None, argrepr='', offset=174, starts_line=None, is_jump_target=False, positions=None),
Instruction(opname='POP_JUMP_IF_TRUE', opcode=115, arg=93, argval=186, argrepr='to 186', offset=176, starts_line=None, is_jump_target=False, positions=None),
Expand All @@ -1211,28 +1211,36 @@ def _prepare_test_cases():
Instruction(opname='POP_EXCEPT', opcode=89, arg=None, argval=None, argrepr='', offset=188, starts_line=None, is_jump_target=False, positions=None),
Instruction(opname='POP_TOP', opcode=1, arg=None, argval=None, argrepr='', offset=190, starts_line=None, is_jump_target=False, positions=None),
Instruction(opname='POP_TOP', opcode=1, arg=None, argval=None, argrepr='', offset=192, starts_line=None, is_jump_target=False, positions=None),
Instruction(opname='LOAD_GLOBAL', opcode=116, arg=1, argval='print', argrepr='print', offset=194, starts_line=28, is_jump_target=True, positions=None),
Instruction(opname='LOAD_CONST', opcode=100, arg=10, argval="OK, now we're done", argrepr='"OK, now we\'re done"', offset=196, starts_line=None, is_jump_target=False, positions=None),
Instruction(opname='CALL_NO_KW', opcode=169, arg=1, argval=1, argrepr='', offset=198, starts_line=None, is_jump_target=False, positions=None),
Instruction(opname='POP_TOP', opcode=1, arg=None, argval=None, argrepr='', offset=200, starts_line=None, is_jump_target=False, positions=None),
Instruction(opname='LOAD_CONST', opcode=100, arg=0, argval=None, argrepr='None', offset=202, starts_line=None, is_jump_target=False, positions=None),
Instruction(opname='RETURN_VALUE', opcode=83, arg=None, argval=None, argrepr='', offset=204, starts_line=None, is_jump_target=False, positions=None),
Instruction(opname='NOP', opcode=9, arg=None, argval=None, argrepr='', offset=206, starts_line=23, is_jump_target=True, positions=None),
Instruction(opname='LOAD_GLOBAL', opcode=116, arg=1, argval='print', argrepr='print', offset=208, starts_line=28, is_jump_target=False, positions=None),
Instruction(opname='LOAD_CONST', opcode=100, arg=10, argval="OK, now we're done", argrepr='"OK, now we\'re done"', offset=210, starts_line=None, is_jump_target=False, positions=None),
Instruction(opname='CALL_NO_KW', opcode=169, arg=1, argval=1, argrepr='', offset=212, starts_line=None, is_jump_target=False, positions=None),
Instruction(opname='POP_TOP', opcode=1, arg=None, argval=None, argrepr='', offset=214, starts_line=None, is_jump_target=False, positions=None),
Instruction(opname='LOAD_CONST', opcode=100, arg=0, argval=None, argrepr='None', offset=216, starts_line=None, is_jump_target=False, positions=None),
Instruction(opname='RETURN_VALUE', opcode=83, arg=None, argval=None, argrepr='', offset=218, starts_line=None, is_jump_target=False, positions=None),
Instruction(opname='PUSH_EXC_INFO', opcode=35, arg=None, argval=None, argrepr='', offset=220, starts_line=None, is_jump_target=False, positions=None),
Instruction(opname='LOAD_GLOBAL', opcode=116, arg=1, argval='print', argrepr='print', offset=222, starts_line=None, is_jump_target=False, positions=None),
Instruction(opname='LOAD_CONST', opcode=100, arg=10, argval="OK, now we're done", argrepr='"OK, now we\'re done"', offset=224, starts_line=None, is_jump_target=False, positions=None),
Instruction(opname='CALL_NO_KW', opcode=169, arg=1, argval=1, argrepr='', offset=226, starts_line=None, is_jump_target=False, positions=None),
Instruction(opname='POP_TOP', opcode=1, arg=None, argval=None, argrepr='', offset=228, starts_line=None, is_jump_target=False, positions=None),
Instruction(opname='RERAISE', opcode=119, arg=0, argval=0, argrepr='', offset=230, starts_line=None, is_jump_target=False, positions=None),
Instruction(opname='COPY', opcode=120, arg=3, argval=3, argrepr='', offset=232, starts_line=None, is_jump_target=False, positions=None),
Instruction(opname='POP_EXCEPT', opcode=89, arg=None, argval=None, argrepr='', offset=234, starts_line=None, is_jump_target=False, positions=None),
Instruction(opname='RERAISE', opcode=119, arg=1, argval=1, argrepr='', offset=236, starts_line=None, is_jump_target=False, positions=None),
Instruction(opname='NOP', opcode=9, arg=None, argval=None, argrepr='', offset=194, starts_line=None, is_jump_target=False, positions=None),
Instruction(opname='LOAD_GLOBAL', opcode=116, arg=1, argval='print', argrepr='print', offset=196, starts_line=28, is_jump_target=False, positions=None),
Instruction(opname='LOAD_CONST', opcode=100, arg=10, argval="OK, now we're done", argrepr='"OK, now we\'re done"', offset=198, starts_line=None, is_jump_target=False, positions=None),
Instruction(opname='CALL_NO_KW', opcode=169, arg=1, argval=1, argrepr='', offset=200, starts_line=None, is_jump_target=False, positions=None),
Instruction(opname='POP_TOP', opcode=1, arg=None, argval=None, argrepr='', offset=202, starts_line=None, is_jump_target=False, positions=None),
Instruction(opname='LOAD_CONST', opcode=100, arg=0, argval=None, argrepr='None', offset=204, starts_line=None, is_jump_target=False, positions=None),
Instruction(opname='RETURN_VALUE', opcode=83, arg=None, argval=None, argrepr='', offset=206, starts_line=None, is_jump_target=False, positions=None),
Instruction(opname='NOP', opcode=9, arg=None, argval=None, argrepr='', offset=208, starts_line=23, is_jump_target=True, positions=None),
Instruction(opname='LOAD_GLOBAL', opcode=116, arg=1, argval='print', argrepr='print', offset=210, starts_line=28, is_jump_target=False, positions=None),
Instruction(opname='LOAD_CONST', opcode=100, arg=10, argval="OK, now we're done", argrepr='"OK, now we\'re done"', offset=212, starts_line=None, is_jump_target=False, positions=None),
Instruction(opname='CALL_NO_KW', opcode=169, arg=1, argval=1, argrepr='', offset=214, starts_line=None, is_jump_target=False, positions=None),
Instruction(opname='POP_TOP', opcode=1, arg=None, argval=None, argrepr='', offset=216, starts_line=None, is_jump_target=False, positions=None),
Instruction(opname='LOAD_CONST', opcode=100, arg=0, argval=None, argrepr='None', offset=218, starts_line=None, is_jump_target=False, positions=None),
Instruction(opname='RETURN_VALUE', opcode=83, arg=None, argval=None, argrepr='', offset=220, starts_line=None, is_jump_target=False, positions=None),
Instruction(opname='NOP', opcode=9, arg=None, argval=None, argrepr='', offset=222, starts_line=25, is_jump_target=True, positions=None),
Instruction(opname='LOAD_GLOBAL', opcode=116, arg=1, argval='print', argrepr='print', offset=224, starts_line=28, is_jump_target=False, positions=None),
Instruction(opname='LOAD_CONST', opcode=100, arg=10, argval="OK, now we're done", argrepr='"OK, now we\'re done"', offset=226, starts_line=None, is_jump_target=False, positions=None),
Instruction(opname='CALL_NO_KW', opcode=169, arg=1, argval=1, argrepr='', offset=228, starts_line=None, is_jump_target=False, positions=None),
Instruction(opname='POP_TOP', opcode=1, arg=None, argval=None, argrepr='', offset=230, starts_line=None, is_jump_target=False, positions=None),
Instruction(opname='LOAD_CONST', opcode=100, arg=0, argval=None, argrepr='None', offset=232, starts_line=None, is_jump_target=False, positions=None),
Instruction(opname='RETURN_VALUE', opcode=83, arg=None, argval=None, argrepr='', offset=234, starts_line=None, is_jump_target=False, positions=None),
Instruction(opname='PUSH_EXC_INFO', opcode=35, arg=None, argval=None, argrepr='', offset=236, starts_line=None, is_jump_target=False, positions=None),
Instruction(opname='LOAD_GLOBAL', opcode=116, arg=1, argval='print', argrepr='print', offset=238, starts_line=None, is_jump_target=False, positions=None),
Instruction(opname='LOAD_CONST', opcode=100, arg=10, argval="OK, now we're done", argrepr='"OK, now we\'re done"', offset=240, starts_line=None, is_jump_target=False, positions=None),
Instruction(opname='CALL_NO_KW', opcode=169, arg=1, argval=1, argrepr='', offset=242, starts_line=None, is_jump_target=False, positions=None),
Instruction(opname='POP_TOP', opcode=1, arg=None, argval=None, argrepr='', offset=244, starts_line=None, is_jump_target=False, positions=None),
Instruction(opname='RERAISE', opcode=119, arg=0, argval=0, argrepr='', offset=246, starts_line=None, is_jump_target=False, positions=None),
Instruction(opname='COPY', opcode=120, arg=3, argval=3, argrepr='', offset=248, starts_line=None, is_jump_target=False, positions=None),
Instruction(opname='POP_EXCEPT', opcode=89, arg=None, argval=None, argrepr='', offset=250, starts_line=None, is_jump_target=False, positions=None),
Instruction(opname='RERAISE', opcode=119, arg=1, argval=1, argrepr='', offset=252, starts_line=None, is_jump_target=False, positions=None),

iritkatriel Jan 11, 2022

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

There are 9 more opcodes generated now. Looks like that jump enabled some optimisation.

iritkatriel Jan 11, 2022

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

@gvanrossum this PR fixes a regression from the except* PR and it’s all very odd, both the bug and the impact of the fix.

gvanrossum Jan 11, 2022

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Tracing is Mark's black magic these days (I don't claim to understand the line number table code).

Could it be that test_dis.py wasn't properly regenerated in a while? It's always struck me as odd that there isn't a more direct way to cause this data to be regenerated.

iritkatriel Jan 11, 2022

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

The tests fail if you don’t update it.

iritkatriel Jan 11, 2022

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

I extracted it out of the test. I think with this PR it emits the finally body 4 times, and without it 3 times?

def jumpy():
    try:
        1 / 0
    except ZeroDivisionError:
        print("Here we go, here we go, here we go...")
    else:
        with i as dodgy:
            print("Never reach this")
    finally:
        print("OK, now we're done")

import dis
dis.dis(jumpy)

Old:

  1           0 RESUME                   0

  2           2 NOP

  3           4 LOAD_CONST               1 (1)
              6 LOAD_CONST               2 (0)
              8 BINARY_OP               11 (/)
             10 POP_TOP
             12 JUMP_FORWARD            14 (to 42)
        >>   14 PUSH_EXC_INFO

  4          16 LOAD_GLOBAL              0 (ZeroDivisionError)
             18 JUMP_IF_NOT_EXC_MATCH    17 (to 34)
             20 POP_TOP

  5          22 LOAD_GLOBAL              1 (print)
             24 LOAD_CONST               3 ('Here we go, here we go, here we go...')
             26 CALL_NO_KW               1
             28 POP_TOP
             30 POP_EXCEPT
             32 JUMP_FORWARD            34 (to 102)

  4     >>   34 RERAISE                  0
        >>   36 COPY                     3
             38 POP_EXCEPT
             40 RERAISE                  1

  7     >>   42 LOAD_GLOBAL              2 (i)
             44 BEFORE_WITH
             46 STORE_FAST               0 (dodgy)

  8          48 LOAD_GLOBAL              1 (print)
             50 LOAD_CONST               4 ('Never reach this')
             52 CALL_NO_KW               1
             54 POP_TOP

  7          56 LOAD_CONST               0 (None)
             58 DUP_TOP
             60 DUP_TOP
             62 CALL_NO_KW               3
             64 POP_TOP
             66 JUMP_FORWARD            11 (to 90)
        >>   68 PUSH_EXC_INFO
             70 WITH_EXCEPT_START
             72 POP_JUMP_IF_TRUE        41 (to 82)
             74 RERAISE                  2
        >>   76 COPY                     3
             78 POP_EXCEPT
             80 RERAISE                  1
        >>   82 POP_TOP
             84 POP_EXCEPT
             86 POP_TOP
             88 POP_TOP

 10     >>   90 LOAD_GLOBAL              1 (print)
             92 LOAD_CONST               5 ("OK, now we're done")
             94 CALL_NO_KW               1
             96 POP_TOP
             98 LOAD_CONST               0 (None)
            100 RETURN_VALUE

  5     >>  102 NOP

 10         104 LOAD_GLOBAL              1 (print)
            106 LOAD_CONST               5 ("OK, now we're done")
            108 CALL_NO_KW               1
            110 POP_TOP
            112 LOAD_CONST               0 (None)
            114 RETURN_VALUE
        >>  116 PUSH_EXC_INFO
            118 LOAD_GLOBAL              1 (print)
            120 LOAD_CONST               5 ("OK, now we're done")
            122 CALL_NO_KW               1
            124 POP_TOP
            126 RERAISE                  0
        >>  128 COPY                     3
            130 POP_EXCEPT
            132 RERAISE                  1
ExceptionTable:
  4 to 10 -> 14 [0]
  12 to 12 -> 116 [0]
  14 to 28 -> 36 [1] lasti
  30 to 32 -> 116 [0]
  34 to 34 -> 36 [1] lasti
  36 to 44 -> 116 [0]
  46 to 54 -> 68 [1] lasti
  56 to 66 -> 116 [0]
  68 to 74 -> 76 [3] lasti
  76 to 80 -> 116 [0]
  82 to 82 -> 76 [3] lasti
  84 to 88 -> 116 [0]
  116 to 126 -> 128 [1] lasti

New:

  1           0 RESUME                   0

  2           2 NOP

  3           4 LOAD_CONST               1 (1)
              6 LOAD_CONST               2 (0)
              8 BINARY_OP               11 (/)
             10 POP_TOP
             12 JUMP_FORWARD            14 (to 42)
        >>   14 PUSH_EXC_INFO

  4          16 LOAD_GLOBAL              0 (ZeroDivisionError)
             18 JUMP_IF_NOT_EXC_MATCH    17 (to 34)
             20 POP_TOP

  5          22 LOAD_GLOBAL              1 (print)
             24 LOAD_CONST               3 ('Here we go, here we go, here we go...')
             26 CALL_NO_KW               1
             28 POP_TOP
             30 POP_EXCEPT
             32 JUMP_FORWARD            35 (to 104)

  4     >>   34 RERAISE                  0
        >>   36 COPY                     3
             38 POP_EXCEPT
             40 RERAISE                  1

  7     >>   42 LOAD_GLOBAL              2 (i)
             44 BEFORE_WITH
             46 STORE_FAST               0 (dodgy)

  8          48 LOAD_GLOBAL              1 (print)
             50 LOAD_CONST               4 ('Never reach this')
             52 CALL_NO_KW               1
             54 POP_TOP

  7          56 LOAD_CONST               0 (None)
             58 DUP_TOP
             60 DUP_TOP
             62 CALL_NO_KW               3
             64 POP_TOP
             66 JUMP_FORWARD            25 (to 118)
        >>   68 PUSH_EXC_INFO
             70 WITH_EXCEPT_START
             72 POP_JUMP_IF_TRUE        41 (to 82)
             74 RERAISE                  2
        >>   76 COPY                     3
             78 POP_EXCEPT
             80 RERAISE                  1
        >>   82 POP_TOP
             84 POP_EXCEPT
             86 POP_TOP
             88 POP_TOP
             90 NOP

 10          92 LOAD_GLOBAL              1 (print)
             94 LOAD_CONST               5 ("OK, now we're done")
             96 CALL_NO_KW               1
             98 POP_TOP
            100 LOAD_CONST               0 (None)
            102 RETURN_VALUE

  5     >>  104 NOP

 10         106 LOAD_GLOBAL              1 (print)
            108 LOAD_CONST               5 ("OK, now we're done")
            110 CALL_NO_KW               1
            112 POP_TOP
            114 LOAD_CONST               0 (None)
            116 RETURN_VALUE

  7     >>  118 NOP

 10         120 LOAD_GLOBAL              1 (print)
            122 LOAD_CONST               5 ("OK, now we're done")
            124 CALL_NO_KW               1
            126 POP_TOP
            128 LOAD_CONST               0 (None)
            130 RETURN_VALUE
        >>  132 PUSH_EXC_INFO
            134 LOAD_GLOBAL              1 (print)
            136 LOAD_CONST               5 ("OK, now we're done")
            138 CALL_NO_KW               1
            140 POP_TOP
            142 RERAISE                  0
        >>  144 COPY                     3
            146 POP_EXCEPT
            148 RERAISE                  1
ExceptionTable:
  4 to 10 -> 14 [0]
  12 to 12 -> 132 [0]
  14 to 28 -> 36 [1] lasti
  30 to 32 -> 132 [0]
  34 to 34 -> 36 [1] lasti
  36 to 44 -> 132 [0]
  46 to 54 -> 68 [1] lasti
  56 to 66 -> 132 [0]
  68 to 74 -> 76 [3] lasti
  76 to 80 -> 132 [0]
  82 to 82 -> 76 [3] lasti
  84 to 88 -> 132 [0]
  132 to 142 -> 144 [1] lasti

iritkatriel Jan 12, 2022

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

We sometimes duplicate exit blocks to save a jump, but only if they have 4 or fewer instructions.

Maybe the explicit jump prevents that?

iritkatriel Jan 12, 2022

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

I'm going to assume that this is what happened (this jump prevented an inlining optimization), and remove it as in the first version of this PR. This will being us back to where we were before the except* PR was merged.

iritkatriel Jan 13, 2022

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

I think we should probably merge this before the release so that we don’t get a change in behaviour between 3.11a3 and 311.a4.

markshannon Jan 13, 2022

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

I agree.
This PR merely highlights the excessive duplication of finally blocks. It doesn't cause it.
Do we have an issue to investigate the cause of finally block duplication?

iritkatriel Jan 13, 2022 •
edited
Loading

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

We do now: link
]

# One last piece of inspect fodder to check the default line number handling
Expand Down
30 changes: 24 additions & 6 deletions Lib/test/test_sys_settrace.py
Show comments
View file
Open in desktop
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Original file line number Diff line number Diff line change
Expand Up @@ -644,16 +644,25 @@ def func():
4
else:
6
if False:

markshannon Jan 11, 2022

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Does this cover the original issue?
The compiler will optimize out the if False.
Might be a good idea to add another test with a non-constant predicate.

iritkatriel Jan 11, 2022

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

The test did fail without the fix. But sure, I'll add a test with a non const.
8
else:
10
if func.__name__ == 'Fred':
12
finally:
8
14

self.run_and_compare(func,
[(0, 'call'),
(1, 'line'),
(2, 'line'),
(6, 'line'),
(8, 'line'),
(8, 'return')])
(7, 'line'),
(10, 'line'),
(11, 'line'),
(14, 'line'),
(14, 'return')])

def test_nested_loops(self):

Expand Down Expand Up @@ -1222,16 +1231,25 @@ def func():
4
else:
6
if False:
8
else:
10
if func.__name__ == 'Fred':
12
finally:
8
14

self.run_and_compare(func,
[(0, 'call'),
(1, 'line'),
(2, 'line'),
(6, 'line'),
(8, 'line'),
(8, 'return')])
(7, 'line'),
(10, 'line'),
(11, 'line'),
(14, 'line'),
(14, 'return')])

def test_try_except_star_named_no_exception(self):

Expand Down
2 changes: 0 additions & 2 deletions Python/compile.c
Show comments
View file
Open in desktop
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Original file line number Diff line number Diff line change
Expand Up @@ -3475,7 +3475,6 @@ compiler_try_except(struct compiler *c, stmt_ty s)
POP_EXCEPT_AND_RERAISE(c);
compiler_use_next_block(c, orelse);
VISIT_SEQ(c, stmt, s->v.Try.orelse);
ADDOP_JUMP(c, JUMP_FORWARD, end);
compiler_use_next_block(c, end);
return 1;
}
Expand Down Expand Up @@ -3702,7 +3701,6 @@ compiler_try_star_except(struct compiler *c, stmt_ty s)
POP_EXCEPT_AND_RERAISE(c);
compiler_use_next_block(c, orelse);
VISIT_SEQ(c, stmt, s->v.TryStar.orelse);
ADDOP_JUMP(c, JUMP_FORWARD, end);
compiler_use_next_block(c, end);
return 1;
}
Expand Down
Back | FazBrowse Home | New Git URL