Update dataclasses, inspect from CPython 3.13 #5377
Conversation
youknowone
changed the title
youknowone
force-pushed
the
inspect
branch
3 times, most recently
from
faed11a to
a3579b3
Compare
youknowone
force-pushed
the
inspect
branch
3 times, most recently
from
4937811 to
b1235fe
Compare
WalkthroughExtended AST pattern conversions: added full bidirectional mapping for Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
youknowone
changed the title
youknowone
force-pushed
the
inspect
branch
2 times, most recently
from
d4d8bda to
b6dfa84
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
crates/vm/src/stdlib/ast/pattern.rs (1)
210-227: Correct object-to-singleton conversion with minor formatting nit.The identity checks and error handling are appropriate. The conversion logic correctly distinguishes between the three singleton values.
Consider using display format
{}instead of debug format{:?}in the error message for cleaner output:} else { Err(vm.new_value_error(format!( - "Expected None, True, or False, got {:?}", + "Expected None, True, or False, got {}", object.class().name() ))) }
📜 Review details
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (26)
Lib/dataclasses.pyis excluded by!Lib/**Lib/dis.pyis excluded by!Lib/**Lib/inspect.pyis excluded by!Lib/**Lib/test/__init__.pyis excluded by!Lib/**Lib/test/test__opcode.pyis excluded by!Lib/**Lib/test/test_abc.pyis excluded by!Lib/**Lib/test/test_compile.pyis excluded by!Lib/**Lib/test/test_copy.pyis excluded by!Lib/**Lib/test/test_dataclasses/__init__.pyis excluded by!Lib/**Lib/test/test_dataclasses/dataclass_module_1.pyis excluded by!Lib/**Lib/test/test_dataclasses/dataclass_module_1_str.pyis excluded by!Lib/**Lib/test/test_dataclasses/dataclass_module_2.pyis excluded by!Lib/**Lib/test/test_dataclasses/dataclass_module_2_str.pyis excluded by!Lib/**Lib/test/test_dataclasses/dataclass_textanno.pyis excluded by!Lib/**Lib/test/test_enum.pyis excluded by!Lib/**Lib/test/test_fstring.pyis excluded by!Lib/**Lib/test/test_inspect/__init__.pyis excluded by!Lib/**Lib/test/test_inspect/inspect_fodder.pyis excluded by!Lib/**Lib/test/test_inspect/inspect_fodder2.pyis excluded by!Lib/**Lib/test/test_inspect/inspect_stock_annotations.pyis excluded by!Lib/**Lib/test/test_inspect/inspect_stringized_annotations.pyis excluded by!Lib/**Lib/test/test_inspect/inspect_stringized_annotations_2.pyis excluded by!Lib/**Lib/test/test_inspect/inspect_stringized_annotations_pep695.pyis excluded by!Lib/**Lib/test/test_inspect/test_inspect.pyis excluded by!Lib/**Lib/test/test_unittest/testmock/testhelpers.pyis excluded by!Lib/**Lib/test/test_warnings/__init__.pyis excluded by!Lib/**
📒 Files selected for processing (1)
crates/vm/src/stdlib/ast/pattern.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 by runningcargo fmtto format Rust code
Always run clippy to lint Rust code (cargo clippy) before completing tasks and fix any warnings or lints introduced by 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:
crates/vm/src/stdlib/ast/pattern.rs
⏰ 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). (10)
- GitHub Check: Run snippets and cpython tests (windows-latest)
- GitHub Check: Run snippets and cpython tests (ubuntu-latest)
- GitHub Check: Run snippets and cpython tests (macos-latest)
- GitHub Check: Run rust tests (macos-latest)
- GitHub Check: Run rust tests (ubuntu-latest)
- GitHub Check: Run rust tests (windows-latest)
- GitHub Check: Check the WASM package and demo
- GitHub Check: Check Rust code with clippy
- GitHub Check: Ensure compilation on various targets
- GitHub Check: Run snippets and cpython tests on wasm-wasi
🔇 Additional comments (1)
crates/vm/src/stdlib/ast/pattern.rs (1)
202-208: LGTM! Clean singleton-to-object conversion.The exhaustive pattern match correctly maps each
Singletonvariant to its corresponding VM object representation.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
crates/vm/src/stdlib/ast/pattern.rs (1)
543-558: Avoid cloningkwd_attrs(optional)
split_pattern_match_classcurrently clonesattrbecause it later consumesarguments.keywords. You can split attrs/patterns in a single consuming loop to avoid clones.
📜 Review details
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (25)
Lib/dataclasses.pyis excluded by!Lib/**Lib/dis.pyis excluded by!Lib/**Lib/inspect.pyis excluded by!Lib/**Lib/test/test__opcode.pyis excluded by!Lib/**Lib/test/test_abc.pyis excluded by!Lib/**Lib/test/test_compile.pyis excluded by!Lib/**Lib/test/test_copy.pyis excluded by!Lib/**Lib/test/test_dataclasses/__init__.pyis excluded by!Lib/**Lib/test/test_dataclasses/dataclass_module_1.pyis excluded by!Lib/**Lib/test/test_dataclasses/dataclass_module_1_str.pyis excluded by!Lib/**Lib/test/test_dataclasses/dataclass_module_2.pyis excluded by!Lib/**Lib/test/test_dataclasses/dataclass_module_2_str.pyis excluded by!Lib/**Lib/test/test_dataclasses/dataclass_textanno.pyis excluded by!Lib/**Lib/test/test_enum.pyis excluded by!Lib/**Lib/test/test_fstring.pyis excluded by!Lib/**Lib/test/test_inspect/__init__.pyis excluded by!Lib/**Lib/test/test_inspect/inspect_fodder.pyis excluded by!Lib/**Lib/test/test_inspect/inspect_fodder2.pyis excluded by!Lib/**Lib/test/test_inspect/inspect_stock_annotations.pyis excluded by!Lib/**Lib/test/test_inspect/inspect_stringized_annotations.pyis excluded by!Lib/**Lib/test/test_inspect/inspect_stringized_annotations_2.pyis excluded by!Lib/**Lib/test/test_inspect/inspect_stringized_annotations_pep695.pyis excluded by!Lib/**Lib/test/test_inspect/test_inspect.pyis excluded by!Lib/**Lib/test/test_unittest/testmock/testhelpers.pyis excluded by!Lib/**Lib/test/test_warnings/__init__.pyis excluded by!Lib/**
📒 Files selected for processing (1)
crates/vm/src/stdlib/ast/pattern.rs(3 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.rs
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.rs: Follow the default rustfmt code style by runningcargo fmtto format Rust code
Always run clippy to lint Rust code (cargo clippy) before completing tasks and fix any warnings or lints introduced by 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:
crates/vm/src/stdlib/ast/pattern.rs
🧬 Code graph analysis (1)
crates/vm/src/stdlib/ast/pattern.rs (5)
crates/vm/src/stdlib/itertools.rs (1)
None(1152-1152)crates/vm/src/stdlib/ast/other.rs (1)
object(39-39)crates/vm/src/builtins/genericalias.rs (1)
arguments(66-66)crates/vm/src/frame.rs (1)
kwd_attrs(1122-1122)crates/vm/src/stdlib/functools.rs (1)
keywords(72-74)
⏰ 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). (8)
- GitHub Check: Run rust tests (macos-latest)
- GitHub Check: Run rust tests (ubuntu-latest)
- GitHub Check: Run snippets and cpython tests (ubuntu-latest)
- GitHub Check: Run snippets and cpython tests (macos-latest)
- GitHub Check: Run snippets and cpython tests (windows-latest)
- GitHub Check: Run snippets and cpython tests on wasm-wasi
- GitHub Check: Check the WASM package and demo
- GitHub Check: Ensure compilation on various targets
🔇 Additional comments (1)
crates/vm/src/stdlib/ast/pattern.rs (1)
390-435: Wrapper types look fine; delegation is cleanThe tuple-struct wrappers +
Nodedelegation keep thePatternMatchClassconversion code simpler and should round-trip as long asVec<T>: Nodeandruff::Identifier: Nodebehave as expected.
| /// Merges the pattern match class attributes and patterns, opposite of [`split_pattern_match_class`]. | ||
| fn merge_pattern_match_class( | ||
| _patterns: PatternMatchClassPatterns, | ||
| _kwd_attrs: PatternMatchClassKeywordAttributes, | ||
| _kwd_patterns: PatternMatchClassKeywordPatterns, | ||
| patterns: PatternMatchClassPatterns, | ||
| kwd_attrs: PatternMatchClassKeywordAttributes, | ||
| kwd_patterns: PatternMatchClassKeywordPatterns, | ||
| ) -> (Vec<ruff::Pattern>, Vec<ruff::PatternKeyword>) { | ||
| todo!() | ||
| let keywords = kwd_attrs | ||
| .0 | ||
| .into_iter() | ||
| .zip(kwd_patterns.0) | ||
| .map(|(attr, pattern)| ruff::PatternKeyword { | ||
| range: Default::default(), | ||
| node_index: Default::default(), | ||
| attr, | ||
| pattern, | ||
| }) | ||
| .collect(); | ||
| (patterns.0, keywords) | ||
| } |
There was a problem hiding this comment.
Don’t silently truncate mismatched kwd_attrs / kwd_patterns lengths
zip will drop extras if lengths differ. Since ast_from_object accepts user-constructed AST objects, this should error instead of losing data.
Low-impact fix: validate in ruff::PatternMatchClass::ast_from_object (where vm is available) before calling merge_pattern_match_class:
let kwd_attrs = Node::ast_from_object(
vm, source_file, get_node_field(vm, &object, "kwd_attrs", "MatchClass")?
)?;
let kwd_patterns = Node::ast_from_object(
vm, source_file, get_node_field(vm, &object, "kwd_patterns", "MatchClass")?
)?;
+
+if kwd_attrs.0.len() != kwd_patterns.0.len() {
+ return Err(vm.new_value_error(format!(
+ "MatchClass kwd_attrs and kwd_patterns must have the same length ({} != {})",
+ kwd_attrs.0.len(),
+ kwd_patterns.0.len(),
+ )));
+}
let (patterns, keywords) = merge_pattern_match_class(patterns, kwd_attrs, kwd_patterns);Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In crates/vm/src/stdlib/ast/pattern.rs around lines 560 to 578, the current
merge_pattern_match_class uses zip and silently drops extra entries when
kwd_attrs and kwd_patterns lengths differ; instead, validate that
kwd_attrs.0.len() == kwd_patterns.0.len() before merging (preferably in
ruff::PatternMatchClass::ast_from_object where the VM is available) and
return/raise a TypeError (or appropriate VM error) if they differ with a clear
message indicating mismatched keyword attributes and patterns lengths; only
after validation, construct PatternKeyword entries and return the merged
vectors.
1 participant
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.