Skip to content

gh-156500: Reject non-positional parameters in wrap_dll_function() - #156501

Open
fedonman wants to merge 2 commits into
python:mainfrom
fedonman:fix-wrap-dll-function-param-kinds
Open

gh-156500: Reject non-positional parameters in wrap_dll_function()#156501
fedonman wants to merge 2 commits into
python:mainfrom
fedonman:fix-wrap-dll-function-param-kinds

Conversation

@fedonman

@fedonman fedonman commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

argtypes was built from every annotated parameter, so an annotated keyword-only, *args or **kwargs parameter contributed an extra positional entry. Such a parameter has no positional counterpart to describe, so it now raises ValueError at decoration time.

$ ./python -m test test_ctypes
Total tests: run=628 skipped=41
Result: SUCCESS

No NEWS entry: wrap_dll_function is new in 3.16 and has not been released, so this is folded into the original entry and needs the skip news label.

@read-the-docs-community

read-the-docs-community Bot commented Aug 27, 2026

Copy link
Copy Markdown

Documentation build overview

📚 cpython-previews | 🛠️ Build #34283116 | 📁 Comparing fd94ad9 against main (24e5a55)

  🔍 Preview build  

2 files changed
± library/ctypes.html
± whatsnew/changelog.html

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

Let's raise an exception for these rather than skipping them. Remember, errors should never pass silently.

@bedevere-app

bedevere-app Bot commented Aug 27, 2026

Copy link
Copy Markdown

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

argtypes was built from every annotated parameter, so an annotated
keyword-only, *args or **kwargs parameter contributed an extra positional
entry. Such a parameter has no positional counterpart to describe, so it
now raises ValueError at decoration time.
@fedonman
fedonman force-pushed the fix-wrap-dll-function-param-kinds branch from 77f100d to 453280a Compare August 28, 2026 17:27
@fedonman fedonman changed the title gh-156500: Skip non-positional parameters in wrap_dll_function() gh-156500: Reject non-positional parameters in wrap_dll_function() Aug 28, 2026
@fedonman

Copy link
Copy Markdown
Contributor Author

I have made the requested changes; please review again

@bedevere-app

bedevere-app Bot commented Aug 28, 2026

Copy link
Copy Markdown

Thanks for making the requested changes!

@ZeroIntensity: please review the changes made to this pull request.

@bedevere-app
bedevere-app Bot requested a review from ZeroIntensity August 28, 2026 17:28
Comment thread Doc/library/ctypes.rst Outdated
:exc:`ValueError` is raised. If the name of the function does not exist
in *dll*, an :exc:`AttributeError` is raised.
:exc:`ValueError` is raised. A :exc:`ValueError` is also raised if it has a
keyword-only, ``*args`` or ``**kwargs`` parameter, since

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 Oxford comma significantly improves clarity here:

Suggested change
keyword-only, ``*args`` or ``**kwargs`` parameter, since
keyword-only, ``*args``, or ``**kwargs`` parameter, since

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thanks, done.

Comment on lines +156 to +158
def test_wrap_dll_function_non_positional(self):
# argtypes describes positional arguments only, so a parameter that
# cannot be passed positionally is rejected.

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.

Can you also test that positional-only arguments (def PyObject_GetAttr(op: ctypes.py_object, attr: ctypes.py_object, /)) do not raise an exception?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thanks, done.

@fedonman

Copy link
Copy Markdown
Contributor Author

I have made the requested changes; please review again

@bedevere-app

bedevere-app Bot commented Aug 28, 2026

Copy link
Copy Markdown

Thanks for making the requested changes!

@ZeroIntensity: please review the changes made to this pull request.

@bedevere-app
bedevere-app Bot requested a review from ZeroIntensity August 28, 2026 21:57
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.

2 participants