📦 Library Dependencies

The following Lib/ modules were modified. Here are their dependencies:

[x] test: cpython/Lib/test/test_dict.py (TODO: 3)
[x] test: cpython/Lib/test/test_dictcomps.py (TODO: 1)
[x] test: cpython/Lib/test/test_dictviews.py (TODO: 1)
[x] test: cpython/Lib/test/test_userdict.py
[x] test: cpython/Lib/test/mapping_tests.py

dependencies:

dependent tests: (no tests depend on dict)

Legend:

• [+] path exists in CPython
• [x] up-to-date, [ ] outdated

sequenceDiagram
  participant DictOperation
  participant hash_or_unhashable
  participant Dict
  DictOperation->>hash_or_unhashable: Hash key
  hash_or_unhashable-->>DictOperation: Return hash or exception
  DictOperation->>Dict: Perform known-hash operation
  Dict-->>DictOperation: Return lookup or mutation result

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

_{Comment @coderabbitai help to get the list of available commands.}

Good catch — fixed in the follow-up commit. You're right the wrapper was rewriting comparison TypeErrors too. I now disambiguate on the error path (re-hash only when an operation already failed), so the successful path still hashes exactly once — keeping the do-not-rehash / atomic invariants — while comparison errors propagate unchanged. Added a colliding-key regression test.

@Jorge-Polanco-Roque Please do not remove pull request template and check AI policy

Done — restored the PR template and checked "This PR follows our AI policy", and added an AI-disclosure note (tool: Claude Opus 4.8; the commits carry Assisted-by: trailers). Thanks for the heads-up.

Thanks for the detailed CPython comparison — exactly the review this needed. Addressed in the latest commit:

1. Coverage (constructor / update / |=): right, those go through merge_object_with_override / merge_from_seq2 and bypassed the wrapper. Both now hash the key up front and thread it into contains_known_hash / insert_known_hash, so dict([([],1)]), d.update([([],1)]) and d |= [([],1)] all get the message. merge_dict is left alone since its keys come from a real dict.

2. The re-hash: dropped. Each op hashes once via a hash_or_unhashable helper and threads the hash through the *_known_hash methods (added get_known_hash to the inner map), so a __hash__ failing only on its first call is now reported instead of escaping — covered by a flaky-hash test. pop/setdefault have no *_known_hash entry point, so they hash once up front for the message and then do their own single lookup; happy to add those variants if you'd prefer them fully threaded.

3. Qualified name: now uses fully_qualified_name, so a nested class reports maker.<locals>.Nested.

4. Exact type: the guard checks the exact TypeError type, so a TypeError subclass raised from __hash__ propagates unchanged and except MySubclass still catches it.

Added regression tests for all four to the snippet. set.rs still has 3 and 4 (and doesn't re-hash, so not 2) — happy to do a follow-up there if it's wanted.

Good catch — addressed. Added setdefault_known_hash and pop_known_hash to the inner map and threaded the precomputed hash into both, so every operation now hashes the key exactly once and a stateful __hash__ can't skip the dict-specific message on a second call. Added a regression test asserting setdefault/pop invoke __hash__ only once.

Read the new version. Hashing once up front and threading it through the *_known_hash calls is exactly it, and the intermittent __hash__ case falls out for free rather than needing its own handling. fully_qualified_name and the exact-type check are both in, and merge_object_with_override and merge_from_seq2 go through the same path now, so the constructor, update and |= are covered.

Nothing left from my side.

Merging this PR will degrade performance by 21.44%

⚡ 1 improved benchmark
❌ 1 regressed benchmark
✅ 64 untouched benchmarks

Performance Changes

_{Comparing Jorge-Polanco-Roque:feat/dict-unhashable-key-message (800497d) with main (287dcd9)}

Fixed the CI: {}.pop(CountingHash()) on an empty dict doesn't hash the key under CPython 3.14 (the lookup short-circuits), so the calls == 1 assertion failed its CPython parity run. Switched to a non-empty dict so the lookup must hash exactly once on both CPython and RustPython, and ran ruff format. Verified locally with python3.14 extra_tests/snippets/builtin_dict.py (exit 0) and ruff check clean.

Thanks for the reviews here. All the CodeRabbit feedback has been addressed and CI is green except for CodSpeed, which is a performance benchmark unaffected by a dict error-message change (there's no baseline for the fork branch). Whenever a maintainer has a moment, could this get a look? Happy to rebase if it helps.

The only red check is CodSpeed, and I believe it's a cross-environment false positive rather than a real regression from this change. The single benchmark flagged is gc_collect.py (-19%), while every dict/set-sensitive benchmark (comprehension_dict, frozenset, comprehension_set, cmp, sorted) is unchanged — which is the opposite of what a regression in dict's hot path would look like. CodSpeed's own report notes "Different runtime environments detected ... which may affect the accuracy of the results."

For what it's worth, the new error message is built lazily: the format! and __cause__ assignment live only in the Err arm, so the happy path hashes the key exactly once, same as before. Could a maintainer re-run the workflow (or acknowledge the CodSpeed result) so it re-measures in a consistent environment? Happy to dig further if a real regression shows up on a clean run.

@youknowone Thanks, and apologies for the earlier template removal — that was my mistake. I've brought the PR into compliance with the AI policy:

• The PR template is restored, with "This PR follows our AI policy" checked.
• AI usage is disclosed in the description: the code was implemented with the assistance of Claude (Opus 4.8, Anthropic).
• Every commit carries an Assisted-by: Claude Opus 4.8 (Anthropic) trailer.
• Per the "fully verified with human use" rule, I reviewed the whole diff and verified it locally: the newly enabled test_dict.DictTest.test_unhashable_key, plus the full test_dict (121) and test_set (630) suites pass, and cargo fmt/clippy are clean.

Happy to adjust anything else the policy requires.

Renamed pop_known_hash to pop like you suggested. You're right that the suffix stopped making sense once the plain pop was gone.

And point taken on the earlier comments. That was lazy of me, I leaned on generated text and passed it off as my own words. From here on I'll write my own replies and only quote generated stuff when I actually mean to quote it. Thanks for the patience.