From da4dd398f83d935d4eb8722a505a70362b165476 Mon Sep 17 00:00:00 2001
From: Joe Jevnik <joe@quantopian.com>
Date: Thu, 25 May 2017 04:07:41 -0400
Subject: [PATCH 01/14] support recursive closure cells

---
 cloudpickle/cloudpickle.py | 49 +++++++++++++++++++++++++++++++-------
 tests/cloudpickle_test.py  | 23 +++++++++++++++++-
 2 files changed, 62 insertions(+), 10 deletions(-)

diff --git a/cloudpickle/cloudpickle.py b/cloudpickle/cloudpickle.py
index cb2883f80..8211ab214 100644
--- a/cloudpickle/cloudpickle.py
+++ b/cloudpickle/cloudpickle.py
@@ -69,6 +69,39 @@
     from io import BytesIO as StringIO
     PY3 = True
 
+
+try:
+    from ctypes import pythonapi, py_object, c_int
+except ImportError:
+    supports_recursive_closure = False
+
+    def compress_closure(closure):
+        return closure
+
+    def decompress_closure(compressed_closure):
+        return compressed_closure
+
+    def fill_cells(cells, values):
+        pass
+else:
+    supports_recursive_closure = True
+
+    def compress_closure(closure):
+        return len(closure)
+
+    def decompress_closure(closure):
+        return tuple(_make_cell(None) for _ in range(closure))
+
+    _cell_set = pythonapi.PyCell_Set
+    _cell_set.argtypes = (py_object, py_object)
+    _cell_set.restype = c_int
+
+    def fill_cells(cells, values):
+        if cells is not None:
+            for cell, value in zip(cells, values):
+                _cell_set(cell, value)
+
+
 #relevant opcodes
 STORE_GLOBAL = opcode.opmap['STORE_GLOBAL']
 DELETE_GLOBAL = opcode.opmap['DELETE_GLOBAL']
@@ -305,7 +338,6 @@ def _save_subimports(self, code, top_level_dependencies):
                             # then discards the reference to it
                             self.write(pickle.POP)
 
-
     def save_function_tuple(self, func):
         """  Pickles an actual func object.
 
@@ -335,7 +367,7 @@ def save_function_tuple(self, func):
 
         # create a skeleton function object and memoize it
         save(_make_skel_func)
-        save((code, closure, base_globals))
+        save((code, compress_closure(closure), base_globals))
         write(pickle.REDUCE)
         self.memoize(func)
 
@@ -343,6 +375,7 @@ def save_function_tuple(self, func):
         save(f_globals)
         save(defaults)
         save(dct)
+        save(closure)
         write(pickle.TUPLE)
         write(pickle.REDUCE)  # applies _fill_function on the tuple
 
@@ -799,14 +832,14 @@ def _gen_ellipsis():
 def _gen_not_implemented():
     return NotImplemented
 
-def _fill_function(func, globals, defaults, dict):
+def _fill_function(func, globals, defaults, dict, closure):
     """ Fills in the rest of function data into the skeleton function object
         that were created via _make_skel_func().
          """
     func.__globals__.update(globals)
     func.__defaults__ = defaults
     func.__dict__ = dict
-
+    fill_cells(func.__closure__, closure)
     return func
 
 
@@ -818,19 +851,17 @@ def _reconstruct_closure(values):
     return tuple([_make_cell(v) for v in values])
 
 
-def _make_skel_func(code, closures, base_globals = None):
+def _make_skel_func(code, compressed_closure, base_globals=None):
     """ Creates a skeleton function object that contains just the provided
         code and the correct number of cells in func_closure.  All other
         func attributes (e.g. func_globals) are empty.
     """
-    closure = _reconstruct_closure(closures) if closures else None
-
     if base_globals is None:
         base_globals = {}
     base_globals['__builtins__'] = __builtins__
 
-    return types.FunctionType(code, base_globals,
-                              None, None, closure)
+    closure = decompress_closure(compressed_closure)
+    return types.FunctionType(code, base_globals, None, None, closure)
 
 
 def _find_module(mod_name):
diff --git a/tests/cloudpickle_test.py b/tests/cloudpickle_test.py
index 29deb8cdf..28d1c2e70 100644
--- a/tests/cloudpickle_test.py
+++ b/tests/cloudpickle_test.py
@@ -38,7 +38,7 @@
 from io import BytesIO
 
 import cloudpickle
-from cloudpickle.cloudpickle import _find_module
+from cloudpickle.cloudpickle import _find_module, supports_recursive_closure
 
 from .testutils import subprocess_pickle_echo
 
@@ -133,6 +133,27 @@ def test_nested_lambdas(self):
         f2 = lambda x: f1(x) // b
         self.assertEqual(pickle_depickle(f2)(1), 1)
 
+    @pytest.mark.skipif(
+        not supports_recursive_closure,
+        reason='The C API is needed for recursively defined closures'
+    )
+    def test_recursive_closure(self):
+        def f1():
+            def g():
+                return g
+            return g
+
+        def f2(base):
+            def g(n):
+                return base if n <= 1 else n * g(n - 1)
+            return g
+
+        g1 = pickle_depickle(f1())
+        self.assertEqual(g1(), g1)
+
+        g2 = pickle_depickle(f2(2))
+        self.assertEqual(g2(5), 240)
+
     @pytest.mark.skipif(sys.version_info >= (3, 4)
                         and sys.version_info < (3, 4, 3),
                         reason="subprocess has a bug in 3.4.0 to 3.4.2")

From ba23a20bf42aca0eeaae99f67b0a2e7f85cfdf7a Mon Sep 17 00:00:00 2001
From: Joe Jevnik <joe@quantopian.com>
Date: Thu, 25 May 2017 04:12:46 -0400
Subject: [PATCH 02/14] support unhashable closure values

---
 cloudpickle/cloudpickle.py |  5 ++++-
 tests/cloudpickle_test.py  | 12 ++++++++++++
 2 files changed, 16 insertions(+), 1 deletion(-)

diff --git a/cloudpickle/cloudpickle.py b/cloudpickle/cloudpickle.py
index 8211ab214..8dd5d816f 100644
--- a/cloudpickle/cloudpickle.py
+++ b/cloudpickle/cloudpickle.py
@@ -363,7 +363,10 @@ def save_function_tuple(self, func):
         save(_fill_function)  # skeleton function updater
         write(pickle.MARK)    # beginning of tuple that _fill_function expects
 
-        self._save_subimports(code, set(f_globals.values()) | set(closure))
+        self._save_subimports(
+            code,
+            itertools.chain(f_globals.values(), closure),
+        )
 
         # create a skeleton function object and memoize it
         save(_make_skel_func)
diff --git a/tests/cloudpickle_test.py b/tests/cloudpickle_test.py
index 28d1c2e70..43f8679f0 100644
--- a/tests/cloudpickle_test.py
+++ b/tests/cloudpickle_test.py
@@ -154,6 +154,18 @@ def g(n):
         g2 = pickle_depickle(f2(2))
         self.assertEqual(g2(5), 240)
 
+    def test_unhashable_closure(self):
+        def f():
+            s = set((1, 2))  # mutable set is unhashable
+
+            def g():
+                return len(s)
+
+            return g
+
+        g = pickle_depickle(f())
+        self.assertEqual(g(), 2)
+
     @pytest.mark.skipif(sys.version_info >= (3, 4)
                         and sys.version_info < (3, 4, 3),
                         reason="subprocess has a bug in 3.4.0 to 3.4.2")

From 2c49ba0bfbfcf65bd9e00a27bb106ce5adbbd0c1 Mon Sep 17 00:00:00 2001
From: Joe Jevnik <joe@quantopian.com>
Date: Thu, 25 May 2017 04:45:11 -0400
Subject: [PATCH 03/14] pypy fixes and _cell_set definition update

---
 cloudpickle/cloudpickle.py | 20 ++++++++++++--------
 1 file changed, 12 insertions(+), 8 deletions(-)

diff --git a/cloudpickle/cloudpickle.py b/cloudpickle/cloudpickle.py
index 8dd5d816f..a8aa48b7d 100644
--- a/cloudpickle/cloudpickle.py
+++ b/cloudpickle/cloudpickle.py
@@ -71,7 +71,7 @@
 
 
 try:
-    from ctypes import pythonapi, py_object, c_int
+    from ctypes import pythonapi, py_object, c_int, PYFUNCTYPE
 except ImportError:
     supports_recursive_closure = False
 
@@ -87,14 +87,18 @@ def fill_cells(cells, values):
     supports_recursive_closure = True
 
     def compress_closure(closure):
-        return len(closure)
+        return len(closure) if closure is not None else -1
 
-    def decompress_closure(closure):
-        return tuple(_make_cell(None) for _ in range(closure))
+    def decompress_closure(compressed_closure):
+        return (
+            tuple(_make_cell(None) for _ in range(compressed_closure))
+            if compressed_closure >= 0 else
+            None
+        )
 
-    _cell_set = pythonapi.PyCell_Set
-    _cell_set.argtypes = (py_object, py_object)
-    _cell_set.restype = c_int
+    _cell_set = PYFUNCTYPE(c_int, py_object, py_object)(
+        ('PyCell_Set', pythonapi), ((1, 'cell'), (1, 'value')),
+    )
 
     def fill_cells(cells, values):
         if cells is not None:
@@ -370,7 +374,7 @@ def save_function_tuple(self, func):
 
         # create a skeleton function object and memoize it
         save(_make_skel_func)
-        save((code, compress_closure(closure), base_globals))
+        save((code, compress_closure(func.__closure__), base_globals))
         write(pickle.REDUCE)
         self.memoize(func)
 

From 4b251a062be28dfc8cdb2639285d37e50c9c6fb3 Mon Sep 17 00:00:00 2001
From: Joe Jevnik <joe@quantopian.com>
Date: Thu, 25 May 2017 04:45:24 -0400
Subject: [PATCH 04/14] test the expected failure for pypy

---
 tests/cloudpickle_test.py | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/tests/cloudpickle_test.py b/tests/cloudpickle_test.py
index 43f8679f0..a56aae40d 100644
--- a/tests/cloudpickle_test.py
+++ b/tests/cloudpickle_test.py
@@ -154,6 +154,19 @@ def g(n):
         g2 = pickle_depickle(f2(2))
         self.assertEqual(g2(5), 240)
 
+    @pytest.mark.skipif(
+        supports_recursive_closure,
+        reason="Recursive closures shouldn't raise an exception if supported"
+    )
+    @pytest.mark.xfail
+    def test_recursive_closure_unsupported(self):
+        def f1():
+            def g():
+                return g
+            return g
+
+        pickle_depickle(f1())
+
     def test_unhashable_closure(self):
         def f():
             s = set((1, 2))  # mutable set is unhashable

From 6d8ec33dc24e249657eea93320beef3b9fcb421b Mon Sep 17 00:00:00 2001
From: Joe Jevnik <joe@quantopian.com>
Date: Thu, 25 May 2017 04:53:11 -0400
Subject: [PATCH 05/14] add test for f.__closure__ preservation

---
 tests/cloudpickle_test.py | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/tests/cloudpickle_test.py b/tests/cloudpickle_test.py
index a56aae40d..3d320454d 100644
--- a/tests/cloudpickle_test.py
+++ b/tests/cloudpickle_test.py
@@ -154,6 +154,24 @@ def g(n):
         g2 = pickle_depickle(f2(2))
         self.assertEqual(g2(5), 240)
 
+    @pytest.mark.skipif(
+        not supports_recursive_closure,
+        reason='The C API is needed for recursively defined closures'
+    )
+    def test_closure_none_is_preserved(self):
+        def f():
+            """a function with no closure cells
+            """
+
+        self.assertIsNone(f.__closure__, msg='f actually has closure cells!')
+
+        g = pickle_depickle(f)
+
+        self.assertIsNone(
+            g.__closure__,
+            msg='g now has closure cells even though f does not',
+        )
+
     @pytest.mark.skipif(
         supports_recursive_closure,
         reason="Recursive closures shouldn't raise an exception if supported"

From 9e11439ba59dd9c7013096537176229ddd488abc Mon Sep 17 00:00:00 2001
From: Joe Jevnik <joe@quantopian.com>
Date: Thu, 25 May 2017 04:59:25 -0400
Subject: [PATCH 06/14] 2.6 compat for TestCase.assertIsNone

---
 tests/cloudpickle_test.py | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/tests/cloudpickle_test.py b/tests/cloudpickle_test.py
index 3d320454d..4e50984da 100644
--- a/tests/cloudpickle_test.py
+++ b/tests/cloudpickle_test.py
@@ -163,12 +163,15 @@ def f():
             """a function with no closure cells
             """
 
-        self.assertIsNone(f.__closure__, msg='f actually has closure cells!')
+        self.assertTrue(
+            f.__closure__ is None,
+            msg='f actually has closure cells!',
+        )
 
         g = pickle_depickle(f)
 
-        self.assertIsNone(
-            g.__closure__,
+        self.assertTrue(
+            g.__closure__ is None,
             msg='g now has closure cells even though f does not',
         )
 

From 121604a4ea043c5f79b5a646ef43c449a70a421d Mon Sep 17 00:00:00 2001
From: Joe Jevnik <joe@quantopian.com>
Date: Thu, 25 May 2017 05:02:52 -0400
Subject: [PATCH 07/14] don't send the closure twice on pypy

---
 cloudpickle/cloudpickle.py | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/cloudpickle/cloudpickle.py b/cloudpickle/cloudpickle.py
index a8aa48b7d..b6bbcb183 100644
--- a/cloudpickle/cloudpickle.py
+++ b/cloudpickle/cloudpickle.py
@@ -81,6 +81,9 @@ def compress_closure(closure):
     def decompress_closure(compressed_closure):
         return compressed_closure
 
+    def save_closure(save, closure):
+        pass
+
     def fill_cells(cells, values):
         pass
 else:
@@ -96,6 +99,9 @@ def decompress_closure(compressed_closure):
             None
         )
 
+    def save_closure(save, closure):
+        save(closure)
+
     _cell_set = PYFUNCTYPE(c_int, py_object, py_object)(
         ('PyCell_Set', pythonapi), ((1, 'cell'), (1, 'value')),
     )
@@ -382,7 +388,7 @@ def save_function_tuple(self, func):
         save(f_globals)
         save(defaults)
         save(dct)
-        save(closure)
+        save_closure(save, closure)
         write(pickle.TUPLE)
         write(pickle.REDUCE)  # applies _fill_function on the tuple
 
@@ -839,10 +845,12 @@ def _gen_ellipsis():
 def _gen_not_implemented():
     return NotImplemented
 
-def _fill_function(func, globals, defaults, dict, closure):
+def _fill_function(func, globals, defaults, dict, closure=None):
     """ Fills in the rest of function data into the skeleton function object
         that were created via _make_skel_func().
-         """
+
+        ``closure`` defaults to None because we don't send this value on PyPy.
+    """
     func.__globals__.update(globals)
     func.__defaults__ = defaults
     func.__dict__ = dict

From ac85930bb02a54642e7a2986a4017f005a4c474d Mon Sep 17 00:00:00 2001
From: Joe Jevnik <joe@quantopian.com>
Date: Thu, 25 May 2017 05:17:18 -0400
Subject: [PATCH 08/14] update closure serialization for pypy

---
 cloudpickle/cloudpickle.py | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/cloudpickle/cloudpickle.py b/cloudpickle/cloudpickle.py
index b6bbcb183..b4efed3e3 100644
--- a/cloudpickle/cloudpickle.py
+++ b/cloudpickle/cloudpickle.py
@@ -79,7 +79,11 @@ def compress_closure(closure):
         return closure
 
     def decompress_closure(compressed_closure):
-        return compressed_closure
+        return (
+            tuple(map(_make_cell, compressed_closure))
+            if compressed_closure is not None else
+            None
+        )
 
     def save_closure(save, closure):
         pass
@@ -375,12 +379,12 @@ def save_function_tuple(self, func):
 
         self._save_subimports(
             code,
-            itertools.chain(f_globals.values(), closure),
+            itertools.chain(f_globals.values(), closure or ()),
         )
 
         # create a skeleton function object and memoize it
         save(_make_skel_func)
-        save((code, compress_closure(func.__closure__), base_globals))
+        save((code, compress_closure(closure), base_globals))
         write(pickle.REDUCE)
         self.memoize(func)
 
@@ -443,7 +447,10 @@ def extract_func_data(self, func):
         defaults = func.__defaults__
 
         # process closure
-        closure = [c.cell_contents for c in func.__closure__] if func.__closure__ else []
+        closure = (
+            [c.cell_contents for c in func.__closure__]
+            if func.__closure__ is not None else None
+        )
 
         # save the dict
         dct = func.__dict__

From 55cd6fd12d805c98bfb7a06a80a296ba12e77bf0 Mon Sep 17 00:00:00 2001
From: Joe Jevnik <joe@quantopian.com>
Date: Thu, 25 May 2017 05:17:29 -0400
Subject: [PATCH 09/14] use pytest.raises instead of xfail

---
 tests/cloudpickle_test.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tests/cloudpickle_test.py b/tests/cloudpickle_test.py
index 4e50984da..b8540383e 100644
--- a/tests/cloudpickle_test.py
+++ b/tests/cloudpickle_test.py
@@ -179,14 +179,14 @@ def f():
         supports_recursive_closure,
         reason="Recursive closures shouldn't raise an exception if supported"
     )
-    @pytest.mark.xfail
     def test_recursive_closure_unsupported(self):
         def f1():
             def g():
                 return g
             return g
 
-        pickle_depickle(f1())
+        with pytest.raises(pickle.PicklingError):
+            pickle_depickle(f1())
 
     def test_unhashable_closure(self):
         def f():

From cb524a4bd49a23d69f224247681a7f3098ad748d Mon Sep 17 00:00:00 2001
From: Joe Jevnik <joe@quantopian.com>
Date: Thu, 25 May 2017 15:11:21 -0400
Subject: [PATCH 10/14] support pypy

---
 cloudpickle/cloudpickle.py | 143 ++++++++++++++++++++++++++-----------
 tests/cloudpickle_test.py  |  23 +-----
 2 files changed, 102 insertions(+), 64 deletions(-)

diff --git a/cloudpickle/cloudpickle.py b/cloudpickle/cloudpickle.py
index b4efed3e3..408356693 100644
--- a/cloudpickle/cloudpickle.py
+++ b/cloudpickle/cloudpickle.py
@@ -56,6 +56,7 @@
 import types
 import weakref
 
+
 if sys.version < '3':
     from pickle import Pickler
     try:
@@ -70,50 +71,114 @@
     PY3 = True
 
 
-try:
-    from ctypes import pythonapi, py_object, c_int, PYFUNCTYPE
-except ImportError:
-    supports_recursive_closure = False
+def compress_closure(closure):
+    """Compress the closure by storing only the number of cells.
+    """
+    return len(closure) if closure is not None else -1
 
-    def compress_closure(closure):
-        return closure
 
-    def decompress_closure(compressed_closure):
-        return (
-            tuple(map(_make_cell, compressed_closure))
-            if compressed_closure is not None else
-            None
-        )
+def _make_cell_set_template_code():
+    """Get the Python compiler to emit LOAD_FAST(arg); STORE_DEREF
 
-    def save_closure(save, closure):
-        pass
+    Notes
+    -----
+    In Python 3, we could use an easier function:
 
-    def fill_cells(cells, values):
-        pass
-else:
-    supports_recursive_closure = True
+    .. code-block:: python
+
+       def f():
+           cell = None
+
+           def _stub(value):
+               nonlocal cell
+               cell = value
 
-    def compress_closure(closure):
-        return len(closure) if closure is not None else -1
+           return _stub
 
-    def decompress_closure(compressed_closure):
-        return (
-            tuple(_make_cell(None) for _ in range(compressed_closure))
-            if compressed_closure >= 0 else
-            None
+        _cell_set_template_code = f()
+
+    This function is _only_ a LOAD_FAST(arg); STORE_DEREF, but that is
+    invalid syntax on Python 2. If we use this function we also don't need
+    to do the weird freevars/cellvars swap below
+    """
+    def inner(value):
+        lambda: cell  # make ``cell`` a closure so that we get a STORE_DEREF
+        cell = value
+
+    co = inner.__code__
+
+    # NOTE: we are marking the cell variable as a free variable intentionally
+    # so that we simulate an inner function instead of the outer function. This
+    # is what gives us the ``nonlocal`` behavior in a Python 2 compatible way.
+    if not PY3:
+        return types.CodeType(
+            co.co_argcount,
+            co.co_nlocals,
+            co.co_stacksize,
+            co.co_flags,
+            co.co_code,
+            co.co_consts,
+            co.co_names,
+            co.co_varnames,
+            co.co_filename,
+            co.co_name,
+            co.co_firstlineno,
+            co.co_lnotab,
+            co.co_cellvars,
+            (),
+        )
+    else:
+        return types.CodeType(
+            co.co_argcount,
+            co.co_kwonlyargcount,
+            co.co_nlocals,
+            co.co_stacksize,
+            co.co_flags,
+            co.co_code,
+            co.co_consts,
+            co.co_names,
+            co.co_varnames,
+            co.co_filename,
+            co.co_name,
+            co.co_firstlineno,
+            co.co_lnotab,
+            co.co_cellvars,
+            (),
         )
 
-    def save_closure(save, closure):
-        save(closure)
 
-    _cell_set = PYFUNCTYPE(c_int, py_object, py_object)(
-        ('PyCell_Set', pythonapi), ((1, 'cell'), (1, 'value')),
-    )
+_cell_set_template_code = _make_cell_set_template_code()
+
+
+def _cell_set(cell, value):
+    """Set the value of a closure cell.
+    """
+    return types.FunctionType(
+        _cell_set_template_code,
+        {},
+        '_cell_set_inner',
+        (),
+        (cell,),
+    )(value)
+
 
-    def fill_cells(cells, values):
-        if cells is not None:
-            for cell, value in zip(cells, values):
-                _cell_set(cell, value)
+def fill_cells(cells, values):
+    """If we have a closure, fill the cells with their real values.
+    """
+    if cells is not None:
+        for cell, value in zip(cells, values):
+            _cell_set(cell, value)
+
+
+def decompress_closure(compressed_closure):
+    """Decompress the closure creating ``compressed_closure`` empty cells or
+    returning None.
+    """
+    return (
+        tuple(_make_cell(None) for _ in range(compressed_closure))
+        if compressed_closure >= 0 else
+        None
+    )
 
 
 #relevant opcodes
@@ -392,7 +457,7 @@ def save_function_tuple(self, func):
         save(f_globals)
         save(defaults)
         save(dct)
-        save_closure(save, closure)
+        save(closure)
         write(pickle.TUPLE)
         write(pickle.REDUCE)  # applies _fill_function on the tuple
 
@@ -852,11 +917,9 @@ def _gen_ellipsis():
 def _gen_not_implemented():
     return NotImplemented
 
-def _fill_function(func, globals, defaults, dict, closure=None):
+def _fill_function(func, globals, defaults, dict, closure):
     """ Fills in the rest of function data into the skeleton function object
         that were created via _make_skel_func().
-
-        ``closure`` defaults to None because we don't send this value on PyPy.
     """
     func.__globals__.update(globals)
     func.__defaults__ = defaults
@@ -869,10 +932,6 @@ def _make_cell(value):
     return (lambda: value).__closure__[0]
 
 
-def _reconstruct_closure(values):
-    return tuple([_make_cell(v) for v in values])
-
-
 def _make_skel_func(code, compressed_closure, base_globals=None):
     """ Creates a skeleton function object that contains just the provided
         code and the correct number of cells in func_closure.  All other
diff --git a/tests/cloudpickle_test.py b/tests/cloudpickle_test.py
index b8540383e..72c9bf88b 100644
--- a/tests/cloudpickle_test.py
+++ b/tests/cloudpickle_test.py
@@ -38,7 +38,7 @@
 from io import BytesIO
 
 import cloudpickle
-from cloudpickle.cloudpickle import _find_module, supports_recursive_closure
+from cloudpickle.cloudpickle import _find_module
 
 from .testutils import subprocess_pickle_echo
 
@@ -133,10 +133,6 @@ def test_nested_lambdas(self):
         f2 = lambda x: f1(x) // b
         self.assertEqual(pickle_depickle(f2)(1), 1)
 
-    @pytest.mark.skipif(
-        not supports_recursive_closure,
-        reason='The C API is needed for recursively defined closures'
-    )
     def test_recursive_closure(self):
         def f1():
             def g():
@@ -154,10 +150,6 @@ def g(n):
         g2 = pickle_depickle(f2(2))
         self.assertEqual(g2(5), 240)
 
-    @pytest.mark.skipif(
-        not supports_recursive_closure,
-        reason='The C API is needed for recursively defined closures'
-    )
     def test_closure_none_is_preserved(self):
         def f():
             """a function with no closure cells
@@ -175,19 +167,6 @@ def f():
             msg='g now has closure cells even though f does not',
         )
 
-    @pytest.mark.skipif(
-        supports_recursive_closure,
-        reason="Recursive closures shouldn't raise an exception if supported"
-    )
-    def test_recursive_closure_unsupported(self):
-        def f1():
-            def g():
-                return g
-            return g
-
-        with pytest.raises(pickle.PicklingError):
-            pickle_depickle(f1())
-
     def test_unhashable_closure(self):
         def f():
             s = set((1, 2))  # mutable set is unhashable

From ed7a73cd0973a7bd094969ce103f4fde28bb18aa Mon Sep 17 00:00:00 2001
From: Joe Jevnik <joe@quantopian.com>
Date: Thu, 25 May 2017 17:48:10 -0400
Subject: [PATCH 11/14] clean up names and inline single-use functions

---
 cloudpickle/cloudpickle.py | 58 +++++++++++++++-----------------------
 1 file changed, 23 insertions(+), 35 deletions(-)

diff --git a/cloudpickle/cloudpickle.py b/cloudpickle/cloudpickle.py
index 408356693..98d04b83f 100644
--- a/cloudpickle/cloudpickle.py
+++ b/cloudpickle/cloudpickle.py
@@ -71,12 +71,6 @@
     PY3 = True
 
 
-def compress_closure(closure):
-    """Compress the closure by storing only the number of cells.
-    """
-    return len(closure) if closure is not None else -1
-
-
 def _make_cell_set_template_code():
     """Get the Python compiler to emit LOAD_FAST(arg); STORE_DEREF
 
@@ -150,7 +144,7 @@ def inner(value):
 _cell_set_template_code = _make_cell_set_template_code()
 
 
-def _cell_set(cell, value):
+def cell_set(cell, value):
     """Set the value of a closure cell.
     """
     return types.FunctionType(
@@ -162,25 +156,6 @@ def _cell_set(cell, value):
     )(value)
 
 
-def fill_cells(cells, values):
-    """If we have a closure, fill the cells with their real values.
-    """
-    if cells is not None:
-        for cell, value in zip(cells, values):
-            _cell_set(cell, value)
-
-
-def decompress_closure(compressed_closure):
-    """Decompress the closure creating ``compressed_closure`` empty cells or
-    returning None.
-    """
-    return (
-        tuple(_make_cell(None) for _ in range(compressed_closure))
-        if compressed_closure >= 0 else
-        None
-    )
-
-
 #relevant opcodes
 STORE_GLOBAL = opcode.opmap['STORE_GLOBAL']
 DELETE_GLOBAL = opcode.opmap['DELETE_GLOBAL']
@@ -437,19 +412,23 @@ def save_function_tuple(self, func):
         save = self.save
         write = self.write
 
-        code, f_globals, defaults, closure, dct, base_globals = self.extract_func_data(func)
+        code, f_globals, defaults, closure_values, dct, base_globals = self.extract_func_data(func)
 
         save(_fill_function)  # skeleton function updater
         write(pickle.MARK)    # beginning of tuple that _fill_function expects
 
         self._save_subimports(
             code,
-            itertools.chain(f_globals.values(), closure or ()),
+            itertools.chain(f_globals.values(), closure_values or ()),
         )
 
         # create a skeleton function object and memoize it
         save(_make_skel_func)
-        save((code, compress_closure(closure), base_globals))
+        save((
+            code,
+            len(closure_values) if closure_values is not None else -1,
+            base_globals,
+        ))
         write(pickle.REDUCE)
         self.memoize(func)
 
@@ -457,7 +436,7 @@ def save_function_tuple(self, func):
         save(f_globals)
         save(defaults)
         save(dct)
-        save(closure)
+        save(closure_values)
         write(pickle.TUPLE)
         write(pickle.REDUCE)  # applies _fill_function on the tuple
 
@@ -495,7 +474,7 @@ def extract_code_globals(cls, co):
     def extract_func_data(self, func):
         """
         Turn the function into a tuple of data necessary to recreate it:
-            code, globals, defaults, closure, dict
+            code, globals, defaults, closure_values, dict
         """
         code = func.__code__
 
@@ -917,14 +896,19 @@ def _gen_ellipsis():
 def _gen_not_implemented():
     return NotImplemented
 
-def _fill_function(func, globals, defaults, dict, closure):
+def _fill_function(func, globals, defaults, dict, closure_values):
     """ Fills in the rest of function data into the skeleton function object
         that were created via _make_skel_func().
     """
     func.__globals__.update(globals)
     func.__defaults__ = defaults
     func.__dict__ = dict
-    fill_cells(func.__closure__, closure)
+
+    cells = func.__closure__
+    if cells is not None:
+        for cell, value in zip(cells, closure_values):
+            cell_set(cell, value)
+
     return func
 
 
@@ -932,7 +916,7 @@ def _make_cell(value):
     return (lambda: value).__closure__[0]
 
 
-def _make_skel_func(code, compressed_closure, base_globals=None):
+def _make_skel_func(code, cell_count, base_globals=None):
     """ Creates a skeleton function object that contains just the provided
         code and the correct number of cells in func_closure.  All other
         func attributes (e.g. func_globals) are empty.
@@ -941,7 +925,11 @@ def _make_skel_func(code, compressed_closure, base_globals=None):
         base_globals = {}
     base_globals['__builtins__'] = __builtins__
 
-    closure = decompress_closure(compressed_closure)
+    closure = (
+        tuple(_make_cell(None) for _ in range(cell_count))
+        if cell_count >= 0 else
+        None
+    )
     return types.FunctionType(code, base_globals, None, None, closure)
 
 

From 6656ccaae4d9cb4455e9ae0137ec7323647a48f6 Mon Sep 17 00:00:00 2001
From: Joe Jevnik <joe@quantopian.com>
Date: Thu, 25 May 2017 22:15:55 -0400
Subject: [PATCH 12/14] add comment pointing out my one weird trick for
 mutating a cell

---
 cloudpickle/cloudpickle.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/cloudpickle/cloudpickle.py b/cloudpickle/cloudpickle.py
index 98d04b83f..0760cbe5d 100644
--- a/cloudpickle/cloudpickle.py
+++ b/cloudpickle/cloudpickle.py
@@ -118,7 +118,7 @@ def inner(value):
             co.co_name,
             co.co_firstlineno,
             co.co_lnotab,
-            co.co_cellvars,
+            co.co_cellvars,  # this is the trickery
             (),
         )
     else:
@@ -136,7 +136,7 @@ def inner(value):
             co.co_name,
             co.co_firstlineno,
             co.co_lnotab,
-            co.co_cellvars,
+            co.co_cellvars,  # this is the trickery
             (),
         )
 

From 26c319c77a51ba1811e4b144e304d20b7ec8ee3c Mon Sep 17 00:00:00 2001
From: Joe Jevnik <joe@quantopian.com>
Date: Fri, 26 May 2017 11:46:25 -0400
Subject: [PATCH 13/14] use an empty cell instead of cell(None)

---
 cloudpickle/cloudpickle.py | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/cloudpickle/cloudpickle.py b/cloudpickle/cloudpickle.py
index 0760cbe5d..030d44a3f 100644
--- a/cloudpickle/cloudpickle.py
+++ b/cloudpickle/cloudpickle.py
@@ -912,8 +912,13 @@ def _fill_function(func, globals, defaults, dict, closure_values):
     return func
 
 
-def _make_cell(value):
-    return (lambda: value).__closure__[0]
+def _make_empty_cell():
+    if False:
+        # trick the compiler into creating an empty cell in our lambda
+        cell = None
+        raise AssertionError('this route should not be executed')
+
+    return (lambda: cell).__closure__[0]
 
 
 def _make_skel_func(code, cell_count, base_globals=None):
@@ -926,7 +931,7 @@ def _make_skel_func(code, cell_count, base_globals=None):
     base_globals['__builtins__'] = __builtins__
 
     closure = (
-        tuple(_make_cell(None) for _ in range(cell_count))
+        tuple(_make_empty_cell() for _ in range(cell_count))
         if cell_count >= 0 else
         None
     )

From cf882c6192c3ba5759691fdfe3bf9b9267548cee Mon Sep 17 00:00:00 2001
From: Joe Jevnik <joe@quantopian.com>
Date: Mon, 29 May 2017 20:42:48 -0400
Subject: [PATCH 14/14] add cell manipulation helper unit tests

---
 tests/cloudpickle_test.py | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/tests/cloudpickle_test.py b/tests/cloudpickle_test.py
index 72c9bf88b..19f1faf1f 100644
--- a/tests/cloudpickle_test.py
+++ b/tests/cloudpickle_test.py
@@ -38,7 +38,7 @@
 from io import BytesIO
 
 import cloudpickle
-from cloudpickle.cloudpickle import _find_module
+from cloudpickle.cloudpickle import _find_module, _make_empty_cell, cell_set
 
 from .testutils import subprocess_pickle_echo
 
@@ -494,6 +494,19 @@ def example():
                    "'))()")
         assert not subprocess.call([sys.executable, '-c', command])
 
+    def test_cell_manipulation(self):
+        cell = _make_empty_cell()
+
+        with pytest.raises(ValueError):
+            cell.cell_contents
+
+        ob = object()
+        cell_set(cell, ob)
+        self.assertTrue(
+            cell.cell_contents is ob,
+            msg='cell contents not set correctly',
+        )
+
 
 if __name__ == '__main__':
     unittest.main()