GeezerVison

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

GeezerVison is showing github.com — Move typeid into vtable by coolreader18 · Pull Request #6231 · RustPython/RustPython

Conversation

coolreader18 commented Oct 31, 2025 •

edited by coderabbitai Bot

Loading

Uh oh!

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

Copy link

Copy Markdown

Member

Now that TypeId::of() is const, we can move the typeid into PyObjVTable

Summary by CodeRabbit

Chores
- Updated minimum Rust toolchain requirement to 1.91.0. Users compiling from source must upgrade their Rust toolchain.
Refactor
- Internal type-identification implementation switched from callable lookups to associated constants. No user-visible behavior changes, only internal API shape.

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

Sorry, something went wrong.

Uh oh!

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

All reactions

coderabbitai Bot commented Oct 31, 2025 •

edited

Loading

Uh oh!

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

Copy link

Copy Markdown

Contributor

Caution

Review failed

The pull request is closed.

📝 Walkthrough

Walkthrough

Workspace Rust toolchain minimum bumped from "1.89.0" to "1.91.0" in Cargo.toml. Several implementations of ::rustpython_vm::PyPayload replace the payload_type_id() function with an associated constant PAYLOAD_TYPE_ID (using core::any::TypeId) in generated and builtin types.

Changes

Cohort / File(s)	Summary
Toolchain Version Bump `Cargo.toml`	Updated `workspace.package.rust-version` from `"1.89.0"` to `"1.91.0"`.
Derived PyPayload impls `crates/derive-impl/src/pyclass.rs`, `crates/derive-impl/src/pystructseq.rs`	Replaced `fn payload_type_id() -> ::std::any::TypeId` with an associated `const PAYLOAD_TYPE_ID: ::core::any::TypeId`; constants initialized from base type `PAYLOAD_TYPE_ID`.
Builtin payload const `crates/vm/src/builtins/str.rs`	Removed `payload_type_id()` and added public `PAYLOAD_TYPE_ID: core::any::TypeId = core::any::TypeId::of::<PyStr>();`.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Wtf8-compatible fixes #5985: Changes the PyPayload API by replacing payload_type_id() with an associated constant PAYLOAD_TYPE_ID, which directly relates to the trait-implementation updates here.

Poem

🐰
I nibbled code beneath a moonlit patch,
Replaced a function with a constant hatch,
From std to core the TypeId springs,
Toolchain hops up—new tiny things!

Pre-merge checks

❌ Failed checks (2 warnings)

Check name	Status	Explanation	Resolution
Title check	⚠️ Warning	The PR title 'Move typeid into vtable' does not match the changeset, which only updates the Rust toolchain version from 1.89.0 to 1.91.0 in Cargo.toml.	Update the PR title to accurately reflect the actual change, such as 'Bump Rust toolchain requirement to 1.91.0' or 'Update workspace rust-version to 1.91.0'.
Docstring Coverage	⚠️ Warning	Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%.	You can run `@coderabbitai generate docstrings` to improve docstring coverage.

✅ Passed checks (1 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.

📜 Recent 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 cdb78f6 and 1756a5c.

📒 Files selected for processing (4)

Cargo.toml
crates/derive-impl/src/pyclass.rs
crates/derive-impl/src/pystructseq.rs
crates/vm/src/builtins/str.rs

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.

coderabbitai Bot reviewed

Oct 31, 2025

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.

Actionable comments posted: 1

📜 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 7249af9 and a8b2f9e.

📒 Files selected for processing (1)

Cargo.toml (1 hunks)

🧰 Additional context used

🧠 Learnings (2)

📚 Learning: 2025-09-11T05:06:59.971Z

Learnt from: CR
Repo: RustPython/RustPython PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-11T05:06:59.971Z
Learning: Applies to Lib/**/*.py : Minimize modifications to CPython standard library files; only make minimal changes to work around RustPython limitations

Applied to files:

Cargo.toml

📚 Learning: 2025-09-11T05:06:59.971Z

Learnt from: CR
Repo: RustPython/RustPython PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-11T05:06:59.971Z
Learning: Applies to {vm,stdlib}/**/*.rs : Use RustPython macros (`pyclass`, `pymodule`, `pyfunction`, etc.) when implementing Python functionality in Rust

Applied to files:

Cargo.toml

Sorry, something went wrong.

Uh oh!

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

All reactions

Comment thread

Cargo.toml Resolved

Uh oh!

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

youknowone reviewed

Nov 1, 2025

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 great, the failing CI must not be related, right?

Sorry, something went wrong.

Uh oh!

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

All reactions

youknowone commented Nov 1, 2025

Copy link

Copy Markdown

Member

No idea how they are related, but re-running CI doesn't fix it.

All reactions

Sorry, something went wrong.

Uh oh!

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

youknowone force-pushed the vtable-typeid branch from a8b2f9e to cdb78f6 Compare

December 30, 2025 08:44

youknowone commented Dec 30, 2025

Copy link

Copy Markdown

Member

const PAYLOAD_TYPE_ID evaluates to different values across compilation units

coolreader18 reacted with confused emoji

All reactions

😕 1 reaction

Sorry, something went wrong.

Uh oh!

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

youknowone marked this pull request as draft

January 4, 2026 15:43

coolreader18 added 2 commits

January 6, 2026 11:41


          Move typeid into vtable

45e7509


          Bump rust-version

1756a5c

coolreader18 force-pushed the vtable-typeid branch from cdb78f6 to 1756a5c Compare

January 6, 2026 17:42

youknowone approved these changes

Jan 7, 2026

View reviewed changes

youknowone marked this pull request as ready for review

January 7, 2026 03:32

youknowone approved these changes

Jan 7, 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.

awesome!

Sorry, something went wrong.

Uh oh!

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

All reactions

youknowone merged commit 09bde28 into RustPython:main

Jan 7, 2026

32 of 35 checks passed

Uh oh!

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

coolreader18 deleted the vtable-typeid branch

January 7, 2026 23:03

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

Jan 15, 2026


          Move typeid into vtable (RustPython#6231)

da30dba

* Move typeid into vtable

* Bump rust-version

coolreader18 mentioned this pull request

Jan 23, 2026

Make validate_downcastable_from unsafe #6851

Merged

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

No one assigned

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 01402503 — 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

coolreader18 commented Oct 31, 2025 • 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 Oct 31, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review failed

Walkthrough

Changes

Estimated code review effort

Possibly related PRs

Poem

Pre-merge checks

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

youknowone left a comment

Choose a reason for hiding this comment

Uh oh!

youknowone commented Nov 1, 2025

Uh oh!

youknowone commented Dec 30, 2025

Uh oh!

youknowone left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

coolreader18 commented Oct 31, 2025 •

edited by coderabbitai Bot

Loading

Uh oh!

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

coderabbitai Bot commented Oct 31, 2025 •

edited

Loading

Uh oh!

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