[Python-checkins] bpo-33947: dataclasses no longer can raise RecursionError in repr (GF9916)

[Eric V. Smith]

https://github.com/python/cpython/commit/dd13c88b5371e13fc16b84e2f9b8715d917de269
commit: dd13c88b5371e13fc16b84e2f9b8715d917de269
branch: master
author: Srinivas  Thatiparthy (శ్రీనివాస్  తాటిపర్తి) <srinivasreddy at users.noreply.github.com>
committer: Eric V. Smith <ericvsmith at users.noreply.github.com>
date: 2018-10-19T12:54:50-04:00
summary:

bpo-33947:  dataclasses no longer can raise RecursionError in repr (GF9916)

The reprlib code was copied here instead of importing reprlib. I'm not sure if we really need to avoid the import, but since I expect dataclasses to be more common that reprlib, it seems wise. Plus, the code is small.

files:
A Misc/NEWS.d/next/Library/2018-10-17-02-15-23.bpo-33947.SRuq3T.rst
M Lib/dataclasses.py
M Lib/test/test_dataclasses.py

diff --git a/Lib/dataclasses.py b/Lib/dataclasses.py
index 28e9f75127b1..71d9896a1052 100644
--- a/Lib/dataclasses.py
+++ b/Lib/dataclasses.py
@@ -5,6 +5,9 @@
 import inspect
 import keyword
 import builtins
+import functools
+import _thread
+
 
 __all__ = ['dataclass',
            'field',
@@ -337,6 +340,27 @@ def _tuple_str(obj_name, fields):
     return f'({",".join([f"{obj_name}.{f.name}" for f in fields])},)'
 
 
+# This function's logic is copied from "recursive_repr" function in
+# reprlib module to avoid dependency.
+def _recursive_repr(user_function):
+    # Decorator to make a repr function return "..." for a recursive
+    # call.
+    repr_running = set()
+
+    @functools.wraps(user_function)
+    def wrapper(self):
+        key = id(self), _thread.get_ident()
+        if key in repr_running:
+            return '...'
+        repr_running.add(key)
+        try:
+            result = user_function(self)
+        finally:
+            repr_running.discard(key)
+        return result
+    return wrapper
+
+
 def _create_fn(name, args, body, *, globals=None, locals=None,
                return_type=MISSING):
     # Note that we mutate locals when exec() is called.  Caller
@@ -497,12 +521,13 @@ def _init_fn(fields, frozen, has_post_init, self_name):
 
 
 def _repr_fn(fields):
-    return _create_fn('__repr__',
-                      ('self',),
-                      ['return self.__class__.__qualname__ + f"(' +
-                       ', '.join([f"{f.name}={{self.{f.name}!r}}"
-                                  for f in fields]) +
-                       ')"'])
+    fn = _create_fn('__repr__',
+                    ('self',),
+                    ['return self.__class__.__qualname__ + f"(' +
+                     ', '.join([f"{f.name}={{self.{f.name}!r}}"
+                                for f in fields]) +
+                     ')"'])
+    return _recursive_repr(fn)
 
 
 def _frozen_get_del_attr(cls, fields):
diff --git a/Lib/test/test_dataclasses.py b/Lib/test/test_dataclasses.py
index 6efe785bc328..ff6060c6d283 100755
--- a/Lib/test/test_dataclasses.py
+++ b/Lib/test/test_dataclasses.py
@@ -3169,6 +3169,92 @@ def __post_init__(self, y):
             replace(c, x=3)
         c = replace(c, x=3, y=5)
         self.assertEqual(c.x, 15)
+
+    def test_recursive_repr(self):
+        @dataclass
+        class C:
+            f: "C"
+
+        c = C(None)
+        c.f = c
+        self.assertEqual(repr(c), "TestReplace.test_recursive_repr.<locals>.C(f=...)")
+
+    def test_recursive_repr_two_attrs(self):
+        @dataclass
+        class C:
+            f: "C"
+            g: "C"
+
+        c = C(None, None)
+        c.f = c
+        c.g = c
+        self.assertEqual(repr(c), "TestReplace.test_recursive_repr_two_attrs"
+                                  ".<locals>.C(f=..., g=...)")
+
+    def test_recursive_repr_indirection(self):
+        @dataclass
+        class C:
+            f: "D"
+
+        @dataclass
+        class D:
+            f: "C"
+
+        c = C(None)
+        d = D(None)
+        c.f = d
+        d.f = c
+        self.assertEqual(repr(c), "TestReplace.test_recursive_repr_indirection"
+                                  ".<locals>.C(f=TestReplace.test_recursive_repr_indirection"
+                                  ".<locals>.D(f=...))")
+
+    def test_recursive_repr_indirection_two(self):
+        @dataclass
+        class C:
+            f: "D"
+
+        @dataclass
+        class D:
+            f: "E"
+
+        @dataclass
+        class E:
+            f: "C"
+
+        c = C(None)
+        d = D(None)
+        e = E(None)
+        c.f = d
+        d.f = e
+        e.f = c
+        self.assertEqual(repr(c), "TestReplace.test_recursive_repr_indirection_two"
+                                  ".<locals>.C(f=TestReplace.test_recursive_repr_indirection_two"
+                                  ".<locals>.D(f=TestReplace.test_recursive_repr_indirection_two"
+                                  ".<locals>.E(f=...)))")
+
+    def test_recursive_repr_two_attrs(self):
+        @dataclass
+        class C:
+            f: "C"
+            g: "C"
+
+        c = C(None, None)
+        c.f = c
+        c.g = c
+        self.assertEqual(repr(c), "TestReplace.test_recursive_repr_two_attrs"
+                                  ".<locals>.C(f=..., g=...)")
+
+    def test_recursive_repr_misc_attrs(self):
+        @dataclass
+        class C:
+            f: "C"
+            g: int
+
+        c = C(None, 1)
+        c.f = c
+        self.assertEqual(repr(c), "TestReplace.test_recursive_repr_misc_attrs"
+                                  ".<locals>.C(f=..., g=1)")
+
     ## def test_initvar(self):
     ##     @dataclass
     ##     class C:
diff --git a/Misc/NEWS.d/next/Library/2018-10-17-02-15-23.bpo-33947.SRuq3T.rst b/Misc/NEWS.d/next/Library/2018-10-17-02-15-23.bpo-33947.SRuq3T.rst
new file mode 100644
index 000000000000..bf08bac13cc7
--- /dev/null
+++ b/Misc/NEWS.d/next/Library/2018-10-17-02-15-23.bpo-33947.SRuq3T.rst
@@ -0,0 +1 @@
+dataclasses now handle recursive reprs without raising RecursionError.