Skip to content

Conversation

@youknowone
Copy link
Member

@youknowone youknowone commented Jan 4, 2026

Summary by CodeRabbit

  • New Features

    • Error messages now include precise end-location info for parse and syntax errors, improving pinpointing of issues.
    • Syntax error objects now carry end-position data so editors and tools can display more accurate error spans.
  • Bug Fixes

    • Caret indicator for syntax errors is shown only for same-line errors, avoiding misleading multi-line caret rendering.
    • Improved metaclass/type resolution with a safer fallback that yields more accurate type selection.

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

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 4, 2026

📝 Walkthrough

Walkthrough

Adds parse-error end-location tracking and exposes it through the compiler/VM, restricts SyntaxError caret rendering to same-line errors, and adds object-level fallback subclass checks in metaclass resolution.

Changes

Cohort / File(s) Summary
Parse Error Structure
crates/compiler/src/lib.rs
Added end_location: SourceLocation to ParseError; compute and adjust end location in from_ruff_parse_error. Added CompileError::python_end_location() to expose parse-end as (line, column).
Error Information Propagation
crates/vm/src/stdlib/ast.rs
When parsing fails, compute full source range and populate both location (start) and end_location (end) on ParseError.
SyntaxError Rendering
crates/vm/src/exceptions.rs
Restrict caret rendering to same-line errors by comparing lineno and end_lineno; only compute/use end_offset and render caret when both are on the same line.
VM SyntaxError Object
crates/vm/src/vm/vm_new.rs
When available, set syntax_error.end_lineno and syntax_error.end_offset using error.python_end_location().
Metaclass Resolution Fallback
crates/vm/src/builtins/type.rs
In calculate_meta_class, add fallback is_subclass checks on object-level (as_object().is_subclass(...)) when fast_issubclass checks fail; adjust winner promotion accordingly.

Sequence Diagram

sequenceDiagram
    participant Parser as AST Parser
    participant Comp as CompileError / ParseError
    participant VM as VM Exception Layer
    participant PyErr as Python SyntaxError Obj

    rect rgb(230,245,230)
    Note over Parser,Comp: Parse-time error detection
    Parser->>Comp: produce parse error with start/end range
    Comp->>Comp: compute start_location and end_location
    Comp->>VM: return/raise ParseError (includes end_location)
    end

    rect rgb(230,230,245)
    Note over VM,PyErr: VM constructs Python SyntaxError
    VM->>VM: set lineno and offset from error.location
    VM->>VM: call error.python_end_location()
    alt end location present
        VM->>PyErr: set end_lineno and end_offset
    else no end location
        VM->>PyErr: leave end attributes unset
    end
    end

    rect rgb(245,240,230)
    Note over VM,PyErr: Exception rendering (display)
    VM->>VM: compare lineno and end_lineno
    alt same_line == true
        VM->>VM: compute caret range using offset..end_offset
        VM->>PyErr: render caret under same line
    else same_line == false
        VM->>PyErr: render single-point marker (no caret)
    end
    end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

Poem

🐰 Hop, hop—errors now know where to end,
start to finish, each token I tend.
Carets only dance on the same-lined stage,
subclasses checked by object-level sage.
I nibble bugs and celebrate this mend. 🥕

Pre-merge checks and finishing touches

❌ Failed checks (1 inconclusive)
Check name Status Explanation Resolution
Title check ❓ Inconclusive The title 'ast end_offset' is too vague and generic. While it contains keyword terms related to the changes, it does not clearly convey what was actually implemented—adding end location tracking to parse errors and improving error rendering. Use a more descriptive title such as 'Add end_location tracking to parse errors' or 'Support end offset in syntax error reporting' to better communicate the primary change.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage ✅ Passed Docstring coverage is 87.50% which is sufficient. The required threshold is 80.00%.
✨ Finishing touches
  • 📝 Generate docstrings

📜 Recent 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 5c60163 and 2bb6fd4.

⛔ Files ignored due to path filters (1)
  • Lib/test/test_cmd_line_script.py is excluded by !Lib/**
📒 Files selected for processing (5)
  • crates/compiler/src/lib.rs
  • crates/vm/src/builtins/type.rs
  • crates/vm/src/exceptions.rs
  • crates/vm/src/stdlib/ast.rs
  • crates/vm/src/vm/vm_new.rs
🚧 Files skipped from review as they are similar to previous changes (4)
  • crates/vm/src/exceptions.rs
  • crates/vm/src/stdlib/ast.rs
  • crates/compiler/src/lib.rs
  • crates/vm/src/vm/vm_new.rs
🧰 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 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/builtins/type.rs
🔇 Additional comments (1)
crates/vm/src/builtins/type.rs (1)

2009-2020: LGTM! The fallback subclass checks are correctly implemented.

The logic correctly mirrors CPython's _PyType_CalculateMetaclass by first attempting fast MRO-based checks, then falling back to general is_subclass calls that respect __subclasscheck__ overrides. Error propagation with ? is appropriate.

The comment at lines 2009-2010 stating "metaclasses are not PyType subclasses" could be more precise. Since both winner and base_type are PyTypeRef, the fallback is actually needed when custom metaclasses override __subclasscheck__ or when the MRO-based check doesn't capture the complete subclass relationship.


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 youknowone marked this pull request as ready for review January 4, 2026 15:33
@youknowone youknowone merged commit bdf3b36 into RustPython:main Jan 4, 2026
13 checks passed
@youknowone youknowone deleted the ast-end_offset branch January 4, 2026 23:01
terryluan12 pushed a commit to terryluan12/RustPython that referenced this pull request Jan 5, 2026
terryluan12 pushed a commit to terryluan12/RustPython that referenced this pull request Jan 5, 2026
terryluan12 pushed a commit to terryluan12/RustPython that referenced this pull request Jan 15, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant