gh-99845: Change _PyDict_KeysSize() return type to size_t by vstinner · Pull Request #99848 · python/cpython · GitHub
Skip to content

gh-99845: Change _PyDict_KeysSize() return type to size_t - #99848

Merged
vstinner merged 2 commits into
python:mainfrom
vstinner:dict_sizeof
Nov 29, 2022
Merged

gh-99845: Change _PyDict_KeysSize() return type to size_t#99848
vstinner merged 2 commits into
python:mainfrom
vstinner:dict_sizeof

Conversation

@vstinner

@vstinner vstinner commented Nov 28, 2022 •
edited by bedevere-bot
Loading

Copy link
Copy Markdown
Member
  • Change _PyDict_KeysSize() and shared_keys_usable_size() return type from signed (Py_ssize_t) to unsigned (size_t) type.
  • new_values() argument type is now unsigned (size_t).
  • init_inline_values() now uses size_t rather than int for the 'i' iterator variable.
  • type.sizeof() implementation now uses unsigned (size_t) type.

@bedevere-bot bedevere-bot mentioned this pull request Nov 28, 2022
Closed
@vstinner
gh-99845: Change _PyDict_KeysSize() return type to size_t
b8953e1 
* Change _PyDict_KeysSize() and shared_keys_usable_size() return type
  from signed (Py_ssize_t) to unsigned (size_t) type.
* new_values() argument type is now unsigned (size_t).
* init_inline_values() now uses size_t rather than int for the 'i'
  iterator variable.
* type.__sizeof__() implementation now uses unsigned (size_t) type.
@vstinner

vstinner commented Nov 28, 2022

Copy link
Copy Markdown
Member Author

@vstinner

vstinner commented Nov 28, 2022

Copy link
Copy Markdown
Member Author

I would prefer to change _PyDict_SizeOf() return type to size_t, but it's not required to fix issue #99845, and it can impact the 3rd party frozendict project which uses it.

@vstinner

vstinner commented Nov 28, 2022

Copy link
Copy Markdown
Member Author

I would prefer to change _PyDict_SizeOf() return type to size_t, but it's not required to fix issue #99845, and it can impact the 3rd party frozendict project which uses it.

Another more radical change is to move the private function to internal C API, and frozendict should call dict.__sizeof__() method instead.

methane
methane approved these changes Nov 29, 2022
View reviewed changes

@methane methane 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.

The patch looks good to me.

Is Py_ssize_t recommended for container size and size_t recommended for memory size for now?

@vstinner
vstinner merged commit 4246fe9 into python:main Nov 29, 2022
@vstinner
vstinner deleted the dict_sizeof branch November 29, 2022 11:12
@vstinner

vstinner commented Nov 29, 2022

Copy link
Copy Markdown
Member Author

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@methane methane methane approved these changes
@markshannon markshannon Awaiting requested review from markshannon markshannon is a code owner

Assignees

No one assigned

Labels

skip news

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@vstinner @methane @bedevere-bot