Support freevars and methods that call mangled methods #68

graingert · 2017-03-09T19:45:39Z

Fixes #67

graingert · 2017-03-09T19:46:17Z

patchy/api.py

+
+
+def _indent(new_source):
+    return '\n'.join(['    ' + x for x in new_source.split('\n')])


this is broken in so many ways. It will mess up docstrings and functions that use tabs in py3k.

mess up docstrings only slightly?

Also yes there will be a TabError, you could add a test for that

graingert · 2017-03-09T19:49:29Z

patchy/api.py

    # Fetch the actual function we are changing
+    class_name = _class_name(func)
+    if class_name:
+        new_source = 'class {name}(object):\n{code}'.format(


might be better to parse the code and do a find and replace with identifiers.

Turns out mangling works everywhere:

def _Artist__foo(): return 'yo!' class Artist(object): def method(self): return __foo() def test_mangle(): assert Artist().method() == 'yo!'

find/replace sounds even more hacky

also that test_mangle is a bit of a WAT🦆

adamchainz · 2017-03-10T17:53:06Z

tests/tabs_mangled.py

@@ -0,0 +1,7 @@
+class Artist:


pytest tmpdir instead to keep this inside the test function? you can always write \t in the string. just wary tabs might get complained about by future lint rules

it's complained about by current lint rules

yeah used the pytest tmpdir.

adamchainz · 2017-03-10T17:55:05Z

tests/test_patchy.py

+
+        assert Artist().method() == "Cheese on toast"
+
+    @pytest.mark.xfail


why does this fail? because the new method is compiled without the original _Artist__mangled_name in scope?

yup, needed to fix freevars/closures. Interestingly it doesn't need to be the original _Artist__mangled_name, just any var with the same name.

adamchainz · 2017-03-10T17:56:15Z

patchy/api.py

@@ -148,25 +149,57 @@ def _get_source(func):
        return source


-def _set_source(func, new_source):
+def _class_name(func):


there's nothing in the stnadard library for this? 😢

func.im_class.__name__ but it was replaced with __qualname__ because __qualname__ is 'easier'

Yeah I'm just sad so much parsing is needed on __qualname__

graingert · 2017-03-10T18:49:28Z

@adamchainz worth splitting out the freevars stuff into a separate PR?

graingert · 2017-03-11T13:47:00Z

@adamchainz I think it's a good idea to release this as v2 just in case it breaks anyone

adamchainz · 2017-03-11T21:18:42Z

tests/base.py


 import patchy.api


-class PatchyTestCase(unittest.TestCase):
-    def setUp(self):
+class PatchyTestCase(object):


can you put this unittest -> pytest stuff in a separate PR please, it's adding noise to this commit

adamchainz · 2017-03-11T21:19:11Z

tests/test_patchy.py

+
+        assert Artist().method() == "Cheese on toast"
+
+    @unittest.skipUnless(six.PY3, "Python 3")


the explanation should point out that it's super() that's Python 3 only

adamchainz · 2017-03-11T21:20:37Z

patchy/api.py

+            dont_inherit=True,
+        )
+
+    def _parse(code):


this doesn't need to be an inner function

It does to call _compile with the feature_flags freevar.

so it does, I think I read it as calling compile, my bad

adamchainz · 2017-03-11T21:22:04Z

patchy/api.py

+    def _parse(code):
+        return _compile(code, flags=ast.PyCF_ONLY_AST)
+
+    class_name = _class_name(func)


there is so much going on here now, i think we need it splitting into more functions and some comments added

I've split it into more functions and added doc strings.

adamchainz · 2017-03-11T21:25:16Z

tests/test_patchy.py

+
+        assert sample()() == "Cheese on toast"
+
+    @pytest.mark.xfail(raises=NameError)


I would be surprised if this could ever be made to work, but I guess never say never...

I'm pretty sure it could be done by digging around in the garbage collector. But it would be unreliable

by adding empty statements that force the use of all freevars.

Fixes #31 and #67.

adamchainz · 2017-03-13T15:25:23Z

Merged in 3de5de6, thanks!

graingert commented Mar 9, 2017

View reviewed changes

adamchainz changed the title ~~WIP: support patching methods that call mangled methods Fixes #67~~ WIP: support patching methods that call mangled methods Mar 9, 2017

adamchainz reviewed Mar 10, 2017

View reviewed changes

graingert force-pushed the methods-call-mangled branch from 86aedaa to e84b599 Compare March 10, 2017 18:27

graingert mentioned this pull request Mar 10, 2017

Patching an __init__ that calls super() doesn't work #31

Closed

graingert force-pushed the methods-call-mangled branch 6 times, most recently from c3cf28c to 9087232 Compare March 11, 2017 12:53

graingert changed the title ~~WIP: support patching methods that call mangled methods~~ support patching methods that call mangled methods. Bonus: also support freevars Mar 11, 2017

graingert force-pushed the methods-call-mangled branch 5 times, most recently from 036880f to 9087232 Compare March 11, 2017 14:45

adamchainz reviewed Mar 11, 2017

View reviewed changes

graingert force-pushed the methods-call-mangled branch 3 times, most recently from cc4743d to 9087232 Compare March 13, 2017 10:05

graingert added 4 commits March 13, 2017 15:23

support patching methods that call mangled methods Fixes adamchainz#67

fd02557

use ast to add class

7212d3c

support mangled freevars

9ab7cf6

add test for freevars in func

f9b2e69

graingert added 8 commits March 13, 2017 15:23

freevars for regular functions

4fd2df4

support super() in __init__ methods Fixes adamchainz#31

fe2c7f7

add tests for freevar ordering and nested freevars

19bc787

test removed freevars

2f0b6fa

add test for function attempting to close over new var

722354b

support patched functions that drop use of a freevar

f101af3

by adding empty statements that force the use of all freevars.

slightly more elegant approach to extract compiled method

2d5d75b

split ast-rewriting code into separate functions

d98d403

adamchainz changed the title ~~support patching methods that call mangled methods. Bonus: also support freevars~~ Support freevars and methods that call mangled methods Mar 13, 2017

graingert force-pushed the methods-call-mangled branch 2 times, most recently from d98d403 to fd02557 Compare March 13, 2017 15:23

adamchainz pushed a commit that referenced this pull request Mar 13, 2017

Support freevars and methods that call mangled methods (#68)

3de5de6

Fixes #31 and #67.

adamchainz closed this Mar 13, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support freevars and methods that call mangled methods #68

Support freevars and methods that call mangled methods #68

graingert commented Mar 9, 2017 •

edited by adamchainz

graingert Mar 9, 2017

adamchainz Mar 9, 2017

graingert Mar 9, 2017

adamchainz Mar 9, 2017

adamchainz Mar 10, 2017

graingert Mar 10, 2017

graingert Mar 11, 2017

adamchainz Mar 10, 2017

graingert Mar 11, 2017 •

edited

adamchainz Mar 10, 2017

graingert Mar 10, 2017

adamchainz Mar 11, 2017

graingert commented Mar 10, 2017

graingert commented Mar 11, 2017

adamchainz Mar 11, 2017

adamchainz Mar 11, 2017

adamchainz Mar 11, 2017

graingert Mar 13, 2017

adamchainz Mar 13, 2017

adamchainz Mar 11, 2017

graingert Mar 13, 2017

adamchainz Mar 11, 2017

graingert Mar 11, 2017

adamchainz commented Mar 13, 2017



		def _indent(new_source):
		return '\n'.join([' ' + x for x in new_source.split('\n')])


		assert Artist().method() == "Cheese on toast"

		@pytest.mark.xfail


		assert Artist().method() == "Cheese on toast"

		@unittest.skipUnless(six.PY3, "Python 3")


		assert sample()() == "Cheese on toast"

		@pytest.mark.xfail(raises=NameError)

Support freevars and methods that call mangled methods #68

Support freevars and methods that call mangled methods #68

Conversation

graingert commented Mar 9, 2017 • edited by adamchainz

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

graingert Mar 11, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

graingert commented Mar 10, 2017

graingert commented Mar 11, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

adamchainz commented Mar 13, 2017

graingert commented Mar 9, 2017 •

edited by adamchainz

graingert Mar 11, 2017 •

edited