FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Refactor _ast module for clarity by youknowone · Pull Request #7031 · RustPython/RustPython · GitHub

/ RustPython Public

Refactor _ast module for clarity - #7031

Merged
youknowone merged 1 commit into
RustPython:mainfrom
youknowone:ast
Feb 6, 2026
ConversationCommits (1)ChecksFiles changed
Merged

Refactor _ast module for clarity#7031
youknowone merged 1 commit into
RustPython:mainfrom
youknowone:ast

Conversation

youknowone commented Feb 6, 2026 •
edited by coderabbitai Bot
Loading

Copy link
Copy Markdown
Member

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

coderabbitai Bot commented Feb 6, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

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

  • arihant2math

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 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.

youknowone marked this pull request as ready for review February 6, 2026 08:31
Refactor _ast module for clarity
6ae4f29 
- 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
youknowone changed the title refactor ast Refactor _ast module for clarity Feb 6, 2026
youknowone merged commit 6d29b7f into RustPython:main Feb 6, 2026
12 of 13 checks passed
youknowone deleted the ast branch February 6, 2026 08:45
coderabbitai Bot mentioned this pull request Mar 10, 2026
Merged
coderabbitai Bot mentioned this pull request Jun 23, 2026
Merged
2 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

Back | FazBrowse Home | New Git URL