Wtf8-compatible fixes by youknowone · Pull Request #5985 · RustPython/RustPython

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.

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.

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.

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.

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.

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.

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.

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.

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.

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.