Address review

Author: sergey-miryanov

Lib/test/test_tuple.py

@@ -294,6 +294,9 @@ def test_repr(self):
     def _not_tracked_instantly(self, t):
         self.assertFalse(gc.is_tracked(t), t)
 
+    def _tracked_instantly(self, t):
+        self.assertTrue(gc.is_tracked(t), t)
+
     # Checks that t is not tracked after GC collection.
     def _not_tracked(self, t):
         # Nested tuples can take several collections to untrack
@@ -323,10 +326,14 @@ def test_track_literals(self):
         self._not_tracked_instantly((object(),))
 
         # Test for _PyTuple_Concat
-        self._not_tracked_instantly((1, 2) + (2, 3))
+        self._not_tracked_instantly((1, 2) + (2, 3, 4))
+        self._not_tracked_instantly((x, y,) + (2, 3))
+        self._tracked_instantly((3, 4, 5, [],) + (2, 3))
 
         # Test for _PyTuple_Repeat
         self._not_tracked_instantly((1, 2) * 5)
+        self._not_tracked_instantly((1, x) * 5)
+        self._tracked_instantly((1, []) * 5)
 
         self._not_tracked(((1, x), y, (2, 3)))
         self._not_tracked((1, 2, (None, True, False, ()), int))

Python/flowgraph.c

@@ -12,7 +12,6 @@
 #include "pycore_opcode_metadata.h" // OPCODE_HAS_ARG, etc
 #include "pycore_pystate.h"         // _PyInterpreterState_GET()
 #include "pycore_stackref.h"        // PyStackRef_AsPyObjectBorrow()
-#include "pycore_tuple.h"           // _PyTuple_MaybeUntrack()
 
 #include <stdbool.h>
 
@@ -1553,6 +1552,8 @@ fold_tuple_of_constants(basicblock *bb, int i, PyObject *consts,
     if (const_tuple == NULL) {
         return ERROR;
     }
+    PyObject_GC_UnTrack(const_tuple);
+    bool track_tuple = false;
 
     for (int i = 0; i < seq_size; i++) {
         cfg_instr *inst = const_instrs[i];
@@ -1563,8 +1564,13 @@ fold_tuple_of_constants(basicblock *bb, int i, PyObject *consts,
             return ERROR;
         }
         PyTuple_SET_ITEM(const_tuple, i, element);
+        if (!track_tuple && PyObject_GC_IsTracked(element)) {
+            track_tuple = true;
+        }
+    }
+    if (track_tuple) {
+        _PyObject_GC_TRACK(const_tuple);
     }
-    _PyTuple_MaybeUntrack(const_tuple);
 
     nop_out(const_instrs, seq_size);
     return instr_make_load_const(instr, const_tuple, consts, const_cache, consts_index);
@@ -1634,6 +1640,8 @@ fold_constant_seq_into_load_const(basicblock *bb, int i,
             if (newconst == NULL) {
                 return ERROR;
             }
+            PyObject_GC_UnTrack(newconst);
+            bool track_tuple = false;
 
             int newpos_start = expected_append ? i - 1 : i;
             for (int newpos = newpos_start; newpos >= pos; newpos--) {
@@ -1649,10 +1657,15 @@ fold_constant_seq_into_load_const(basicblock *bb, int i,
                     }
                     assert(consts_found > 0);
                     PyTuple_SET_ITEM(newconst, --consts_found, constant);
+                    if (!track_tuple && PyObject_GC_IsTracked(constant)) {
+                        track_tuple = true;
+                    }
                 }
                 nop_out(&instr, 1);
             }
-            _PyTuple_MaybeUntrack(newconst);
+            if (track_tuple) {
+                _PyObject_GC_TRACK(newconst);
+            }
             assert(consts_found == 0);
 
             if (build_op == BUILD_SET) {
@@ -1729,6 +1742,8 @@ optimize_lists_and_sets(basicblock *bb, int i, int nextop,
     if (const_result == NULL) {
         return ERROR;
     }
+    PyObject_GC_UnTrack(const_result);
+    bool track_tuple = false;
 
     for (int i = 0; i < seq_size; i++) {
         cfg_instr *inst = const_instrs[i];
@@ -1739,8 +1754,13 @@ optimize_lists_and_sets(basicblock *bb, int i, int nextop,
             return ERROR;
         }
         PyTuple_SET_ITEM(const_result, i, element);
+        if (!track_tuple && PyObject_GC_IsTracked(element)) {
+            track_tuple = true;
+        }
+    }
+    if (track_tuple) {
+        _PyObject_GC_TRACK(const_result);
     }
-    _PyTuple_MaybeUntrack(const_result);
 
     if (instr->i_opcode == BUILD_SET) {
         PyObject *frozenset = PyFrozenSet_New(const_result);