Eliminate all remaining uses of Py_SIZE and Py_SET_SIZE on PyLongObject.

python · markshannon · Mar 22, 2023 · Jan 25, 2023 · Jan 25, 2023 · Feb 28, 2023
commit bc14fa66e6497f236f7ef58aa474491a1a9ab9fe
diff --git a/Include/cpython/longintrepr.h b/Include/cpython/longintrepr.h
@@ -80,7 +80,7 @@ typedef long stwodigits; /* signed variant of twodigits */
 */

 typedef struct _PyLongValue {
-    Py_ssize_t ob_size; /* Number of items in variable part */
+    intptr_t ob_size; /* Number of digits, sign and flags */
    digit ob_digit[1];
 } _PyLongValue;


diff --git a/Include/internal/pycore_long.h b/Include/internal/pycore_long.h
@@ -112,9 +112,7 @@ PyAPI_FUNC(char*) _PyLong_FormatBytesWriter(
 /* Return 1 if the argument is positive single digit int */
 static inline int
 _PyLong_IsNonNegativeSingleDigit(const PyLongObject* op) {
-    /*  For a positive single digit int, the value of Py_SIZE(sub) is 0 or 1.
-
-        We perform a fast check using a single comparison by casting from int
+    /*  We perform a fast check using a single comparison by casting from int
        to uint which casts negative numbers to large positive numbers.
        For details see Section 14.2 "Bounds Checking" in the Agner Fog
        optimization manual found at:
@@ -124,51 +122,51 @@ _PyLong_IsNonNegativeSingleDigit(const PyLongObject* op) {
        compiler options of GCC and clang
    */
    assert(PyLong_Check(op));
-    Py_ssize_t signed_size = Py_SIZE(op);
+    Py_ssize_t signed_size = op->long_value.ob_size;
    return ((size_t)signed_size) <= 1;
 }


 static inline int
 _PyLong_IsSingleDigit(const PyLongObject* op) {
    assert(PyLong_Check(op));
-    Py_ssize_t signed_size = Py_SIZE(op);
+    Py_ssize_t signed_size = op->long_value.ob_size;
    return ((size_t)(signed_size+1)) <= 2;
 }

 static inline Py_ssize_t
 _PyLong_SingleDigitValue(const PyLongObject *op)
 {
    assert(PyLong_Check(op));
-    assert(Py_SIZE(op) >= -1 && Py_SIZE(op) <= 1);
-    return Py_SIZE(op) * op->long_value.ob_digit[0];
+    assert(op->long_value.ob_size >= -1 && op->long_value.ob_size <= 1);
+    return op->long_value.ob_size * op->long_value.ob_digit[0];
 }

 static inline bool
 _PyLong_IsPositive(const PyLongObject *op)
 {
-    return Py_SIZE(op) > 0;
+    return op->long_value.ob_size > 0;
 }

 static inline bool
 _PyLong_IsNegative(const PyLongObject *op)
 {
-    return Py_SIZE(op) < 0;
+    return op->long_value.ob_size < 0;
 }

 static inline Py_ssize_t
 _PyLong_DigitCount(const PyLongObject *op)
 {
    assert(PyLong_Check(op));
-    return Py_ABS(Py_SIZE(op));
+    return Py_ABS(op->long_value.ob_size);
 }

 /* Equivalent to _PyLong_DigitCount(op) * _PyLong_NonZeroSign(op) */
 static inline Py_ssize_t
 _PyLong_SignedDigitCount(const PyLongObject *op)
 {
    assert(PyLong_Check(op));
-    return Py_SIZE(op);
+    return op->long_value.ob_size;
 }

 /* Like _PyLong_DigitCount but asserts that op is non-negative */
@@ -177,28 +175,28 @@ _PyLong_UnsignedDigitCount(const PyLongObject *op)
 {
    assert(PyLong_Check(op));
    assert(!_PyLong_IsNegative(op));
-    return Py_SIZE(op);
+    return op->long_value.ob_size;
 }

 static inline bool
 _PyLong_IsZero(const PyLongObject *op)
 {
-    return Py_SIZE(op) == 0;
+    return op->long_value.ob_size == 0;
 }

 static inline int
 _PyLong_NonZeroSign(const PyLongObject *op)
 {
    assert(PyLong_Check(op));
    assert(!_PyLong_IsZero(op));
-    return ((Py_SIZE(op) > 0) << 1) - 1;
+    return ((op->long_value.ob_size > 0) << 1) - 1;
 }

 /* Do a and b have the same sign? Zero counts as positive. */
 static inline int
 _PyLong_SameSign(const PyLongObject *a, const PyLongObject *b)
 {
-    return (Py_SIZE(a) ^ Py_SIZE(b)) >= 0;
+    return (a->long_value.ob_size ^ b->long_value.ob_size) >= 0;
 }

 static inline void
@@ -214,6 +212,20 @@ _PyLong_FlipSign(PyLongObject *op) {
    op->long_value.ob_size = -op->long_value.ob_size;
 }

+#define _PyLong_DIGIT_INIT(val) \
+    { \
+        .ob_base = _PyObject_IMMORTAL_INIT(&PyLong_Type), \
+        .long_value  = { \
+            .ob_size = (val) == 0 ? 0 : ((val) < 0 ? -1 : 1), \
+            { ((val) >= 0 ? (val) : -(val)) }, \
+        } \
+    }
+
+#define TAG_FROM_SIGN_AND_SIZE(neg, size) (neg ? -(size) : (size))
+
+#define _PyLong_FALSE_TAG 0
+#define _PyLong_TRUE_TAG 1
+

 #ifdef __cplusplus
 }

diff --git a/Include/internal/pycore_object.h b/Include/internal/pycore_object.h
@@ -118,8 +118,9 @@ static inline void
 _PyObject_InitVar(PyVarObject *op, PyTypeObject *typeobj, Py_ssize_t size)
 {
    assert(op != NULL);
-    Py_SET_SIZE(op, size);
+    assert(typeobj != &PyLong_Type);
    _PyObject_Init((PyObject *)op, typeobj);
+    Py_SET_SIZE(op, size);
 }



@@ -8,6 +8,7 @@ extern "C" {
 #  error "this header requires Py_BUILD_CORE define"
 #endif

+#include "pycore_long.h"
 #include "pycore_object.h"
 #include "pycore_parser.h"
 #include "pycore_pymem_init.h"
@@ -134,15 +135,6 @@ extern "C" {

 // global objects

-#define _PyLong_DIGIT_INIT(val) \
-    { \
-        .ob_base = _PyObject_IMMORTAL_INIT(&PyLong_Type), \
-        .long_value  = { \
-            ((val) == 0 ? 0 : ((val) > 0 ? 1 : -1)), \
-            { ((val) >= 0 ? (val) : -(val)) }, \
-        } \
-    }
-
 #define _PyBytes_SIMPLE_INIT(CH, LEN) \
    { \
        _PyVarObject_IMMORTAL_INIT(&PyBytes_Type, (LEN)), \

diff --git a/Include/object.h b/Include/object.h
@@ -138,8 +138,11 @@ static inline PyTypeObject* Py_TYPE(PyObject *ob) {
 #  define Py_TYPE(ob) Py_TYPE(_PyObject_CAST(ob))
 #endif

+extern PyTypeObject PyLong_Type;
+
 // bpo-39573: The Py_SET_SIZE() function must be used to set an object size.
 static inline Py_ssize_t Py_SIZE(PyObject *ob) {
+    assert(ob->ob_type != &PyLong_Type);
    PyVarObject *var_ob = _PyVarObject_CAST(ob);
    return var_ob->ob_size;
 }
@@ -171,8 +174,8 @@ static inline void Py_SET_TYPE(PyObject *ob, PyTypeObject *type) {
 #  define Py_SET_TYPE(ob, type) Py_SET_TYPE(_PyObject_CAST(ob), type)
 #endif

-
 static inline void Py_SET_SIZE(PyVarObject *ob, Py_ssize_t size) {
+    assert(ob->ob_base.ob_type != &PyLong_Type);
    ob->ob_size = size;
 }
 #if !defined(Py_LIMITED_API) || Py_LIMITED_API+0 < 0x030b0000

diff --git a/Modules/_testcapi/mem.c b/Modules/_testcapi/mem.c
@@ -347,7 +347,7 @@ test_pyobject_new(PyObject *self, PyObject *Py_UNUSED(ignored))
 {
    PyObject *obj;
    PyTypeObject *type = &PyBaseObject_Type;
-    PyTypeObject *var_type = &PyLong_Type;
+    PyTypeObject *var_type = &PyBytes_Type;

    // PyObject_New()
    obj = PyObject_New(PyObject, type);

diff --git a/Modules/_tkinter.c b/Modules/_tkinter.c
@@ -32,6 +32,8 @@ Copyright (C) 1994 Steen Lumholt.
 #  include "pycore_fileutils.h"   // _Py_stat()
 #endif

+#include "pycore_long.h"
+
 #ifdef MS_WINDOWS
 #include <windows.h>
 #endif
@@ -887,8 +889,8 @@ asBignumObj(PyObject *value)
    PyObject *hexstr;
    const char *hexchars;
    mp_int bigValue;
-
-    neg = Py_SIZE(value) < 0;
+    assert(PyLong_Check(value));
+    neg = _PyLong_IsNegative((PyLongObject *)value);
-    assert(PyLong_Check(value));
-    neg = _PyLong_IsNegative((PyLongObject *)value);
+
+    assert(PyLong_Check(value));
+    neg = _PyLong_IsNegative((PyLongObject *)value);
-    assert(PyLong_Check(value));
-    neg = _PyLong_IsNegative((PyLongObject *)value);
+
+    assert(PyLong_Check(value));
+    neg = _PyLong_IsNegative((PyLongObject *)value);
    hexstr = _PyLong_Format(value, 16);
    if (hexstr == NULL)
        return NULL;
@@ -1960,7 +1962,7 @@ _tkinter_tkapp_getboolean(TkappObject *self, PyObject *arg)
    int v;

    if (PyLong_Check(arg)) { /* int or bool */
-        return PyBool_FromLong(Py_SIZE(arg) != 0);
+        return PyBool_FromLong(!_PyLong_IsZero((PyLongObject *)arg));
    }

    if (PyTclObject_Check(arg)) {

diff --git a/Objects/boolobject.c b/Objects/boolobject.c
@@ -2,6 +2,7 @@

 #include "Python.h"
 #include "pycore_object.h"      // _Py_FatalRefcountError()
+#include "pycore_long.h"        // FALSE_TAG TRUE_TAG
 #include "pycore_runtime.h"       // _Py_ID()

 #include <stddef.h>
@@ -198,10 +199,14 @@ PyTypeObject PyBool_Type = {

 struct _longobject _Py_FalseStruct = {
    PyObject_HEAD_INIT(&PyBool_Type)
-    { 0, { 0 } }
+    { .ob_size = _PyLong_FALSE_TAG,
+        { 0 }
+    }
 };

 struct _longobject _Py_TrueStruct = {
    PyObject_HEAD_INIT(&PyBool_Type)
-    { 1, { 1 } }
+    { .ob_size = _PyLong_TRUE_TAG,
+        { 1 }
+    }
 };
diff --git a/Objects/longobject.c b/Objects/longobject.c
@@ -172,8 +172,7 @@ _PyLong_FromDigits(int negative, Py_ssize_t digit_count, digit *digits)
        PyErr_NoMemory();
        return NULL;
    }
-    int sign = negative ? -1 : 1;
-    result->long_value.ob_size = sign * digit_count;
+    _PyLong_SetSignAndSize(result, negative, digit_count);
    memcpy(result->long_value.ob_digit, digits, digit_count * sizeof(digit));
    return result;
 }
@@ -204,9 +203,9 @@ _PyLong_FromMedium(sdigit x)
        PyErr_NoMemory();
        return NULL;
    }
-    Py_ssize_t sign = x < 0 ? -1: 1;
    digit abs_x = x < 0 ? -x : x;
-    _PyObject_InitVar((PyVarObject*)v, &PyLong_Type, sign);
+    _PyLong_SetSignAndSize(v, x < 0, 1);
+    _PyObject_Init((PyObject*)v, &PyLong_Type);
    v->long_value.ob_digit[0] = abs_x;
    return (PyObject*)v;
 }

@@ -8,6 +8,7 @@
 #include "pycore_initconfig.h"    // _PyStatus_OK()
 #include "pycore_moduleobject.h"  // _PyModule_GetDef()
 #include "pycore_object.h"        // _PyType_HasFeature()
+#include "pycore_long.h"          // _PyLong_IsNegative()
 #include "pycore_pyerrors.h"      // _PyErr_Occurred()
 #include "pycore_pystate.h"       // _PyThreadState_GET()
 #include "pycore_typeobject.h"    // struct type_cache
@@ -7850,7 +7851,7 @@ slot_sq_length(PyObject *self)
        return -1;

    assert(PyLong_Check(res));
-    if (Py_SIZE(res) < 0) {
+    if (_PyLong_IsNegative((PyLongObject *)res)) {
        Py_DECREF(res);
        PyErr_SetString(PyExc_ValueError,
                        "__len__() should return >= 0");

diff --git a/Python/ast_opt.c b/Python/ast_opt.c
@@ -2,6 +2,7 @@
 #include "Python.h"
 #include "pycore_ast.h"           // _PyAST_GetDocString()
 #include "pycore_compile.h"       // _PyASTOptimizeState
+#include "pycore_long.h"           // _PyLong
 #include "pycore_pystate.h"       // _PyThreadState_GET()
 #include "pycore_format.h"        // F_LJUST

@@ -152,7 +153,9 @@ check_complexity(PyObject *obj, Py_ssize_t limit)
 static PyObject *
 safe_multiply(PyObject *v, PyObject *w)
 {
-    if (PyLong_Check(v) && PyLong_Check(w) && Py_SIZE(v) && Py_SIZE(w)) {
+    if (PyLong_Check(v) && PyLong_Check(w) &&
+        !_PyLong_IsZero((PyLongObject *)v) && !_PyLong_IsZero((PyLongObject *)w)
+    ) {
        size_t vbits = _PyLong_NumBits(v);
        size_t wbits = _PyLong_NumBits(w);
        if (vbits == (size_t)-1 || wbits == (size_t)-1) {
@@ -198,7 +201,9 @@ safe_multiply(PyObject *v, PyObject *w)
 static PyObject *
 safe_power(PyObject *v, PyObject *w)
 {
-    if (PyLong_Check(v) && PyLong_Check(w) && Py_SIZE(v) && Py_SIZE(w) > 0) {
+    if (PyLong_Check(v) && PyLong_Check(w) &&
+        !_PyLong_IsZero((PyLongObject *)v) && _PyLong_IsPositive((PyLongObject *)w)
+    ) {
        size_t vbits = _PyLong_NumBits(v);
        size_t wbits = PyLong_AsSize_t(w);
        if (vbits == (size_t)-1 || wbits == (size_t)-1) {
@@ -215,7 +220,9 @@ safe_power(PyObject *v, PyObject *w)
 static PyObject *
 safe_lshift(PyObject *v, PyObject *w)
 {
-    if (PyLong_Check(v) && PyLong_Check(w) && Py_SIZE(v) && Py_SIZE(w)) {
+    if (PyLong_Check(v) && PyLong_Check(w) &&
+        !_PyLong_IsZero((PyLongObject *)v) && !_PyLong_IsZero((PyLongObject *)w)
+    ) {
        size_t vbits = _PyLong_NumBits(v);
        size_t wbits = PyLong_AsSize_t(w);
        if (vbits == (size_t)-1 || wbits == (size_t)-1) {

diff --git a/Python/marshal.c b/Python/marshal.c
@@ -840,7 +840,7 @@ r_PyLong(RFILE *p)
    if (ob == NULL)
        return NULL;

-    Py_SET_SIZE(ob, n > 0 ? size : -size);
+    _PyLong_SetSignAndSize(ob, n < 0, size);

    for (i = 0; i < size-1; i++) {
        d = 0;

@@ -309,7 +309,7 @@ def generate_tuple(self, name: str, t: Tuple[object, ...]) -> str:
        return f"& {name}._object.ob_base.ob_base"

    def _generate_int_for_bits(self, name: str, i: int, digit: int) -> None:
-        sign = -1 if i < 0 else 0 if i == 0 else +1
+        negative = int(i < 0)
        i = abs(i)
        digits: list[int] = []
        while i:
@@ -318,10 +318,12 @@ def _generate_int_for_bits(self, name: str, i: int, digit: int) -> None:
        self.write("static")
        with self.indent():
            with self.block("struct"):
-                self.write("PyObject_VAR_HEAD")
+                self.write("PyObject ob_base;")
+                self.write("uintptr_t ob_size;")
                self.write(f"digit ob_digit[{max(1, len(digits))}];")
        with self.block(f"{name} =", ";"):
-            self.object_var_head("PyLong_Type", sign*len(digits))
+            self.object_head("PyLong_Type")
+            self.write(f".ob_size = TAG_FROM_SIGN_AND_SIZE({negative}, {len(digits)}),")
            if digits:
                ds = ", ".join(map(str, digits))
                self.write(f".ob_digit = {{ {ds} }},")
@@ -345,7 +347,7 @@ def generate_int(self, name: str, i: int) -> str:
            self.write('#error "PYLONG_BITS_IN_DIGIT should be 15 or 30"')
            self.write("#endif")
            # If neither clause applies, it won't compile
-        return f"& {name}.ob_base.ob_base"
+        return f"& {name}.ob_base"

    def generate_float(self, name: str, x: float) -> str:
        with self.block(f"static PyFloatObject {name} =", ";"):
-Original file line number
+Diff line change
@@ -80,7 +80,7 @@ typedef long stwodigits; /* signed variant of twodigits */
     */
             Copy link

  
      
    
  

  
      

  
    Contributor


      

  

  
    
      

      
            verhovsky
  

      

      

      


        Apr 29, 2023


      
  •

  
    
      
        
          edited
          
        
        
      
    
    
      
  
        
      Loading


  
    
  
    
      

              Uh oh!

              
There was an error while loading. Please reload this page.


  
  
    
  


    

  


        
      
  
  
  
    

    There was a problem hiding this comment.


  

 
  
    

    Choose a reason for hiding this comment

    
      The reason will be displayed to describe this comment to others. Learn more.
    

    
      
      


  


  
    
      You didn't update this comment that documents _longobject, it's still talking about ob_size and PyVarObject
/* Long integer representation.
   The absolute value of a number is equal to
        SUM(for i=0 through abs(ob_size)-1) ob_digit[i] * 2**(SHIFT*i)

    
  
  


    

        
      
  
  
    
  
  
  
    
    Sorry, something went wrong.
  

  
    
  
    
      

              Uh oh!

              
There was an error while loading. Please reload this page.


  
  


          
      
  
    
    
      
        
            
    All reactions
     typedef struct _PyLongValue {
-        Py_ssize_t ob_size; /* Number of items in variable part */
+        intptr_t ob_size; /* Number of digits, sign and flags */
         digit ob_digit[1];
     } _PyLongValue;
-          Expand Down