Skip to content

Conversation

@youknowone
Copy link
Member

@youknowone youknowone commented Jan 16, 2026

incomplete but enough for #6718

Summary by CodeRabbit

  • New Features

    • Template (t-) strings with embedded interpolations are now implemented end-to-end.
    • New Interpolation and Template types exposed: support conversion flags, format specs, concatenation, iteration, pickling, and rich repr/compare.
    • AST and stdlib integration now represent template expressions and interpolations.
  • Bug Fixes

    • Previously unimplemented template-string paths now produce runtime Template objects and work correctly.

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

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 16, 2026

📝 Walkthrough

Walkthrough

Adds full support for template strings (t-strings / PEP 750) end-to-end: AST representation, codegen emitting new bytecode ops, new bytecode instructions, VM execution handlers, and runtime types PyInterpolation and PyTemplate.

Changes

Cohort / File(s) Summary
Code generation
crates/codegen/src/compile.rs
Implemented compile_expr_tstring and compile_tstring_into, replacing NotImplemented paths to emit BuildInterpolation and BuildTemplate.
Symbol scanning
crates/codegen/src/symboltable.rs
Scan t-string interpolation expressions for symbols (mirrors f-string logic) instead of erroring.
Bytecode / Instruction set
crates/compiler-core/src/bytecode/instruction.rs
Added BuildTemplate (opcode 125) and BuildInterpolation { oparg } (opcode 126); updated decoding, metadata (stack effects), and disassembly formatting.
VM frame / execution
crates/vm/src/frame.rs
Implemented handlers for BuildInterpolation (construct PyInterpolation from stack, decode oparg) and BuildTemplate (build PyTemplate from tuples).
Runtime types — Interpolation
crates/vm/src/builtins/interpolation.rs
New PyInterpolation type exposed as string.templatelib.Interpolation with constructor, validation, equality, hashing, repr, pickling, getters, and registration.
Runtime types — Template
crates/vm/src/builtins/template.rs
New PyTemplate and PyTemplateIter types with construction/validation, concatenation, iteration, equality, repr, pickling, and getters; registered in builtins.
Builtins exports
crates/vm/src/builtins/mod.rs
Added modules and re-exports for interpolation and template (PyInterpolation, PyTemplate, PyTemplateIter).
AST integration
crates/vm/src/stdlib/ast/string.rs, crates/vm/src/stdlib/ast/expression.rs, crates/vm/src/stdlib/ast/pyast.rs
Added tstring_to_object, TemplateStr/TemplateStrPart/TStringInterpolation nodes and conversions; registered new AST node classes TemplateStr and Interpolation; replaced previous TString unimplemented flows.
Type zoo
crates/vm/src/types/zoo.rs
Added interpolation_type, template_type, and template_iter_type fields and init wiring.
VM ops (minor)
crates/vm/src/vm/vm_ops.rs
Refactored concat/add dispatch to prefer direct slot-based concat/inplace-slot paths over try_sequence.

Sequence Diagram(s)

sequenceDiagram
    participant Parser
    participant Codegen as Code Generator
    participant Bytecode
    participant VM as Virtual Machine
    participant Runtime as Builtins

    Parser->>Codegen: Emit TString AST node
    Codegen->>Codegen: compile_expr_tstring()
    Codegen->>Codegen: collect literal parts and interpolations
    Codegen->>Bytecode: emit BuildInterpolation (per interpolation)
    Codegen->>Bytecode: emit BuildTemplate

    VM->>VM: execute BuildInterpolation
    VM->>Runtime: construct PyInterpolation(value, expr, conversion, format_spec)
    Runtime-->>VM: PyInterpolation object

    VM->>VM: execute BuildTemplate
    VM->>Runtime: construct PyTemplate(strings_tuple, interpolations_tuple)
    Runtime-->>VM: PyTemplate object
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~50 minutes

Possibly related PRs

Suggested reviewers

  • ShaharNaveh

Poem

🐇
I stitched the strings with hops and cheer,
Interpolations snug and near,
BuildInterpolation, BuildTemplate too,
PyTemplate blooms — a rabbit’s view,
T-strings prance, all bright and clear.

🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 38.60% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'PEP 750 tstring' clearly summarizes the main change: implementing PEP 750 template string support, as evidenced by widespread tstring/template-related additions across compilation, VM, AST, and builtins modules.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ 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

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions
Copy link
Contributor

Code has been automatically formatted

The code in this PR has been formatted using:

  • python scripts/generate_opcode_metadata.py
    Please pull the latest changes before pushing again:
git pull origin tstring

@youknowone youknowone marked this pull request as ready for review January 16, 2026 15:57
@fanninpm
Copy link
Contributor

@coderabbitai review

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 16, 2026

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

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🤖 Fix all issues with AI agents
In `@crates/vm/src/builtins/template.rs`:
- Around line 297-304: The __reduce__ for PyTemplateIter (method reduce)
currently returns (TemplateIterType, (template,)) which fails because
PyTemplateIter lacks a Constructor; update reduce to return the same
reconstruction pattern used by PyTemplate by returning the helper function
_template_unpickle as the callable and the template tuple as its args (i.e.,
return (_template_unpickle, (zelf.template.clone(),))) so unpickling calls the
helper instead of trying to call TemplateIterType directly.

In `@crates/vm/src/frame.rs`:
- Around line 706-755: PyTemplate::new and PyInterpolation::new currently skip
the invariant checks performed in their py_new implementations; update these
simple constructors to validate the same invariants: in PyTemplate::new, assert
strings.len() == interpolations.len() + 1 (use a debug_assert! or panic with a
clear message if you prefer non-debug enforcement) and in PyInterpolation::new
validate that conversion is either vm.ctx.none() or a single-character string in
{'s','r','a'} (mirror the checks and error text from interpolation.rs:py_new and
template.rs:py_new), failing fast with a clear message when violated so
directly-constructed objects cannot be malformed.
🧹 Nitpick comments (2)
crates/vm/src/stdlib/ast/string.rs (2)

329-356: Avoid quadratic iteration when consuming f-string parts.

iter_mut().nth(i) inside the loop re-traverses the slice each iteration. Iterating directly keeps this O(n) and reads cleaner.

♻️ Proposed refactor
-    for i in 0..value.as_slice().len() {
-        let part = core::mem::replace(value.iter_mut().nth(i).unwrap(), default_part.clone());
+    for part in value.iter_mut() {
+        let part = core::mem::replace(part, default_part.clone());

575-589: Avoid quadratic iteration in tstring_to_object.

Same pattern as above—direct iteration over value.iter_mut() avoids O(n²) traversal.

♻️ Proposed refactor
-    for i in 0..value.as_slice().len() {
-        let tstring = core::mem::replace(value.iter_mut().nth(i).unwrap(), default_tstring.clone());
+    for tstring in value.iter_mut() {
+        let tstring = core::mem::replace(tstring, default_tstring.clone());
📜 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 c934265 and b1114b4.

⛔ Files ignored due to path filters (1)
  • Lib/_opcode_metadata.py is excluded by !Lib/**
📒 Files selected for processing (12)
  • crates/codegen/src/compile.rs
  • crates/codegen/src/symboltable.rs
  • crates/compiler-core/src/bytecode/instruction.rs
  • crates/vm/src/builtins/interpolation.rs
  • crates/vm/src/builtins/mod.rs
  • crates/vm/src/builtins/template.rs
  • crates/vm/src/frame.rs
  • crates/vm/src/stdlib/ast/expression.rs
  • crates/vm/src/stdlib/ast/pyast.rs
  • crates/vm/src/stdlib/ast/string.rs
  • crates/vm/src/types/zoo.rs
  • crates/vm/src/vm/vm_ops.rs
🧰 Additional context used
📓 Path-based instructions (1)
**/*.rs

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

**/*.rs: Follow the default rustfmt code style using 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/types/zoo.rs
  • crates/vm/src/stdlib/ast/expression.rs
  • crates/codegen/src/symboltable.rs
  • crates/vm/src/builtins/mod.rs
  • crates/vm/src/vm/vm_ops.rs
  • crates/vm/src/stdlib/ast/pyast.rs
  • crates/vm/src/builtins/interpolation.rs
  • crates/compiler-core/src/bytecode/instruction.rs
  • crates/vm/src/builtins/template.rs
  • crates/vm/src/frame.rs
  • crates/codegen/src/compile.rs
  • crates/vm/src/stdlib/ast/string.rs
🧠 Learnings (1)
📚 Learning: 2026-01-14T14:52:10.778Z
Learnt from: CR
Repo: RustPython/RustPython PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-01-14T14:52:10.778Z
Learning: Applies to **/*.rs : Use the macro system (`pyclass`, `pymodule`, `pyfunction`, etc.) when implementing Python functionality in Rust

Applied to files:

  • crates/vm/src/builtins/mod.rs
  • crates/vm/src/frame.rs
🧬 Code graph analysis (10)
crates/vm/src/types/zoo.rs (1)
crates/vm/src/builtins/template.rs (1)
  • iter (234-236)
crates/vm/src/stdlib/ast/expression.rs (1)
crates/vm/src/stdlib/ast/string.rs (1)
  • tstring_to_object (565-596)
crates/codegen/src/symboltable.rs (2)
crates/vm/src/builtins/interpolation.rs (1)
  • format_spec (128-130)
crates/vm/src/frame.rs (1)
  • element (650-650)
crates/vm/src/builtins/mod.rs (1)
crates/vm/src/stdlib/sre.rs (1)
  • template (140-165)
crates/vm/src/vm/vm_ops.rs (2)
crates/vm/src/protocol/mapping.rs (1)
  • slots (108-110)
crates/vm/src/protocol/sequence.rs (1)
  • slots (134-136)
crates/vm/src/builtins/interpolation.rs (1)
crates/vm/src/types/zoo.rs (1)
  • init (109-203)
crates/compiler-core/src/bytecode/instruction.rs (1)
crates/compiler-core/src/bytecode/oparg.rs (2)
  • from (25-27)
  • from (68-70)
crates/vm/src/builtins/template.rs (1)
crates/vm/src/types/slot.rs (1)
  • sequence_downcast (1982-1984)
crates/vm/src/frame.rs (2)
crates/vm/src/builtins/template.rs (2)
  • interpolations (103-105)
  • strings (98-100)
crates/vm/src/builtins/interpolation.rs (4)
  • format_spec (128-130)
  • expression (118-120)
  • value (113-115)
  • conversion (123-125)
crates/vm/src/stdlib/ast/string.rs (1)
crates/vm/src/stdlib/ast/expression.rs (16)
  • 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)
  • ast_to_object (728-742)
  • ast_to_object (761-775)
🔇 Additional comments (16)
crates/vm/src/stdlib/ast/expression.rs (1)

39-39: TString conversion is now wired in cleanly.

This removes the unimplemented path and routes t-strings through the dedicated converter as expected.

crates/codegen/src/symboltable.rs (1)

1444-1456: T-string interpolation scanning mirrors f-strings correctly.

Nice alignment with the f-string path, including format-spec traversal.

crates/compiler-core/src/bytecode/instruction.rs (2)

267-279: Opcode definitions and decode registration are consistent.

Good to see BuildTemplate/BuildInterpolation added to both the enum and custom opcode list.

Also applies to: 441-445


802-809: Stack-effect and disassembly updates look consistent.

The stack deltas and disassembly names align with the new template/interpolation ops.

Also applies to: 1004-1005

crates/vm/src/builtins/interpolation.rs (1)

49-203: Interpolation constructor and protocol impls look solid.

Validation, reduce, equality, hashing, and repr are consistent and well-scoped.

crates/vm/src/builtins/template.rs (1)

17-197: Template normalization, concat, and iteration logic look consistent.

Constructor normalization and concat boundary handling read clean, and the iterator skips empty strings as intended.

Also applies to: 199-267, 269-293, 308-341, 343-346

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

366-563: TemplateStr/Interpolation AST wiring looks consistent.

Nice alignment with the existing JoinedStr/FormattedValue flow.

crates/vm/src/builtins/mod.rs (1)

39-41: New interpolation/template exports are wired cleanly.

Also applies to: 81-82

crates/vm/src/types/zoo.rs (1)

6-9: TypeZoo registration for template/interpolation looks complete.

Also applies to: 96-98, 194-196, 252-253

crates/vm/src/vm/vm_ops.rs (1)

383-387: sequence_unchecked() is safe for non-sequence operands—it's a thin wrapper without assertions.

The method simply wraps the object in a PySequence struct without validation. Safety is guaranteed by subsequent .slots().concat.load() checks, which return Option<T> and are guarded by if let Some(f) conditionals. Non-sequences safely return None from .load(), preventing execution.

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

935-936: LGTM! Module registration is correctly placed and follows established patterns.

The new nodes are appropriately registered after JoinedStr and FormattedValue, grouping related string expression types together.


451-463: LGTM! New AST nodes correctly implement PEP 750 template string support.

The TemplateStr and Interpolation node definitions follow the established impl_node! macro pattern and correctly mirror their f-string counterparts (JoinedStr and FormattedValue). Field definitions align with PEP 750:

  • TemplateStr.values: sequence of constants and interpolations
  • Interpolation: includes value, str (original expression source text), conversion, and format_spec fields, matching the PEP 750 specification exactly
crates/codegen/src/compile.rs (4)

27-33: Import updates look good.
The added ExprTString/TString imports align with the new t-string codegen path.


6285-6287: TString expression dispatch is wired correctly.
Routing Expr::TString into the new compiler path is the right integration point.


7495-7514: Stack contract and oparg encoding are correctly implemented.

The BuildTemplate/BuildInterpolation stack order and bit layout match across compiler and VM:

  • BuildTemplate receives [strings_tuple, interpolations_tuple] after the swap (line 7511)
  • BuildInterpolation oparg encoding (conversion << 2) | has_format_spec matches VM expectations
  • Conversion bits 2+ encode None (0), Str (1), Repr (2), Ascii (3); has_format_spec is bit 0

7526-7531: The suggested code references a non-existent field and will not compile.

InterpolatedStringLiteralElement (the type of lit) has only three fields: node_index, range, and value. It does not have a flags field. The flags information is stored on the parent TString struct, not on individual literal elements.

For this change to work correctly, compile_tstring_into would need to be modified to:

  1. Accept the tstring.flags parameter
  2. Use tstring.flags (not lit.flags) when calling parse_fstring_literal_element
  3. Update the function signature accordingly

The core concern about parity with f-string surrogate handling is reasonable, but the proposed implementation is incorrect.

Likely an incorrect or invalid review comment.

✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.

@youknowone youknowone merged commit f4363a6 into RustPython:main Jan 17, 2026
23 of 24 checks passed
@youknowone youknowone deleted the tstring branch January 17, 2026 09:41
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.

2 participants