GeezerVison is showing github.com — Refactor _ast module for clarity by youknowone · Pull Request #7031 · RustPython/RustPython
Conversation
youknowone
commented
Feb 6, 2026
•
edited by coderabbitai
Bot
Loading
Uh oh!
There was an error while loading. Please reload this page.
Summary by CodeRabbit
-
Performance Improvements
- Improved AST node object caching for faster repeated access.
-
Refactor
- Consolidated AST node object creation and serialization across implementations for maintainability.
- Simplified how node metadata is populated: switched to a representation-focused population and removed some automatic attribute/match-args population.
-
Chores
- Renamed the compile-time AST-only flag (public API name updated).
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
coderabbitai
Bot
commented
Feb 6, 2026
•
edited
Loading
Uh oh!
There was an error while loading. Please reload this page.
Caution
Review failed
The pull request is closed.
📝 Walkthrough
Walkthrough
This PR adds a helper singleton_node_to_object, renames PY_COMPILE_FLAG_AST_ONLY → PY_CF_ONLY_AST (and updates derived flags/masks), and consistently renames underscore-prefixed AST API parameters (_vm, _object, _cls, _range) to vm, object, cls, range across AST modules.
Changes
| Cohort / File(s) |
Summary |
AST Core & Flag Refactoring
crates/vm/src/stdlib/ast.rs |
Adds singleton_node_to_object(vm, node_type) and renames PY_COMPILE_FLAG_AST_ONLY → PY_CF_ONLY_AST; updates PY_CF_OPTIMIZED_AST and PY_COMPILE_FLAGS_MASK to use the new constant. |
Pattern & Type Parameter Refactoring
crates/vm/src/stdlib/ast/pattern.rs, crates/vm/src/stdlib/ast/type_parameters.rs |
Renames parameters/signatures from _vm/_object/_range → vm/object/range and updates all nested calls, dict population, and node location handling across pattern and type-parameter nodes. |
AST Node Implementations
crates/vm/src/stdlib/ast/operator.rs, crates/vm/src/stdlib/ast/expression.rs, crates/vm/src/stdlib/ast/type_ignore.rs, crates/vm/src/stdlib/ast/exception.rs |
Replaces local parameter names (_vm/_object/_cls) with vm/object/cls, consolidates singleton retrieval to singleton_node_to_object, and standardizes error construction and repr usage via vm. |
AST Module Population / repr
crates/vm/src/stdlib/ast/pyast.rs, crates/vm/src/stdlib/ast/python.rs |
Removes populate_match_args_and_attributes, adds/uses populate_repr to set types' repr slot, assigns _attributes for extended classes, and updates imports/constant binding to PY_CF_ONLY_AST. |
Compiler Flag Usage
crates/vm/src/stdlib/builtins.rs |
Replaces references to PY_COMPILE_FLAG_AST_ONLY with PY_CF_ONLY_AST in compile() AST gating checks. |
Estimated code review effort
🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
Suggested reviewers
Poem
🐇 I hopped through code with nimble feet,
Renamed each underscore to make things neat.
Singletons cached in a comfy spot,
Flags reborn and reprs now taught.
Hooray — the AST's tidy and sweet! 🎉
🚥 Pre-merge checks | ✅ 1 | ❌ 2
❌ 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%. |
Write docstrings for the functions missing them to satisfy the coverage threshold. |
| Title check |
❓ Inconclusive |
The title 'refactor ast' is vague and lacks specificity about the scope and nature of the refactoring, using a generic term that doesn't convey the actual changes. |
Consider a more descriptive title such as 'Refactor AST parameter naming and introduce singleton caching' or 'Refactor AST: standardize parameter names and add compile flag constants' to better reflect the primary changes. |
✅ Passed checks (1 passed)
| Check name |
Status |
Explanation |
| Description Check |
✅ Passed |
Check skipped - CodeRabbit’s high-level summary is enabled. |
✏️ Tip: You can configure your own custom pre-merge checks in the settings.
✨ Finishing touches
🧪 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.
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
- Extract singleton_node_to_object() helper for operator/context nodes
- Rename PY_COMPILE_FLAG_AST_ONLY to PY_CF_ONLY_AST
- Rename populate_match_args_and_attributes to populate_repr
- Set _attributes in impl_base_node! no-args variant
- Simplify ast_reduce with .is_some() instead of drop(value)
- Remove _ prefix from used parameters across ast/ files
- Fix slot_new comment to explain why slot_init is called
Uh oh!
There was an error while loading. Please reload this page.
youknowone
added a commit
that referenced
this pull request
Sep 16, 2026
- Extract singleton_node_to_object() helper for operator/context nodes
- Rename PY_COMPILE_FLAG_AST_ONLY to PY_CF_ONLY_AST
- Rename populate_match_args_and_attributes to populate_repr
- Set _attributes in impl_base_node! no-args variant
- Simplify ast_reduce with .is_some() instead of drop(value)
- Remove _ prefix from used parameters across ast/ files
- Fix slot_new comment to explain why slot_init is called
Uh oh!
There was an error while loading. Please reload this page.
Assignees
No one assigned
Milestone
No milestone
Development
Successfully merging this pull request may close these issues.
Uh oh!
There was an error while loading. Please reload this page.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
|