◐ Shell
clean mode source ↗

more PEP695 by youknowone · Pull Request #5917 · RustPython/RustPython

gemini-code-assist

@youknowone

@youknowone youknowone commented

Jul 9, 2025

•

edited by coderabbitai Bot

Loading

Summary by CodeRabbit

  • Refactor

    • Improved the handling of class definitions for better separation between compiling class bodies and class definitions, resulting in clearer and more maintainable code.
    • Updated the creation of generic type aliases to use a more dynamic approach when handling type parameters.

  • New Features

    • Added support for a new class attribute __firstlineno__ and enhanced handling of type parameters and annotations in class definitions.

@coderabbitai

Walkthrough

The changes refactor class compilation by separating class body code generation into a new compile_class_body method and restructuring compile_class_def for clearer orchestration, especially around generic classes and type parameter handling. The generic alias creation logic in subscript_generic now dynamically calls Python's _GenericAlias instead of constructing the alias directly.

Changes

File(s) Change Summary
compiler/codegen/src/compile.rs Refactored class compilation: added compile_class_body, rewrote compile_class_def to manage scopes and generics.
vm/src/builtins/genericalias.rs Changed subscript_generic to use dynamic call to _GenericAlias for generic alias creation.

Sequence Diagram(s)

sequenceDiagram
    participant Compiler
    participant VM
    participant TypingModule

    Compiler->>Compiler: compile_class_def()
    alt Generic class
        Compiler->>Compiler: Push type param scope
        Compiler->>Compiler: compile type params
        Compiler->>Compiler: compile_class_body()
        Compiler->>VM: subscript_generic()
        VM->>TypingModule: import typing, get Generic, _GenericAlias
        VM->>VM: Call _GenericAlias(Generic, type_params)
        Compiler->>Compiler: Prepare and call __build_class__ with generic base
        Compiler->>Compiler: Exit type param scope
    else Non-generic class
        Compiler->>Compiler: compile_class_body()
        Compiler->>Compiler: Prepare and call __build_class__
    end
    Compiler->>Compiler: Apply decorators and store class name
Loading

Possibly related PRs

Poem

In the warren, code refines,
Classes split in tidy lines.
Generics hop with Python grace,
GenericAlias takes its place.
Scopes are entered, left with flair—
This bunny’s proud of code so rare!
🐇✨

📜 Recent review details

Configuration used: .coderabbit.yml
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 3ef0cfc and 9403919.

⛔ Files ignored due to path filters (2)
  • Lib/test/test_descr.py is excluded by !Lib/**
  • Lib/test/test_typing.py is excluded by !Lib/**
📒 Files selected for processing (2)
  • compiler/codegen/src/compile.rs (4 hunks)
  • vm/src/builtins/genericalias.rs (1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.rs

Instructions used from:

Sources:
📄 CodeRabbit Inference Engine

  • .github/copilot-instructions.md
⏰ 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). (11)
  • GitHub Check: Run tests under miri
  • GitHub Check: Run snippets and cpython tests on wasm-wasi
  • GitHub Check: Check the WASM package and demo
  • GitHub Check: Check Rust code with rustfmt and clippy
  • GitHub Check: Run rust tests (windows-latest)
  • GitHub Check: Run rust tests (macos-latest)
  • GitHub Check: Run snippets and cpython tests (windows-latest)
  • GitHub Check: Run rust tests (ubuntu-latest)
  • GitHub Check: Run snippets and cpython tests (macos-latest)
  • GitHub Check: Run snippets and cpython tests (ubuntu-latest)
  • GitHub Check: Ensure compilation on various targets
🔇 Additional comments (4)
vm/src/builtins/genericalias.rs (2)

619-635: LGTM! Good alignment with CPython's typing module behavior.

The refactor successfully changes from direct PyGenericAlias construction to dynamically calling Python's _GenericAlias class, which better aligns with CPython's implementation for PEP 695 support. The error handling through the ? operator is appropriate for the VM calls.

625-625: Accessing _GenericAlias matches CPython’s PEP 695 user-defined generics

  • CPython 3.12+ uses typing._GenericAlias for user-defined generics (e.g. class Foo[T]: …; Foo[int]) and types.GenericAlias for built-in generics (list[int]).
  • The lookup in vm/src/builtins/genericalias.rs:625 
    let generic_alias_class = typing_module.get_attr("_GenericAlias", vm)?;
    correctly mirrors CPython’s implementation for PEP 695.
  • Caveat: _GenericAlias is a private API and may change in future Python releases. Consider adding a fallback or a comment to track this dependency.
compiler/codegen/src/compile.rs (2)

2072-2164: LGTM! Good separation of concerns.

The new compile_class_body method cleanly separates the class body compilation logic from the overall class definition orchestration. The implementation correctly handles:

  • Class scope management
  • Namespace setup with all required dunder attributes
  • The new __firstlineno__ attribute (Python 3.12+ feature)
  • Type parameters integration
  • Proper __classcell__ handling

The method signature is clear and the logic flow follows Python's class compilation semantics properly.

2166-2331: Excellent refactoring for generic class handling.

The refactored compile_class_def method provides clear orchestration of class definition compilation with proper handling of both generic and non-generic classes:

Strengths:

  • Clean separation between generic and non-generic paths
  • Proper type parameters scope management for generic classes
  • Correct use of SubscriptGeneric intrinsic for generic base creation
  • Maintains all existing functionality while improving code organization

Implementation details handled correctly:

  • Decorator preparation and application
  • Function flags management (TYPE_PARAMS, CLOSURE)
  • Stack management for different compilation paths
  • Proper cleanup of type parameter scopes

The integration with the new compile_class_body method works well and the overall structure is much cleaner than having everything in one large method.

✨ Finishing Touches
  • 📝 Generate Docstrings

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
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Explain this complex logic.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai explain this code block.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and explain its main purpose.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

gemini-code-assist[bot]

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Summary of Changes

Hello @youknowone, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request refactors the compilation process for Python's PEP 695 generic classes, introducing a new wrapper-based approach that better manages type parameters and their scope. It also includes improvements to symbol table handling for type parameters and aligns the Generic type subscripting with CPython's internal implementation, enhancing overall compatibility and correctness.

Highlights

  • Enhanced PEP 695 Support: I've significantly advanced the implementation of PEP 695 (Type Parameter Syntax) for generic classes. This includes a new compilation strategy that aligns more closely with CPython's behavior.
  • Wrapper Function for Generic Classes: For generic classes, a new compilation approach is introduced where the class definition is wrapped in an outer function. This wrapper handles the creation and management of type parameters, then compiles and calls the actual class body, ensuring proper scope and attribute setting (__type_params__).
  • Refined Symbol Table Management: The compiler's symbol table logic has been updated to correctly identify and handle type parameters as cell variables. This ensures they are properly captured and accessible in nested scopes, which is crucial for the new wrapper-based compilation.
  • Improved Generic Subscripting: The subscript_generic intrinsic function in the VM has been updated to create Generic type aliases by calling typing._GenericAlias directly, mirroring CPython's internal mechanism for better compatibility.
  • LoadClassDeref Instruction Correction: A bug in the LoadClassDeref bytecode instruction has been fixed. It now correctly resolves variables by first checking cell variables and then free variables, which is essential for correctly loading __class__ and other captured variables in class scopes.
Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in issue comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist is currently in preview and may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments to provide feedback.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

gemini-code-assist[bot]

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request implements support for PEP 695 generic classes. The changes involve significant additions to the compiler to handle the new syntax, including creating wrapper functions for generic classes and managing symbol tables accordingly. The implementation seems to correctly follow the logic required by PEP 695.

I've identified an area in the compiler with significant code duplication that could be refactored to improve maintainability. Other than that, the changes look good and the bug fix in frame.rs is a great catch.

Comment on lines +1768 to +1849

for type_param in &type_params.type_params {
match type_param {
TypeParam::TypeVar(TypeParamTypeVar {
name,
bound,
default,
..
}) => {
if let Some(expr) = bound {
self.compile_expression(expr)?;
self.emit_load_const(ConstantData::Str {
value: name.as_str().into(),
});
emit!(self, Instruction::TypeVarWithBound);
} else {
self.emit_load_const(ConstantData::Str {
value: name.as_str().into(),
});
emit!(self, Instruction::TypeVar);
}

if let Some(default_expr) = default {
self.compile_expression(default_expr)?;
emit!(
self,
Instruction::CallIntrinsic2 {
func: bytecode::IntrinsicFunction2::SetTypeparamDefault
}
);
}

// Store as cell variable
self.store_name(name.as_str())?;
}
TypeParam::ParamSpec(TypeParamParamSpec { name, default, .. }) => {
self.emit_load_const(ConstantData::Str {
value: name.as_str().into(),
});
emit!(self, Instruction::ParamSpec);

if let Some(default_expr) = default {
self.compile_expression(default_expr)?;
emit!(
self,
Instruction::CallIntrinsic2 {
func: bytecode::IntrinsicFunction2::SetTypeparamDefault
}
);
}

// Store as cell variable
self.store_name(name.as_str())?;
}
TypeParam::TypeVarTuple(TypeParamTypeVarTuple { name, default, .. }) => {
self.emit_load_const(ConstantData::Str {
value: name.as_str().into(),
});
emit!(self, Instruction::TypeVarTuple);

if let Some(default_expr) = default {
self.compile_expression(default_expr)?;
emit!(
self,
Instruction::CallIntrinsic2 {
func: bytecode::IntrinsicFunction2::SetTypeparamDefault
}
);
}

// Store as cell variable
self.store_name(name.as_str())?;
}
}
}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

There's significant code duplication in the for loop that compiles type parameters. The match arms for TypeParam::TypeVar, TypeParam::ParamSpec, and TypeParam::TypeVarTuple are very similar. Each arm:

  1. Loads the name as a string constant.
  2. Emits a specific instruction (TypeVar, ParamSpec, etc.).
  3. Handles an optional default value by compiling an expression and calling an intrinsic.
  4. Stores the created type parameter as a cell variable.

This repetition makes the code harder to maintain. Consider refactoring this logic to reduce duplication. For example, you could extract the common parts, like handling the default value and storing the name, into a helper function or restructure the loop to handle the differences more concisely.

A possible refactoring could look something like this:

for type_param in &type_params.type_params {
    let (name, default) = match type_param {
        TypeParam::TypeVar(tp) => {
            if let Some(expr) = &tp.bound {
                self.compile_expression(expr)?;
                self.emit_load_const(ConstantData::Str { value: tp.name.as_str().into() });
                emit!(self, Instruction::TypeVarWithBound);
            } else {
                self.emit_load_const(ConstantData::Str { value: tp.name.as_str().into() });
                emit!(self, Instruction::TypeVar);
            }
            (tp.name.as_str(), &tp.default)
        }
        TypeParam::ParamSpec(tp) => {
            self.emit_load_const(ConstantData::Str { value: tp.name.as_str().into() });
            emit!(self, Instruction::ParamSpec);
            (tp.name.as_str(), &tp.default)
        }
        TypeParam::TypeVarTuple(tp) => {
            self.emit_load_const(ConstantData::Str { value: tp.name.as_str().into() });
            emit!(self, Instruction::TypeVarTuple);
            (tp.name.as_str(), &tp.default)
        }
    };

    if let Some(default_expr) = default {
        self.compile_expression(default_expr)?;
        emit!(
            self,
            Instruction::CallIntrinsic2 {
                func: bytecode::IntrinsicFunction2::SetTypeparamDefault
            }
        );
    }

    // Store as cell variable
    self.store_name(name)?;
}

This is just an idea, and there might be even cleaner ways to structure this.

@youknowone youknowone changed the title Update types.py + more PEP695 more PEP695

Jul 12, 2025

@youknowone

@coderabbitai

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.