Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use the new large PyLong conversion functions in Py3.13. #5997

Merged
merged 3 commits into from
May 4, 2024

Conversation

scoder
Copy link
Contributor

@scoder scoder commented Feb 12, 2024

Untested since it's not released yet.

See python/cpython#111140

Copy link
Contributor

@da-woods da-woods left a comment

Choose a reason for hiding this comment

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

Looks fine apart from being untested.

I kind of wonder if any of this "large integer" type code is reliable covered by the test-suite at all.

@scoder
Copy link
Contributor Author

scoder commented Feb 18, 2024

I kind of wonder if any of this "large integer" type code is reliable covered by the test-suite at all.

This should exercise it:
https://github.com/cython/cython/blob/master/tests/run/int128.pyx

@scoder
Copy link
Contributor Author

scoder commented Feb 19, 2024

FAIL: signed_conversion (int128)
Doctest: int128.signed_conversion
----------------------------------------------------------------------
File "/home/runner/work/cython/cython/TEST_TMP/0/run/cpp/int128/int128.cpython-313-x86_64-linux-gnu.so", line ?, in int128.signed_conversion
Failed example:
    bigint(signed_conversion(-2**127))
Exception raised:
    Traceback (most recent call last):
      File "/opt/hostedtoolcache/Python/3.13.0-alpha.4/x64/lib/python3.13/doctest.py", line 1378, in __run
        exec(compile(example.source, filename, "single",
        ~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
                     compileflags, True), test.globs)
                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
      File "<doctest int128.signed_conversion[19]>", line 1, in <module>
        bigint(signed_conversion(-2**127))
               ~~~~~~~~~~~~~~~~~^^^^^^^^^
      File "tests/run/int128.pyx", line 139, in int128.signed_conversion (int128.cpp:2833)
        cdef int128_t n = x
    OverflowError: value too large to convert to __int128_t


======================================================================
FAIL: unsigned_conversion (int128)
Doctest: int128.unsigned_conversion
----------------------------------------------------------------------
File "/home/runner/work/cython/cython/TEST_TMP/0/run/cpp/int128/int128.cpython-313-x86_64-linux-gnu.so", line ?, in int128.unsigned_conversion
Failed example:
    bigint(unsigned_conversion(2**128-1))
Exception raised:
    Traceback (most recent call last):
      File "/opt/hostedtoolcache/Python/3.13.0-alpha.4/x64/lib/python3.13/doctest.py", line 1378, in __run
        exec(compile(example.source, filename, "single",
        ~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
                     compileflags, True), test.globs)
                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
      File "<doctest int128.unsigned_conversion[19]>", line 1, in <module>
        bigint(unsigned_conversion(2**128-1))
               ~~~~~~~~~~~~~~~~~~~^^^^^^^^^^
      File "tests/run/int128.pyx", line 75, in int128.unsigned_conversion (int128.cpp:2670)
        cdef uint128_t n = x
    OverflowError: value too large to convert to __uint128_t

@scoder scoder force-pushed the py313_pylong_native_bytes branch from f203050 to 1ef92d8 Compare May 3, 2024 19:38
@scoder scoder marked this pull request as ready for review May 3, 2024 19:39
@scoder
Copy link
Contributor Author

scoder commented May 4, 2024

I found several ref-leak issues in the shift+mask fallback conversion implementation while working on this that should also be fixed in 3.0.x. It's probably easier to backport the PR than to try to extract them separately.

@scoder scoder merged commit dc63743 into cython:master May 4, 2024
64 checks passed
@scoder scoder deleted the py313_pylong_native_bytes branch May 4, 2024 20:21
scoder added a commit that referenced this pull request May 4, 2024
See python/cpython#111140

Also clean up and simplify the fallback implementation, fixing some reference leaks along the way.
@scoder scoder modified the milestones: 3.1, 3.0.11 Jun 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants