This thread is based on https://bugs.python.org/issue37500
to get the thoughts of a broader range of people.
On 7/5/2019 5:28 PM, Pablo Galindo Salgado wrote:
Recently, we moved the optimization for the removal of dead code of the form
if 0:
....
to the ast
My impression is that you stopped at least the (complete) removal of
such code.
An example use case for "if 0:"
if 0:
<code for algorithm version A>
else:
<code for algorithm version B>
It is easy to switch between branches. Both branches should pass and be
covered by tests, but this is impossible when using a constant, while
using a constant results in leaner, cleaner .pyc.
(I combined here paraphrases of Tim Peters' msg347383 concerns and Net
Batchhelder's msg347296 coverage concerns.)
so we use JUMP bytecodes instead (being completed in
Are you leaving the code and just jumping over it?
PR14116). The reason is that
currently, any syntax error in the block will never be reported. For
example:
if 0:
return
if 1:
pass
else:
return
while 0:
return
at module level do not raise any syntax error (just some examples), In
https://bugs.python.org/issue37500 it was reported
that after that, code coverage will decrease as coverage.py sees these
new bytecodes (even if they are not executed). In general,
the code object is a bit bigger and the optimization now it requires an
JUMP instruction to be executed, but syntax errors are reported.
The discussion on issue 37500 is about if we should prioritize the
optimization or the correctness of reporting syntax errors.
This is only an issue if we cannot do both.
In my opinion,
SyntaxErrors should be reported with independence of the value of
variables (__debug__)
__debug__ is defined as a boolean constant, set on startup
or constant as is a property of the code being written
not of the code being executed. Also, as CPython is the reference
implementation of Python, the danger here is that it could be
interpreted that
this optimization is part of the language and its behavior should be
mirrored in every other Python implementation. Elsewhere we have always
prioritize correctness over speed or optimizations.
I am not sure I see your point here. The python language documentation
for __debug__ and assert make no mention of CPython.
https://docs.python.org/3/library/constants.html#__debug__
https://docs.python.org/3/reference/simple_stmts.html#the-assert-statement
As I said on the issue, it is not unreasonable to me to infer from the
above that skipping 'if 0' clauses is at least close to being a language
feature, and certainly not a minor CPython implementation detail to be
casually changed.
https://docs.python.org/3/using/cmdline.html does say
"Other implementations’ command line schemes may differ."
but you are patching CPython, not some other implementation.
https://docs.python.org/3/using/cmdline.html#cmdoption-o
says -O removes "assert statements and any code conditional on the value
of __debug__". Your initial patch appears to violate this. As for
other implementations, I would think that any implementation with -O or
the claimed equivalent should do the same.
I am writing this email to know what other people think. Should we
revert the change and not report Syntax Errors on optimized blocks? Someone
sees a viable way of reporting the errors and not emitting the bytecode
for these block?
I presume that most of us agree that the last option, doing both, would
be best, or at least agreeable. So I prefer that this be explored more
before we fight too hard over a binary choice than might not be
necessary. Right now, I would say that the 15-year status quo, bytecode
deletion, should remain for 3.8 and that the Steering Council should
ultimately decide for 3.9 should a decision be necessary.
--
Terry Jan Reedy
_______________________________________________
Python-Dev mailing list -- python-dev@python.org
To unsubscribe send an email to python-dev-le...@python.org
https://mail.python.org/mailman3/lists/python-dev.python.org/
Message archived at
https://mail.python.org/archives/list/python-dev@python.org/message/HDGHKDECPO5IUNKGSUGHF7YJBN25ACKN/
