Skip to content

code nits#7908

Merged
youknowone merged 6 commits into
RustPython:mainfrom
ShaharNaveh:nits-0
May 19, 2026
Merged

code nits#7908
youknowone merged 6 commits into
RustPython:mainfrom
ShaharNaveh:nits-0

Conversation

@ShaharNaveh

@ShaharNaveh ShaharNaveh commented May 18, 2026 •
edited by coderabbitai Bot
Loading

Copy link
Copy Markdown
Contributor

Summary by CodeRabbit

  • Refactor
    • Internal VM optimizations enabling more compile-time evaluation and reducing temporary allocations for improved performance.
    • Stronger equality/type semantics for internal enums.
    • Minor internal formatting and error-message construction improvements that do not change user-visible behavior.

Review Change Stack

@coderabbitai

coderabbitai Bot commented May 18, 2026 •
edited
Loading

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: c60d5bdc-51c6-4c9f-830e-26e272396e2a

📥 Commits

Reviewing files that changed from the base of the PR and between ba19f78 and 264ae0e.

📒 Files selected for processing (1)
  • crates/vm/src/builtins/tuple.rs
🚧 Files skipped from review as they are similar to previous changes (1)
  • crates/vm/src/builtins/tuple.rs
📝 Walkthrough

Walkthrough

This PR converts several helpers to const fn, adds Eq derives to buffer enums, replaces a safe unwrap with unwrap_unchecked() after peek() in Endianness::parse, refactors tuple hashing and super() docs, and makes minor control-flow and formatting tweaks across builtins.

Changes

VM Builtin Improvements

Layer / File(s) Summary
Buffer enum traits and unsafe optimization
crates/vm/src/buffer.rs		 Endianness and FormatType gain Eq derives. Endianness::parse now consumes the marker with unwrap_unchecked() after a peek() guard; surrounding whitespace/macro boundaries adjusted.
Const fn conversions (frame, tuple, bound-method accessors)
crates/vm/src/builtins/frame.rs, crates/vm/src/builtins/function.rs, crates/vm/src/builtins/tuple.rs		 Frame stack-analysis helpers (push_value, pop_value, top_of_stack, compatible_stack, explain_incompatible_stack) and PyBoundMethod accessors become const fn. PyTuple::len and is_empty are now const fn. Minor whitespace edits in function.rs.
Control flow and error/formatting refinements
crates/vm/src/builtins/bool.rs, crates/vm/src/builtins/function.rs, crates/vm/src/builtins/weakref.rs, crates/vm/src/builtins/weakproxy.rs		 try_to_bool uses else if for the false sentinel; PyBool::slot_new formats type name directly; format_missing_args uses last.to_string(); PyWeak::slot_new reuses a computed got value; PyWeak::repr_str returns directly from if let; PyWeakProxy::IterNext removes allocation.
Tuple hashing refactor and super documentation
crates/vm/src/builtins/tuple.rs, crates/vm/src/builtins/super.rs		 tuple_hash now uses cfg_select! for pointer-width constants and uses an intermediate acc_pyhash for finalization. super() builtin doc extracted into const SUPER_DOC multiline string.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Suggested reviewers

  • youknowone
  • fanninpm

Poem

A rabbit hops through builtins bright,
Converting functions to const for speed and light,
Peeked then unchecked, a tiny daring trick,
Tuples, frames, and docs now tidy and slick,
🐰✨

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 48.65% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check ❓ Inconclusive The title 'code nits' is vague and generic, using non-descriptive terminology that does not convey meaningful information about the specific changes made across eight different files. Consider a more descriptive title that highlights the main changes, such as 'Add const/Eq derives and refactor stack analysis helpers' or 'Add derives and convert helpers to const fn'.
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

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

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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 youknowone merged commit 67e66bd into RustPython:main May 19, 2026
26 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@youknowone youknowone youknowone approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@ShaharNaveh @youknowone