Apply some clippy lints #6045
Apply some clippy lints #6045
Conversation
WalkthroughThis update applies broad stylistic and idiomatic improvements across the codebase, focusing on making iteration more concise, using the Changes
Sequence Diagram(s)Omitted (changes are primarily stylistic, trait/method signature adjustments, and do not introduce new control flow or features). Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. 📜 Recent review detailsConfiguration used: .coderabbit.yml 📒 Files selected for processing (6)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (5)
⏰ 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). (11)
✨ 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. 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)
Other keywords and placeholders
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (2)
vm/sre_engine/src/string.rs (2)
226-243: Incorrect pointer arithmetic –ptr.offset(1)advances the outer pointer
ptris a&mut *const u8.
Usingptr.offset(1)increments the address of the pointer-variable itself, not the inner*const u8.
The intended behaviour is to move the inner pointer one byte forward.All
offset(±1)calls in this function (and the reverse variant below) therefore exhibit Undefined Behaviour and will crash or read past the buffer.-*ptr = unsafe { ptr.offset(1) }; +*ptr = unsafe { (*ptr).add(1) }; // advance inner pointer(The same fix applies to every subsequent
offset(1)in this function.)
274-300: Same pointer-arithmetic bug innext_code_point_reverseThe reverse reader decrements the outer pointer:
*ptr = unsafe { ptr.offset(-1) };It must mutate the inner pointer instead:
-*ptr = unsafe { ptr.offset(-1) }; +*ptr = unsafe { (*ptr).sub(1) }; // move inner pointer backApply the same substitution to every
offset(-1)in this function to prevent memory corruption.
🧹 Nitpick comments (5)
src/settings.rs (1)
28-31: Consider accepting case-insensitive valuesUsing
Self::Ensurepip/Self::GetPipis fine, but the CLI currently rejectsENSUREPIP,EnsurePip, etc. You may wish to normalise the input with.to_ascii_lowercase()to match CPython’s behaviour.compiler/codegen/src/compile.rs (4)
885-891: Duplicate “forbidden name” logic – consider deduplication
varname()now callsSelf::is_forbidden_arg_name.
However we already haveis_forbidden_nameplus scattered ad-hoc checks. Maintaining three parallel helpers invites drift.- if Self::is_forbidden_arg_name(name) { + if is_forbidden_name(name) {Unless you expect the two helpers to diverge semantically, prefer one canonical check.
3438-3452: Avoid the extra allocation when pushing attribute names
name.as_str().to_string().into()allocates a freshString.
Wtf8BufimplementsFrom<&str>, so you can eliminate the copy:- attr_names.push(ConstantData::Str { - value: name.as_str().to_string().into(), - }); + attr_names.push(ConstantData::Str { + value: name.as_str().into(), + });Minor, but this sits in a hot path for class-pattern matching.
3670-3673: Rotation loop can be collapsed
for _ in 0..=i_stores { pattern_helper_rotate(i_control + 1) }issues the sameSWAPsequencei_stores+1times.
A single call withcount = (i_control + 1) * (i_stores + 1)would generate fewer instructions:-for _ in 0..=i_stores { - self.pattern_helper_rotate(i_control + 1)?; -} +self.pattern_helper_rotate((i_control + 1) * (i_stores + 1))?;Not critical, but it trims bytecode size for deeply nested OR-patterns.
4580-4592: Nit: computedefault_kw_countonceGood readability tweak. You could inline the
to_u32()conversion to avoid two casts:-let default_kw_count = kw_with_defaults.len(); +let default_kw_count = kw_with_defaults.len().to_u32(); ... - Instruction::BuildMap { - size: default_kw_count.to_u32(), - } + Instruction::BuildMap { size: default_kw_count }
📜 Review details
Configuration used: .coderabbit.yml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (34)
common/src/boxvec.rs(1 hunks)common/src/format.rs(2 hunks)compiler/codegen/src/compile.rs(7 hunks)compiler/codegen/src/unparse.rs(1 hunks)compiler/core/src/bytecode.rs(25 hunks)compiler/core/src/frozen.rs(2 hunks)compiler/core/src/marshal.rs(6 hunks)compiler/literal/src/escape.rs(10 hunks)compiler/literal/src/float.rs(1 hunks)examples/call_between_rust_and_python.rs(1 hunks)src/settings.rs(1 hunks)src/shell/helper.rs(1 hunks)stdlib/src/unicodedata.rs(1 hunks)vm/src/builtins/code.rs(3 hunks)vm/src/builtins/dict.rs(1 hunks)vm/src/builtins/genericalias.rs(4 hunks)vm/src/builtins/namespace.rs(1 hunks)vm/src/builtins/slice.rs(1 hunks)vm/src/builtins/str.rs(1 hunks)vm/src/builtins/super.rs(1 hunks)vm/src/builtins/traceback.rs(1 hunks)vm/src/builtins/tuple.rs(3 hunks)vm/src/frame.rs(3 hunks)vm/src/stdlib/ast/python.rs(1 hunks)vm/src/stdlib/io.rs(1 hunks)vm/src/stdlib/itertools.rs(1 hunks)vm/src/stdlib/posix.rs(3 hunks)vm/src/stdlib/symtable.rs(1 hunks)vm/src/stdlib/thread.rs(1 hunks)vm/src/stdlib/typing.rs(1 hunks)vm/sre_engine/src/engine.rs(5 hunks)vm/sre_engine/src/string.rs(4 hunks)wtf8/src/core_str_count.rs(1 hunks)wtf8/src/lib.rs(27 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/stdlib/ast/python.rscompiler/codegen/src/unparse.rssrc/settings.rsvm/src/stdlib/thread.rscompiler/core/src/frozen.rsstdlib/src/unicodedata.rsvm/src/builtins/super.rsvm/src/builtins/dict.rsvm/src/stdlib/typing.rsvm/src/builtins/namespace.rscommon/src/boxvec.rscompiler/literal/src/float.rsvm/src/stdlib/itertools.rswtf8/src/core_str_count.rsvm/src/stdlib/io.rsvm/src/builtins/traceback.rsvm/src/builtins/genericalias.rsvm/src/builtins/str.rsvm/src/builtins/slice.rscommon/src/format.rssrc/shell/helper.rsexamples/call_between_rust_and_python.rsvm/src/stdlib/symtable.rsvm/src/builtins/code.rsvm/src/stdlib/posix.rsvm/sre_engine/src/string.rscompiler/core/src/marshal.rsvm/src/builtins/tuple.rscompiler/core/src/bytecode.rsvm/src/frame.rscompiler/codegen/src/compile.rsvm/sre_engine/src/engine.rscompiler/literal/src/escape.rswtf8/src/lib.rs
🧠 Learnings (8)
📓 Common learnings
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 : Follow Rust best practices for error handling and memory management
vm/src/builtins/super.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
vm/src/stdlib/typing.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.
examples/call_between_rust_and_python.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
vm/src/stdlib/posix.rs (3)
Learnt from: moreal
PR: #5847
File: vm/src/stdlib/stat.rs:547-567
Timestamp: 2025-06-27T14:47:28.810Z
Learning: In RustPython's stat module implementation, platform-specific constants like SF_SUPPORTED and SF_SYNTHETIC should be conditionally declared only for the platforms where they're available (e.g., macOS), following CPython's approach of optional declaration using #ifdef checks rather than providing fallback values for other platforms.
Learnt from: moreal
PR: #5847
File: vm/src/stdlib/stat.rs:547-567
Timestamp: 2025-06-27T14:47:28.810Z
Learning: In RustPython's stat module implementation, platform-specific constants like SF_SUPPORTED and SF_SYNTHETIC should be conditionally declared only for the platforms where they're available (e.g., macOS), following CPython's approach of optional declaration rather than providing fallback values for other platforms.
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
vm/sre_engine/src/string.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 : Follow Rust best practices for error handling and memory management
compiler/core/src/marshal.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 : Follow Rust best practices for error handling and memory management
wtf8/src/lib.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 : Follow Rust best practices for error handling and memory management
🧬 Code Graph Analysis (3)
vm/src/stdlib/itertools.rs (1)
vm/src/builtins/zip.rs (1)
zelf(50-54)
vm/src/builtins/genericalias.rs (1)
vm/src/protocol/sequence.rs (1)
tuple(279-289)
vm/sre_engine/src/string.rs (1)
wtf8/src/core_str.rs (2)
next_code_point(33-67)next_code_point_reverse(76-110)
⏰ 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). (7)
- GitHub Check: Check Rust code with rustfmt and clippy
- GitHub Check: Check the WASM package and demo
- GitHub Check: Run snippets and cpython tests on wasm-wasi
- GitHub Check: Run tests under miri
- GitHub Check: Run rust tests (windows-latest)
- GitHub Check: Run snippets and cpython tests (windows-latest)
- GitHub Check: Ensure compilation on various targets
🔇 Additional comments (93)
examples/call_between_rust_and_python.rs (1)
90-93: IdiomaticSelfusage looks goodSwitching to
Ok(Self { name })is concise and fully equivalent to the previous explicit type. No functional change introduced.compiler/core/src/frozen.rs (2)
60-60: Pure formatting – no issues
88-88: Pure formatting – no issuesstdlib/src/unicodedata.rs (1)
19-28: Iteration simplification approvedDropping the explicit
.into_iter()makes the loop cleaner without changing semantics.vm/src/builtins/super.rs (1)
173-178: Minor style improvement acknowledgedIterating with
for cls in &mrois the preferred idiom. Behaviour remains unchanged.vm/src/stdlib/itertools.rs (1)
1383-1386: Iteration now uses&zelf.pools– idiomatic and fineSwitching from
self.pools.iter()tofor element in &zelf.poolsis equivalent, more concise, and keeps the borrow checker happy. No functional or performance concerns here.vm/src/builtins/dict.rs (1)
279-285: Loop overkwargsis correct and more idiomaticDropping the explicit
.into_iter()simplifies the code without changing semantics—thefor (key, value) in kwargspattern consumes theKwArgsvalue exactly as before. Insertions intoself.entriesremain unchanged.vm/src/builtins/slice.rs (1)
231-231: LGTM! Idiomatic iteration syntax improvement.This change from iterating over an array to iterating over a slice reference follows Rust best practices and maintains identical semantics while using more concise syntax.
vm/src/builtins/str.rs (1)
1524-1524: LGTM! Improved type consistency usingSelf.Replacing the explicit type
PyRef<PyStr>withSelfimproves code maintainability and follows Rust conventions for self-referential types. The transmute logic and safety invariants remain unchanged.wtf8/src/core_str_count.rs (2)
98-98: LGTM! Enabling compile-time evaluation.Converting to
const fnallows this function to be evaluated at compile time when arguments are known, providing a performance benefit without changing runtime behavior.
106-106: LGTM! Enabling compile-time evaluation.Converting to
const fnallows this function to be evaluated at compile time when arguments are known, providing a performance benefit without changing runtime behavior.src/shell/helper.rs (1)
57-57: LGTM! Enabling compile-time construction.Converting the constructor to
const fnallowsShellHelperinstances to be created at compile time when parameters are constant, following the PR's pattern of increasing const-correctness throughout the codebase.vm/src/stdlib/posix.rs (7)
519-519: LGTM: Idiomatic iteration simplification.Removing the explicit
.into_iter()call is more idiomatic since vectors automatically implementIntoIteratorfor direct iteration.
1606-1608: LGTM: Appropriate const fn for libc macro.The
WIFSIGNALEDlibc macro performs bit manipulation operations that are const-evaluable, making thisconst fnannotation appropriate.
1611-1613: LGTM: Appropriate const fn for libc macro.The
WIFSTOPPEDlibc macro is const-evaluable, making thisconst fnannotation appropriate.
1616-1618: LGTM: Appropriate const fn for libc macro.The
WIFEXITEDlibc macro is const-evaluable, making thisconst fnannotation appropriate.
1621-1623: LGTM: Appropriate const fn for libc macro.The
WTERMSIGlibc macro is const-evaluable, making thisconst fnannotation appropriate.
1626-1628: LGTM: Appropriate const fn for libc macro.The
WSTOPSIGlibc macro is const-evaluable, making thisconst fnannotation appropriate.
1631-1633: LGTM: Appropriate const fn for libc macro.The
WEXITSTATUSlibc macro is const-evaluable, making thisconst fnannotation appropriate.compiler/core/src/marshal.rs (6)
117-143: LGTM: Well-designed convenience methods.These convenience methods provide type-safe ways to read fixed-size data types with proper error handling and const generic array sizes. The consistent use of little-endian byte order aligns with the marshaling format requirements.
149-151: LGTM: Consistent borrowed string reading method.The
read_str_borrowmethod follows the established pattern of theReadBorrowedtrait and includes proper UTF-8 validation with appropriate error handling.
277-277: LGTM: Improved type relationships through associated types.Adding the
ConstantBagassociated type and updating theconstant_bagmethod signature improves type safety and makes the relationship betweenMarshalBagandConstantBagexplicit in the type system.Also applies to: 315-315
320-320: LGTM: Proper implementation of associated type constraint.The implementation correctly handles the new
ConstantBagassociated type by setting it toSelfand returningselfin theconstant_bagmethod, which is appropriate for the default case.Also applies to: 388-390
465-465: LGTM: Enhanced type constraints in Dumpable trait.Adding the
Constant: Constantassociated type constraint improves type safety and makes the relationship betweenDumpableandConstanttypes explicit.
518-532: LGTM: Consistent write convenience methods.These convenience methods mirror the
Readtrait additions and provide type-safe ways to write fixed-size data types with consistent little-endian byte order. The implementation follows established patterns and improves API usability.wtf8/src/lib.rs (6)
94-96: LGTM! Idiomatic use ofSelfin constructor.Using
Selfinstead of the explicit type name improves code clarity and maintainability.
102-107: LGTM! Consistent use ofSelfin return type.The change from
Option<CodePoint>toOption<Self>maintains consistency with the constructor pattern improvements throughout the codebase.
113-117: LGTM! Constructor consistency improved.Using
Self { value }instead ofCodePoint { value }follows Rust best practices for constructor implementations.
246-248: LGTM! Trait implementation improvement.Using
&Self::Targetinstead of the explicit type is more idiomatic and flexible.
639-643: LGTM! Clean trait implementation.The
AsRef<Self>implementation usingSelfis idiomatic and improves code consistency.
949-952: LGTM! Improved constructor method.Using
Box::default()instead ofDefault::default()and returningBox<Self>makes the code more concise and idiomatic.vm/src/stdlib/typing.rs (1)
168-168: LGTM! Idiomatic constructor call.Using
Self::newinstead ofTypeAliasType::newis more idiomatic within the impl block and improves code consistency.vm/src/stdlib/thread.rs (1)
289-291: LGTM! Performance improvement withconst fn.Converting
allocate_lockto aconst fnenables compile-time evaluation while maintaining the same functionality. This aligns with the broader pattern in the PR of making eligible functionsconst.compiler/codegen/src/unparse.rs (1)
467-470: LGTM! Simplified iteration syntax.Replacing
.iter()with direct iteration over&args.kwonlyargsis more idiomatic and concise while maintaining the same functionality.vm/src/stdlib/ast/python.rs (1)
74-74: LGTM! Idiomatic method call.Using
Self::__init__instead ofNodeAst::__init__is more idiomatic within the impl block and improves code consistency with the broader codebase changes.vm/src/stdlib/io.rs (1)
3603-3606:const fnconversion is soundThe getter is pure and independent of runtime state, so making it
const fnis perfectly safe and aligns with the broader const-ification effort.vm/src/builtins/namespace.rs (1)
64-64: LGTM: Idiomatic iteration simplification.Removing the explicit
.into_iter()call is correct and more idiomatic when the collection already implementsIntoIterator.vm/src/stdlib/symtable.rs (1)
72-77: LGTM: Appropriate const fn conversion.Converting this method to
const fnis correct since it only performs pattern matching on enum variants, which can be evaluated at compile time.common/src/boxvec.rs (1)
168-168: LGTM: More idiomatic range syntax.Using
index..=indexinstead ofindex..index + 1is more readable and idiomatic for representing a single-element range, while maintaining identical functionality.compiler/literal/src/float.rs (1)
47-52: LGTM: Appropriate const fn conversion.Converting this function to
const fnis correct since it only performs pattern matching on primitive types and returns string literals, enabling beneficial compile-time evaluation.vm/src/builtins/traceback.rs (2)
72-72: LGTM: Idiomatic use of Self keyword.Replacing explicit type references with
Selfis more idiomatic and maintainable Rust code.
78-78: LGTM: Consistent Self keyword usage.Using
Self::newinstead of explicit type name maintains consistency and follows Rust best practices.vm/src/builtins/genericalias.rs (4)
341-341: LGTM: Idiomatic iteration simplification.The direct iteration over the tuple reference is more idiomatic than explicitly calling
.iter().
390-390: LGTM: Consistent iteration style improvement.Direct iteration over the tuple reference maintains consistency with the idiomatic Rust style being applied throughout the codebase.
487-487: LGTM: Maintains consistency in iteration style.This change aligns with the pattern of simplifying iteration syntax throughout the file.
575-575: LGTM: Idiomatic array iteration.Direct iteration over the array reference is more concise and idiomatic than explicitly calling
.iter().common/src/format.rs (2)
392-392: LGTM: More idiomatic inclusive range syntax.Using
1..=sep_cntis clearer and more idiomatic than1..sep_cnt + 1when including the end value.
757-757: LGTM: Idiomatic use of Self.Using
Self::instead of the explicit type name is more idiomatic and maintainable within impl blocks.vm/src/builtins/code.rs (4)
110-110: LGTM: Minor formatting improvement.The additional blank line improves code readability.
127-145: LGTM: Simplified match arms improve readability.Removing the redundant
bytecode::prefix makes the match arms cleaner and more readable. The consistency across all arms is good.
147-147: LGTM: Formatting improvement.
175-175: LGTM: Idiomatic use of Self in trait implementation.Using
Selfas the return type is more idiomatic and maintainable than the explicit type name in trait implementations.vm/src/builtins/tuple.rs (6)
234-234: LGTM: Idiomatic use of Self in return type.Using
PyRef<Self>is more concise and maintainable than the explicit generic type in the return position.
240-240: LGTM: Consistent use of Self in transmute.Using
Selfin the transmute maintains consistency with the return type and improves maintainability.
341-341: LGTM: Idiomatic iteration over slice reference.Direct iteration over
&self.elementsis more concise than explicitly calling.iter().
485-485: LGTM: Consistent Self usage in transmute.Using
Selfin the transmute operation maintains consistency with the broader pattern of idiomatic type references.
492-492: LGTM: Maintains consistency in transmute operations.
499-499: LGTM: Completes consistent Self usage pattern.This final transmute change maintains consistency with the idiomatic use of
Selfthroughout the implementation.vm/src/frame.rs (1)
344-353:const fnlikely fails to compile on stable Rust
Cell::<u32>::get()is not yet aconst fnon stable.
MarkingExecutingFrame::lastiasconst fnwill therefore break the build (unless you compile with nightly and the relevant feature flags).Consider reverting to a plain
#[inline(always)]fn, or gate theconstwith a feature flag.-#[inline(always)] -const fn lasti(&self) -> u32 { +#[inline(always)] +fn lasti(&self) -> u32 {Please run
cargo check --all-targetsto verify compilation.compiler/codegen/src/compile.rs (3)
363-379: Initialization looks good – no actionable issues spottedThe refactor to use
Self { .. }keeps the constructor readable and aligns with idiomatic struct-literal style. No functional impact.
383-385:is_two_element_slicepromoted toconst fnGood call; this allows the pattern-check to be evaluated in a const‐context if ever needed and is zero-cost at runtime.
No concerns.
4551-4567: Reliable fallback to emptyParametersIntroducing a cached
default_paramsavoids repeatedParameters::default()construction – nice micro-optimisation.
No further comments.compiler/literal/src/escape.rs (9)
11-16: LGTM! Proper const fn conversion and Self usage.The conversion to
const fnis appropriate here since this is a simple pattern match with no side effects. UsingSelf::variants instead ofQuote::improves code consistency and maintainability.
21-31: LGTM! Consistent Self usage in pattern matches.The updated pattern matches using
Self::SingleandSelf::Doublemaintain consistency with theswapmethod changes and follow Rust idioms.
98-101: LGTM! Appropriate const fn conversion.The
with_forced_quoteconstructor can be safely marked asconst fnsince it only performs struct initialization without any runtime computation.
112-114: LGTM! Valid const fn conversion for accessor method.The
str_reprmethod is a simple wrapper that returns a struct, making it suitable forconst fn.
172-181: LGTM! Appropriate const fn for nested helper.The nested
stopfunction is correctly marked asconst fnsince it only constructs and returns anEscapeLayoutstruct without side effects.
286-293: LGTM! Proper const fn conversions for constructors.All three AsciiEscape constructor methods (
new,with_forced_quote, andbytes_repr) are appropriately converted toconst fnas they only perform struct initialization.Also applies to: 290-293, 304-306
332-332: LGTM! Simplified iteration syntax.Removing the explicit
.iter()calls and iterating directly over slices is more idiomatic Rust and slightly more efficient.Also applies to: 416-416
346-355: LGTM! Consistent const fn pattern for nested helper.The nested
stopfunction inAsciiEscapefollows the same pattern as inUnicodeEscape, properly marked asconst fn.
373-379: LGTM! Appropriate const fn for utility function.The
escaped_char_lenfunction performs only compile-time deterministic pattern matching, making it suitable forconst fn.vm/sre_engine/src/engine.rs (9)
71-73: LGTM! Simple getter method appropriately marked const.The
last_indexmethod is a straightforward field accessor that can be safely evaluated at compile time.
1057-1059: LGTM! Arithmetic computation suitable for const fn.The
remaining_codesmethod performs simple arithmetic on compile-time accessible fields, making it appropriate forconst fn.
1061-1063: LGTM! Simple arithmetic operation for const fn.The
remaining_charsmethod calculates the difference between twousizevalues, which is a const-safe operation.
1100-1102: LGTM! State mutation method correctly marked const.While this method mutates
self.code_position, theconstkeyword here refers to the ability to call this method in const contexts when the receiver is mutable, which is correct.
1108-1111: LGTM! Simple comparison suitable for const fn.The
at_beginningmethod performs a simple field comparison that can be evaluated at compile time.
1113-1115: LGTM! Field comparison appropriate for const fn.The
at_endmethod compares cursor position with request end, which is const-safe.
1147-1158: LGTM! Complex logic correctly implemented as const fn.The
can_successmethod contains conditional logic that can all be evaluated at compile time since it only involves field comparisons and boolean operations.
1166-1168: LGTM! Method delegation appropriate for const fn.The
next_offsetmethod delegates tonext_atwith simple arithmetic, suitable for const evaluation.
1171-1179: LGTM! Struct construction with field update syntax.The
next_atmethod creates a new instance using struct update syntax, which is const-safe. The field assignments and struct literal are all compile-time operations.compiler/core/src/bytecode.rs (13)
45-45: LGTM! Formatting improvements enhance readability.The spacing adjustments around trait implementations and function definitions improve code consistency and readability without affecting functionality.
Also applies to: 48-48, 67-67, 69-69, 71-71, 73-73, 75-75, 81-81, 88-88, 99-99, 103-103, 107-107, 113-113, 119-119
182-184: LGTM! Appropriate const fn conversion.Converting
OpArgByte::null()toconst fnis correct as it's a simple constructor with no side effects that can be evaluated at compile time.
199-201: LGTM! Appropriate const fn conversions.Both
OpArg::null()andOpArg::instr_size()are correctly converted toconst fnas they perform simple operations without side effects that can be evaluated at compile time.Also applies to: 205-207
247-249: LGTM! Appropriate const fn conversion.Converting
OpArgState::reset()toconst fnis correct as it performs a simple assignment operation that can be evaluated at compile time.
254-254: LGTM! Consistent formatting improvements.The spacing adjustments in trait implementations enhance code readability and maintain consistency across the codebase.
Also applies to: 264-264, 276-276, 288-288
317-319: LGTM! Appropriate const fn conversion.Converting
Arg<T>::marker()toconst fnis correct as it only constructs aPhantomDatawhich is const-evaluable.
325-333: LGTM! Const fn conversions appear valid.The const fn conversions for
Arg<T>methods are appropriate, assuming the underlying trait implementations (Into,OpArgType) support const evaluation. Since this code compiles, these conversions are valid.Also applies to: 334-337, 339-342, 346-350
358-358: LGTM! Consistent formatting improvements.The spacing adjustments continue to improve code readability and maintain consistency.
Also applies to: 378-378, 402-402
715-715: LGTM! Appropriate const fn conversion and formatting.Converting
CodeUnit::new()toconst fnis correct as it's a simple struct constructor, and the spacing adjustments improve readability.Also applies to: 718-718, 752-754
769-769: LGTM! Consistent formatting improvements.The spacing adjustments in trait implementations maintain code consistency.
Also applies to: 775-775
886-886: LGTM! Improved byte literal display formatting.The change from Debug formatting to using
escape_ascii()with raw string literal provides more consistent and readable output for byte constants, aligning better with Python's byte literal representation.
1253-1269: LGTM! Appropriate const fn conversions for pattern matching methods.Converting
Instruction::label_arg()andInstruction::unconditional_branch()toconst fnis correct as they perform simple pattern matching operations that can be evaluated at compile time.Also applies to: 1280-1290
808-808: LGTM! Comprehensive formatting improvements.All the spacing adjustments throughout the file improve code readability and maintain consistent formatting without affecting functionality.
Also applies to: 834-834, 869-869, 905-905, 908-908, 1018-1018, 1024-1024, 1616-1616, 1618-1618, 1620-1620, 1622-1622, 1624-1624, 1628-1628, 1632-1632, 1636-1636, 1640-1640
wtf8/src/lib.rs
Outdated
| #[inline] | ||
| pub const unsafe fn from_u32_unchecked(value: u32) -> CodePoint { | ||
| CodePoint { value } | ||
| pub const unsafe fn from_u32_unchecked(value: u32) -> Self { |
There was a problem hiding this comment.
wtf8 crate is mostly clone of outer project. I'd like to keep the diff as small as possible
2 participants
Summary by CodeRabbit
Refactor
Selfin method calls and type references for consistency.New Features
Style
const fnwhere possible, enabling compile-time evaluation and enhancing performance in certain contexts.Documentation
No changes to existing functionality or user-facing behavior.