Repository navigation
Disable type caching for custom MRO hierarchies - #9477
Conversation
Track permanent version-tag ineligibility after custom MRO publication and bases changes, propagate it through version assignment, and retain existing MCACHE invalidation. Restore canonical metaclass doctests through update_lib. Assisted-by: OpenAI Codex: model version unavailable
|
Caution Review failedThe pull request is closed. Finishing Touches
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 |
📦 Library DependenciesThe following Lib/ modules were modified. Here are their dependencies: [x] test: cpython/Lib/test/test_metaclass.py dependencies: dependent tests: (no tests depend on metaclass) Legend:
|
Merging this PR will not alter performance
Comparing 
Footnotes
|
| pub tp_version_tag: AtomicU32, | ||
| /// Permanent cache exclusion after an unsafe MRO, like CPython's | ||
| /// _Py_ATTR_CACHE_UNUSED state. A zero version alone is only temporary. | ||
| pub(crate) version_tag_disabled: AtomicBool, |
There was a problem hiding this comment.
is this encoded in same way in CPython too? isnt' this integrated into tp_version_tag?
There was a problem hiding this comment.
CPython keeps these states separate: type_mro_modified() clears tp_version_tag to 0 and sets tp_versions_used = _Py_ATTR_CACHE_UNUSED. assign_version_tag() rejects that sentinel through its version-budget check.
The bool here models only permanent custom-MRO cache exclusion, not CPython’s per-type version budget. A zero tag remains temporary invalidation; the existing cache retirement/invalidation semantics are unchanged.
There was a problem hiding this comment.
align field names to cpython in that case
There was a problem hiding this comment.
Updated in ddec470: the field is now tp_versions_used, using AtomicU16 and CPython's _Py_ATTR_CACHE_UNUSED value (30000) as ATTR_CACHE_UNUSED. The existing zero/sentinel state transitions and atomic ordering are unchanged. Per-type version counting/budgeting is still not implemented, and the field comment states that explicitly.
Scoped Cargo check and strict Clippy passed; the existing canonical metaclass coverage will run again in CI.
Use tp_versions_used and the CPython ATTR_CACHE_UNUSED sentinel for permanent cache exclusion. Keep the existing zero/sentinel state behavior and atomic ordering; per-type version budgeting remains unimplemented. Assisted-by: OpenAI Codex: model version unavailable
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Summary
Prevent cached type attributes from becoming stale when a custom MRO includes a class outside the physical base hierarchy.
__bases__changes, using raw metaclassmrodescriptor identity and CPython's MRO/base subtype conditions.test_metaclass.pywithscripts/update_lib migrate, removing all three skipped attribute reads and the custom doctest loader. No test assertions were edited manually.CPython basis
Compared directly with
Objects/typeobject.cat d220409a13b77de7a6c30abe16ea90db01528edc:has_custom_mro,type_mro_modified,mro_internalandassign_version_tag.The permanent eligibility state mirrors CPython's
_Py_ATTR_CACHE_UNUSEDsentinel, including conservative exclusion after a failed bases transition. Raw lookup bypasses descriptor binding, but dictionary key comparisons can still invoke callbacks; this patch makes no general callback-free or concurrency-safety claim.Validation
AI assistance
OpenAI Codex assisted with implementation, CPython source comparison, update_lib synchronization and validation preparation. Model version is unavailable.