Closed gdesmar closed 2 months ago
Thanks for the detailed report, and proposed fix (which is correct).
I have a feeling this may be too naive of an approach.
Happily, the simple approach I think is correct. It will be fixed soon.
There may be deeper issues with allowing _stmts directly into the whilestmt38 codeblock. I may think that it would be part of l_stmts, or that the parser/ast need a tweak to understand that it is a l_stmts instead of a _stmts.
Let me explain the difference between stmts
, _stmts
and l_stmts
; stmts
is one or more statements (stmt
) while _stmts
is zero or more statements. (stmts_opt
might be a clearer name). And l_stmts
was supposed to be stmts
augmented with the kinds of statements that can only be found in loops, such as break
and continue
.
Right now, there is a bit of looseness here in the gramma. The distinction between the various kinds of "stmt" was done a while ago. I am currently in the process of redoing the entire grammar where I hope to tighten things up.
In the way old Python 2.3 code which made its way to Python 2.7 before I picked this up, there were not the reduction checks. That is, instead of:
... (2, ("l_stmts", "l_stmts_opt", "pass", "_stmts")), ...
what written was:
... 2, ...
Over time I have been adding in those additional checks. I sometimes get things wrong though. That's why I work on debuggers.
Description
Pysource's template_engine function triggers an assertion error because it found a
_stmts
instead of one ofl_stmts
,l_stmts_opt
,pass
.How to Reproduce
The example can be generated using python 3.8 and compiling the following, with
python -m compileall brokenwhile.py
The assertion on pysource's template_engine() fails with the follwing message:
Output Given
Expected behavior
See section workarounds: with the proposed workaround, the original code is successfully recovered.
Environment
Uncompyle console output headers:
pydisasm, version 6.1.0
Python version used to compile the bytecode: Python 3.8.19
Executing on Debian 11 (Bullseye)
Workarounds
The cleanest workaround I am seeing is to modify the table for
whilestmt38
in customize_for_version38. (I would personally prefer to have the_stmts
as the first item, if I may, but this is simply to show the modification.)Additional Context
I have a feeling this may be too naive of an approach. There may be deeper issues with allowing
_stmts
directly into thewhilestmt38
codeblock. I may think that it would be part ofl_stmts
, or that the parser/ast need a tweak to understand that it is al_stmts
instead of a_stmts
.If the workaround is the solution, I am more than happy to open a Pull Request for it. If not, and you have a hint where to look into to have a better understanding of the problem, I am interested to help, if needed.