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
@coderabbitai coderabbitai Bot mentioned this pull request Aug 12, 2026
Merged
1 task
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