Wtf8-compatible fixes #5985

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Merged

youknowone merged 7 commits into RustPython:main from youknowone:wtf8

Jul 29, 2025

Member

youknowone commented Jul 15, 2025 •

edited by coderabbitai bot

Loading

Summary by CodeRabbit

Refactor
- Improved internal string and byte handling for various functions, leading to more consistent and efficient processing of byte data.
- Deprecated certain object methods in favor of updated alternatives, with clearer guidance for future usage.
- Simplified and clarified internal code for object representation and conversion.
Style
- Enhanced code clarity and maintainability without altering user-facing behavior.

No changes to user-facing features or documentation.

Contributor

coderabbitai bot commented Jul 15, 2025 •

edited

Loading

Walkthrough

The changes update several string-processing functions and methods to operate on byte slices (&[u8]) instead of string slices (&str). Associated logic and function signatures are adjusted accordingly, including Levenshtein distance calculations and hexadecimal parsing. Additional refactoring improves code clarity, removes unnecessary trait bounds, and introduces deprecation annotations for certain object payload methods.

Changes

Cohort / File(s)	Change Summary
Levenshtein and Suggestion Byte Slice Refactor `common/src/str.rs`, `vm/src/suggestion.rs`	Updated Levenshtein distance and suggestion logic to operate on byte slices (`&[u8]`) instead of string slices (`&str`). Adjusted function signatures and internal comparisons accordingly.
Hexadecimal Parsing Byte Slice Refactor `vm/src/bytes_inner.rs`, `vm/src/builtins/bytearray.rs`, `vm/src/builtins/bytes.rs`	Changed `fromhex` and related calls to accept and process byte slices rather than string slices. Updated iterators and pattern matching to handle byte references. Adjusted buffer capacity calculations.
String Conversion Simplification `vm/src/builtins/genericalias.rs`, `vm/src/builtins/union.rs`	Simplified conversion of `PyStr` references to owned `String` by using `.to_string()` directly, replacing `.as_str().to_string()`. Applied to generic alias and union representation logic.
Bound Method Representation Refactor `vm/src/builtins/function.rs`	Refactored `repr_str` for `PyBoundMethod` to precompute and store function name and object representation, improving clarity and efficiency.
PyInterned Trait Bound Simplification `vm/src/intern.rs`	Removed the `T: PyPayload` trait bound from the `PyInterned<T>` struct and related implementations. Reordered trait bounds in `Debug` implementation.
Object Payload API Deprecation and Refactor `vm/src/object/core.rs`	Added deprecation annotations to legacy payload methods, recommended newer downcasting methods, and reduced unsafe code usage. Simplified type checking and added inline annotation for payload access.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~18 minutes

Poem

In bytes and hex, we hop along,
Refactoring code to make it strong.
Strings to slices, clear and neat,
Deprecations in a tidy suite.
Rabbits cheer with every stride,
As cleaner code now hops inside!
🐇✨

Note

⚡️ Unit Test Generation is now available in beta!

Learn more here, or try it out under "Finishing Touches" below.

📜 Recent review details

Configuration used: .coderabbit.yml
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 57029f6 and 34b9562.

📒 Files selected for processing (10)

common/src/str.rs (2 hunks)
vm/src/builtins/bytearray.rs (1 hunks)
vm/src/builtins/bytes.rs (1 hunks)
vm/src/builtins/function.rs (1 hunks)
vm/src/builtins/genericalias.rs (1 hunks)
vm/src/builtins/union.rs (1 hunks)
vm/src/bytes_inner.rs (1 hunks)
vm/src/intern.rs (3 hunks)
vm/src/object/core.rs (5 hunks)
vm/src/suggestion.rs (2 hunks)

🧰 Additional context used

📓 Path-based instructions (1)

**/*.rs

📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)

**/*.rs: Follow the default rustfmt code style (cargo fmt to format)
Always run clippy to lint code (cargo clippy) before completing tasks. Fix any warnings or lints that are introduced by your changes
Follow Rust best practices for error handling and memory management
Use the macro system (pyclass, pymodule, pyfunction, etc.) when implementing Python functionality in Rust

Files:

vm/src/suggestion.rs
vm/src/builtins/genericalias.rs
vm/src/builtins/union.rs
vm/src/builtins/bytearray.rs
vm/src/builtins/bytes.rs
common/src/str.rs
vm/src/bytes_inner.rs
vm/src/intern.rs
vm/src/builtins/function.rs
vm/src/object/core.rs

🧠 Learnings (2)

vm/src/builtins/genericalias.rs (1)

Learnt from: youknowone
PR: #5945
File: vm/src/stdlib/typing.rs:132-136
Timestamp: 2025-07-11T07:16:06.225Z
Learning: In Python 3.12, TypeAliasType's repr method returns just the alias name as a string (e.g., repr(A) returns 'A'), not the "" format. This applies to type aliases created with the type statement syntax.

vm/src/object/core.rs (1)

Learnt from: CR
PR: RustPython/RustPython#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-07-19T03:16:56.511Z
Learning: Applies to **/*.rs : Use the macro system (pyclass, pymodule, pyfunction, etc.) when implementing Python functionality in Rust

🧬 Code Graph Analysis (4)

vm/src/builtins/genericalias.rs (5)

vm/src/builtins/union.rs (3)

obj (156-156)

o (67-67)

o (69-69)

vm/src/builtins/function.rs (5)

obj (511-511)

__qualname__ (549-551)

__qualname__ (822-841)

__module__ (529-531)

__module__ (817-819)

vm/src/builtins/type.rs (5)

obj (215-215)

obj (496-496)

obj (946-946)

__qualname__ (652-659)

__module__ (750-764)

vm/src/builtins/classmethod.rs (2)

__qualname__ (146-148)

__module__ (141-143)

vm/src/builtins/staticmethod.rs (2)

__qualname__ (105-107)

__module__ (100-102)

vm/src/builtins/union.rs (4)

vm/src/builtins/type.rs (5)

obj (215-215)

obj (496-496)

obj (946-946)

__qualname__ (652-659)

__module__ (750-764)

vm/src/builtins/function.rs (4)

__qualname__ (549-551)

__qualname__ (822-841)

__module__ (529-531)

__module__ (817-819)

vm/src/builtins/builtin_func.rs (3)

__qualname__ (89-109)

__qualname__ (162-167)

__module__ (79-81)

vm/src/builtins/descriptor.rs (2)

__qualname__ (118-120)

__qualname__ (257-267)

vm/src/builtins/bytearray.rs (2)

vm/src/bytes_inner.rs (2)

bytes (113-113)

fromhex (462-502)

vm/src/builtins/bytes.rs (1)

fromhex (266-270)

vm/src/object/core.rs (1)

vm/src/object/payload.rs (1)

payload_type_id (23-25)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)

GitHub Check: Run snippets and cpython tests (ubuntu-latest)
GitHub Check: Run snippets and cpython tests (windows-latest)

🔇 Additional comments (23)

vm/src/builtins/union.rs (3)

62-62: LGTM! Cleaner string conversion.

The direct call to .to_string() on the PyStr result is more efficient than the previous .as_str().to_string() chain.

67-69: LGTM! Consistent string conversion improvement.

The removal of .as_str() calls makes the attribute extraction and string conversion more direct and efficient.

71-71: LGTM! Consistent with other string conversion improvements.

The direct .to_string() call maintains consistency with the other changes in this function.

vm/src/builtins/function.rs (1)

862-869: LGTM! Good refactoring for clarity and efficiency.

The extraction of function name and object representation into separate variables improves code readability and avoids repeated calls. The logic remains unchanged while being more maintainable.

vm/src/object/core.rs (5)

636-640: LGTM! Good deprecation strategy.

The deprecation annotation guides users toward the safer downcastable method while maintaining backward compatibility.

646-653: LGTM! Appropriate deprecation with safety guidance.

Deprecating payload_unchecked in favor of downcast_unchecked_ref promotes safer downcasting patterns.

658-664: LGTM! Proper handling of deprecated method usage.

The #[allow(deprecated)] annotations are appropriate since this deprecated method needs to use the other deprecated methods internally for backward compatibility.

725-727: LGTM! Performance improvement.

The direct typeid comparison is more efficient than calling the deprecated payload_is method and achieves the same result.

906-909: LGTM! Safer payload access.

The change from unsafe payload_unchecked to direct safe access to self.0.payload removes unnecessary unsafe code. The inline attribute is also appropriate for this simple accessor.

vm/src/intern.rs (3)

131-133: LGTM! Simplified type constraints.

Removing the unnecessary T: PyPayload trait bound from the struct declaration reduces complexity without affecting functionality.

173-178: LGTM! Consistent trait bound simplification.

The removal of the T: PyPayload bound aligns with the struct declaration change and maintains consistency.

180-186: LGTM! Clean trait bound removal.

The Deref implementation doesn't require the PyPayload trait bound, so removing it simplifies the type constraints appropriately.

vm/src/builtins/bytes.rs (1)

267-267: LGTM! Correct adaptation to byte-based hex parsing.

The change from string.as_str() to string.as_bytes() correctly aligns with the updated PyBytesInner::fromhex function signature that now accepts &[u8] instead of &str. This maintains functionality while operating at the byte level, which is more appropriate for hex string parsing.

vm/src/builtins/bytearray.rs (1)

331-331: LGTM! Consistent with bytes.rs changes.

The change from string.as_str() to string.as_bytes() correctly adapts to the updated PyBytesInner::fromhex signature and maintains consistency with the identical change in vm/src/builtins/bytes.rs. This ensures both bytes and bytearray types handle hex parsing consistently at the byte level.

vm/src/suggestion.rs (2)

29-29: LGTM! Consistent byte-level string comparison.

The change from as_str() to as_bytes() for string equality comparison is correct and maintains the same functionality while operating at the byte level, which aligns with the broader codebase changes toward byte-based string processing.

38-38: LGTM! Adapted to updated levenshtein_distance signature.

The change correctly adapts to the updated levenshtein_distance function signature that now accepts byte slices (&[u8]) instead of string slices (&str). This maintains the same distance calculation functionality while operating at the byte level.

vm/src/builtins/genericalias.rs (4)

130-130: LGTM! Simplified string conversion from PyStr.

The change from n.as_str().to_string() to n.to_string() is correct and cleaner, as it directly converts the PyStr reference to a String without the intermediate step.

132-132: LGTM! Simplified string conversion from PyStr.

The change from m.as_str().to_string() to m.to_string() is correct and cleaner, as it directly converts the PyStr reference to a String without the intermediate step.

134-134: Verify that PyObjectRef implements ToString correctly.

The change from obj.repr(vm)?.as_str().to_string() to obj.repr(vm)?.to_string() assumes that PyObjectRef has a direct to_string() implementation. Since repr() returns a PyObjectRef (not a PyStr), please verify this compiles and produces the same result.

This is the same concern as line 125 - the verification script above should cover both cases.

125-125: No changes required: .repr() returns a Rust String

The call to .to_string() is invoked on the String returned by obj.repr(vm)?, not on PyObjectRef itself. Rust’s String type implements ToString (via Display), so this compiles and produces the same result as before. The original concern about PyObjectRef::to_string does not apply here.

Likely an incorrect or invalid review comment.

vm/src/bytes_inner.rs (1)

462-464: LGTM! Clean conversion from string to byte slice processing.

The function signature update and corresponding iterator changes are well-coordinated. The shift from .bytes().enumerate() to .iter().enumerate() is correct for byte slice input, and the capacity calculation using bytes.len() / 2 remains appropriate for hex parsing.

common/src/str.rs (2)

490-490: LGTM! Clean function signature update for byte slice processing.

The parameter type change from &str to &[u8] is consistent with the codebase's shift towards byte-oriented processing.

502-502: LGTM! Simplified variable assignment eliminates unnecessary conversion.

The removal of .as_bytes() calls is correct since the parameters are already byte slices. This simplification improves efficiency while maintaining the same functionality.

✨ Finishing Touches

📝 Generate Docstrings

🧪 Generate unit tests

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

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

🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

Review comments: Directly reply to a review comment made by CodeRabbit. Example:
- I pushed a fix in commit <commit_id>, please review it.
- Explain this complex logic.
- Open a follow-up GitHub issue for this discussion.
Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
- @coderabbitai explain this code block.
- @coderabbitai modularize this function.
PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
- @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
- @coderabbitai read src/utils.ts and explain its main purpose.
- @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
- @coderabbitai help me debug CodeRabbit configuration file.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

@coderabbitai pause to pause the reviews on a PR.
@coderabbitai resume to resume the paused reviews.
@coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
@coderabbitai full review to do a full review from scratch and review all the files again.
@coderabbitai summary to regenerate the summary of the PR.
@coderabbitai generate docstrings to generate docstrings for this PR.
@coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
@coderabbitai generate unit tests to generate unit tests for this PR.
@coderabbitai resolve resolve all the CodeRabbit review comments.
@coderabbitai configuration to show the current CodeRabbit configuration for the repository.
@coderabbitai help to get help.

Other keywords and placeholders

Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
Add @coderabbitai anywhere in the PR title to generate the title automatically.

Documentation and Community

Visit our Documentation for detailed information on how to use CodeRabbit.
Join our Discord Community to get help, request features, and share feedback.
Follow us on X/Twitter for updates and announcements.

youknowone force-pushed the wtf8 branch 4 times, most recently from 069830b to f08ca8e Compare

July 16, 2025 01:31

youknowone force-pushed the wtf8 branch from f08ca8e to d1f3c2a Compare

July 28, 2025 14:47

youknowone added 7 commits

July 30, 2025 00:24


          deprecate more payload_* functions

ed36ac8


          loose trait bount for PyInterned

9019b6f


          Fix levenstein

95bf98e


          Fix genericalias

a581614


          Fix PyBoundMethod::repr

9c649eb


          fix repr

fc2b97d


          Fix fromhex

34b9562

youknowone force-pushed the wtf8 branch from d1f3c2a to 34b9562 Compare

July 29, 2025 15:28

youknowone changed the title ~~Wtf8~~ Wtf8-compatible fixes

youknowone marked this pull request as ready for review

July 29, 2025 15:42

youknowone merged commit 59a8a56 into RustPython:main

12 checks passed

youknowone deleted the wtf8 branch

July 29, 2025 16:03

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet