Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork34k
gh-143189: fix insertdict() for non-Unicode key#143285
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
When iserting non unicode key into split table, matching Unicode keymay be in the shared split table without its value.
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.
Pull request overview
This PR fixes a crash in the CPython dictionary implementation when inserting a non-str key (specifically astr subclass) into a split table dictionary where the key matches an existing key in the shared split table but has no corresponding value in the instance dictionary yet.
Key Changes:
- Fixed the logic in
insertdict()to checkold_value == NULLinstead ofix == DKIX_EMPTYto properly handle split tables where a key exists without a value - Added explanatory comments documenting this edge case behavior
- Added test coverage for the specific scenario that was causing the crash
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| Objects/dictobject.c | Fixed the condition check ininsertdict() to useold_value == NULL instead ofix == DKIX_EMPTY and added clarifying comments about split table behavior |
| Misc/NEWS.d/next/Core_and_Builtins/2025-12-30-06-48-48.gh-issue-143189.in_sv2.rst | Added changelog entry describing the crash fix |
| Lib/test/test_dict.py | Added new test casetest_split_table_insert_with_str_subclass to verify the fix works correctly |
💡Add Copilot custom instructions for smarter, more guided reviews.Learn how to get started.
Lib/test/test_dict.py Outdated
| obj2 = MyClass() | ||
| d = obj2.__dict__ | ||
| d[MyStr("attr1")] = 2 | ||
| assert isinstance(list(d)[0], MyStr) |
CopilotAIDec 30, 2025
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.
Use unittest assertion method instead of bare assert statement. This ensures the test properly reports failures with helpful messages and follows unittest best practices. Replace withself.assertTrue(isinstance(list(d)[0], MyStr)).
| assertisinstance(list(d)[0],MyStr) | |
| self.assertTrue(isinstance(list(d)[0],MyStr)) |
43c7658 intopython:mainUh oh!
There was an error while loading.Please reload this page.
Sorry,@methane and@hugovk, I could not cleanly backport this to |
Sorry,@methane and@hugovk, I could not cleanly backport this to |
hugovk commentedJan 12, 2026
@methane Please can you check the backports? |
GH-143771 is a backport of this pull request to the3.14 branch. |
GH-143772 is a backport of this pull request to the3.13 branch. |
Uh oh!
There was an error while loading.Please reload this page.
When iserting non unicode key into split table, matching Unicode key may be in the shared split table without its value.
insertdict#143189