PEP 757: PyLong_Export() can fail

[vstinner]

---

📚 Documentation preview 📚: https://pep-previews--4025.org.readthedocs.build/

[vstinner]

cc @encukou @skirpichev

[skirpichev]

Obviously, I would prefer to keep this constraints, unless other workarounds for @encukou concerns are impossible. Removing it makes API less convenient.

[encukou]

Workarounds are possible, but IMO worse. I can think of:

• put a version in PyLongLayout. Then we can expand the PyLongLayout struct in the future, or even (as a big stretch) mark the whole concept as non-functional.
• add this as Unstable API, so it can be removed without deprecation.

[skirpichev]

put a version in PyLongLayout.

I would reiterate, that PyLongLayout has nothing to say about small integers. I hope #4026 will clarify this. We can add a version field to this structure as well, but why? To allow bigint representations, different from best libraries in the field?

But we could put (you original proposal) version to the PyLongExport structure.

add this as Unstable API, so it can be removed without deprecation.

The version field looks much better. Users just have to put upper bound for CPython version. If new one is coming with a new variant of PyLongExport - they just will have to add a new if (layout->version == 123) { ... }...

But suspect, that it's overkill and int64_t value will fit our needs for 20+ years.

[vstinner]

The alternative is to keep the "cannot fail" sentence and implementation a deprecation by using Py_DEPRECATED() macro on the function, and remove the function after a deprecation cycle (ex: 2 Python releases). It has been done in the past. It's not perfect, but better than nothing.

[skirpichev]

Assuming this removal, how this helps for possible deprecation of this API?

The alternative is to keep the "cannot fail"

In fact, the whole API is about big integers. @encukou, do you think we can do API for reading big integers with such constraint? If we can't offer zero-copy mechanism (i.e. mpz_limbs_read-like for reading) - such API will be useless.

[vstinner]

Assuming this removal, how this helps for possible deprecation of this API?

When a function is modified to emit a DeprecationWarning, the warning can be treated as error in which case, the function starts failing.

[encukou]

If we can't offer zero-copy mechanism (i.e. mpz_limbs_read-like for reading) - such API will be useless.

Indeed. If the internal representation changes, this API will be useless.

However, I realized the user must already branch to two cases (compact and big integers). How onerous would it be to add two more (failure with exception, and an “unknown/future format” case that can keep the API from becoming entirely useless if the representation changes)? Please see the discussion thread: https://discuss.python.org/t/63895/51

[vstinner]

How onerous would it be to add two more (failure with exception, and an “unknown/future format” case that can keep the API from becoming entirely useless if the representation changes)?

Currently, in the gmpy2 project, there are 27 calls to the mpz_set_PyLong() function which convert a Python int to a GMP mpz. mpz_set_PyLong() would call PyLong_Export() with PEP 757. Currently, mpz_set_PyLong() cannot fail.

If PyLong_Export() can fail, the 27 calls should be adapted to handle error which is not convenient. It's the same in other projects which currently rely on Python internals (access directly PyLongObject members).

[encukou]

gmpy2 may choose to modify mpz_set_PyLong so that on failure it aborts the process, or returns zero. That way, the expected behaviour, on new CPython versions that change the internal representation, would remain the same as with the internal API.

I guess we can guarantee the export won't fail in 3.14. But the point here is that it's not version-specific API any more.

[skirpichev]

That does make sense if PyLong_Export() can fail only if internal representation was changed with the new release. So, eventually, API will just stop to work abruptly.

I like more your approach in the d.p.o thread, where API doesn't fail on integers and we just return some export code (might be new in the new release).

[skirpichev]

I like more your approach in the d.p.o thread, where API doesn't fail on integers and we just return some export code (might be new in the new release).

PR #4026 updated to follow that proposal.

[skirpichev]

LGTM

Well, I would prefer if we put some message to users like "this function either fail or not fail - for all integers". At least this could be a CPython implementation detail. But it seems this not worth to debate. People will have to figure out such stuff from sources - this is still better than private API.

But you should somehow adapt PyLong_Export example. Now it's just ignores the return value, assuming integer input. I'll adjust gmpy2 pr.

[vstinner]

But you should somehow adapt PyLong_Export example.

I made this change to abort the process if the function fails:

-        PyLong_Export(obj, &long_export);
+        if (PyLong_Export(obj, &long_export) < 0) {
+            // cannot report errors to the caller
+            Py_FatalError("PyLong_Export failed");
+        }

Does it look good to you?

[skirpichev]

Does it look good to you?

Lets just change function signature and return -1 here and 0 otherwise. I will have to propagate this error up, so people will not loose their work with interpreter crash.

[vstinner]

Lets just change function signature and return -1 here and 0 otherwise.

Ok, done.

[vstinner]

Merged, thanks for the review @skirpichev.