Skip to content

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

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

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

AA-Turner
Copy link
Member

@AA-Turner AA-Turner commented Aug 12, 2025

serhiy-storchaka

This comment was marked as resolved.

@ericsnowcurrently
Copy link
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.

@ericsnowcurrently
Copy link
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.

@AA-Turner

This comment was marked as resolved.

@AA-Turner
Copy link
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

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