Skip to content

GH-145247: Use _PyTuple_FromPair[Steal] in Objects - #145884

Merged
vstinner merged 16 commits into
python:mainfrom
sergey-miryanov:feat/145247-pytuple-from-pair-use-2
Mar 28, 2026
Merged

GH-145247: Use _PyTuple_FromPair[Steal] in Objects#145884
vstinner merged 16 commits into
python:mainfrom
sergey-miryanov:feat/145247-pytuple-from-pair-use-2

Conversation

@sergey-miryanov

@sergey-miryanovsergey-miryanov commented Mar 12, 2026

Copy link
Copy Markdown
Contributor

It is a second PR for usage of _PyTuple_FromPair[Steal] in the codebase.

Comment threadObjects/stringlib/unicode_format.h Outdated
Comment threadObjects/stringlib/unicode_format.h Outdated
Comment threadObjects/dictobject.c
Comment threadObjects/listobject.c Outdated

@sergey-miryanovsergey-miryanov left a comment

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Address review

Comment threadObjects/stringlib/unicode_format.h Outdated
Comment threadObjects/stringlib/unicode_format.h Outdated
Comment threadObjects/listobject.c Outdated

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

This change introduces a leak in the dict type (see my comment below about a leak).

Comment threadObjects/stringlib/unicode_format.h Outdated
Comment threadObjects/stringlib/unicode_format.h Outdated
Comment threadObjects/dictobject.c
Comment threadObjects/enumobject.c Outdated
Comment threadObjects/enumobject.c Outdated
Comment threadObjects/longobject.c Outdated
Comment threadObjects/longobject.c Outdated
Comment threadObjects/longobject.c Outdated
Comment threadObjects/odictobject.c Outdated
Comment threadObjects/odictobject.c Outdated
Co-authored-by: Victor Stinner <vstinner@python.org>
@vstinner

Copy link
Copy Markdown
Member

Ah, there are now compiler warnings, about unused variables if I understood correctly:

Objects/odictobject.c expected 0 warnings, found 1
{'file': 'Objects/odictobject.c', 'line': '1159', 'column': '29', 'message': 'unused variable ‘item’', 'option': '-Wunused-variable'}
Objects/longobject.c expected 0 warnings, found 3
{'file': 'Objects/longobject.c', 'line': '4882', 'column': '15', 'message': 'unused variable ‘z’', 'option': '-Wunused-variable'}
{'file': 'Objects/longobject.c', 'line': '6113', 'column': '27', 'message': 'unused variable ‘result’', 'option': '-Wunused-variable'}
{'file': 'Objects/longobject.c', 'line': '6356', 'column': '15', 'message': 'unused variable ‘ratio_tuple’', 'option': '-Wunused-variable'}

@sergey-miryanov

Copy link
Copy Markdown
ContributorAuthor

Ah, there are now compiler warnings, about unused variables if I understood correctly:

Yeah, I fixed it. Working on left review comments.

@sergey-miryanov

Copy link
Copy Markdown
ContributorAuthor

@vstinner Thanks for review! Could you please take a look?

@vstinnervstinner 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. I have have a few more coding style suggestions.

Comment threadObjects/dictobject.c Outdated
Comment threadObjects/dictobject.c Outdated
Comment threadObjects/floatobject.c Outdated
Comment threadObjects/frameobject.c
Comment threadObjects/odictobject.c

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

Sorry, more sugestions for dictitems_xor_lock_held() which is the most complex function of this change. I propose further changes to make the code easier to follow.

Comment threadObjects/dictobject.c
Comment threadObjects/dictobject.c Outdated
@vstinner
vstinner enabled auto-merge (squash) March 27, 2026 21:31
@vstinner

Copy link
Copy Markdown
Member

Thanks for all updates! I enabled auto-merge on the PR. IMO this change makes the code easier to follow/review.

@sergey-miryanov

Copy link
Copy Markdown
ContributorAuthor

Thanks for review!

@vstinner
vstinner merged commit a933e9c into python:mainMar 28, 2026
88 of 90 checks passed
clin1234 pushed a commit to clin1234/cpython that referenced this pull request Apr 16, 2026
Co-authored-by: Victor Stinner <vstinner@python.org>
ljfp pushed a commit to ljfp/cpython that referenced this pull request Apr 25, 2026
Co-authored-by: Victor Stinner <vstinner@python.org>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@sergey-miryanov@vstinner@methane