GeezerVison

The World Wide Web, restored to 800 pixels of glory.

GeezerVison is showing github.com — Implement missing slots for NoneType by moreal · Pull Request #7437 · RustPython/RustPython

Conversation

moreal commented Mar 15, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link

Copy Markdown

Contributor

This pull request does two things. First, as the title, it implements missing slots (e.g., __eq__) for NoneType.

And it lets slot traits (e.g., Comparable) implement only the slot (wrapper_descriptor), not the method. This task was ongoing, and this commit may complete it (#6486).

It will remove NoneType from whats_left.

NoneType tp_hash: https://github.com/python/cpython/blob/2f4e4ec2e7292901cab0c1466b78f5ddff48208d/Objects/object.c#L2276-L2279

static Py_hash_t none_hash(PyObject *v)
{
    return 0xFCA86420;
}

Summary by CodeRabbit

Improvements
- None now has a stable, consistent hash value (0xFCA86420 masked to 32 bits) for reliable use in sets and as dict keys.
- Rich comparisons between None and non-None values now return NotImplemented as expected.
Behavior Changes
- Some explicit Python-level dunder wrappers (comparison, descriptor, repr, and attribute-call bridges) were removed; behavior now relies on core slot implementations.
Tests
- Added tests validating None's hashing, comparison behaviors, and presence/type of comparison/hash descriptors.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

JaehunYoon reacted with thumbs up emoji

All reactions

👍 1 reaction

moreal added 2 commits

March 15, 2026 23:49


          Define only wrapper_descriptor

b31b812


          Implement missing slots for NoneType

6d863fb

coderabbitai Bot commented Mar 15, 2026 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link

Copy Markdown

Contributor

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

Run ID: 4cd61388-6679-45c5-9782-254efe25d0cc

📥 Commits

Reviewing files that changed from the base of the PR and between 4ababea and 07b792b.

📒 Files selected for processing (1)

crates/vm/src/builtins/singletons.rs

📝 Walkthrough

Walkthrough

Adds Comparable and Hashable implementations for the PyNone singleton (including a fixed NONE_HASH); removes many Python-level dunder wrapper method signatures from slot-related traits; removes explicit repr wrappers for struct sequences; and expands tests to assert None's comparison behaviors and hash value.

Changes

Cohort / File(s)	Summary
PyNone Trait Implementations `crates/vm/src/builtins/singletons.rs`	Adds `Comparable` and `Hashable` to `PyNone` (pyclass attribute), implements `Comparable` (identical optimization → equality, otherwise `NotImplemented`) and `Hashable` returning constant `NONE_HASH` = `0xFCA86420`.
Slot Trait API Cleanup `crates/vm/src/types/slot.rs`	Removes Python-level dunder method signatures from multiple traits (Destructor, Callable, GetDescriptor, GetAttr, SetAttr, Comparable rich comparisons); trims related public imports and reduces pymethod wrapper declarations.
StructSequence repr Removal `crates/vm/src/types/structseq.rs`	Removes explicit `__repr__` pymethod wrappers for `PyStructSequence` and related data block, relying on slot-based `slot_repr` instead.
Tests: None behavior `extra_tests/snippets/builtin_none.py`	Adds assertions that None's rich comparisons with non-None return `NotImplemented`; verifies presence of comparison/hash `wrapper_descriptor`s in `type(None).__dict__` distinct from `object.__dict__`; asserts `hash(None)` masked to 32 bits equals `0xFCA86420`.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

RichCompare contains pymethod. No AtomicCell for slots #6562: Modifies the Comparable/rich-comparison machinery and slot wrapper wiring closely related to adding Comparable for PyNone.
More slot impl #6618: Adjusts slot trait exposures and dunder wrapper handling similar to the slot API cleanup here.
__hash__ to slot_wrapper #6480: Changes hashing/Hashable slot surface and __hash__ handling; relates to introducing Hashable and the NONE_HASH constant.

Suggested reviewers

ShaharNaveh

Poem

🐰 I nibble slots and hop through traits,
None gets a hash in tidy slates.
When comparisons meet a stranger's face,
I whisper "NotImplemented" with gentle grace.
Happy hops and tiny bytes of delight. 🥕

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 62.50% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The PR title directly and clearly describes the main objective: implementing missing type slots for NoneType, which is the primary focus of all changes across the modified files.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

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

📝 Coding Plan

Generate coding plan for human review comments

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.}

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.


          Unmark fixed tests

6a08fda

github-actions Bot commented Mar 15, 2026 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link

Copy Markdown

Contributor

📦 Library Dependencies

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

[x] lib: cpython/Lib/types.py
[ ] test: cpython/Lib/test/test_types.py (TODO: 6)

dependencies:

types

dependent tests: (52 tests)

types: test_annotationlib test_ast test_asyncgen test_asyncio test_builtin test_call test_code test_collections test_compile test_compiler_assemble test_coroutines test_decorators test_descr test_dis test_doctest test_dtrace test_dynamicclassattribute test_email test_enum test_exception_group test_fstring test_funcattrs test_generators test_genericalias test_hmac test_importlib test_inspect test_listcomps test_marshal test_monitoring test_opcache test_os test_positional_only_arg test_pprint test_pyclbr test_pydoc test_raise test_string test_subclassinit test_subprocess test_tempfile test_threading test_trace test_traceback test_type_aliases test_type_annotations test_type_params test_types test_typing test_unittest test_xml_etree test_xml_etree_c

Legend:

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

moreal commented Mar 15, 2026

Copy link

Copy Markdown

Contributor Author

As a memo, it seems 765133c commit added pymethods to Comparable again but I can't guess the reason. It may be okay if all tests pass.

youknowone reacted with thumbs up emoji

All reactions

👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

moreal marked this pull request as ready for review

March 15, 2026 16:04

moreal marked this pull request as draft

March 15, 2026 16:09


          Inline PyNone hash value

4ababea

moreal self-assigned this

Mar 15, 2026

moreal marked this pull request as ready for review

March 15, 2026 16:12

coderabbitai Bot reviewed

Mar 15, 2026

View reviewed changes

coderabbitai Bot left a comment

Copy link

Copy Markdown

Contributor

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.

🧹 Nitpick comments (1)

crates/vm/src/builtins/singletons.rs (1)
53-54: Consider adding IMMUTABLETYPE flag for consistency.

The Comparable and Hashable additions are correct. However, note that PyNotImplemented (line 122) includes flags(IMMUTABLETYPE) while PyNone does not. Both are singletons with similar semantics, so you may want to add this flag here for consistency.
Optional consistency fix
-#[pyclass(with(Constructor, AsNumber, Comparable, Hashable, Representable))]
+#[pyclass(with(Constructor, AsNumber, Comparable, Hashable, 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` around lines 53 - 54, Add the
IMMUTABLETYPE flag to PyNone's pyclass attribute to match PyNotImplemented:
update the PyNone declaration (#[pyclass(with(Constructor, AsNumber, Comparable,
Hashable, Representable))] impl PyNone {}) to include flags(IMMUTABLETYPE) so
the singleton is marked immutable like PyNotImplemented.

🤖 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`:
- Around line 53-54: Add the IMMUTABLETYPE flag to PyNone's pyclass attribute to
match PyNotImplemented: update the PyNone declaration
(#[pyclass(with(Constructor, AsNumber, Comparable, Hashable, Representable))]
impl PyNone {}) to include flags(IMMUTABLETYPE) so the singleton is marked
immutable like PyNotImplemented.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

Run ID: f03bde30-5a15-45cc-a93f-89952ea2194b

📥 Commits

Reviewing files that changed from the base of the PR and between 6a08fda and 4ababea.

📒 Files selected for processing (1)

crates/vm/src/builtins/singletons.rs

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

moreal requested a review from youknowone

March 15, 2026 16:21

youknowone approved these changes

Mar 16, 2026

View reviewed changes

youknowone left a comment

Copy link

Copy Markdown

Member

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.

Looks good, and thank you for catching legacy python magic methods.
There must be a bunch more of those unnecessary magic methods. Could you investigate more about other methods too?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

youknowone requested changes

Mar 16, 2026

View reviewed changes

Comment thread

crates/vm/src/builtins/singletons.rs Resolved

Uh oh!

There was an error while loading. Please reload this page.


          Improve searchability for NoneType hash value

07b792b

Co-authored-by: Jeong, YunWon <[email protected]>

moreal commented Mar 16, 2026

Copy link

Copy Markdown

Contributor Author

Looks good, and thank you for catching legacy python magic methods. There must be a bunch more of those unnecessary magic methods. Could you investigate more about other methods too?

Okay, I will do in the next pull request.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

moreal requested a review from youknowone

March 16, 2026 02:54

youknowone approved these changes

Mar 16, 2026

View reviewed changes

youknowone merged commit 97790a8 into RustPython:main

Mar 16, 2026

16 checks passed

Uh oh!

There was an error while loading. Please reload this page.

youknowone pushed a commit to youknowone/RustPython that referenced this pull request

Mar 19, 2026


          Implement missing slots for NoneType (RustPython#7437)

52c0588

* Define only wrapper_descriptor

* Implement missing slots for NoneType

youknowone pushed a commit to youknowone/RustPython that referenced this pull request

Mar 22, 2026


          Implement missing slots for NoneType (RustPython#7437)

9d4ceee

* Define only wrapper_descriptor

* Implement missing slots for NoneType

youknowone pushed a commit that referenced this pull request

Sep 16, 2026


          Implement missing slots for NoneType (#7437)

1614e88

* Define only wrapper_descriptor

* Implement missing slots for NoneType

Reviewers

coderabbitai[bot]

coderabbitai[bot] left review comments

youknowone

youknowone approved these changes

Uh oh!

There was an error while loading. Please reload this page.

Assignees

moreal

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

2 participants

Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

You are visitor 01402520 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.

GeezerVison

Conversation

moreal commented Mar 15, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Mar 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Possibly related PRs

Suggested reviewers

Poem

❌ Failed checks (1 warning)

Uh oh!

github-actions Bot commented Mar 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

📦 Library Dependencies

Uh oh!

moreal commented Mar 15, 2026

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

youknowone left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

moreal commented Mar 16, 2026

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

moreal commented Mar 15, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

There was an error while loading. Please reload this page.

coderabbitai Bot commented Mar 15, 2026 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

github-actions Bot commented Mar 15, 2026 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.