FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

GH-132775: Fix argument parsing for ``_interpqueues.put()`` by AA-Turner · Pull Request #137686 · python/cpython · GitHub

/ cpython Public

GH-132775: Fix argument parsing for _interpqueues.put() - #137686

Merged
AA-Turner merged 3 commits into
python:mainfrom
AA-Turner:clinic/interpqueues-put
Aug 21, 2025
Merged

GH-132775: Fix argument parsing for _interpqueues.put()#137686
AA-Turner merged 3 commits into
python:mainfrom
AA-Turner:clinic/interpqueues-put

Conversation

AA-Turner commented Aug 12, 2025
edited by bedevere-app Bot
Loading

Copy link
Copy Markdown
Member

This comment was marked as resolved.

Copy link
Copy Markdown
Member

Just be sure I understood, the problem is that the arg spec for _queues.put() has one too many items in it. Is that right?

I'm pretty sure the $p shouldn't be there. The "fallback" arg is an int flag 1, so it should have an "i" in the spec. Thus, the spec for queuesmod_create() should not change and for queuesmod_put() it should be "O&O|ii:put".

FWIW, at the time, with the feature freeze at hand, I didn't worry about ironing out little API quirks (e.g. placeholder args) in the low-level module. It isn't exposed to users, right?

Footnotes

  1. See resolve_fallback() in Modules/_interpreters_common.h.

Copy link
Copy Markdown
Member

To be clear, I left room for other "fallback" options later, rather than it being a definite binary choice. Sorry that wasn't more clear. Then again, that decision was probably a premature accommodation for a possible future. I figure it doesn't matter much though.

This comment was marked as resolved.

Copy link
Copy Markdown
Member Author

Just be sure I understood, the problem is that the arg spec for _queues.put() has one too many items in it. Is that right?

Yes, specifically that the format spec for PyArg_ParseTupleAndKeywords expected five args, but kwlist gave only four.

I'm pretty sure the $p shouldn't be there. The "fallback" arg is an int flag 1, so it should have an "i" in the spec. Thus, the spec for queuesmod_create() should not change and for queuesmod_put() it should be "O&O|ii:put".

I've updated the PR. I don't think we need the test anymore given this is the current behaviour -- we are just removing the errant $p. Please would you be able to review/approve the updated PR?

FWIW, at the time, with the feature freeze at hand, I didn't worry about ironing out little API quirks (e.g. placeholder args) in the low-level module. It isn't exposed to users, right?

Definitely the correct choice. _interpqueues is technically exposed to users by dint of being importable, but is entirely undocumented and of course a private module. This PR is just a correctness fix that Serhiy spotted, but it would nevertheless be good to include it in 3.14.0 candidate 3 / final.

A

ericsnowcurrently 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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

LGTM

AA-Turner merged commit 79aeeb8 into python:main Aug 21, 2025
46 checks passed

Copy link
Copy Markdown

Thanks @AA-Turner for the PR 🌮🎉.. I'm working now to backport this PR to: 3.14.
🐍🍒⛏🤖

AA-Turner deleted the clinic/interpqueues-put branch August 21, 2025 22:01
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Aug 21, 2025
…thonGH-137686)

(cherry picked from commit 79aeeb8)

Co-authored-by: Adam Turner <9087854+AA-Turner@users.noreply.github.com>

bedevere-app Bot commented Aug 21, 2025

Copy link
Copy Markdown

GH-138034 is a backport of this pull request to the 3.14 branch.

bedevere-app Bot removed the needs backport to 3.14 bugs and security fixes label Aug 21, 2025

Copy link
Copy Markdown
Member

And I think that it would be better to remove passing explicit -1 as the fallback argument to _queue.create().

hugovk pushed a commit that referenced this pull request Aug 27, 2025
…H-137686) (#138034)

Co-authored-by: Adam Turner <9087854+AA-Turner@users.noreply.github.com>
kumaraditya303 pushed a commit to miss-islington/cpython that referenced this pull request Sep 9, 2025
…)`` (pythonGH-137686) (python#138034)

Co-authored-by: Adam Turner <9087854+AA-Turner@users.noreply.github.com>
This file contains hidden or 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
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.

3 participants


Back | FazBrowse Home | New Git URL