Actionable comments posted: 6

Cargo.toml (1)

161-164: Ruff crates aligned to 0.13.1 — good. Consider pinning to a commit for reproducibility.

Keeping all ruff crates on the same tag is correct for API coherence. For hermetic builds, consider adding rev = "<commit-sha>" alongside tag (or instead of tag). Optional, but helps avoid tag retargeting surprises.

compiler/codegen/src/lib.rs (1)

58-60: Handled Expr::TString — nice coverage update.

This keeps error messages consistent with new AST variants introduced by Ruff 0.13.x.

If feasible, replace the IpyEscapeCommand(_) => todo!() arm nearby with a stable name to avoid panics in diagnostics.

compiler/codegen/src/compile.rs (6)

239-256: Utf8 location printing looks correct; consider a small helper to avoid drift.

You call source_location(..., PositionEncoding::Utf8) here and in a few other places. A tiny helper (e.g., to_utf8_loc(offset)) would keep encoding consistent across all call sites.

---

530-540: Error locations switched to Utf8 — good. Centralize encoding selection.

Same note as above: a single helper on Compiler for building SourceLocation would prevent future mismatches.

---

5550-5571: Recursive await detection inside format specs — solid approach.

The recursion over nested format_spec elements mirrors CPython behavior; early-returns should keep it cheap.

If this gets hot, consider short-circuiting on first true at the outer iterator, but current code already does .any() which is fine.

---

5337-5345: Instruction location emission uses Utf8 — unify with other call sites.

Same helper recommendation to keep encoding usage consistent in _emit/eprint/error paths.

---

5869-5987: Nice tests for await-in-fstrings; add one for debug text and one for nested debug+spec.

Consider adding:

• f"{x=}" (no await)
• f"{x={y=}}" (nested debug text in spec)

---

1-1: Run rustfmt and clippy.

Given the scope of refactors, please run cargo fmt and cargo clippy and fix any new lints before merge.

vm/src/stdlib/ast/string.rs (1)

13-16: Avoid O(n^2) traversal by repeated nth(i) on fresh iterators.

Both iterators rebuild from the start per element. Consider draining in one pass to avoid quadratic behavior.

Example (no extra allocations on elements, collects output once):

let mut out = Vec::with_capacity(fstring_value.as_slice().len());
for slot in fstring_value.iter_mut() {
    out.push(std::mem::replace(slot, default.clone()));
}
out.into_iter()

Apply similarly to ruff_fstring_element_into_iter using (&mut fstring_element).into_iter(). As per coding guidelines, ensure clippy is clean.

Also applies to: 28-31

vm/src/stdlib/ast/argument.rs (1)

160-166: Preserve range when merging class-def args.

range: Default::default() loses the span. Consider covering the spans of provided positional/keyword arguments, mirroring merge_function_call_arguments.

-    Some(Box::new(ruff::Arguments {
-        node_index: Default::default(),
-        range: Default::default(), // TODO
+    // Compute a best-effort range from provided parts
+    let range = match (positional_arguments.as_ref(), keyword_arguments.as_ref()) {
+        (Some(p), Some(k)) => p.range.cover(k.range),
+        (Some(p), None) => p.range,
+        (None, Some(k)) => k.range,
+        (None, None) => TextRange::default(),
+    };
+    Some(Box::new(ruff::Arguments {
+        node_index: Default::default(),
+        range,
         args,
         keywords,
     }))

vm/src/stdlib/ast/statement.rs (1)

963-969: Avoid redundant class lookup and shadowing in Try::ast_from_object.

You fetch _object.class() twice and shadow _cls. Fold into one lookup and reuse in the debug assertion.

-        let _cls = _object.class();
-        let is_star = _cls.is(pyast::NodeStmtTryStar::static_type());
-        let _cls = _object.class();
-        debug_assert!(
+        let _cls = _object.class();
+        let is_star = _cls.is(pyast::NodeStmtTryStar::static_type());
+        debug_assert!(
             _cls.is(pyast::NodeStmtTry::static_type())
                 || _cls.is(pyast::NodeStmtTryStar::static_type())
         );

vm/src/stdlib/ast/pattern.rs (1)

358-371: PatternArguments constructed with defaulted range.

If range info matters downstream, consider threading a meaningful range into PatternArguments; otherwise this is fine as a stopgap.

Ensure consumers of PatternArguments don’t rely on a non-default range.

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

Reviewing files that changed from the base of the PR and between 1aea146 and 3887ae9.

• Cargo.lock is excluded by !**/*.lock

• Cargo.toml (1 hunks)
• compiler/codegen/src/compile.rs (15 hunks)
• compiler/codegen/src/ir.rs (2 hunks)
• compiler/codegen/src/lib.rs (1 hunks)
• compiler/codegen/src/symboltable.rs (16 hunks)
• compiler/codegen/src/unparse.rs (24 hunks)
• compiler/core/src/bytecode.rs (1 hunks)
• compiler/core/src/lib.rs (1 hunks)
• compiler/core/src/marshal.rs (2 hunks)
• compiler/src/lib.rs (3 hunks)
• src/shell.rs (2 hunks)
• stdlib/src/faulthandler.rs (1 hunks)
• vm/src/builtins/frame.rs (1 hunks)
• vm/src/frame.rs (2 hunks)
• vm/src/stdlib/ast.rs (4 hunks)
• vm/src/stdlib/ast/argument.rs (4 hunks)
• vm/src/stdlib/ast/constant.rs (10 hunks)
• vm/src/stdlib/ast/elif_else_clause.rs (4 hunks)
• vm/src/stdlib/ast/exception.rs (2 hunks)
• vm/src/stdlib/ast/expression.rs (51 hunks)
• vm/src/stdlib/ast/module.rs (4 hunks)
• vm/src/stdlib/ast/other.rs (4 hunks)
• vm/src/stdlib/ast/parameter.rs (8 hunks)
• vm/src/stdlib/ast/pattern.rs (18 hunks)
• vm/src/stdlib/ast/statement.rs (46 hunks)
• vm/src/stdlib/ast/string.rs (10 hunks)
• vm/src/stdlib/ast/type_parameters.rs (7 hunks)
• vm/src/vm/vm_new.rs (2 hunks)
• wasm/lib/src/convert.rs (1 hunks)

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

**/*.rs: Format Rust code with the default rustfmt style (run cargo fmt)
Run clippy and fix any warnings or lints introduced by your changes
Follow Rust best practices for error handling and memory management

Files:

• compiler/core/src/bytecode.rs
• compiler/codegen/src/ir.rs
• compiler/core/src/marshal.rs
• compiler/core/src/lib.rs
• vm/src/stdlib/ast/argument.rs
• compiler/codegen/src/symboltable.rs
• vm/src/stdlib/ast/parameter.rs
• vm/src/stdlib/ast/module.rs
• compiler/codegen/src/unparse.rs
• vm/src/stdlib/ast/pattern.rs
• wasm/lib/src/convert.rs
• vm/src/stdlib/ast/elif_else_clause.rs
• vm/src/stdlib/ast/other.rs
• src/shell.rs
• vm/src/stdlib/ast.rs
• vm/src/stdlib/ast/string.rs
• compiler/codegen/src/compile.rs
• stdlib/src/faulthandler.rs
• vm/src/vm/vm_new.rs
• vm/src/stdlib/ast/type_parameters.rs
• vm/src/stdlib/ast/constant.rs
• compiler/src/lib.rs
• compiler/codegen/src/lib.rs
• vm/src/builtins/frame.rs
• vm/src/stdlib/ast/statement.rs
• vm/src/frame.rs
• vm/src/stdlib/ast/expression.rs
• vm/src/stdlib/ast/exception.rs

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Use RustPython macros (pyclass, pymodule, pyfunction, etc.) when implementing Python functionality in Rust

Files:

• vm/src/stdlib/ast/argument.rs
• vm/src/stdlib/ast/parameter.rs
• vm/src/stdlib/ast/module.rs
• vm/src/stdlib/ast/pattern.rs
• vm/src/stdlib/ast/elif_else_clause.rs
• vm/src/stdlib/ast/other.rs
• vm/src/stdlib/ast.rs
• vm/src/stdlib/ast/string.rs
• stdlib/src/faulthandler.rs
• vm/src/vm/vm_new.rs
• vm/src/stdlib/ast/type_parameters.rs
• vm/src/stdlib/ast/constant.rs
• vm/src/builtins/frame.rs
• vm/src/stdlib/ast/statement.rs
• vm/src/frame.rs
• vm/src/stdlib/ast/expression.rs
• vm/src/stdlib/ast/exception.rs

Learnt from: CR
PR: RustPython/RustPython#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-11T05:06:59.971Z
Learning: Applies to {vm,stdlib}/**/*.rs : Use RustPython macros (`pyclass`, `pymodule`, `pyfunction`, etc.) when implementing Python functionality in Rust

Applied to files:

• src/shell.rs

vm/src/stdlib/ast/expression.rs (12)

• ast_from_object (55-143)
• ast_from_object (167-186)
• ast_from_object (210-229)
• ast_from_object (256-280)
• ast_from_object (303-322)
• ast_from_object (346-365)
• ast_from_object (392-416)
• ast_from_object (447-472)
• ast_from_object (492-506)
• ast_from_object (530-549)
• ast_from_object (573-592)
• ast_from_object (619-643)

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

• get_node_field (56-59)
• range_from_object (163-186)

vm/src/stdlib/ast/expression.rs (14)

• ast_to_object (11-53)
• ast_to_object (148-165)
• ast_to_object (191-208)
• ast_to_object (234-254)
• ast_to_object (285-302)
• ast_to_object (327-344)
• ast_to_object (370-390)
• ast_to_object (421-445)
• ast_to_object (477-491)
• ast_to_object (511-528)
• ast_to_object (554-571)
• ast_to_object (597-617)
• ast_to_object (648-666)
• ast_to_object (694-708)

compiler/src/lib.rs (1)

• location (58-63)

vm/src/stdlib/ast/constant.rs (1)

• new_str (13-23)

vm/src/frame.rs (1)

• current_location (173-175)

vm/src/stdlib/ast/exception.rs (2)

• ast_to_object (6-10)
• ast_to_object (36-59)

vm/src/stdlib/ast/expression.rs (14)

• ast_to_object (11-53)
• ast_to_object (148-165)
• ast_to_object (191-208)
• ast_to_object (234-254)
• ast_to_object (285-302)
• ast_to_object (327-344)
• ast_to_object (370-390)
• ast_to_object (421-445)
• ast_to_object (477-491)
• ast_to_object (511-528)
• ast_to_object (554-571)
• ast_to_object (597-617)
• ast_to_object (648-666)
• ast_to_object (694-708)

vm/src/stdlib/ast.rs (1)

• range_from_object (163-186)

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

vm/src/builtins/frame.rs (1)

61-64: Switch to line looks right; confirm 1-based semantics and early-frame safety.

This aligns with the row→line migration. Please confirm SourceLocation.line is 1-based to match CPython’s f_lineno. Also note: if lasti() == 0, current_location() will index with -1 and panic; see suggested fix in vm/src/frame.rs.

vm/src/frame.rs (1)

391-400: Traceback lasti vs idx off-by-one — please confirm intent.

Here, idx is computed before update_lasti(|i| *i += 1), so frame.lasti() equals idx + 1. You’re recording loc for idx but passing frame.lasti() to PyTraceback::new. If traceback expects the index of the instruction that raised, consider passing idx as u32 + 1 explicitly or idx depending on that API’s contract. Please verify.

stdlib/src/faulthandler.rs (1)

11-15: LGTM: switched to line for consistency with new location fields.

compiler/codegen/src/ir.rs (2)

185-187: Leverage SourceLocation’s Copy semantics

Nice touch swapping info.location.clone() for the by-value repeat; it keeps the iterator allocation-free while still duplicating the metadata for multi-word instructions.

---

425-493: Column math matches the UTF-8 offset semantics

Using loc.character_offset.to_zero_indexed() keeps the short/long encodings in sync with PositionEncoding::Utf8, so downstream consumers will continue to get zero-based columns without needing any extra conversions.

wasm/lib/src/convert.rs (1)

254-260: JS error surface stays aligned with the new location fields

Confirming that wiring line.get() / character_offset.get() through preserves the 1-based numbers the browser tooling expects, so there’s no behavioral regression for WASM consumers.

vm/src/vm/vm_new.rs (1)

323-417: Parser error mapping follows the renamed lexer diagnostics

Thanks for updating both the enum path and the line.to_zero_indexed() helper—error classification and snippet extraction remain consistent with the parser rename.

compiler/core/src/marshal.rs (2)

201-206: Deserialization handles zero-indexed character offsets

Reading the offset back via OneIndexed::from_zero_indexed mirrors the data written on disk, so existing marshaled blobs round-trip without loss.

---

659-661: Serialized payload mirrors the new location schema

Writing line and zero-based character_offset keeps marshal format compatibility with the broader SourceLocation rename while avoiding any off-by-one surprises.

compiler/core/src/lib.rs (1)

11-13: Re-exporting PositionEncoding smooths downstream transitions

Pulling the new enum into the public surface saves dependents an extra import churn during the Ruff 0.13 bump—nice proactive step.

compiler/codegen/src/compile.rs (9)

633-637: Confirm character_offset initialization semantics.

character_offset is set to OneIndexed::MIN. Please confirm this is the intended “column 1” equivalent with Utf8 encoding for all consumers of SourceLocation.

---

769-775: Module-scope RESUME location: line/character MIN — verify downstream expectations.

Using OneIndexed::MIN for both fields diverges from CPython’s “-1” semantics. If any tooling assumes 1-based positions, this is fine; otherwise, worth double-checking.

---

416-473: Subscript/slice lowering: two-element slice path looks right; add coverage for Del and step.

The special-case for [lower:upper] builds a slice and selects Subscript/StoreSubscript. For Del and for [: :step], you fall through to the generic path. Please add tests for:

• a[1:3] (load), a[1:3] = v (store), del a[1:3] (delete)
• a[1:3:2] (step present)

---

4873-4877: Named expression: ignoring node_index in codegen is fine.

Plumbing the new field without affecting semantics is appropriate here.

---

5529-5540: Await detection for FString/TString integrated via helper — LGTM.

---

5612-5700: F-string lowering: debug text => repr, empty format-spec sentinel, BuildString sizing — looks correct; add a couple of tests.

Please add coverage for:

• f"{x=}" (ensures debug_text forces repr)
• nested format specs and surrogate-literal reparse paths

---

27-33: AST/type import updates align with Ruff 0.13.x — good.

Interpolated* types and PositionEncoding adoption here match the rest of the changes.

Also applies to: 36-37

---

1-1: Refine sanity checks and verify slice-lowering tests

• The lone .source_location( call without PositionEncoding::Utf8 is the SourceCode::index.source_location(offset, self.text) invocation in compiler/source/src/lib.rs; it’s an instance method that doesn’t take an encoding parameter—static calls all use PositionEncoding::Utf8.
• All Expr::TString occurrences are confined to codegen/src/{unparse.rs,symboltable.rs,compile.rs}, so no stray handlers outside codegen.
• No slice-lowering tests detected in Rust sources; please confirm that Python-level tests for slice deletion (e.g. del a[1:3]) exist under /tests or add them, and update your regex to target .py files rather than Rust code.

---

4885-4887: TString returns NotImplementedYet — gate or test this path.

TString is present in the AST and referenced by symboltable/unparse/vm, but the compiler currently returns NotImplemented (compiler/codegen/src/compile.rs:4885–4887). Either:

• reuse the existing interpolated-string lowering (compile_fstring_elements at compiler/codegen/src/compile.rs:5613) to handle TString, or
• add a focused test that asserts the exact NotImplemented error for TString until full support is implemented.

I can sketch the minimal wiring to reuse compile_fstring_elements.

vm/src/stdlib/ast/argument.rs (3)

59-65: LGTM: Arguments now initializes node_index.

Initialization with Default::default() is consistent across the PR.

---

70-75: LGTM: Destructuring ignores node_index safely.

Discarding node_index here has no behavioral impact.

---

110-114: LGTM: Destructuring in class-def split ignores node_index.

Matches the function-call split behavior.

vm/src/stdlib/ast/module.rs (4)

69-74: LGTM: ModModule ignores node_index in object conversion.

Consistent with treating it as an internal field.

---

98-101: LGTM: ModModule::ast_from_object initializes node_index.

Matches the new field requirements.

---

152-156: LGTM: ModExpression destructuring includes node_index and preserves range.

No concerns.

---

173-180: LGTM: ModExpression::ast_from_object sets node_index default.

Aligned with other nodes.

vm/src/stdlib/ast/elif_else_clause.rs (1)

10-15: LGTM: node_index destructuring is consistent.

Ignoring node_index in to_object and defaulting it when constructing new clauses matches the pattern used across the AST conversions.

vm/src/stdlib/ast/constant.rs (2)

250-345: LGTM: node_index initialization across Constant→Expr is thorough.

All literal variants now initialize node_index and destructuring ignores it as intended.

---

366-437: LGTM: literal-to-object helpers handle the new field safely.

Destructuring with node_index: _ avoids churn; behavior unchanged.

vm/src/stdlib/ast/expression.rs (2)

148-186: LGTM: BoolOp conversion updated correctly with node_index.

---

286-301: LGTM: UnaryOp conversion mirrors the established pattern.

Actionable comments posted: 1

vm/src/stdlib/ast/argument.rs (3)

60-64: Consider preserving node_index when re-merging Arguments.

You default the node_index on construction. If these args originated from an existing ruff::Arguments, this loses stable identity. If feasible, thread node_index through split_* and accept it here to preserve it. Otherwise, confirm that defaulting is intentional.

---

71-75: node_index is dropped on split.

You explicitly ignore node_index. If callers later re-merge, the index is lost. Consider returning it (e.g., an extra Option<NodeIndex> or embedding it in a wrapper) so merge_* can restore it.

---

110-114: Same as above: node_index ignored in class-def arg split.

Preserve and propagate node_index to enable lossless round‑trips.

vm/src/vm/vm_new.rs (1)

413-418: Use to_zero_indexed() to index lines.

Good fix for off‑by‑one with 1‑based line. Optionally, prefer .lines() over .split('\n') to handle CRLF uniformly (you still append the trailing newline explicitly).

vm/src/stdlib/ast/string.rs (2)

20-33: Iterator pattern is hard to follow; align with FStringValue version.

Current use of into_iter() twice and shadowing fstring_element is easy to misread. Consider mirroring ruff_fstring_value_into_iter with len() + iter_mut() (or expose as_slice()/iter_mut() on InterpolatedStringElements) to avoid repeated into_iter() calls and shadowing.

---

50-57: Flags are intentionally dropped; re‑confirm.

The flags: _, // TODO in FStringPart::FString branch discards string flags. If this is still TODO, consider threading them through to avoid losing raw-prefix/case info.

compiler/codegen/src/lib.rs (1)

59-60: LGTM; optional naming tweak

Mapping TString to “t-string expression” looks fine. Optionally consider “interpolated string” to keep error text consistent with f-strings.

compiler/src/lib.rs (1)

88-96: Normalize CRLF consistently in compile_symtable() too.

compile() replaces CRLF on Windows but compile_symtable() doesn’t, which can desync locations/symtable offsets. Mirror the normalization to avoid skewed ranges.

 pub fn compile_symtable(
     source: &str,
     mode: Mode,
     source_path: &str,
 ) -> Result<symboltable::SymbolTable, CompileError> {
-    let source_file = SourceFileBuilder::new(source_path, source).finish();
+    #[cfg(windows)]
+    let source = source.replace("\r\n", "\n");
+    #[cfg(windows)]
+    let source = source.as_str();
+    let source_file = SourceFileBuilder::new(source_path, source).finish();
     _compile_symtable(source_file, mode)
 }

Also applies to: 133-140

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

375-427: Remove dead wrappers and helpers or fully implement them

PatternMatchClassPatterns/KeywordAttributes/KeywordPatterns structs and the split/merge helpers are unused after inlining and currently all todo!(). Delete them to avoid dead code, or fully implement them if you keep the helper design.

Also applies to: 534-552

---

47-48: Populate MatchCase.range (avoid losing locations)

range is Default::default() here; prefer range_from_object(vm, source_file, object, "match_case") to preserve locations for error reporting.

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

Reviewing files that changed from the base of the PR and between 3887ae9 and de8c06d.

• Cargo.lock is excluded by !**/*.lock

• Cargo.toml (1 hunks)
• compiler/codegen/src/compile.rs (15 hunks)
• compiler/codegen/src/ir.rs (2 hunks)
• compiler/codegen/src/lib.rs (1 hunks)
• compiler/codegen/src/symboltable.rs (16 hunks)
• compiler/codegen/src/unparse.rs (24 hunks)
• compiler/core/src/bytecode.rs (1 hunks)
• compiler/core/src/lib.rs (1 hunks)
• compiler/core/src/marshal.rs (2 hunks)
• compiler/src/lib.rs (3 hunks)
• src/shell.rs (2 hunks)
• stdlib/src/faulthandler.rs (1 hunks)
• vm/src/builtins/frame.rs (1 hunks)
• vm/src/frame.rs (2 hunks)
• vm/src/stdlib/ast.rs (4 hunks)
• vm/src/stdlib/ast/argument.rs (4 hunks)
• vm/src/stdlib/ast/constant.rs (10 hunks)
• vm/src/stdlib/ast/elif_else_clause.rs (4 hunks)
• vm/src/stdlib/ast/exception.rs (2 hunks)
• vm/src/stdlib/ast/expression.rs (51 hunks)
• vm/src/stdlib/ast/module.rs (4 hunks)
• vm/src/stdlib/ast/other.rs (4 hunks)
• vm/src/stdlib/ast/parameter.rs (8 hunks)
• vm/src/stdlib/ast/pattern.rs (18 hunks)
• vm/src/stdlib/ast/statement.rs (46 hunks)
• vm/src/stdlib/ast/string.rs (10 hunks)
• vm/src/stdlib/ast/type_parameters.rs (7 hunks)
• vm/src/vm/vm_new.rs (2 hunks)
• wasm/lib/src/convert.rs (1 hunks)

• vm/src/builtins/frame.rs
• compiler/core/src/bytecode.rs
• vm/src/stdlib/ast/elif_else_clause.rs
• vm/src/stdlib/ast/parameter.rs
• Cargo.toml
• compiler/core/src/lib.rs
• vm/src/stdlib/ast.rs
• compiler/codegen/src/symboltable.rs
• compiler/codegen/src/ir.rs
• vm/src/stdlib/ast/exception.rs
• vm/src/stdlib/ast/module.rs

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

**/*.rs: Format Rust code with the default rustfmt style (run cargo fmt)
Run clippy and fix any warnings or lints introduced by your changes
Follow Rust best practices for error handling and memory management

Files:

• wasm/lib/src/convert.rs
• vm/src/stdlib/ast/constant.rs
• compiler/codegen/src/lib.rs
• vm/src/stdlib/ast/other.rs
• src/shell.rs
• vm/src/stdlib/ast/type_parameters.rs
• vm/src/stdlib/ast/argument.rs
• vm/src/stdlib/ast/statement.rs
• vm/src/stdlib/ast/expression.rs
• vm/src/frame.rs
• stdlib/src/faulthandler.rs
• compiler/codegen/src/unparse.rs
• vm/src/stdlib/ast/pattern.rs
• vm/src/vm/vm_new.rs
• vm/src/stdlib/ast/string.rs
• compiler/codegen/src/compile.rs
• compiler/core/src/marshal.rs
• compiler/src/lib.rs

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Use RustPython macros (pyclass, pymodule, pyfunction, etc.) when implementing Python functionality in Rust

Files:

• vm/src/stdlib/ast/constant.rs
• vm/src/stdlib/ast/other.rs
• vm/src/stdlib/ast/type_parameters.rs
• vm/src/stdlib/ast/argument.rs
• vm/src/stdlib/ast/statement.rs
• vm/src/stdlib/ast/expression.rs
• vm/src/frame.rs
• stdlib/src/faulthandler.rs
• vm/src/stdlib/ast/pattern.rs
• vm/src/vm/vm_new.rs
• vm/src/stdlib/ast/string.rs

Learnt from: CR
PR: RustPython/RustPython#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-11T05:06:59.971Z
Learning: Applies to {vm,stdlib}/**/*.rs : Use RustPython macros (`pyclass`, `pymodule`, `pyfunction`, etc.) when implementing Python functionality in Rust

Applied to files:

• src/shell.rs

vm/src/stdlib/ast/expression.rs (14)

• ast_to_object (11-53)
• ast_to_object (148-165)
• ast_to_object (191-208)
• ast_to_object (234-254)
• ast_to_object (285-302)
• ast_to_object (327-344)
• ast_to_object (370-390)
• ast_to_object (421-445)
• ast_to_object (477-491)
• ast_to_object (511-528)
• ast_to_object (554-571)
• ast_to_object (597-617)
• ast_to_object (648-666)
• ast_to_object (694-708)

vm/src/stdlib/ast/exception.rs (2)

• ast_to_object (6-10)
• ast_to_object (36-59)

vm/src/stdlib/ast/expression.rs (14)

• ast_to_object (11-53)
• ast_to_object (148-165)
• ast_to_object (191-208)
• ast_to_object (234-254)
• ast_to_object (285-302)
• ast_to_object (327-344)
• ast_to_object (370-390)
• ast_to_object (421-445)
• ast_to_object (477-491)
• ast_to_object (511-528)
• ast_to_object (554-571)
• ast_to_object (597-617)
• ast_to_object (648-666)
• ast_to_object (694-708)

vm/src/stdlib/ast.rs (1)

• range_from_object (163-186)

vm/src/frame.rs (1)

• current_location (173-175)

vm/src/stdlib/ast/expression.rs (12)

• ast_from_object (55-143)
• ast_from_object (167-186)
• ast_from_object (210-229)
• ast_from_object (256-280)
• ast_from_object (303-322)
• ast_from_object (346-365)
• ast_from_object (392-416)
• ast_from_object (447-472)
• ast_from_object (492-506)
• ast_from_object (530-549)
• ast_from_object (573-592)
• ast_from_object (619-643)

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

• get_node_field (56-59)
• range_from_object (163-186)

vm/src/stdlib/ast/constant.rs (1)

• new_str (13-23)

• GitHub Check: Run snippets and cpython tests on wasm-wasi
• GitHub Check: Run rust tests (windows-2025)
• GitHub Check: Run rust tests (macos-latest)
• GitHub Check: Run rust tests (ubuntu-latest)
• GitHub Check: Check the WASM package and demo
• GitHub Check: Check Rust code with rustfmt and clippy
• GitHub Check: Run snippets and cpython tests (windows-2025)
• 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

vm/src/frame.rs (2)

391-399: LGTM! Consistent migration to line field.

The traceback construction correctly uses loc.line instead of loc.row and the logging statement is updated consistently. This aligns with the SourceLocation API changes in ruff 0.13.1.

Based on learnings

---

173-175: Fix underflow in current_location when lasti()==0.

This is the same critical underflow issue identified in previous reviews that remains unfixed. Indexing with self.lasti() as usize - 1 will underflow for a brand‑new frame (lasti==0), causing OOB panic if current_location() is queried early (e.g., tracing/diagnostics).

As per coding guidelines

stdlib/src/faulthandler.rs (1)

13-13: LGTM! Consistent API migration to line field.

The change from frame.current_location().row to frame.current_location().line correctly follows the SourceLocation API update in ruff 0.13.1. However, note that this depends on the critical underflow fix needed in Frame::current_location() method.

Based on learnings

vm/src/vm/vm_new.rs (1)

324-326: Correct enum rename to InterpolatedStringErrorType.

This aligns with the parser’s new error type. LGTM.

vm/src/stdlib/ast/other.rs (2)

59-63: Decorator mapping: defaulting node_index.

Defaulting and ignoring node_index in the Python object path is reasonable since Python’s AST has no Decorator node; only expressions are exposed. LGTM.

---

70-84: Alias: include node_index but ignore on export.

Pattern matches and node_add_location use are consistent with other nodes. LGTM.

vm/src/stdlib/ast/string.rs (3)

358-363: Destructure ExprFString: ignoring node_index is fine; keep range.

This path now carries range to JoinedStr correctly; with the fix above, end‑to‑end locations are preserved. LGTM after range fix.

---

166-169: ExprFString drops its source range.

range: Default::default() loses location info. Use the actual range captured from JoinedStr.

-        ruff::Expr::FString(ruff::ExprFString {
-            node_index: Default::default(),
-            range: Default::default(),
+        ruff::Expr::FString(ruff::ExprFString {
+            node_index: Default::default(),
+            range,

---

126-133: Use-after-move: element moved by match, then used.

match element { ... } moves element. Referencing it in vec![element] uses a moved value. Bind the inner value and reconstruct.

-        ruff::InterpolatedStringElement::Interpolation(ruff::InterpolatedElement {
-            range, ..
-        }) => ruff::FStringPart::FString(ruff::FString {
-            node_index: Default::default(),
-            range,
-            elements: vec![element].into(),
-            flags: ruff::FStringFlags::empty(),
-        }),
+        ruff::InterpolatedStringElement::Interpolation(el) => {
+            let range = el.range;
+            ruff::FStringPart::FString(ruff::FString {
+                node_index: Default::default(),
+                range,
+                elements: vec![ruff::InterpolatedStringElement::Interpolation(el)].into(),
+                flags: ruff::FStringFlags::empty(),
+            })
+        }

compiler/core/src/marshal.rs (2)

197-206: Location rename read path looks correct; confirm on-disk compatibility

Deserialization now reads (line: 1-based) and (character_offset: stored 0-based → OneIndexed). This matches the serialize path below. Please confirm the marshal format version (FORMAT_VERSION = 4) didn’t change semantics from previous row/column storage; otherwise a version bump is warranted.

---

659-661: Write path matches read path (1-based line, 0-based char offset)

The round-trip for SourceLocation is consistent: write line.get() and character_offset.to_zero_indexed(). LGTM.

Consider adding a quick round‑trip test for mixed ASCII/UTF‑8 to validate character_offset behavior.

src/shell.rs (1)

3-6: LGTM: updated to InterpolatedStringErrorType

Matches the parser API change; keeps the unterminated triple‑quoted detection working.

Please run the REPL quick check: enter an unterminated t/f‑triple‑quoted string and confirm it prompts for continuation.

Also applies to: 53-59

compiler/codegen/src/unparse.rs (1)

368-369: Preserve TString contents when unparsing (don’t emit empty t"")

Current branch drops literal text and embedded expressions.

Minimal safe fix: print the original source slice for this node.

-            Expr::TString(_) => self.p("t\"\"")?,
+            Expr::TString(ruff::ExprTString { range, .. }) => {
+                // Emit exactly as written to preserve contents/interpolations.
+                self.p(self.source.slice(*range))?
+            }

Preferred fix: mirror FString handling by rendering the TString value via the same interpolated‑string machinery (prefix with "t" and feed elements to unparse_fstring_body), so escapes/normalization match f-strings. If you provide the exact ExprTString shape, I can draft that version too.

wasm/lib/src/convert.rs (1)

249-260: Guard optional error location to avoid WASM panic
CompileError::location() returns an Option<SourceLocation>, so calling unwrap() can abort the WASM module when there’s no location (e.g., non-parse errors). Wrap the Reflect::set calls in if let Some(loc) = err.location() or supply a default (e.g. 0) for row/col when None.

compiler/src/lib.rs (1)

46-56: Confirm encoding/offset semantics match Python expectations.

We now compute locations via PositionEncoding::Utf8 and expose python_location as (line.get(), character_offset.get()). Please confirm this aligns with how vm/src/stdlib/ast.rs expects lineno/col_offset (1‑based line, 0‑based UTF‑8 column), to avoid off‑by‑one or encoding mismatches when round‑tripping.

Based on learnings

Also applies to: 65-71

vm/src/stdlib/ast/statement.rs (1)

171-186: node_index plumbed correctly through FunctionDef.

Destructuring ignores node_index in ast_to_object and initializes it via Default in ast_from_object; consistent with the rest of the module.

Also applies to: 221-265

vm/src/stdlib/ast/type_parameters.rs (1)

18-22: node_index initialization looks correct

Defaulting node_index here aligns with the broader migration; range is derived from first/last and defaults on empty, which is fine. Please confirm no downstream code assumes a non-empty range for TypeParams.

compiler/codegen/src/compile.rs (3)

239-256: Good: switched source_location to PositionEncoding::Utf8

The move to Utf8 for start/end and instruction locations is consistent and LGTM.

Also applies to: 530-540, 5340-5348

---

632-637: Confirm character_offset semantics

You set SourceLocation { line: OneIndexed, character_offset: OneIndexed::MIN }. Verify character_offset is intended to be 1-based here; if it’s a zero-based character index, this should be 0 (or an appropriate type) rather than OneIndexed::MIN.

Also applies to: 771-775

---

5550-5571: Nice helper: await detection in interpolated strings + element compiler

The recursive contains_await for Interpolated* and the updated compile_fstring_elements logic look solid.

Also applies to: 5616-5681

vm/src/stdlib/ast/expression.rs (1)

39-41: Handle TString instead of panicking

Self::TString(_) => unimplemented!() will panic. Implement the conversion (e.g., mirror FString’s mapping to PyAST) or temporarily map to the closest supported representation until full support lands.

#!/bin/bash
# Locate any existing TString helpers to reuse
rg -nP -C2 '\btstring_to_object\b|TString' vm/src/stdlib/ast