Fix future clippy warnings #6103
Conversation
WalkthroughUpdates workspace Rust toolchain requirement to 1.87.0. Refactors several divisibility checks from modulo operations to is_multiple_of across array processing, struct unpacking, memory view casting, and UTF-8 counting code paths. No public API changes or behavioral adjustments reported. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate 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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
Status, Documentation and Community
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
vm/src/builtins/memory.rs (1)
746-748: Consider guarding against zero-sized item formats before the divisibility check.Using is_multiple_of is fine; however, a zero itemsize would now bypass a panic and reach the division that follows (Line 760), which could divide by zero if a zero-sized format were ever parsed. If FormatSpec guarantees size > 0, this is a non-issue—but an explicit guard is cheap and future-proof.
Proposed minimal guard:
fn cast_to_1d(&self, format: PyStrRef, vm: &VirtualMachine) -> PyResult<Self> { let format_spec = Self::parse_format(format.as_str(), vm)?; let itemsize = format_spec.size(); + if itemsize == 0 { + return Err(vm.new_type_error("memoryview: itemsize must be non-zero")); + } if !self.desc.len.is_multiple_of(itemsize) { return Err(vm.new_type_error("memoryview: length is not a multiple of itemsize")); }If you prefer to rely on the invariant from FormatSpec, please confirm that size() can never be zero.
📜 Review details
Configuration used: .coderabbit.yml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (5)
Cargo.toml(1 hunks)stdlib/src/array.rs(3 hunks)stdlib/src/pystruct.rs(1 hunks)vm/src/builtins/memory.rs(1 hunks)wtf8/src/core_str_count.rs(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.rs
📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
**/*.rs: Follow the default rustfmt code style (cargo fmtto 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/builtins/memory.rsstdlib/src/pystruct.rswtf8/src/core_str_count.rsstdlib/src/array.rs
🧬 Code Graph Analysis (1)
stdlib/src/array.rs (2)
vm/src/builtins/memory.rs (2)
itemsize(582-584)format(628-631)vm/src/protocol/buffer.rs (1)
format(188-201)
🔇 Additional comments (6)
Cargo.toml (1)
141-141: Toolchain bump to 1.87.0 is appropriate for using is_multiple_of (incl. const contexts). LGTM.This aligns the workspace with the new integer helpers used across the PR and should satisfy future clippy suggestions.
wtf8/src/core_str_count.rs (1)
38-38: Const-time divisibility check via is_multiple_of is correct.This maintains the original invariant and compiles under the bumped toolchain.
stdlib/src/pystruct.rs (1)
174-175: Switch to is_multiple_of improves readability; semantics unchanged.Zero-size is already rejected just above (Line 169), so there’s no behavioral risk here.
stdlib/src/array.rs (3)
854-856: Use of is_multiple_of for length/itemsize check is correct.Matches prior behavior without modulo; clearer intent.
1476-1476: Big-endian bit via !code.is_multiple_of(2) is equivalent and clear.No behavior change for u8; safe and explicit.
1618-1620: Divisibility check with is_multiple_of reads better; behavior unchanged.Error path and message remain identical.
Summary by CodeRabbit
Chores
Refactor
Style