bpo-44654: Refactor and clean up the union type implementation (GH-27196)

Author: serhiy-storchaka

Diff for: Include/genericaliasobject.h

@@ -5,11 +5,6 @@
 extern "C" {
 #endif
 
-#ifndef Py_LIMITED_API
-PyAPI_FUNC(PyObject *) _Py_subs_parameters(PyObject *, PyObject *, PyObject *, PyObject *);
-PyAPI_FUNC(PyObject *) _Py_make_parameters(PyObject *);
-#endif
-
 PyAPI_FUNC(PyObject *) Py_GenericAlias(PyObject *, PyObject *);
 PyAPI_DATA(PyTypeObject) Py_GenericAliasType;
 

Diff for: Include/internal/pycore_unionobject.h

@@ -8,9 +8,13 @@ extern "C" {
 #  error "this header requires Py_BUILD_CORE define"
 #endif
 
-PyAPI_FUNC(PyObject *) _Py_Union(PyObject *args);
-PyAPI_DATA(PyTypeObject) _Py_UnionType;
-PyAPI_FUNC(PyObject *) _Py_union_type_or(PyObject* self, PyObject* param);
+PyAPI_DATA(PyTypeObject) _PyUnion_Type;
+#define _PyUnion_Check(op) Py_IS_TYPE(op, &_PyUnion_Type)
+PyAPI_FUNC(PyObject *) _Py_union_type_or(PyObject *, PyObject *);
+
+#define _PyGenericAlias_Check(op) PyObject_TypeCheck(op, &Py_GenericAliasType)
+PyAPI_FUNC(PyObject *) _Py_subs_parameters(PyObject *, PyObject *, PyObject *, PyObject *);
+PyAPI_FUNC(PyObject *) _Py_make_parameters(PyObject *);
 
 #ifdef __cplusplus
 }

Diff for: Lib/test/test_types.py

@@ -611,6 +611,18 @@ def test_method_descriptor_types(self):
         self.assertIsInstance(int.from_bytes, types.BuiltinMethodType)
         self.assertIsInstance(int.__new__, types.BuiltinMethodType)
 
+    def test_ellipsis_type(self):
+        self.assertIsInstance(Ellipsis, types.EllipsisType)
+
+    def test_notimplemented_type(self):
+        self.assertIsInstance(NotImplemented, types.NotImplementedType)
+
+    def test_none_type(self):
+        self.assertIsInstance(None, types.NoneType)
+
+
+class UnionTests(unittest.TestCase):
+
     def test_or_types_operator(self):
         self.assertEqual(int | str, typing.Union[int, str])
         self.assertNotEqual(int | list, typing.Union[int, str])
@@ -657,18 +669,23 @@ def test_or_types_operator(self):
         with self.assertRaises(TypeError):
             Example() | int
         x = int | str
-        self.assertNotEqual(x, {})
+        self.assertEqual(x, int | str)
+        self.assertEqual(x, str | int)
+        self.assertNotEqual(x, {})  # should not raise exception
         with self.assertRaises(TypeError):
-            (int | str) < typing.Union[str, int]
+            x < x
         with self.assertRaises(TypeError):
-            (int | str) < (int | bool)
+            x <= x
+        y = typing.Union[str, int]
         with self.assertRaises(TypeError):
-            (int | str) <= (int | str)
+            x < y
+        y = int | bool
         with self.assertRaises(TypeError):
-            # Check that we don't crash if typing.Union does not have a tuple in __args__
-            x = typing.Union[str, int]
-            x.__args__ = [str, int]
-            (int | str ) == x
+            x < y
+        # Check that we don't crash if typing.Union does not have a tuple in __args__
+        y = typing.Union[str, int]
+        y.__args__ = [str, int]
+        self.assertEqual(x, y)
 
     def test_hash(self):
         self.assertEqual(hash(int | str), hash(str | int))
@@ -873,15 +890,6 @@ def test_or_type_operator_reference_cycle(self):
         self.assertLessEqual(sys.gettotalrefcount() - before, leeway,
                              msg='Check for union reference leak.')
 
-    def test_ellipsis_type(self):
-        self.assertIsInstance(Ellipsis, types.EllipsisType)
-
-    def test_notimplemented_type(self):
-        self.assertIsInstance(NotImplemented, types.NotImplementedType)
-
-    def test_none_type(self):
-        self.assertIsInstance(None, types.NoneType)
-
 
 class MappingProxyTests(unittest.TestCase):
     mappingproxy = types.MappingProxyType

Diff for: Lib/typing.py

@@ -1184,7 +1184,7 @@ def copy_with(self, params):
         return Union[params]
 
     def __eq__(self, other):
-        if not isinstance(other, _UnionGenericAlias):
+        if not isinstance(other, (_UnionGenericAlias, types.Union)):
             return NotImplemented
         return set(self.__args__) == set(other.__args__)
 

Diff for: Objects/abstract.c

@@ -6,7 +6,7 @@
 #include "pycore_object.h"        // _Py_CheckSlotResult()
 #include "pycore_pyerrors.h"      // _PyErr_Occurred()
 #include "pycore_pystate.h"       // _PyThreadState_GET()
-#include "pycore_unionobject.h"   // _Py_UnionType && _Py_Union()
+#include "pycore_unionobject.h"   // _PyUnion_Check()
 #include <ctype.h>
 #include <stddef.h>               // offsetof()
 #include "longintrepr.h"
@@ -2623,9 +2623,7 @@ recursive_issubclass(PyObject *derived, PyObject *cls)
                      "issubclass() arg 1 must be a class"))
         return -1;
 
-    PyTypeObject *type = Py_TYPE(cls);
-    int is_union = (PyType_Check(type) && type == &_Py_UnionType);
-    if (!is_union && !check_class(cls,
+    if (!_PyUnion_Check(cls) && !check_class(cls,
                             "issubclass() arg 2 must be a class,"
                             " a tuple of classes, or a union.")) {
         return -1;

Diff for: Objects/genericaliasobject.c

@@ -2,7 +2,7 @@
 
 #include "Python.h"
 #include "pycore_object.h"
-#include "pycore_unionobject.h"   // _Py_union_as_number
+#include "pycore_unionobject.h"   // _Py_union_type_or, _PyGenericAlias_Check
 #include "structmember.h"         // PyMemberDef
 
 typedef struct {
@@ -441,8 +441,7 @@ ga_getattro(PyObject *self, PyObject *name)
 static PyObject *
 ga_richcompare(PyObject *a, PyObject *b, int op)
 {
-    if (!PyObject_TypeCheck(a, &Py_GenericAliasType) ||
-        !PyObject_TypeCheck(b, &Py_GenericAliasType) ||
+    if (!_PyGenericAlias_Check(b) ||
         (op != Py_EQ && op != Py_NE))
     {
         Py_RETURN_NOTIMPLEMENTED;
@@ -622,7 +621,7 @@ ga_new(PyTypeObject *type, PyObject *args, PyObject *kwds)
 }
 
 static PyNumberMethods ga_as_number = {
-        .nb_or = (binaryfunc)_Py_union_type_or, // Add __or__ function
+        .nb_or = _Py_union_type_or, // Add __or__ function
 };
 
 // TODO:

Diff for: Objects/object.c

@@ -11,7 +11,7 @@
 #include "pycore_pymem.h"         // _PyMem_IsPtrFreed()
 #include "pycore_pystate.h"       // _PyThreadState_GET()
 #include "pycore_symtable.h"      // PySTEntry_Type
-#include "pycore_unionobject.h"   // _Py_UnionType
+#include "pycore_unionobject.h"   // _PyUnion_Type
 #include "frameobject.h"
 #include "interpreteridobject.h"
 
@@ -1878,7 +1878,7 @@ _PyTypes_Init(void)
     INIT_TYPE(_PyWeakref_CallableProxyType);
     INIT_TYPE(_PyWeakref_ProxyType);
     INIT_TYPE(_PyWeakref_RefType);
-    INIT_TYPE(_Py_UnionType);
+    INIT_TYPE(_PyUnion_Type);
 
     return _PyStatus_OK();
 #undef INIT_TYPE

Diff for: Objects/typeobject.c

@@ -9,7 +9,7 @@
 #include "pycore_object.h"
 #include "pycore_pyerrors.h"
 #include "pycore_pystate.h"       // _PyThreadState_GET()
-#include "pycore_unionobject.h"   // _Py_Union(), _Py_union_type_or
+#include "pycore_unionobject.h"   // _Py_union_type_or
 #include "frameobject.h"
 #include "opcode.h"               // MAKE_CELL
 #include "structmember.h"         // PyMemberDef