GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — gh-87613: Pass NULL for an empty **kwargs in Argument Clinic by cmaloney · Pull Request #161382 · python/cpython

Conversation

@cmaloney

@cmaloney cmaloney commented Nov 5, 2026 •
edited by bedevere-app Bot
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

The generated parser allocated an empty dict for **kwargs when there were no keyword arguments. Pass NULL instead, like the caller does, and handle NULL in the impls that used the dict unconditionally.

This is preparation work for supporting **kwargs in Argument Clinic Vectorcall where allocating a dict instead of passing NULL makes a significant performance impact.

All **kwargs impls were audited: _csv and zoneinfo already accept NULL, and the rest (structseq and SimpleNamespace __replace__, _interpreters.new_config, atexit.register, SubElement) now check for it. atexit stores None, which the run loop already maps back to NULL.

pyperf, PGO+LTO, t = time.gmtime(0), ns = SimpleNamespace(a=1):

Benchmark main this change
struct_time.__replace__() 229 ns 47.0 ns: 4.87x faster
SimpleNamespace.__replace__() 72.2 ns 65.5 ns: 1.10x faster
  • Issue: Generate vectorcall code to parse arguments using Argument Clinic #87613

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@cmaloney @claude
pythongh-87613: Pass NULL for an empty **kwargs in Argument Clinic
82837ae
The parser allocated an empty dict for **kwargs when there were no
keyword arguments.  Pass NULL instead, like the caller does, and handle
NULL in the impls that used the dict unconditionally.

All **kwargs impls were audited: _csv and zoneinfo already accept NULL,
and the rest (structseq and SimpleNamespace `__replace__`,
_interpreters.new_config, atexit.register, SubElement) now check for it.
atexit stores None, which the run loop already maps back to NULL.

pyperf, PGO+LTO, `t = time.gmtime(0)`, `ns = SimpleNamespace(a=1)`:

| Benchmark                       | main   | this change           |
|---------------------------------|:------:|:---------------------:|
| struct_time.`__replace__`()     | 229 ns | 47.0 ns: 4.87x faster |
| SimpleNamespace.`__replace__`() | 72.2 ns | 65.5 ns: 1.10x faster |

Co-Authored-By: Claude Opus 5.5 <[email protected]>
@cmaloney
cmaloney requested review from AA-Turner, ericsnowcurrently and erlend-aasland as code owners November 5, 2026 07:03
@bedevere-app bedevere-app Bot added awaiting core review type-feature A feature request or enhancement labels Nov 5, 2026
@bedevere-app bedevere-app Bot mentioned this pull request Nov 5, 2026
Generate vectorcall code to parse arguments using Argument Clinic #87613
Open
@cmaloney cmaloney added the skip news label Nov 5, 2026
msullivan
msullivan reviewed Nov 5, 2026
View reviewed changes
Comment thread Tools/clinic/libclinic/converters.py
Comment on lines +1452 to +1453
# NULL when there are no keywords: no call allocates an empty dict.
return f"{self.parser_name} = Py_XNewRef(kwargs);\n"

@msullivan msullivan Nov 5, 2026 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Is there anywhere to document the new behavior, to make sure this isn't a footgun?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

@cmaloney cmaloney Nov 5, 2026

Copy link
Copy Markdown
Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

https://devguide.python.org/development-tools/clinic/ is all the docs and doesn't go into that much detail. For https://docs.python.org/3.13/c-api/call.html#c.vectorcallfunc it is documented as able to be NULL, so this to me is bringing AC parsers into that same state.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
msullivan
msullivan approved these changes Nov 5, 2026
View reviewed changes
@cmaloney

cmaloney commented Nov 6, 2026

Copy link
Copy Markdown
Contributor Author

@eendebakpt could you take a look at this one?

This + the "support **kwargs in AC" that comes after enable using AC vectorcall for a speedup on most objects, in particular BaseException. It also should enable converting dict and frozendict from handwritten.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

eendebakpt
eendebakpt approved these changes Nov 7, 2026
View reviewed changes

@eendebakpt eendebakpt left a comment

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Looks good to me. The new behavior of AC is fine (it is a behavior change for external users of AC, but AC is supported to be only an internal tool).

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@bedevere-app bedevere-app Bot added awaiting merge and removed awaiting core review labels Nov 7, 2026
@cmaloney
cmaloney merged commit 9ea60a5 into python:main Nov 7, 2026
68 of 72 checks passed

Uh oh!

There was an error while loading. Please reload this page.

@bedevere-app bedevere-app Bot removed the awaiting merge label Nov 7, 2026
@cmaloney
cmaloney deleted the clinic_kwargs_null_pr branch November 7, 2026 22:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@msullivan msullivan msullivan approved these changes
@eendebakpt eendebakpt eendebakpt approved these changes
@ericsnowcurrently ericsnowcurrently Awaiting requested review from ericsnowcurrently ericsnowcurrently is a code owner
@erlend-aasland erlend-aasland Awaiting requested review from erlend-aasland erlend-aasland is a code owner
@AA-Turner AA-Turner Awaiting requested review from AA-Turner AA-Turner is a code owner

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

skip news type-feature A feature request or enhancement

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

3 participants

@cmaloney @msullivan @eendebakpt
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402520 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.