add PyLong* API (3.14+)

[chirizxc]

/close #6015

main (3.13):

into_u128_zero          time:   [33.711 ns 33.971 ns 34.252 ns]
into_u128_small         time:   [34.602 ns 34.859 ns 35.157 ns]
into_u128_u32_max       time:   [34.700 ns 35.231 ns 35.684 ns]
into_u128_u64_max       time:   [40.138 ns 40.795 ns 41.375 ns]
into_u128_max           time:   [39.668 ns 40.320 ns 40.891 ns]
into_i128_zero          time:   [33.628 ns 33.947 ns 34.335 ns]
into_i128_small_pos     time:   [34.595 ns 34.900 ns 35.250 ns]
into_i128_small_neg     time:   [35.660 ns 36.223 ns 36.743 ns]
into_i128_pos_max       time:   [39.891 ns 40.513 ns 41.041 ns]
into_i128_neg_min       time:   [43.970 ns 44.582 ns 45.093 ns]
extract_u128_zero       time:   [20.255 ns 20.341 ns 20.439 ns]
extract_u128_small      time:   [20.208 ns 20.330 ns 20.466 ns]
extract_u128_u32_max    time:   [26.269 ns 26.458 ns 26.674 ns]
extract_u128_u64_max    time:   [30.125 ns 30.322 ns 30.549 ns]
extract_u128_max        time:   [35.379 ns 35.644 ns 35.933 ns]
extract_i128_zero       time:   [20.521 ns 20.602 ns 20.702 ns]
extract_i128_small_pos  time:   [20.630 ns 20.736 ns 20.858 ns]
extract_i128_small_neg  time:   [20.357 ns 20.442 ns 20.539 ns]
extract_i128_pos_max    time:   [34.338 ns 34.568 ns 34.837 ns]
extract_i128_neg_min    time:   [49.427 ns 49.754 ns 50.169 ns]

this PR (3.14):

into_u128_zero          time:   [17.348 ns 17.475 ns 17.617 ns]
into_u128_small         time:   [18.758 ns 18.950 ns 19.163 ns]
into_u128_u32_max       time:   [27.941 ns 28.377 ns 28.762 ns]
into_u128_u64_max       time:   [33.655 ns 34.173 ns 34.633 ns]
into_u128_max           time:   [34.166 ns 34.739 ns 35.296 ns]
into_i128_zero          time:   [17.017 ns 17.154 ns 17.301 ns]
into_i128_small_pos     time:   [19.636 ns 19.818 ns 20.019 ns]
into_i128_small_neg     time:   [29.753 ns 30.247 ns 30.675 ns]
into_i128_pos_max       time:   [34.856 ns 35.374 ns 35.835 ns]
into_i128_neg_min       time:   [34.675 ns 35.254 ns 35.785 ns]
extract_u128_zero       time:   [18.302 ns 18.434 ns 18.571 ns]
extract_u128_small      time:   [18.654 ns 18.771 ns 18.904 ns]
extract_u128_u32_max    time:   [19.758 ns 19.915 ns 20.084 ns]
extract_u128_u64_max    time:   [23.769 ns 23.975 ns 24.193 ns]
extract_u128_max        time:   [23.988 ns 24.128 ns 24.284 ns]
extract_i128_zero       time:   [19.126 ns 19.261 ns 19.405 ns]
extract_i128_small_pos  time:   [19.004 ns 19.157 ns 19.316 ns]
extract_i128_small_neg  time:   [18.819 ns 18.933 ns 19.058 ns]
extract_i128_pos_max    time:   [24.424 ns 24.593 ns 24.776 ns]
extract_i128_neg_min    time:   [24.007 ns 24.147 ns 24.299 ns]

cmd:

cargo bench --bench bench_int128 -- --warm-up-time 3 --measurement-time 5 2>& | grep -E '^(into|extract)_[a-z0-9_]+\s+time:'

benchmark	3.13 (ns)	3.14 (ns)	speedup
into_u128_zero	33.97	17.48	1.94×
into_u128_small	34.86	18.95	1.84×
into_u128_u32_max	35.23	28.38	1.24×
into_u128_u64_max	40.80	34.17	1.19×
into_u128_max	40.32	34.74	1.16×
into_i128_zero	33.95	17.15	1.98×
into_i128_small_pos	34.90	19.82	1.76×
into_i128_small_neg	36.22	30.25	1.20×
into_i128_pos_max	40.51	35.37	1.15×
into_i128_neg_min	44.58	35.25	1.26×
extract_u128_zero	20.34	18.43	1.10×
extract_u128_small	20.33	18.77	1.08×
extract_u128_u32_max	26.46	19.91	1.33×
extract_u128_u64_max	30.32	23.98	1.26×
extract_u128_max	35.64	24.13	1.48×
extract_i128_zero	20.60	19.26	1.07×
extract_i128_small_pos	20.74	19.16	1.08×
extract_i128_small_neg	20.44	18.93	1.08×
extract_i128_pos_max	34.57	24.59	1.41×
extract_i128_neg_min	49.75	24.15	2.06×

[ngoldbaum]

Did a quick pass and spotted a couple issues.

[ngoldbaum]

I looked over the FFI changes.

While we're touching longobject.rs, I'd appreciate it if you could make sure that your updates are ordered following CPython's Include/cpython/longobject.h as much as possible. For example, there are comments about skipping PyUnstable_Long_IsCompact and PyUnstable_Long_CompactValue, but those comments should follow the definition for PyLong_FromUnicodeObject because that's how the corresponding CPython header is organized in 3.14. Our bindings are also missing Py_ASNATIVEBYTES_ALLOW_INDEX, PyLong_IsPositive, PyLong_IsNegative, and PyLong_IsZero, which we should add.

It also looks like the PyLongLayout and PyLongExport structs are in cpython/longintrepr.h, so the definitions you're adding here should go into a new cpython/longintrepr.rs rust module, along with the C API definitions that are in the corresponding header.

The principle you should keep in mind when touching PyO3's FFI bindings is that it should be organized exactly like the upstream headers, as of the newest Python we support (3.14 right now, but it'll be 3.15 in the next few weeks).

[chirizxc]

I looked over the FFI changes.

While we're touching longobject.rs, I'd appreciate it if you could make sure that your updates are ordered following CPython's Include/cpython/longobject.h as much as possible. For example, there are comments about skipping PyUnstable_Long_IsCompact and PyUnstable_Long_CompactValue, but those comments should follow the definition for PyLong_FromUnicodeObject because that's how the corresponding CPython header is organized in 3.14. Our bindings are also missing Py_ASNATIVEBYTES_ALLOW_INDEX, PyLong_IsPositive, PyLong_IsNegative, and PyLong_IsZero, which we should add.

It also looks like the PyLongLayout and PyLongExport structs are in cpython/longintrepr.h, so the definitions you're adding here should go into a new cpython/longintrepr.rs rust module, along with the C API definitions that are in the corresponding header.

The principle you should keep in mind when touching PyO3's FFI bindings is that it should be organized exactly like the upstream headers, as of the newest Python we support (3.14 right now, but it'll be 3.15 in the next few weeks).

In branch 3.14, PyLong_FromNativeBytes, Py_ASNATIVEBYTES_BIG_ENDIAN, etc. , were moved from cpython/Include/longobject.h to cpython/longobject.h (see: 3.13* | 3.14*). Should I move them as well? I mean, do I need to fully synchronize the current files with branch 3.14?

[ngoldbaum]

Should I move them as well? I mean, do I need to fully synchronize the current files with branch 3.14?

Yes, that's generally what we do. It's hard to keep the FFI bindings perfectly up-to-date, so it's a chore for whoever needs to touch them.

[chirizxc]

I split up some of the extern_libpython! blocks so that the order remains the same as in the *.h files

[ngoldbaum]

I spotted some issues, see below.

[bschoenmaeckers]

Should we apply this to the num_bigint conversions as well?

[davidhewitt]

Should we apply this to the num_bigint conversions as well?

Good idea. It might be possible to avoid an intermediate allocation on the to-python direction, on the from-python direction I think the int_to_u32_vec is probably close to optimal because we ask Python to export into a buffer which we pass to BigInt constructors? But investigation is welcome.

[ngoldbaum]

Should we apply this to the num_bigint conversions as well?

Yes, but as I alluded to above IMO further work to add more conversions should happen in followups, this PR has already had a bit of scope creep (mea culpa on that).

[bschoenmaeckers]

Should we apply this to the num_bigint conversions as well?

Yes, but as I alluded to above IMO further work to add more conversions should happen in followups, this PR has already had a bit of scope creep (mea culpa on that).

Sorry missed your comment.

[davidhewitt]

In which case let's move to merge this, and leave an issue to follow up with the num_bigint conversions later.

Merge conflict has arisen, sorry.

[davidhewitt]

I guess we still need a decision here. Would you be amenable to leaving these implementations to panic for now, and maybe we open an issue to decide if we change this for all implementations separately?

[chirizxc]

https://github.com/search?q=repo%3APyO3%2Fpyo3%20libc%3A%3Asize_t&type=code

Should there be another alias for libc::size_t, such as pub type Py_uhash_t = ::libc::size_t;?

Something like pub type Py_size_t = ::libc::size_t;?

[ngoldbaum]

No, we don't like to create symbols that look like they're exposed by Python unless they actually are.

[davidhewitt]

One tiny last thing, otherwise LGTM. Thanks, and also noted you created the follow up issues 👍

[chirizxc]

I think I forgot to change the test after 3fdcb06

[chirizxc]

                    #[cfg(Py_3_13)]
                    {
                        let mut flags = ffi::Py_ASNATIVEBYTES_NATIVE_ENDIAN;
                        if !$is_signed {
                            flags |= ffi::Py_ASNATIVEBYTES_UNSIGNED_BUFFER
                                | ffi::Py_ASNATIVEBYTES_REJECT_NEGATIVE;
                        }

On CPython 3.13 / 3.14 / 3.15 Py_ASNATIVEBYTES_REJECT_NEGATIVE flag raises ValueError:

PyErr_SetString(PyExc_ValueError, "Cannot convert negative int");

[chirizxc]

It seems to me that OverflowError is semantically more appropriate for "the number does not fit the type"

< 3.13: ffi::_PyLong_AsByteArray -> OverflowError
= 3.13: ffi::PyLong_AsNativeBytes + ffi::Py_ASNATIVEBYTES_REJECT_NEGATIVE -> ValueError
≥ 3.14: pylong_visit_digits -> OverflowError

Maybe we should change error type for =3.13?

[davidhewitt]

Related - #5179

I am not entirely decided whether OverflowError or ValueError is better. I think in other places in this file we use ValueError for the negative case already?

[chirizxc]

Related - #5179

I am not entirely decided whether OverflowError or ValueError is better. I think in other places in this file we use ValueError for the negative case already?

Overall, I don't really care the main thing is that it's consistent and that the same type of error occurs across different versions.