Skip to content

gh-120389: Add PyLong_FromInt64() and PyLong_AsInt64() - #120390

Merged
vstinner merged 12 commits into
python:mainfrom
vstinner:stdint
Aug 28, 2024
Merged

gh-120389: Add PyLong_FromInt64() and PyLong_AsInt64()#120390
vstinner merged 12 commits into
python:mainfrom
vstinner:stdint

Conversation

@vstinner

@vstinnervstinner commented Jun 12, 2024•
edited
Loading

Copy link
Copy Markdown
Member

Add new functions to convert C <stdint.h> numbers from/to Python int:

  • PyLong_FromInt32()
  • PyLong_FromUInt32()
  • PyLong_FromInt64()
  • PyLong_FromUInt64()
  • PyLong_AsInt32()
  • PyLong_AsUInt32()
  • PyLong_AsInt64()
  • PyLong_AsUInt64()

@bedevere-appbedevere-appBot mentioned this pull request Jun 12, 2024
Closed
@vstinner

vstinner commented Jun 12, 2024

Copy link
Copy Markdown
MemberAuthor

Draft PR to see how #120389 can be implemented. I will complete the PR once we agree on #120389 design.

@vstinnervstinner changed the title gh-120389: Add PyLong_FromInt64() and PyLong_AsInt64()gh-120389: Add PyLong_FromInt64() and PyLong_ToInt64()Jun 17, 2024
@vstinner
vstinnerforce-pushed the stdint branch 4 times, most recently from 93e27a1 to 684360eCompareJune 19, 2024 15:29
@vstinner
pythongh-120389: Add PyLong_FromInt64() and PyLong_ToInt64()
3e50341
Add new functions to convert C <stdint.h> numbers from/to Python int:
* PyLong_FromInt32()
* PyLong_FromUInt32()
* PyLong_FromInt64()
* PyLong_FromUInt64()
* PyLong_ToInt32()
* PyLong_ToUInt32()
* PyLong_ToInt64()
* PyLong_ToUInt64()
@vstinner
vstinner marked this pull request as ready for review June 19, 2024 16:36
@vstinner
vstinner requested review from a team and encukou as code ownersJune 19, 2024 16:36
@vstinner

vstinner commented Jun 19, 2024

Copy link
Copy Markdown
MemberAuthor

This PR is now ready for review.

@vstinnervstinner mentioned this pull request Jun 19, 2024
Closed
zooba
zooba reviewed Jun 19, 2024
View reviewed changes
Comment threadObjects/longobject.c Outdated
@vstinner
Use PyLong_AsNativeBytes()
c239d18
PyLong_ToUInt32() and PyLong_ToUInt64() can now use the __index__()
method if the object has the method.
zooba
zooba reviewed Jun 20, 2024
View reviewed changes
Comment threadDoc/c-api/long.rst Outdated
zooba
zooba approved these changes Jun 20, 2024
View reviewed changes

@zoobazooba left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM. Would like it if we don't have to have docs like "like this other function", but if that's the best we can do then I'll survive.

@vstinner

vstinner commented Jun 20, 2024

Copy link
Copy Markdown
MemberAuthor

Would like it if we don't have to have docs like "like this other function", but if that's the best we can do then I'll survive.

I grouped functions by pairs in the documentation.

@zooba

zooba commented Jun 20, 2024•
edited
Loading

Copy link
Copy Markdown
Member

The formatting in the docs preview looks good 👍

image
image

@vstinnervstinner mentioned this pull request Jun 24, 2024
Closed
@serhiy-storchaka

serhiy-storchaka commented Jun 24, 2024

Copy link
Copy Markdown
Member

Calling __index__ can release the GIL and make values saved in C variables (borrowed references, the size of a list, etc) invalid. Existing code can rely on atomicity of PyLong_AsUnsignedLong(), so calling __index__ in it is an unsafe change.

@vstinner

vstinner commented Jun 24, 2024

Copy link
Copy Markdown
MemberAuthor

Existing code can rely on atomicity of PyLong_AsUnsignedLong(), so calling index in it is an unsafe change.

This PR doesn't change PyLong_AsUnsignedLong(). Are you referring to code replacing PyLong_AsUnsignedLong() with one of these functions?

Calling index can release the GIL and make values saved in C variables (borrowed references, the size of a list, etc) invalid.

For me, it's strange that signed and unsigned integers are treated diffferently. I would like to treat them the same.

The GIL issue can be mentioned in the doc? For example:

This function can release the GIL temporarily indirectly while calling an __index__() method.

@vstinner

vstinner commented Jun 24, 2024

Copy link
Copy Markdown
MemberAuthor

I created capi-workgroup/decisions#32 "Add PyLong_FromInt64() and PyLong_ToInt64()" in the C API WG Decisions project.

@vstinnervstinner mentioned this pull request Jul 4, 2024
Open
serhiy-storchaka
serhiy-storchaka approved these changes Jul 22, 2024
View reviewed changes

@serhiy-storchakaserhiy-storchaka left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM.

Comment threadModules/_testlimitedcapi/long.c
Comment threadModules/_testcapimodule.c
Comment on lines +4050 to +4055
PyModule_AddObject(m, "INT32_MIN", PyLong_FromInt32(INT32_MIN));
PyModule_AddObject(m, "INT32_MAX", PyLong_FromInt32(INT32_MAX));
PyModule_AddObject(m, "UINT32_MAX", PyLong_FromUInt32(UINT32_MAX));
PyModule_AddObject(m, "INT64_MIN", PyLong_FromInt64(INT64_MIN));
PyModule_AddObject(m, "INT64_MAX", PyLong_FromInt64(INT64_MAX));
PyModule_AddObject(m, "UINT64_MAX", PyLong_FromUInt64(UINT64_MAX));

@serhiy-storchakaserhiy-storchakaJul 22, 2024

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is not needed. You can define them in the Python code as INT32_MAX = 2**31 - 1 etc.

@vstinnervstinnerAug 27, 2024

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

I prefer to reuse <limits.h> C constants to avoid any typo.

Comment threadLib/test/test_capi/test_long.py
@serhiy-storchakaserhiy-storchaka mentioned this pull request Jul 22, 2024
Merged
picnixz
picnixz reviewed Jul 22, 2024
View reviewed changes
Comment threadDoc/c-api/long.rst Outdated
Comment threadDoc/c-api/long.rst Outdated
Comment threadObjects/longobject.c Outdated
Comment threadObjects/longobject.c
@vstinnervstinner changed the title gh-120389: Add PyLong_FromInt64() and PyLong_ToInt64()gh-120389: Add PyLong_FromInt64() and PyLong_AsInt64()Aug 27, 2024
@vstinner

vstinner commented Aug 27, 2024

Copy link
Copy Markdown
MemberAuthor

@serhiy-storchaka@zooba@encukou: Would you mind to review the updated PR? I updated the PR to the API approved the C API WG. It now uses the __index__() method and functions are called "As" (instead of "To").

serhiy-storchaka
serhiy-storchaka approved these changes Aug 27, 2024
View reviewed changes

@serhiy-storchakaserhiy-storchaka left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM, but check_long_asunsignedint is now superseded by check_long_asint.

Comment threadLib/test/test_capi/test_long.py Outdated
@vstinner

vstinner commented Aug 28, 2024

Copy link
Copy Markdown
MemberAuthor

but check_long_asunsignedint is now superseded by check_long_asint.

I removed check_long_asunsignedint() to reuse check_long_asint().

encukou
encukou approved these changes Aug 28, 2024
View reviewed changes
Comment threadDoc/c-api/long.rst Outdated
Comment threadDoc/c-api/long.rst Outdated
vstinnerand others added 2 commits August 28, 2024 11:51
@vstinner@encukou
Update Doc/c-api/long.rst
dd2194e
Co-authored-by: Petr Viktorin <encukou@gmail.com>
@vstinner@encukou
Update Doc/c-api/long.rst
7070514
Co-authored-by: Petr Viktorin <encukou@gmail.com>
@vstinner
vstinner enabled auto-merge (squash) August 28, 2024 09:52
@vstinner
vstinner merged commit 4c6dca8 into python:mainAug 28, 2024
@vstinner
vstinner deleted the stdint branch August 28, 2024 10:16
@KRRT7KRRT7 mentioned this pull request May 28, 2026
Open
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@encukouencukouencukou approved these changes
@zoobazoobazooba approved these changes
@serhiy-storchakaserhiy-storchakaserhiy-storchaka approved these changes
@picnixzpicnixzpicnixz approved these changes

Assignees

No one assigned

Labels

skip news

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@vstinner@zooba@serhiy-storchaka@encukou@picnixz