Visitar URL original
Inline with-suppress return blocks and extend return duplication by youknowone · Pull Request #7622 · RustPython/RustPython · GitHub
Skip to content

Inline with-suppress return blocks and extend return duplication #7622

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
youknowone merged 4 commits into from
Apr 18, 2026
Merged

Inline with-suppress return blocks and extend return duplication #7622

Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
f0bf810
Inline with-suppress return blocks and extend return duplication
youknowone Apr 18, 2026
c79baa3
Fix for-loop target NOP and t-string stack order
youknowone Apr 18, 2026
caf8d55
Bytecode parity - folding, class prologue, except cleanup
youknowone Apr 18, 2026
a5b9f0e
refactro
youknowone Apr 18, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Fix for-loop target NOP and t-string stack order 
- Remove unnecessary NOP between FOR_ITER and unpack/store
  by compiling loop target directly on target range
- Fix t-string compilation to match stack order: build
  strings tuple first, then evaluate interpolations
- Split compile_tstring_into into collect_tstring_strings
  and compile_tstring_interpolations
- Handle debug text literals and default repr conversion
  for debug specifier in t-strings
- Always set bit 1 in BUILD_INTERPOLATION oparg encoding
  • Loading branch information
@youknowone
youknowone committed Apr 18, 2026
commit c79baa3317962675d793dba2ead6febd14b0b72a
277 changes: 202 additions & 75 deletions crates/codegen/src/compile.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5931,15 +5931,13 @@ impl Compiler {

emit!(self, Instruction::ForIter { delta: else_block });

// Match CPython codegen_for(): keep a line anchor on the target line
// so multiline/single-line `for ...: pass` bodies preserve tracing layout.
// Match CPython's line attribution by compiling the loop target on
// the target range directly instead of leaving a synthetic anchor
// NOP between FOR_ITER and the unpack/store sequence.
let saved_range = self.current_source_range;
self.set_source_range(target.range());
emit!(self, Instruction::Nop);
self.set_source_range(saved_range);

// Start of loop iteration, set targets:
self.compile_store(target)?;
self.set_source_range(saved_range);
};

let was_in_loop = self.ctx.loop_data.replace((for_block, after_block));
Expand Down Expand Up @@ -10734,128 +10732,128 @@ impl Compiler {
}

fn compile_expr_tstring(&mut self, expr_tstring: &ast::ExprTString) -> CompileResult<()> {
// ast::TStringValue can contain multiple ast::TString parts (implicit concatenation)
// Each ast::TString part should be compiled and the results merged into a single Template
// ast::TStringValue can contain multiple ast::TString parts (implicit
// concatenation). Match CPython's stack order by materializing the
// strings tuple first, then evaluating interpolations left-to-right.
let tstring_value = &expr_tstring.value;

// Collect all strings and compile all interpolations
let mut all_strings: Vec<Wtf8Buf> = Vec::new();
let mut current_string = Wtf8Buf::new();
let mut interp_count: u32 = 0;

for tstring in tstring_value.iter() {
self.compile_tstring_into(
self.collect_tstring_strings(
tstring,
&mut all_strings,
&mut current_string,
&mut interp_count,
)?;
);
}

// Add trailing string
all_strings.push(core::mem::take(&mut current_string));

// Now build the Template:
// Stack currently has all interpolations from compile_tstring_into calls

// 1. Build interpolations tuple from the interpolations on the stack
emit!(
self,
Instruction::BuildTuple {
count: interp_count
}
);

// 2. Load all string parts
let string_count: u32 = all_strings
.len()
.try_into()
.expect("t-string string count overflowed");
for s in &all_strings {
self.emit_load_const(ConstantData::Str { value: s.clone() });
}

// 3. Build strings tuple
emit!(
self,
Instruction::BuildTuple {
count: string_count
}
);

// 4. Swap so strings is below interpolations: [interps, strings] -> [strings, interps]
emit!(self, Instruction::Swap { i: 2 });
for tstring in tstring_value.iter() {
self.compile_tstring_interpolations(tstring)?;
}

// 5. Build the Template
emit!(
self,
Instruction::BuildTuple {
count: interp_count
}
);
emit!(self, Instruction::BuildTemplate);

Ok(())
}

fn compile_tstring_into(
&mut self,
fn collect_tstring_strings(
&self,
tstring: &ast::TString,
strings: &mut Vec<Wtf8Buf>,
current_string: &mut Wtf8Buf,
interp_count: &mut u32,
) -> CompileResult<()> {
) {
for element in &tstring.elements {
match element {
ast::InterpolatedStringElement::Literal(lit) => {
// Accumulate literal parts into current_string
current_string.push_str(&lit.value);
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
ast::InterpolatedStringElement::Interpolation(interp) => {
// Finish current string segment
if let Some(ast::DebugText { leading, trailing }) = &interp.debug_text {
let range = interp.expression.range();
let source = self.source_file.slice(range);
let text = [
strip_fstring_debug_comments(leading).as_str(),
source,
strip_fstring_debug_comments(trailing).as_str(),
]
.concat();
current_string.push_str(&text);
}
strings.push(core::mem::take(current_string));
*interp_count += 1;
}
}
}
}

// Compile the interpolation value
self.compile_expression(&interp.expression)?;
fn compile_tstring_interpolations(&mut self, tstring: &ast::TString) -> CompileResult<()> {
for element in &tstring.elements {
let ast::InterpolatedStringElement::Interpolation(interp) = element else {
continue;
};

// Load the expression source string, including any
// whitespace between '{' and the expression start
let expr_range = interp.expression.range();
let expr_source = if interp.range.start() < expr_range.start()
&& interp.range.end() >= expr_range.end()
{
let after_brace = interp.range.start() + TextSize::new(1);
self.source_file
.slice(TextRange::new(after_brace, expr_range.end()))
} else {
// Fallback for programmatically constructed ASTs with dummy ranges
self.source_file.slice(expr_range)
};
self.emit_load_const(ConstantData::Str {
value: expr_source.to_string().into(),
});
self.compile_expression(&interp.expression)?;

// Determine conversion code
let conversion: u32 = match interp.conversion {
ast::ConversionFlag::None => 0,
ast::ConversionFlag::Str => 1,
ast::ConversionFlag::Repr => 2,
ast::ConversionFlag::Ascii => 3,
};
let expr_range = interp.expression.range();
let expr_source = if interp.range.start() < expr_range.start()
&& interp.range.end() >= expr_range.end()
{
let after_brace = interp.range.start() + TextSize::new(1);
self.source_file
.slice(TextRange::new(after_brace, expr_range.end()))
} else {
self.source_file.slice(expr_range)
};
self.emit_load_const(ConstantData::Str {
value: expr_source.to_string().into(),
});

// Handle format_spec
let has_format_spec = interp.format_spec.is_some();
if let Some(format_spec) = &interp.format_spec {
// Compile format_spec as a string using fstring element compilation
// Use default ast::FStringFlags since format_spec syntax is independent of t-string flags
self.compile_fstring_elements(
ast::FStringFlags::empty(),
&format_spec.elements,
)?;
}
let mut conversion: u32 = match interp.conversion {
ast::ConversionFlag::None => 0,
ast::ConversionFlag::Str => 1,
ast::ConversionFlag::Repr => 2,
ast::ConversionFlag::Ascii => 3,
};

// Emit BUILD_INTERPOLATION
// oparg encoding: (conversion << 2) | has_format_spec
let format = (conversion << 2) | u32::from(has_format_spec);
emit!(self, Instruction::BuildInterpolation { format });
if interp.debug_text.is_some() && conversion == 0 && interp.format_spec.is_none() {
conversion = 2;
}

*interp_count += 1;
}
let has_format_spec = interp.format_spec.is_some();
if let Some(format_spec) = &interp.format_spec {
self.compile_fstring_elements(ast::FStringFlags::empty(), &format_spec.elements)?;
}

// CPython keeps bit 1 set in BUILD_INTERPOLATION's oparg and uses
// bit 0 for the optional format spec.
let format = 2 | (conversion << 2) | u32::from(has_format_spec);
emit!(self, Instruction::BuildInterpolation { format });
}

Ok(())
Expand Down Expand Up @@ -12298,6 +12296,135 @@ elif maxsize == 9223372036854775807:
);
}

#[test]
fn test_for_tuple_target_does_not_leave_loop_header_nop() {
let code = compile_exec(
"\
def f(pairs):
for left, right in pairs:
pass
",
);
let f = find_code(&code, "f").expect("missing function code");
let ops: Vec<_> = f
.instructions
.iter()
.map(|unit| unit.op)
.filter(|op| !matches!(op, Instruction::Cache))
.collect();

assert!(
ops.windows(2).any(|window| {
matches!(
window,
[
Instruction::ForIter { .. },
Instruction::UnpackSequence { .. }
]
)
}),
"expected FOR_ITER to flow directly into UNPACK_SEQUENCE, got ops={ops:?}"
);
assert!(
!ops.windows(3).any(|window| {
matches!(
window,
[
Instruction::ForIter { .. },
Instruction::Nop,
Instruction::UnpackSequence { .. },
]
)
}),
"unexpected loop-header NOP before tuple unpack, got ops={ops:?}"
);
}

#[test]
fn test_tstring_build_template_matches_cpython_stack_order() {
let code = compile_exec("t = t\"{0}\"");
let units: Vec<_> = code
.instructions
.iter()
.copied()
.filter(|unit| !matches!(unit.op, Instruction::Cache))
.collect();

assert!(
units.windows(6).any(|window| {
matches!(
window,
[
a,
b,
c,
d,
e,
f,
]
if matches!(a.op, Instruction::LoadConst { .. })
&& matches!(b.op, Instruction::LoadSmallInt { .. })
&& matches!(c.op, Instruction::LoadConst { .. })
&& matches!(d.op, Instruction::BuildInterpolation { .. })
&& u8::from(d.arg) == 2
&& matches!(e.op, Instruction::BuildTuple { .. })
&& u8::from(e.arg) == 1
&& matches!(f.op, Instruction::BuildTemplate)
)
}),
"expected CPython-style t-string lowering, got units={units:?}"
);
assert!(
!units
.iter()
.any(|unit| matches!(unit.op, Instruction::Swap { .. })),
"unexpected SWAP in t-string lowering, got units={units:?}"
);
}

#[test]
fn test_tstring_debug_specifier_uses_debug_literal_and_repr_default() {
let code = compile_exec(
"\
value = 42
t = t\"Value: {value=}\"
",
);

let string_consts = code
.instructions
.iter()
.filter_map(|unit| match unit.op {
Instruction::LoadConst { consti } => {
Some(&code.constants[consti.get(OpArg::new(u32::from(u8::from(unit.arg))))])
}
_ => None,
})
.collect::<Vec<_>>();

assert!(
string_consts.iter().any(|constant| matches!(
constant,
ConstantData::Tuple { elements }
if matches!(
&elements[..],
[
ConstantData::Str { value: first },
ConstantData::Str { value: second },
] if first.to_string() == "Value: value=" && second.is_empty()
)
)),
"expected debug literal prefix in t-string constants, got {string_consts:?}"
);
assert!(
code.instructions.iter().any(|unit| matches!(
unit.op,
Instruction::BuildInterpolation { .. }
) && u8::from(unit.arg) == 10),
"expected default repr conversion for debug t-string"
);
}

#[test]
fn test_break_in_finally_after_return_keeps_load_fast_check_for_loop_locals() {
let code = compile_exec(
Expand Down