slots xor dict , name mangling by youknowone · Pull Request #6392 · RustPython/RustPython

youknowone

youknowone commented
Dec 10, 2025
•

edited by coderabbitai Bot

Loading

Summary by CodeRabbit

Bug Fixes
- Improved object attribute initialization based on class configuration
- Fixed type inheritance to properly handle base class attributes
- Enhanced support for generic typing and parameterized types
- Corrected Python name mangling for private attributes in class definitions

_{✏️ Tip: You can customize this high-level summary in your review settings.}

Walkthrough

The pull request modifies object and type construction in RustPython. It replaces a hardcoded object-type check with a HAS_DICT flag check for dict allocation, inherits HAS_DICT from all bases in the MRO during type creation, introduces Python name mangling for private slot attributes, and adds new __slots__ and __init_subclass__ methods to the typing.Generic class.

Changes

Cohort / File(s)	Summary
Object dict allocation `crates/vm/src/builtins/object.rs`	Replaced special-case check for object_type with HAS_DICT flag check; dict creation now depends on whether the class has the HAS_DICT flag set.
Type creation and slot handling `crates/vm/src/builtins/type.rs`	Inherit HAS_DICT from all bases in MRO (not just first base); introduce name mangling helper for private slot attributes; apply mangling to slot definitions and descriptors; add `__dict__` descriptor conditionally based on HAS_DICT flag.
Generic typing class `crates/vm/src/stdlib/typevar.rs`	Added Context import; added `__slots__()` method returning empty tuple; added `__init_subclass__()` method that delegates to `_generic_init_subclass` via `call_typing_args_kwargs`.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

type.rs requires careful review of the name mangling logic, MRO-based flag inheritance, and the conditional __dict__ descriptor addition to ensure slot handling is correct
object.rs change is straightforward (flag-based check) but should be verified to work correctly with the type.rs modifications
typevar.rs additions are simple and low-risk

Possibly related PRs

feat(vm/slot): implement Py_TPFLAGS_MANAGED_DICT for class objects #5949: Modifies how the HAS_DICT flag is set during type/object creation; directly affects dict allocation logic
Fix __dict__ getset type #6010: Modifies __dict__ descriptor detection and HAS_DICT inheritance in type.rs; overlaps significantly with flag propagation changes
relocate Generic under typevar.rs #5889: Relocates Generic to typevar.rs and introduces helper to forward __init_subclass__; directly related to typing.Generic changes

Poem

🐰 With HAS_DICT flags dancing through the MRO,
Private names now mangled from high to low,
Objects gain dicts with a flag's gentle say,
Generic subclasses now find their way! ✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 71.43% which is insufficient. The required threshold is 80.00%.	You can run `@coderabbitai generate docstrings` to improve docstring coverage.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately reflects the main changes: implementing slots xor dict logic and name mangling for private attributes across multiple files.

✨ Finishing touches

📝 Generate docstrings

🧪 Generate unit tests (beta)

Create PR with unit tests
Post copyable unit tests in a comment

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.

❤️ Share

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

youknowone changed the title ~~__slots__ xor __dict__~~ __slots__ xor __dict__ , name mangling

Dec 10, 2025

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (1)

crates/vm/src/builtins/type.rs (1)
1798-1808: Handle edge case when class name consists entirely of underscores.

When class_name consists only of underscores (e.g., _, __, ___), trim_start_matches('_') returns an empty string. This causes the code to produce "___x" for a name like "__x" in such a class, which diverges from CPython's behavior—CPython does not mangle names in classes whose names consist only of underscores and returns the original name unchanged.

Add a check after stripping the leading underscores:
 fn mangle_name(class_name: &str, name: &str) -> String {
     // Only mangle names starting with __ and not ending with __
     if !name.starts_with("__") || name.ends_with("__") || name.contains('.') {
         return name.to_string();
     }
     // Strip leading underscores from class name
     let class_name = class_name.trim_start_matches('_');
+    if class_name.is_empty() {
+        return name.to_string();
+    }
     format!("_{}{}", class_name, name)
 }

📜 Review details

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 98fff96 and 8718dc4.

⛔ Files ignored due to path filters (10)

Lib/test/datetimetester.py is excluded by !Lib/**
Lib/test/test_contextlib.py is excluded by !Lib/**
Lib/test/test_contextlib_async.py is excluded by !Lib/**
Lib/test/test_dataclasses.py is excluded by !Lib/**
Lib/test/test_decimal.py is excluded by !Lib/**
Lib/test/test_fractions.py is excluded by !Lib/**
Lib/test/test_functools.py is excluded by !Lib/**
Lib/test/test_os.py is excluded by !Lib/**
Lib/test/test_statistics.py is excluded by !Lib/**
Lib/test/test_typing.py is excluded by !Lib/**

📒 Files selected for processing (3)

crates/vm/src/builtins/object.rs (1 hunks)
crates/vm/src/builtins/type.rs (4 hunks)
crates/vm/src/stdlib/typevar.rs (2 hunks)

🧰 Additional context used

📓 Path-based instructions (1)

**/*.rs

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

**/*.rs: Follow the default rustfmt code style by running cargo fmt to format Rust code
Always run clippy to lint Rust code (cargo clippy) before completing tasks and fix any warnings or lints introduced by changes
Follow Rust best practices for error handling and memory management
Use the macro system (pyclass, pymodule, pyfunction, etc.) when implementing Python functionality in Rust

Files:

crates/vm/src/builtins/object.rs
crates/vm/src/stdlib/typevar.rs
crates/vm/src/builtins/type.rs

🧠 Learnings (1)

📚 Learning: 2025-11-29T12:17:28.606Z

Learnt from: CR
Repo: RustPython/RustPython PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-29T12:17:28.606Z
Learning: Applies to **/*.rs : Use the macro system (`pyclass`, `pymodule`, `pyfunction`, etc.) when implementing Python functionality in Rust

Applied to files:

crates/vm/src/builtins/type.rs

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)

GitHub Check: Run snippets and cpython tests (windows-latest)

🔇 Additional comments (7)

crates/vm/src/builtins/type.rs (3)

310-317: LGTM! Correct HAS_DICT inheritance from MRO.

The change from checking only the first base to checking any base in the MRO aligns with CPython's behavior. If any base class in the MRO has HAS_DICT, the subclass should inherit it too since it needs to support instance dictionaries from any base.

1188-1213: LGTM! Name mangling correctly applied to slot-backed member descriptors.

The implementation properly:

Extracts the class name before processing slots

Applies mangling to each slot attribute name

Uses the mangled name consistently for both the PyMemberDef and the attribute key on the type

This matches CPython's behavior where __x in __slots__ becomes _ClassName__x as the actual attribute.

1233-1246: LGTM! Correct conditional addition of __dict__ descriptor.

The updated condition ensures the __dict__ descriptor is only added when:

The base is not type (type subclasses inherit __dict__ from type)

The class has the HAS_DICT flag (meaning __slots__ was not defined, or __dict__ was explicitly in __slots__)

This correctly implements the __slots__ xor __dict__ semantics.

crates/vm/src/builtins/object.rs (1)

69-79: LGTM! Dict creation now driven by HAS_DICT flag.

The change correctly ties instance dict creation to the HAS_DICT flag rather than a hardcoded type check. This ensures:

Classes with __slots__ (no __dict__ in slots) → no instance dict

Classes without __slots__ or with __dict__ in __slots__ → instance dict created

This aligns with the type initialization changes and properly implements Python's __slots__ semantics.

crates/vm/src/stdlib/typevar.rs (3)

3-3: LGTM!

The Context import is required for the new __slots__ method signature.

975-978: LGTM! Empty __slots__ on Generic matches CPython.

Defining __slots__ = () on Generic prevents instances from having an instance dictionary, which is the expected behavior for this typing construct. Using #[pyattr] correctly defines this as a class attribute.

986-988: LGTM! __init_subclass__ delegation to typing module.

The implementation correctly delegates to _generic_init_subclass in the typing module, consistent with the existing __class_getitem__ pattern and CPython's behavior for handling Generic subclass initialization.

This was referenced

Dec 26, 2025