Make singleton builtin types immutable#7423
Conversation
📦 Library DependenciesThe following Lib/ modules were modified. Here are their dependencies: [x] test: cpython/Lib/test/test_builtin.py (TODO: 18) dependencies: dependent tests: (no tests depend on builtin) Legend:
|
📝 WalkthroughWalkthroughTwo singleton type declarations in the virtual machine's built-in types are updated to include the IMMUTABLETYPE flag, marking PyNotImplemented and PyEllipsis as immutable types. No functional behavior or method implementations are changed. Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~5 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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 Tip You can make CodeRabbit's review stricter and more nitpicky using the `assertive` profile, if that's what you prefer.Change the |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
crates/vm/src/builtins/singletons.rs (1)
101-101: LGTM, but consider consistency withPyNone.The
IMMUTABLETYPEflag correctly marksNotImplementedTypeas immutable. However,PyNone(defined in this same file at lines 52-53) is also a singleton type but doesn't have theIMMUTABLETYPEflag. In CPython,NoneTypeis likewise immutable.Consider adding
IMMUTABLETYPEtoPyNonefor consistency:-#[pyclass(with(Constructor, AsNumber, Representable))] +#[pyclass(with(Constructor, AsNumber, Representable), flags(IMMUTABLETYPE))] impl PyNone {},
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/vm/src/builtins/singletons.rs` at line 101, The PyNone type should be marked immutable like NotImplementedType for consistency; update the #[pyclass(...)] attribute on PyNone to include flags(IMMUTABLETYPE) (mirror the usage from NotImplementedType) so the PyNone singleton is treated as immutable by the VM.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@crates/vm/src/builtins/singletons.rs`:
- Line 101: The PyNone type should be marked immutable like NotImplementedType
for consistency; update the #[pyclass(...)] attribute on PyNone to include
flags(IMMUTABLETYPE) (mirror the usage from NotImplementedType) so the PyNone
singleton is treated as immutable by the VM.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: 467ef5bd-73ea-4fc0-9be9-944719979dd9
⛔ Files ignored due to path filters (1)
Lib/test/test_builtin.pyis excluded by!Lib/**
📒 Files selected for processing (2)
crates/vm/src/builtins/singletons.rscrates/vm/src/builtins/slice.rs
This pull request just marks
NotImplementedandEllipsisas immutable type (IMMUTABLETYPEflag).Summary by CodeRabbit