Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for CPython 3.5+ #24

Closed
wants to merge 1 commit into from
Closed

Conversation

Vgr255
Copy link
Contributor

@Vgr255 Vgr255 commented Oct 26, 2015

The package fails on import with a AttributeError: module 'codetransformer.instructions' has no attribute 'STORE_MAP'. After some debugging, digging and research, I found out that this specific opcode was removed in CPython 3.5. I am proposing a (hopefully temporary) fix to this. This allows the package to not fail on import, but the code might fail in other places. If there's a better way to fix this, I'll be glad to do so.

I really love this package. Expect to see a lot of contributions from me in the coming days/weeks.

@llllllllll
Copy link
Owner

Hmm, I think the correct fix would be to look into what a dict comprehension compiles into in 3.5 and then use that instead of just patching it out. This would cause the tests to fail.

@Vgr255
Copy link
Contributor Author

Vgr255 commented Oct 26, 2015

Alright, I wasn't sure what this opcode was about. I'll look into it and update the PR.

On an unrelated note, are you on IRC? This would be really convenient.

@llllllllll
Copy link
Owner

We are not. Currently @ssanderson and I work together in person so there was not really a need. We could set something up if you would like. Thanks for showing interest in codetransformer

@Vgr255
Copy link
Contributor Author

Vgr255 commented Oct 26, 2015

It's up to you. I set up a ##codetransformer channel on irc.freenode.net if you're interested.

I'm currently trying to understand the code a bit more so I can properly fix it. A dict comprehension is the equivalent of a generator comprehension with a dict() cast on it (it does have a CALL_FUNCTION opcode).

@ssanderson
Copy link
Collaborator

I think dict comprehensions are more interesting than you think: they get compiled into a local function that creates an empty map and then does a sequence of MAP_ADD instructions. The newly-added d function in the top-level namespace is an easy way to recursively inspect code objects.

In [1]: from codetransformer import d

In [2]: d("def foo(): return {a:b for a, b in c}")
<module>
--------
  1           0 LOAD_CONST               0 (<code object foo at 0x7f37dfa93810, file "<show>", line 1>)
              3 LOAD_CONST               1 ('foo')
              6 MAKE_FUNCTION            0
              9 STORE_NAME               0 (foo)
             12 LOAD_CONST               2 (None)
             15 RETURN_VALUE

<module>.foo
------------
  1           0 LOAD_CONST               1 (<code object <dictcomp> at 0x7f37e0605780, file "<show>", line 1>)
              3 LOAD_CONST               2 ('foo.<locals>.<dictcomp>')
              6 MAKE_FUNCTION            0
              9 LOAD_GLOBAL              0 (c)
             12 GET_ITER
             13 CALL_FUNCTION            1 (1 positional, 0 keyword pair)
             16 RETURN_VALUE

<module>.foo.<locals>.<dictcomp>
--------------------------------
  1           0 BUILD_MAP                0
              3 LOAD_FAST                0 (.0)
        >>    6 FOR_ITER                21 (to 30)
              9 UNPACK_SEQUENCE          2
             12 STORE_FAST               1 (a)
             15 STORE_FAST               2 (b)
             18 LOAD_FAST                2 (b)
             21 LOAD_FAST                1 (a)
             24 MAP_ADD                  2
             27 JUMP_ABSOLUTE            6
        >>   30 RETURN_VALUE

The above says that the string "def foo(): return {a:b for a, b in c}", generates a code object named <module>, which has a code constant named <module>.foo, which has a code constant named <module>.foo.<dictcomp>. That last one is the bytecode for the actual comprehension.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants