impl more ast #6419

youknowone · 2025-12-12T12:11:37Z

Summary by CodeRabbit

Refactor
- Enhanced serialization and deserialization support for Python singleton values in pattern matching.
- Modernized pattern matching class structures for more reliable argument handling and improved consistency.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

coderabbitai · 2025-12-12T12:11:50Z

Walkthrough

This PR refactors AST pattern handling in the Rust VM stdlib, replacing placeholder implementations with concrete vector-based structs. Three PatternMatchClass related structures now wrap vectors instead of ranges. Functions for splitting and merging pattern arguments are updated to handle actual data conversions for Singleton values and pattern components.

Changes

Cohort / File(s)	Summary
Pattern AST Refactoring `crates/vm/src/stdlib/ast/pattern.rs`	Refactored `PatternMatchClassPatterns`, `PatternMatchClassKeywordAttributes`, and `PatternMatchClassKeywordPatterns` from placeholder range-based structs to vector-based newtype wrappers. Implemented trait methods (`ast_to_object`, `ast_from_object`) for `ruff::Singleton` to convert between Python objects (None, True, False) and Rust enum variants. Updated `split_pattern_match_class` and `merge_pattern_match_class` to construct and deconstruct public vector-based components with proper keyword pattern handling.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Struct representation changes: Verify that the newtype wrapper conversions correctly delegate to inner vectors and handle edge cases.
Singleton implementation: Ensure ast_to_object and ast_from_object properly map between Python objects and enum variants, and that error handling is complete.
split/merge function logic: Carefully review the merge_pattern_match_class construction of PatternKeyword items with proper range and node_index defaults to ensure correct round-tripping of AST data.

Poem

🐰 From ranges sparse to vectors full of might,
We pack our patterns tight, a refactor's delight.
Singletons sing true—None, False, and True—
Now serialized and round-tripped, the old TODOs are through! ✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning, 1 inconclusive)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 9.09% which is insufficient. The required threshold is 80.00%.	You can run `@coderabbitai generate docstrings` to improve docstring coverage.
Title check	❓ Inconclusive	The title 'impl more ast' is vague and generic, using non-descriptive terms that don't convey meaningful information about the specific changes made.	Consider using a more descriptive title that reflects the specific implementation, such as 'Implement proper serialization for Singleton and vector-based PatternMatchClass structures' or similar.

✅ Passed checks (1 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.

✨ Finishing touches

📝 Generate docstrings

🧪 Generate unit tests (beta)

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

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

coderabbitai

Actionable comments posted: 0

🧹 Nitpick comments (1)

crates/vm/src/stdlib/ast/pattern.rs (1)
566-577: Consider adding a debug assertion for length mismatch.

The zip() will silently truncate if kwd_attrs and kwd_patterns have different lengths, which could mask upstream bugs or malformed AST input. A debug_assert_eq! would catch this during development without runtime cost in release builds.
 fn merge_pattern_match_class(
     patterns: PatternMatchClassPatterns,
     kwd_attrs: PatternMatchClassKeywordAttributes,
     kwd_patterns: PatternMatchClassKeywordPatterns,
 ) -> (Vec<ruff::Pattern>, Vec<ruff::PatternKeyword>) {
+    debug_assert_eq!(
+        kwd_attrs.0.len(),
+        kwd_patterns.0.len(),
+        "kwd_attrs and kwd_patterns length mismatch"
+    );
     let keywords = kwd_attrs
         .0
         .into_iter()
         .zip(kwd_patterns.0)

📜 Review details

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 35f8860 and 3f06a4b.

📒 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 running cargo fmt to 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 (4)

crates/vm/src/stdlib/itertools.rs (1)

None (1152-1152)

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). (10)

GitHub Check: Ensure compilation on various targets
GitHub Check: Run snippets and cpython tests (windows-latest)
GitHub Check: Run snippets and cpython tests (macos-latest)
GitHub Check: Check the WASM package and demo
GitHub Check: Run rust tests (windows-latest)
GitHub Check: Run rust tests (macos-latest)
GitHub Check: Check Rust code with clippy
GitHub Check: Run rust tests (ubuntu-latest)
GitHub Check: Run snippets and cpython tests (ubuntu-latest)
GitHub Check: Run snippets and cpython tests on wasm-wasi

🔇 Additional comments (2)

crates/vm/src/stdlib/ast/pattern.rs (2)

201-228: LGTM! Clean implementation of Singleton conversion.

The identity-based checks using object.is() for True and False are correct for Python singletons, and vm.is_none() properly handles the None case.

390-436: LGTM! Clean newtype wrapper pattern.

The tuple structs provide type-safe wrappers around the vectors, and the Node implementations correctly delegate to the inner collection's serialization/deserialization.

impl more ast

3f06a4b

youknowone marked this pull request as ready for review December 12, 2025 12:11

youknowone marked this pull request as draft December 12, 2025 12:11

coderabbitai bot reviewed Dec 12, 2025

View reviewed changes

youknowone marked this pull request as ready for review December 12, 2025 12:50

youknowone merged commit c11a72a into RustPython:main Dec 12, 2025
12 of 13 checks passed

youknowone deleted the ast branch December 12, 2025 12:50

coderabbitai bot mentioned this pull request Jan 18, 2026

import ruff_python_ast only and don't imort its subitems #6769

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

impl more ast #6419

impl more ast #6419

Uh oh!

youknowone commented Dec 12, 2025 •

edited by coderabbitai bot

Loading

Uh oh!

coderabbitai bot commented Dec 12, 2025 •

edited

Loading

Uh oh!

coderabbitai bot left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

impl more ast #6419

impl more ast #6419

Uh oh!

Conversation

youknowone commented Dec 12, 2025 • edited by coderabbitai bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Uh oh!

coderabbitai bot commented Dec 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Poem

Pre-merge checks and finishing touches

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

youknowone commented Dec 12, 2025 •

edited by coderabbitai bot

Loading

coderabbitai bot commented Dec 12, 2025 •

edited

Loading