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.

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

python3.8: unable to detect break in while loop as covered #772

Closed
asottile opened this issue Feb 7, 2019 · 1 comment
Closed

python3.8: unable to detect break in while loop as covered #772

asottile opened this issue Feb 7, 2019 · 1 comment
Labels
bug Something isn't working not our bug The problem was elsewhere

Comments

@asottile
Copy link
Contributor

asottile commented Feb 7, 2019

Describe the bug
python has changed how loops work in python3.8. there are no longer explicit opcodes for break / continue / etc. Some of these seem handled in coverage but others are not.

To Reproduce

def f(start):
    end = 3
    while start < end:
        if start == 1:
            start += 1
            continue
        else:
            break  # not covered in py38 :'(

f(1)
f(3)
$ coverage erase && coverage run --branch t.py && coverage report --include t.py --show-missing
Name    Stmts   Miss Branch BrPart  Cover   Missing
---------------------------------------------------
t.py        9      1      4      1    85%   4->8, 8
$ python --version --version
Python 3.8.0a1+ (heads/master:e9bc4172d1, Feb  6 2019, 17:49:51) 
[GCC 7.3.0]

Expected behavior

This is what happens in python3.7:

$ coverage erase && coverage run --branch t.py && coverage report --include t.py
Name    Stmts   Miss Branch BrPart  Cover
-----------------------------------------
t.py        9      0      4      0   100%
$ python3.7 --version --version
Python 3.7.2 (default, Dec 25 2018, 03:50:46) 
[GCC 7.3.0]

Additional context
https://bugs.python.org/issue17611 appears to be the bpo that changed this behaviour

I've verified that the break is covered by adding a print(0) above it, however this causes the break to magically become covered. It only reproduces when there's nothing else in that block.

Here's the disassembly from python3.7:

Disassembly of <code object f at 0x7f4c8460b300, file "t.py", line 1>:
  2           0 LOAD_CONST               1 (3)
              2 STORE_FAST               1 (end)

  3           4 SETUP_LOOP              34 (to 40)
        >>    6 LOAD_FAST                0 (start)
              8 LOAD_FAST                1 (end)
             10 COMPARE_OP               0 (<)
             12 POP_JUMP_IF_FALSE       38

  4          14 LOAD_FAST                0 (start)
             16 LOAD_CONST               2 (1)
             18 COMPARE_OP               2 (==)
             20 POP_JUMP_IF_FALSE       34

  5          22 LOAD_FAST                0 (start)
             24 LOAD_CONST               2 (1)
             26 INPLACE_ADD
             28 STORE_FAST               0 (start)

  6          30 JUMP_ABSOLUTE            6
             32 JUMP_ABSOLUTE            6

  8     >>   34 BREAK_LOOP
             36 JUMP_ABSOLUTE            6
        >>   38 POP_BLOCK
        >>   40 LOAD_CONST               0 (None)
             42 RETURN_VALUE

Here's the disassembly from python3.8

Disassembly of <code object f at 0x7f97cde9eae0, file "t.py", line 1>:
  2           0 LOAD_CONST               1 (3)
              2 STORE_FAST               1 (end)

  3     >>    4 LOAD_FAST                0 (start)
              6 LOAD_FAST                1 (end)
              8 COMPARE_OP               0 (<)
             10 POP_JUMP_IF_FALSE       36

  4          12 LOAD_FAST                0 (start)
             14 LOAD_CONST               2 (1)
             16 COMPARE_OP               2 (==)
             18 POP_JUMP_IF_FALSE       36

  5          20 LOAD_FAST                0 (start)
             22 LOAD_CONST               2 (1)
             24 INPLACE_ADD
             26 STORE_FAST               0 (start)

  6          28 JUMP_ABSOLUTE            4
             30 JUMP_ABSOLUTE            4

  8          32 JUMP_ABSOLUTE           36
             34 JUMP_ABSOLUTE            4
        >>   36 LOAD_CONST               0 (None)
             38 RETURN_VALUE

The main difference here being line 8 which goes from BREAK_LOOP (3.7) to JUMP_ABSOLUTE (3.8)

@asottile asottile added the bug Something isn't working label Feb 7, 2019
@nedbat
Copy link
Owner

nedbat commented Feb 7, 2019

The problem here isn't that the opcodes have changed, but that the peephole optimizer has become cleverer. If you look at the bytecode, you see that if start != 1 on line 4, then the jump is to bytecode 36, not bytecode 32. That is, the optimizer recognizes that the if statement could go to a break, which itself goes to the return, so just make the if go straight to the return.

This is another instance of the problem first reported over 10 years ago in bpo 2506. The debate there ended long ago, but needs someone to implement a way to disable the peephole optimizer for times when you are running code for analysis rather than performance. Perhaps your voice will help?

5j9 added a commit to 5j9/wikitextparser that referenced this issue Nov 7, 2019
@nedbat nedbat added the not our bug The problem was elsewhere label Jan 26, 2020
@nedbat nedbat closed this as completed Jan 26, 2020
pganssle added a commit to pganssle/zoneinfo that referenced this issue Mar 3, 2020
This is a long-running bug in coverage that manifests in many ways. When
the peephole optimizer eliminates a part of the code, it doesn't leave a
trace that coverage can see, so even if a line's effects are seen, the
code shows as uncovered.

It appears that this is happening here, but we can disable the optimizer
in this test by assigning to a random variable to force the branch to be
executed in a way that coverage can see it.

We could also add # pragma: nocover here, but I'd like to make sure that
this line is seen as covered, since otherwise it may mean that some of
our tests are erroneously being treated as folds or gaps.

More background reading:

- bpo-2506 https://bugs.python.org/issue2506
- nedbat/coveragepy#772
pganssle added a commit to pganssle/zoneinfo that referenced this issue Mar 3, 2020
This is a long-running bug in coverage that manifests in many ways. When
the peephole optimizer eliminates a part of the code, it doesn't leave a
trace that coverage can see, so even if a line's effects are seen, the
code shows as uncovered.

It appears that this is happening here, but we can disable the optimizer
in this test by assigning to a random variable to force the branch to be
executed in a way that coverage can see it.

We could also add # pragma: nocover here, but I'd like to make sure that
this line is seen as covered, since otherwise it may mean that some of
our tests are erroneously being treated as folds or gaps.

More background reading:

- bpo-2506 https://bugs.python.org/issue2506
- nedbat/coveragepy#772
facebook-github-bot pushed a commit to facebookincubator/antlir that referenced this issue Jan 12, 2021
Summary: It turns out that with python 3.8 (platform009 version of python) the coverage of `break` immediately followed by a return is not handled.  Turns out to be some peephole optimizer thing: nedbat/coveragepy#772  So, we'll just add `#pragma: no cover` to these statements and cry a little.

Reviewed By: justintrudell

Differential Revision: D25876993

fbshipit-source-id: 041eca915bbc73a1b7360d5e80f794772d82ba96
rajulkumar added a commit to release-engineering/pubtools-ami that referenced this issue Nov 8, 2021
1. break statement doesn't gets covered by coverage in py38
onwards due to some optimizations in the peephole optimizer.
nedbat/coveragepy#772
Hence, it was excluded from the coverage.
2. Region specific accounts were added apart from default
for aws publish to cover all the cases in the code.
nstockton added a commit to nstockton/mud-protocols that referenced this issue Apr 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working not our bug The problem was elsewhere
Projects
None yet
Development

No branches or pull requests

2 participants