
This issue trackerhas been migrated toGitHub, and is currentlyread-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.
Created on2016-04-29 06:50 byvstinner, last changed2022-04-11 14:58 byadmin. This issue is nowclosed.
| Files | ||||
|---|---|---|---|---|
| File name | Uploaded | Description | Edit | |
| dis_unpack_args.patch | serhiy.storchaka,2016-04-29 07:43 | review | ||
| dis_unpack_args_2.patch | serhiy.storchaka,2016-05-04 07:43 | review | ||
| dis_unpack_args_3.patch | serhiy.storchaka,2016-05-05 11:53 | review | ||
| Messages (11) | |||
|---|---|---|---|
| msg264468 -(view) | Author: STINNER Victor (vstinner)*![]() | Date: 2016-04-29 06:50 | |
The scan_opcodes_25() method of the modulefinder module implements a disassembler of Python bytecode. The implementation is incomplete, it doesn't support EXTENDED_ARG. I suggest to drop the disassembler and reuse the dis module.See also the issue#26647 "Wordcode" changes the bytecode. | |||
| msg264471 -(view) | Author: Serhiy Storchaka (serhiy.storchaka)*![]() | Date: 2016-04-29 07:43 | |
Proposed patch adds private helper function dis._unpack_args() that unpacks long arguments. This function is now reused in two places in dis and in modulefinder. | |||
| msg264645 -(view) | Author: Serhiy Storchaka (serhiy.storchaka)*![]() | Date: 2016-05-02 12:50 | |
This is pretty simple patch and I'm going to push it in short time if there are no objections. This can make the patch forissue26647 simpler since handling extended args is isolated in one function.Alternative names for _unpack_args() are welcome.An alternative way is to use public function get_instructions(), but this adds more overhead and don't work for 2.7. | |||
| msg264787 -(view) | Author: Serhiy Storchaka (serhiy.storchaka)*![]() | Date: 2016-05-04 07:43 | |
modulefinder in 2.7 should be compatible with Python 2.2 (seePEP 291). Thus we can't just add new function in dis and reuse it in modulefinder. We should duplicate this function in modulefinder.I don't know if there is corresponding rule for 3.x. Should modulefinder keep compatibility with say 3.1? | |||
| msg264870 -(view) | Author: Meador Inge (meador.inge)*![]() | Date: 2016-05-05 01:50 | |
Overall, this patch LGTM. Some new tests would be nice. Especially since the NEWS entry claims that we now support extended args.As for for the compatibility concerns, I don't know. | |||
| msg264905 -(view) | Author: Serhiy Storchaka (serhiy.storchaka)*![]() | Date: 2016-05-05 11:53 | |
Added test.But this test is fragile. Clever optimizer can made it passing even with modulefinder not supporting extended args (I'm going to provide such optimization). | |||
| msg265168 -(view) | Author: Roundup Robot (python-dev)![]() | Date: 2016-05-08 21:02 | |
New changeset3579cdaf56d9 by Serhiy Storchaka in branch '3.5':Issue#26881: The modulefinder module now supports extended opcode arguments.https://hg.python.org/cpython/rev/3579cdaf56d9New changesetc27e3773d0f9 by Serhiy Storchaka in branch 'default':Issue#26881: The modulefinder module now supports extended opcode arguments.https://hg.python.org/cpython/rev/c27e3773d0f9New changesetf06baed1bb0c by Serhiy Storchaka in branch '2.7':Issue#26881: modulefinder now works with bytecode with extended args.https://hg.python.org/cpython/rev/f06baed1bb0c | |||
| msg265332 -(view) | Author: STINNER Victor (vstinner)*![]() | Date: 2016-05-11 17:58 | |
It looks like the change is mentionned twice inMisc/NEWS.IMHO it's worth to add an optional argument to skip extended opcodes in_unpack_opcodes() to simplify the code in modulefinder.Is the scancode method public? If yes, I'm not sure that it's ok to renameit in a minor release.Thanks for the change, it prepares the work for wordcode. | |||
| msg265338 -(view) | Author: Roundup Robot (python-dev)![]() | Date: 2016-05-11 19:21 | |
New changesetf0a4d563b32e by Serhiy Storchaka in branch '3.5':Issue#26881: Restored the name of scan_opcodes_25().https://hg.python.org/cpython/rev/f0a4d563b32e | |||
| msg265341 -(view) | Author: Roundup Robot (python-dev)![]() | Date: 2016-05-11 19:26 | |
New changesetd60040b3bb8c by Serhiy Storchaka in branch '3.5':Removed duplicated NEWS entity for issue#26881.https://hg.python.org/cpython/rev/d60040b3bb8c | |||
| msg265342 -(view) | Author: Serhiy Storchaka (serhiy.storchaka)*![]() | Date: 2016-05-11 19:30 | |
Thank you Victor. Fixed.> IMHO it's worth to add an optional argument to skip extended opcodes in_unpack_opcodes() to simplify the code in modulefinder.But this would complicate the code in dis.If the disassembler would changed to skip extended opcodes, we would add this option in _unpack_opargs().If there would be some Python 3 analogue ofPEP 291, modulefinder would be changed to use public dis API. | |||
| History | |||
|---|---|---|---|
| Date | User | Action | Args |
| 2022-04-11 14:58:30 | admin | set | github: 71068 |
| 2016-06-05 15:30:34 | serhiy.storchaka | link | issue26448 superseder |
| 2016-05-11 19:30:44 | serhiy.storchaka | set | messages: +msg265342 |
| 2016-05-11 19:26:04 | python-dev | set | messages: +msg265341 |
| 2016-05-11 19:21:37 | python-dev | set | messages: +msg265338 |
| 2016-05-11 17:58:13 | vstinner | set | messages: +msg265332 |
| 2016-05-08 21:03:49 | serhiy.storchaka | set | status: open -> closed resolution: fixed stage: patch review -> resolved |
| 2016-05-08 21:02:33 | python-dev | set | nosy: +python-dev messages: +msg265168 |
| 2016-05-05 11:53:43 | serhiy.storchaka | set | files: +dis_unpack_args_3.patch messages: +msg264905 |
| 2016-05-05 01:50:21 | meador.inge | set | nosy: +meador.inge messages: +msg264870 |
| 2016-05-04 07:43:50 | serhiy.storchaka | set | files: +dis_unpack_args_2.patch nosy: +theller,jvr messages: +msg264787 |
| 2016-05-02 12:50:06 | serhiy.storchaka | set | assignee:serhiy.storchaka messages: +msg264645 versions: + Python 2.7, Python 3.5 |
| 2016-04-30 16:07:32 | serhiy.storchaka | link | issue26647 dependencies |
| 2016-04-29 07:43:34 | serhiy.storchaka | set | files: +dis_unpack_args.patch messages: +msg264471 components: + Library (Lib) keywords: +patch type: behavior stage: patch review |
| 2016-04-29 06:50:49 | vstinner | create | |