GeezerVison

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

GeezerVison is showing github.com — Remove `Reverse` bytecode by ShaharNaveh · Pull Request #6675 · RustPython/RustPython

Conversation

ShaharNaveh commented Jan 9, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

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

Copy link

Copy Markdown

Contributor

Summary by CodeRabbit

Chores
- Removed a legacy bytecode instruction and its runtime handling, simplifying the instruction set and trimming related formatting/diagnostic paths.
- Simplified opcode metadata/name generation logic by removing redundant code paths.
- CI validation tightened to fail when generated metadata is out of date.
Breaking Changes
- Potential bytecode compatibility impact for programs or artifacts that relied on the removed instruction; affected items should be regenerated or recompiled.

_{✏️ 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


          Remove Reverse bytecode

2aba3fc

coderabbitai Bot commented Jan 9, 2026 •

edited

Loading

Uh oh!

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

Copy link

Copy Markdown

Contributor

📝 Walkthrough

Walkthrough

Removed the RustPython-only Reverse { amount: Arg<u32> } opcode variant from the Instruction enum, deleted its execution handling in the VM, and removed related stack-effect, disassembly/formatting, and dead-code paths; also fixed opcode-name generation and inverted a CI workflow check.

Changes

Cohort / File(s)	Change Summary
Bytecode instruction definition `crates/compiler-core/src/bytecode.rs`	Removed `Reverse { amount: Arg<u32> }` from `Instruction`; removed its `TryFrom<u8>` mapping, stack-effect branch, and disassembly/formatting entry.
Frame execution handler `crates/vm/src/frame.rs`	Removed `Reverse` handling from `ExecutingFrame::execute_instruction` (stack slicing and in-place reversal logic deleted).
Opcode metadata & tooling `scripts/generate_opcode_metadata.py`	Removed redundant/alternative return paths in `Opcode.cpython_name`, consolidating name derivation to a single code path.
CI workflow `.github/workflows/ci.yaml`	Inverted the condition in the "Ensure Lib/_opcode_metadata is updated" step so the job fails when the working tree is dirty (non-empty `git status --porcelain`).

Sequence Diagram(s)

(omitted)

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Assign opcode ids #6637: Modifies the same Instruction enum and related encoding/decoding/disassembly areas.
Ci gen opcode metadata #6677: Touches opcode metadata generation and CI workflow files (scripts/generate_opcode_metadata.py, .github/workflows/*).
Sort Instruction enum & related match arms #6322: Changes Instruction variants and VM handling related to the Reverse opcode.

Suggested reviewers

youknowone

Poem

🐇 I nudged the opcodes, gave them a glance,

The Reverse hop has hopped its last dance.
Stacks now flow in straighter streams,
I twitch my nose and polish dreams.
Carrots for tidy, humming machines! 🎉

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and directly describes the main change—removing the Reverse bytecode operation from the codebase.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

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

📜 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 ab43e94 and d879bf1.

📒 Files selected for processing (1)

.github/workflows/ci.yaml

🧰 Additional context used

🧠 Learnings (1)

📓 Common learnings

Learnt from: CR
Repo: RustPython/RustPython PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-27T14:03:49.034Z
Learning: When modifying bytecode instructions, perform a full clean build by running `rm -r target/debug/build/rustpython-* && find . | grep -E '\.pyc$' | xargs rm -r`

🔇 Additional comments (1)

.github/workflows/ci.yaml (1)

334-339: LGTM! Correct enforcement of metadata synchronization.

The inverted condition now properly fails the CI when the opcode metadata generation produces uncommitted changes, ensuring developers commit updated metadata alongside opcode definition changes. This is a standard best practice for keeping generated files synchronized with source code.

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.

youknowone reviewed

Jan 9, 2026

View reviewed changes

Comment thread

crates/compiler-core/src/bytecode.rs Resolved

Uh oh!

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

ShaharNaveh and others added 2 commits

January 9, 2026 17:22


          Update crates/compiler-core/src/bytecode.rs

2cf54f8

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

Gen

ad05793

ShaharNaveh mentioned this pull request

Jan 9, 2026

Replace CallMethod/LoadMethod opcodes with plain Call #6674

Merged

ShaharNaveh and others added 3 commits

January 10, 2026 10:36


          Merge remote-tracking branch 'upstream/main' into bytecode-remove-rev…

fb01dab

…erse


          Remove Reverse

0ec4e61


          Auto-format: cargo fmt --all

f442954

github-actions Bot commented Jan 10, 2026

Copy link

Copy Markdown

Contributor

Code has been automatically formatted

The code in this PR has been formatted using cargo fmt --all.
Please pull the latest changes before pushing again:

git pull origin bytecode-remove-reverse

All reactions

Sorry, something went wrong.

Uh oh!

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

ShaharNaveh commented Jan 10, 2026 •

edited

Loading

Uh oh!

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

Copy link

Copy Markdown

Contributor Author

Code has been automatically formatted

The code in this PR has been formatted using cargo fmt --all. Please pull the latest changes before pushing again:
git pull origin bytecode-remove-reverse

Yay #6677 works

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.

ShaharNaveh added 3 commits

January 10, 2026 10:51


          Revert comment

e847ff4


          Remove debug code

ab43e94


          Fix CI

d879bf1

youknowone approved these changes

Jan 10, 2026

View reviewed changes

youknowone commented Jan 10, 2026

Copy link

Copy Markdown

Member

Great! The message looks like need to be changed. it is not by fmt --all

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

youknowone merged commit 6fe0598 into RustPython:main

Jan 10, 2026

13 checks passed

Uh oh!

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

ShaharNaveh added a commit to ShaharNaveh/RustPython that referenced this pull request

Jan 10, 2026


          Remove Reverse bytecode (RustPython#6675)

d448753

* Remove `Reverse` bytecode

* Update crates/compiler-core/src/bytecode.rs

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

* Gen

* Remove Reverse

* Auto-format: cargo fmt --all

* Revert comment

* Remove debug code

* Fix CI

---------

Co-authored-by: Jeong, YunWon <[email protected]>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

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

Jan 15, 2026


          Remove Reverse bytecode (RustPython#6675)

dc39bd6

* Remove `Reverse` bytecode

* Update crates/compiler-core/src/bytecode.rs

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

* Gen

* Remove Reverse

* Auto-format: cargo fmt --all

* Revert comment

* Remove debug code

* Fix CI

---------

Co-authored-by: Jeong, YunWon <[email protected]>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

Reviewers

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

ShaharNaveh commented Jan 9, 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 Jan 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Sequence Diagram(s)

Estimated code review effort

Possibly related PRs

Suggested reviewers

Poem

Uh oh!

Uh oh!

github-actions Bot commented Jan 10, 2026

Uh oh!

ShaharNaveh commented Jan 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

youknowone commented Jan 10, 2026

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

ShaharNaveh commented Jan 9, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

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

coderabbitai Bot commented Jan 9, 2026 •

edited

Loading

Uh oh!

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

ShaharNaveh commented Jan 10, 2026 •

edited

Loading

Uh oh!

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