Change PyLong_Sign() API per review request

python · vstinner · Jun 3, 2024 · Mar 10, 2024 · Mar 10, 2024 · Mar 11, 2024
commit 2c0b118a01a06026706b2a331d8a3775daeea06f
@@ -442,12 +442,13 @@ distinguished from a number.  Use :c:func:`PyErr_Occurred` to disambiguate.
    .. versionadded:: 3.13
 
 
-.. c:function:: int PyLong_Sign(PyObject *obj)
+.. c:function:: int PyLong_Sign(PyObject *obj, int *sign)
 
-   Return ``-1``, ``0`` or ``+1`` if the integer object *obj* is negative, zero
-   or positive, respectively.
+   Retrieve the sign of integer object *obj* (``0``, ``-1`` or ``+1`` for zero,
+   negative or positive integer, respectively) in a variable *sign*.
 
-   Returns ``-1`` on error.  Use :c:func:`PyErr_Occurred` to disambiguate.
+   Return ``0`` on success, else ``-1`` with an exception set.  This function
+   always succeeds if *obj* is a :c:type:`PyLongObject` or its subtype.
 
    .. versionadded:: 3.13
 

diff --git a/Doc/whatsnew/3.13.rst b/Doc/whatsnew/3.13.rst
@@ -1645,8 +1645,8 @@ New Features
   between native integer types and Python :class:`int` objects.
   (Contributed by Steve Dower in :gh:`111140`.)
 
-* Add :c:func:`PyLong_Sign` function (former private :c:func:`!_PyLong_Sign`)
-  to test sign of :class:`int` objects.  Contributed by Sergey B Kirpichev.
+* Add :c:func:`PyLong_Sign` function to test sign of :class:`int` objects.
+  Contributed by Sergey B Kirpichev.
 
 
 Porting to Python 3.13

diff --git a/Include/cpython/longobject.h b/Include/cpython/longobject.h
@@ -41,12 +41,11 @@ PyAPI_FUNC(PyObject*) PyLong_FromUnsignedNativeBytes(const void* buffer,
 PyAPI_FUNC(int) PyUnstable_Long_IsCompact(const PyLongObject* op);
 PyAPI_FUNC(Py_ssize_t) PyUnstable_Long_CompactValue(const PyLongObject* op);
 
-/* PyLong_Sign.  Return the sign of the integer value: 0, -1 or +1 for
-   zero, negative and positive integer, respectively.  */
-PyAPI_FUNC(int) PyLong_Sign(PyObject *v);
+/* PyLong_Sign.  Retrieve the sign of the integer value (0, -1 or +1) in a
+   variable sign.  Return 0 on success, else -1 with an exception set. */
+PyAPI_FUNC(int) PyLong_Sign(PyObject *v, int* sign);
 
-/* Alias kept for backward compatibility with Python 3.12 */
-#define _PyLong_Sign PyLong_Sign
+PyAPI_FUNC(int) _PyLong_Sign(PyObject *v);
 
 /* _PyLong_FromByteArray:  View the n unsigned bytes as a binary integer in
    base 256, and return a Python int with the same numeric value.

diff --git a/Misc/NEWS.d/next/C API/2024-03-10-14-55-51.gh-issue-116560.x2mZaO.rst b/Misc/NEWS.d/next/C API/2024-03-10-14-55-51.gh-issue-116560.x2mZaO.rst
@@ -1,2 +1 @@
-Add :c:func:`PyLong_Sign` function (former private :c:func:`!_PyLong_Sign`).
-Patch by Sergey B Kirpichev.
+Add :c:func:`PyLong_Sign` function.  Patch by Sergey B Kirpichev.
diff --git a/Modules/_ctypes/_ctypes.c b/Modules/_ctypes/_ctypes.c
@@ -1440,7 +1440,7 @@ PyCArrayType_new(PyTypeObject *type, PyObject *args, PyObject *kwds)
         goto error;
     }
 
-    if (PyLong_Sign(length_attr) == -1) {
+    if (_PyLong_Sign(length_attr) == -1) {
         Py_DECREF(length_attr);
         PyErr_SetString(PyExc_ValueError,
                         "The '_length_' attribute must not be negative");

diff --git a/Modules/_io/_iomodule.c b/Modules/_io/_iomodule.c
@@ -10,6 +10,7 @@
 #include "Python.h"
 #include "pycore_abstract.h"      // _PyNumber_Index()
 #include "pycore_initconfig.h"    // _PyStatus_OK()
+#include "pycore_long.h"          // _PyLong_Sign()
 #include "pycore_pyerrors.h"      // _PyErr_ChainExceptions1()
 #include "pycore_pystate.h"       // _PyInterpreterState_GET()
 
@@ -546,7 +547,7 @@ PyNumber_AsOff_t(PyObject *item, PyObject *err)
         /* Whether or not it is less than or equal to
            zero is determined by the sign of ob_size
         */
-        if (PyLong_Sign(value) < 0)
+        if (_PyLong_Sign(value) < 0)
             result = PY_OFF_T_MIN;
         else
             result = PY_OFF_T_MAX;

diff --git a/Modules/_pickle.c b/Modules/_pickle.c
@@ -2124,7 +2124,7 @@ save_long(PicklerObject *self, PyObject *obj)
         unsigned char *pdata;
         char header[5];
         int i;
-        int sign = PyLong_Sign(obj);
+        int sign = _PyLong_Sign(obj);
 
         if (sign == 0) {
             header[0] = LONG1;

diff --git a/Modules/_testcapi/long.c b/Modules/_testcapi/long.c
@@ -824,11 +824,11 @@ static PyObject *
 pylong_sign(PyObject *module, PyObject *arg)
 {
     NULLABLE(arg);
-    int ret = PyLong_Sign(arg);
-    if (ret == -1 && PyErr_Occurred()) {
+    int sign;
+    if (PyLong_Sign(arg, &sign) == -1) {
         return NULL;
     }
-    return PyLong_FromLong(ret);
+    return PyLong_FromLong(sign);
 }
 
 

diff --git a/Modules/_testinternalcapi.c b/Modules/_testinternalcapi.c
@@ -22,7 +22,7 @@
 #include "pycore_hashtable.h"     // _Py_hashtable_new()
 #include "pycore_initconfig.h"    // _Py_GetConfigsAsDict()
 #include "pycore_interp.h"        // _PyInterpreterState_GetConfigCopy()
-#include "pycore_long.h"          // _PyLong_NumBits()
+#include "pycore_long.h"          // _PyLong_Sign()
 #include "pycore_object.h"        // _PyObject_IsFreed()
 #include "pycore_optimizer.h"     // _Py_UopsSymbol, etc.
 #include "pycore_pathconfig.h"    // _PyPathConfig_ClearGlobal()
@@ -1587,7 +1587,7 @@ _testinternalcapi_test_long_numbits_impl(PyObject *module)
         if (plong == NULL)
             return NULL;
         nbits = _PyLong_NumBits(plong);
-        sign = PyLong_Sign(plong);
+        sign = _PyLong_Sign(plong);
 
         Py_DECREF(plong);
         if (nbits != testcases[i].nbits)

diff --git a/Objects/floatobject.c b/Objects/floatobject.c
@@ -426,7 +426,7 @@ float_richcompare(PyObject *v, PyObject *w, int op)
 
     else if (PyLong_Check(w)) {
         int vsign = i == 0.0 ? 0 : i < 0.0 ? -1 : 1;
-        int wsign = PyLong_Sign(w);
+        int wsign = _PyLong_Sign(w);
         size_t nbits;
         int exponent;
 

diff --git a/Objects/longobject.c b/Objects/longobject.c
@@ -758,7 +758,20 @@ PyLong_AsUnsignedLongMask(PyObject *op)
 }
 
 int
-PyLong_Sign(PyObject *vv)
+_PyLong_Sign(PyObject *vv)
+{
+    PyLongObject *v = (PyLongObject *)vv;
+
+    assert(v != NULL);
+    assert(PyLong_Check(v));
+    if (_PyLong_IsCompact(v)) {
+        return _PyLong_CompactSign(v);
+    }
+    return _PyLong_NonCompactSign(v);
+}
+
+int
+PyLong_Sign(PyObject *vv, int *sign)
 {
     if (vv == NULL) {
         PyErr_BadInternalCall();
@@ -769,12 +782,8 @@ PyLong_Sign(PyObject *vv)
         return -1;
     }
 
-    PyLongObject *v = (PyLongObject *)vv;
-
-    if (_PyLong_IsCompact(v)) {
-        return _PyLong_CompactSign(v);
-    }
-    return _PyLong_NonCompactSign(v);
+    *sign = _PyLong_Sign(vv);
+    return 0;
 }
 
 static int

diff --git a/Objects/sliceobject.c b/Objects/sliceobject.c
@@ -429,7 +429,7 @@ _PySlice_GetLongIndices(PySliceObject *self, PyObject *length,
         step = evaluate_slice_index(self->step);
         if (step == NULL)
             goto error;
-        step_sign = PyLong_Sign(step);
+        step_sign = _PyLong_Sign(step);
         if (step_sign == 0) {
             PyErr_SetString(PyExc_ValueError,
                             "slice step cannot be zero");