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.

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.

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 for issue26647 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.

modulefinder in 2.7 should be compatible with Python 2.2 (see PEP 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?

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.

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).

New changeset 3579cdaf56d9 by Serhiy Storchaka in branch '3.5':
Issue #26881: The modulefinder module now supports extended opcode arguments.
https://hg.python.org/cpython/rev/3579cdaf56d9

New changeset c27e3773d0f9 by Serhiy Storchaka in branch 'default':
Issue #26881: The modulefinder module now supports extended opcode arguments.
https://hg.python.org/cpython/rev/c27e3773d0f9

New changeset f06baed1bb0c by Serhiy Storchaka in branch '2.7':
Issue #26881: modulefinder now works with bytecode with extended args.
https://hg.python.org/cpython/rev/f06baed1bb0c

It looks like the change is mentionned twice in Misc/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 rename
it in a minor release.

Thanks for the change, it prepares the work for wordcode.

New changeset f0a4d563b32e by Serhiy Storchaka in branch '3.5':
Issue #26881: Restored the name of scan_opcodes_25().
https://hg.python.org/cpython/rev/f0a4d563b32e

New changeset d60040b3bb8c by Serhiy Storchaka in branch '3.5':
Removed duplicated NEWS entity for issue #26881.
https://hg.python.org/cpython/rev/d60040b3bb8c

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 of PEP 291, modulefinder would be changed to use public dis API.

messages: + msg264905

components: + Library (Lib)
keywords: + patch
type: behavior
stage: patch review