On 22 May 2014 00:24, "Ned Batchelder" <ned@nedbatchelder.com> wrote:
>
> On 5/21/14 8:21 AM, Jonas Wielicki wrote:
>>
>> On 21.05.2014 14:13, Steven D'Aprano wrote:
>>>
>>> > On Wed, May 21, 2014 at 07:05:49AM -0400, Ned Batchelder wrote:
>>>>
>>>> >> ** The problem
>>>> >>
>>>> >> A long-standing problem with CPython is that the peephole optimizer
>>>> >> cannot be completely disabled.  Normally, peephole optimization is a
>>>> >> good thing, it improves execution speed.  But in some situations, like
>>>> >> coverage testing, it's more important to be able to reason about the
>>>> >> code's execution.  I propose that we add a way to completely disable the
>>>> >> optimizer.
>>>
>>> >
>>> > I'm not sure whether this is an argument for or against your proposal,
>>> > but the continue statement shown below is *not* dead code and should not
>>> > be optimized out. The assert fails if you remove the continue statement.
>>> >
>>> > I don't have 3.4 on this machine to test with, but using 3.3, I can see
>>> > no evidence that `continue` is optimized away.
>>
>> The logical continue is still there -- what happens is that the
>> optimizer rewrites the `else` jump at the preceding `if` condition,
>> which would normally point at the `continue` statement, to the beginning
>> of the loop, because it would be a jump (to the continue) to a jump (to
>> the for loop header).
>>
>> Thus, the actual continue statement is not reached, but logically the
>> code does the same, because the only way continue would have been
>> reached was transformed to a continue itself.
>>
> To make the details more explicit, here is the source again, and the disassembled code, with the original source interspersed:
>
>> a = b = c = 0
>> for n in range(100):
>>     if n % 2:
>>         if n % 4:
>>             a += 1
>>         continue
>>     else:
>>         b += 1
>>     c += 1
>> assert a == 50 and b == 50 and c == 50
>
> Disassembled (Python 3.4, but the same effect is visible in 2.7, 3.3, etc):
>
>
> a = b = c = 0
>   1           0 LOAD_CONST               0 (0)
>               3 DUP_TOP
>               4 STORE_NAME               0 (a)
>               7 DUP_TOP
>               8 STORE_NAME               1 (b)
>              11 STORE_NAME               2 (c)
>
> for n in range(100):
>   2          14 SETUP_LOOP              79 (to 96)
>              17 LOAD_NAME                3 (range)
>              20 LOAD_CONST               1 (100)
>              23 CALL_FUNCTION            1 (1 positional, 0 keyword pair)
>              26 GET_ITER
>         >>   27 FOR_ITER                65 (to 95)
>              30 STORE_NAME               4 (n)
>
>     if n % 2:
>   3          33 LOAD_NAME                4 (n)
>              36 LOAD_CONST               2 (2)
>              39 BINARY_MODULO
>              40 POP_JUMP_IF_FALSE       72
>
>         if n % 4:
>   4          43 LOAD_NAME                4 (n)
>              46 LOAD_CONST               3 (4)
>              49 BINARY_MODULO
>              50 POP_JUMP_IF_FALSE       27
>
>             a += 1
>   5          53 LOAD_NAME                0 (a)
>              56 LOAD_CONST               4 (1)
>              59 INPLACE_ADD
>              60 STORE_NAME               0 (a)
>              63 JUMP_ABSOLUTE           27
>
>         continue
>   6          66 JUMP_ABSOLUTE           27
>              69 JUMP_FORWARD            10 (to 82)
>
>         b += 1
>   8     >>   72 LOAD_NAME                1 (b)
>              75 LOAD_CONST               4 (1)
>              78 INPLACE_ADD
>              79 STORE_NAME               1 (b)
>
>     c += 1
>   9     >>   82 LOAD_NAME                2 (c)
>              85 LOAD_CONST               4 (1)
>              88 INPLACE_ADD
>              89 STORE_NAME               2 (c)
>              92 JUMP_ABSOLUTE           27
>         >>   95 POP_BLOCK
>
>
> assert a == 50 and b == 50 and c == 50
>  10     >>   96 LOAD_NAME                0 (a)
>              99 LOAD_CONST               5 (50)
>             102 COMPARE_OP               2 (==)
>             105 POP_JUMP_IF_FALSE      132
>             108 LOAD_NAME                1 (b)
>             111 LOAD_CONST               5 (50)
>             114 COMPARE_OP               2 (==)
>             117 POP_JUMP_IF_FALSE      132
>             120 LOAD_NAME                2 (c)
>             123 LOAD_CONST               5 (50)
>             126 COMPARE_OP               2 (==)
>             129 POP_JUMP_IF_TRUE       138
>         >>  132 LOAD_GLOBAL              5 (AssertionError)
>             135 RAISE_VARARGS            1
>         >>  138 LOAD_CONST               6 (None)
>             141 RETURN_VALUE
>
> Notice that line 6 (the continue) is unreachable, because the else-jump from line 4 has been turned into a jump to bytecode offset 27 (the for loop), and the end of line 5 has also been turned into a jump to 27, rather than letting it flow to line 6.   So line 6 still exists in the bytecode, but is never executed, leading tracing tools to indicate that line 6 is never executed.

So isn't this just a bug in the dead code elimination? Fixing that (so there's no bytecode behind that line and coverage tools can know it has been optimised out) sounds better than adding an obscure config option.

Potentially less risky would be to provide a utility in the dis module to flag such lines after the fact.

Cheers,
Nick.


>
> --Ned.
>
>
> _______________________________________________
> Python-ideas mailing list
> Python-ideas@python.org
> https://mail.python.org/mailman/listinfo/python-ideas
> Code of Conduct: http://python.org/psf/codeofconduct/