-
-
Notifications
You must be signed in to change notification settings - Fork 30.8k
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
gh-111140: Adds PyLong_AsNativeBytes and PyLong_FromNative[Unsigned]Bytes functions #114886
Merged
Merged
Changes from 20 commits
Commits
Show all changes
22 commits
Select commit
Hold shift + click to select a range
c86821e
gh-111140: Adds PyLong_AsByteArray functions
zooba cd6754a
Statics
zooba bf86f85
Better options handling and tests
zooba 22f3d14
Rework into PyLong_CopyBits
zooba 0dc0bcc
Merge remote-tracking branch 'upstream/main' into gh-111140
zooba c30e65a
Not static
zooba 9661b65
Add motivating example to tests
zooba 79d4942
Add FromBits functions
zooba bc9e48c
A test and a const
zooba b4761be
Space
zooba 043947f
Merge remote-tracking branch 'upstream/main' into gh-111140
zooba 761db5c
Docs
zooba 22c2a64
Merge remote-tracking branch 'upstream/main' into gh-111140
zooba 6e1a89a
Rename to PyLong_AsNativeBytes
zooba 7b44648
Buffer overflow in big endian
zooba 79dd452
Review feedback
zooba 9693afe
_resolve_endianness
zooba 376e358
More reliable test
zooba 1f2cf3a
Doc tweaks
zooba 7f4431d
Merge remote-tracking branch 'upstream/main' into gh-111140
zooba 2649d94
Merge remote-tracking branch 'upstream/main' into gh-111140
zooba 39962de
Switch to Py_ssize_t consistently instead of int and size_t
zooba File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
2 changes: 2 additions & 0 deletions
2
Misc/NEWS.d/next/C API/2024-02-05-17-11-15.gh-issue-111140.WMEjid.rst
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,2 @@ | ||
Adds :c:func:`PyLong_AsNativeBytes`, :c:func:`PyLong_FromNativeBytes` and | ||
:c:func:`PyLong_FromUnsignedNativeBytes` functions. |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Shouldn't this return
size_t
rather thanint
?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.
Negative values are required, so we'd have to do
Py_ssize_t
and notsize_t
. The cast is just as annoying either way unless we maken_bytes
also signed, which is then inconsistent with other APIs (but probably less bad than making it acceptint
).We might need some agreed upon guidelines for choosing types for these kinds of purposes.
int
is a very common return type, and IMHO that makes things easier for people trying to call this from non-C languages, but they probably have no choice but to supportPy_ssize_t
as a return type too and so it really wouldn't make a huge difference.I pity the poor CPU that has to convert a 32 billion bit number 🙃
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.
I really need to focus on not mixing up signed/unsigned when I write a comment...
Yes,
Py_ssize_t
is one of the types that users need to support.I'd much prefer using
Py_ssize_t
for sizes. These are arbitrary-sized integers, after all; limited by available memory.