Skip to content

Commit af963bc

Browse files
youknowoneyouknowone
committed
Bytecode parity phase 3
Compiler changes: - Emit TO_BOOL in and/or short-circuit evaluation (COPY+TO_BOOL+JUMP) - Add module-level __conditional_annotations__ cell (PEP 649) - Only set conditional annotations for AnnAssign, not function params - Skip __classdict__ cell when future annotations are active - Convert list literals to tuples in for-loop iterables - Fix cell variable ordering: parameters first, then alphabetical - Fix RESUME DEPTH1 flag for yield-from/await - Don't propagate __classdict__/__conditional_annotations__ freevar through regular functions — only annotation/type-param scopes - Inline string compilation path
1 parent 9282a87 commit af963bc

File tree

5 files changed

+123
-39
lines changed

5 files changed

+123
-39
lines changed

‎Lib/test/test_peepholer.py‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -645,7 +645,6 @@ def containtest():
645645
self.assertEqual(count_instr_recursively(containtest, 'BUILD_LIST'), 0)
646646
self.check_lnotab(containtest)
647647

648-
@unittest.expectedFailure # TODO: RUSTPYTHON; no BUILD_LIST to BUILD_TUPLE optimization
649648
def test_iterate_literal_list(self):
650649
def forloop():
651650
for x in [a, b]:

‎crates/codegen/src/compile.rs‎

Lines changed: 49 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1074,12 +1074,12 @@ impl Compiler {
10741074
.filter(|(_, s)| {
10751075
s.scope == SymbolScope::Cell || s.flags.contains(SymbolFlags::COMP_CELL)
10761076
})
1077-
.map(|(name, _)| name.clone())
1077+
.map(|(name, sym)| (name.clone(), sym.flags))
10781078
.collect();
10791079
let mut param_cells = Vec::new();
10801080
let mut nonparam_cells = Vec::new();
1081-
for name in cell_symbols {
1082-
if varname_cache.contains(&name) {
1081+
for (name, flags) in cell_symbols {
1082+
if flags.contains(SymbolFlags::PARAMETER) {
10831083
param_cells.push(name);
10841084
} else {
10851085
nonparam_cells.push(name);
@@ -1110,8 +1110,9 @@ impl Compiler {
11101110
}
11111111

11121112
// Handle implicit __conditional_annotations__ cell if needed
1113-
// Only for class scope - module scope uses NAME operations, not DEREF
1114-
if ste.has_conditional_annotations && scope_type == CompilerScope::Class {
1113+
if ste.has_conditional_annotations
1114+
&& matches!(scope_type, CompilerScope::Class | CompilerScope::Module)
1115+
{
11151116
cellvar_cache.insert("__conditional_annotations__".to_string());
11161117
}
11171118

@@ -1794,8 +1795,27 @@ impl Compiler {
17941795
let size_before = self.code_stack.len();
17951796
// Set future_annotations from symbol table (detected during symbol table scan)
17961797
self.future_annotations = symbol_table.future_annotations;
1798+
1799+
// Module-level __conditional_annotations__ cell
1800+
let has_module_cond_ann = symbol_table.has_conditional_annotations;
1801+
if has_module_cond_ann {
1802+
self.current_code_info()
1803+
.metadata
1804+
.cellvars
1805+
.insert("__conditional_annotations__".to_string());
1806+
}
1807+
17971808
self.symbol_table_stack.push(symbol_table);
17981809

1810+
// Emit MAKE_CELL for module-level cells (before RESUME)
1811+
if has_module_cond_ann {
1812+
let ncells = self.code_stack.last().unwrap().metadata.cellvars.len();
1813+
for i in 0..ncells {
1814+
let i_varnum: oparg::VarNum = u32::try_from(i).expect("too many cellvars").into();
1815+
emit!(self, Instruction::MakeCell { i: i_varnum });
1816+
}
1817+
}
1818+
17991819
self.emit_resume_for_scope(CompilerScope::Module, 1);
18001820

18011821
let (doc, statements) = split_doc(&body.body, &self.opts);
@@ -5437,7 +5457,24 @@ impl Compiler {
54375457
let mut end_async_for_target = BlockIdx::NULL;
54385458

54395459
// The thing iterated:
5440-
self.compile_expression(iter)?;
5460+
// Optimize: `for x in [a, b, c]` → use tuple instead of list
5461+
// (list creation is wasteful for iteration)
5462+
// Skip optimization if any element is starred (e.g., `[a, *b, c]`)
5463+
if let ast::Expr::List(ast::ExprList { elts, .. }) = iter
5464+
&& !elts.iter().any(|e| matches!(e, ast::Expr::Starred(_)))
5465+
{
5466+
for elt in elts {
5467+
self.compile_expression(elt)?;
5468+
}
5469+
emit!(
5470+
self,
5471+
Instruction::BuildTuple {
5472+
count: u32::try_from(elts.len()).expect("too many elements"),
5473+
}
5474+
);
5475+
} else {
5476+
self.compile_expression(iter)?;
5477+
}
54415478

54425479
if is_async {
54435480
if self.ctx.func != FunctionContext::AsyncFunction {
@@ -7033,6 +7070,7 @@ impl Compiler {
70337070
/// For `And`, emits `PopJumpIfFalse`; for `Or`, emits `PopJumpIfTrue`.
70347071
fn emit_short_circuit_test(&mut self, op: &ast::BoolOp, target: BlockIdx) {
70357072
emit!(self, Instruction::Copy { i: 1 });
7073+
emit!(self, Instruction::ToBool);
70367074
match op {
70377075
ast::BoolOp::And => {
70387076
emit!(self, Instruction::PopJumpIfFalse { delta: target });
@@ -8554,11 +8592,11 @@ impl Compiler {
85548592

85558593
// fn block_done()
85568594

8557-
/// Convert a string literal AST node to Wtf8Buf, handling surrogates correctly.
8595+
/// Convert a string literal AST node to Wtf8Buf, handling surrogate literals correctly.
85588596
fn compile_string_value(&self, string: &ast::ExprStringLiteral) -> Wtf8Buf {
85598597
let value = string.value.to_str();
85608598
if value.contains(char::REPLACEMENT_CHARACTER) {
8561-
// Might have a surrogate literal; reparse from source to preserve them
8599+
// Might have a surrogate literal; reparse from source to preserve them.
85628600
string
85638601
.value
85648602
.iter()
@@ -8601,8 +8639,9 @@ impl Compiler {
86018639
}
86028640
},
86038641
ast::Expr::StringLiteral(s) => {
8604-
let value = self.compile_string_value(s);
8605-
constants.push(ConstantData::Str { value });
8642+
constants.push(ConstantData::Str {
8643+
value: self.compile_string_value(s),
8644+
});
86068645
}
86078646
ast::Expr::BytesLiteral(b) => {
86088647
constants.push(ConstantData::Bytes {

‎crates/codegen/src/ir.rs‎

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2172,11 +2172,19 @@ pub(crate) fn label_exception_targets(blocks: &mut [Block]) {
21722172
blocks[bi].instructions[i].except_handler = handler_info;
21732173

21742174
// Track YIELD_VALUE except stack depth
2175-
if matches!(
2176-
blocks[bi].instructions[i].instr.real(),
2177-
Some(Instruction::YieldValue { .. })
2178-
) {
2179-
last_yield_except_depth = stack.len() as i32;
2175+
// Only count for direct yield (arg=0), not yield-from/await (arg=1)
2176+
// The yield-from's internal SETUP_FINALLY is not an external except depth
2177+
if let Some(Instruction::YieldValue { .. }) =
2178+
blocks[bi].instructions[i].instr.real()
2179+
{
2180+
let yield_arg = u32::from(blocks[bi].instructions[i].arg);
2181+
if yield_arg == 0 {
2182+
// Direct yield: count actual except depth
2183+
last_yield_except_depth = stack.len() as i32;
2184+
} else {
2185+
// yield-from/await: subtract 1 for the internal SETUP_FINALLY
2186+
last_yield_except_depth = (stack.len() as i32) - 1;
2187+
}
21802188
}
21812189

21822190
// Set RESUME DEPTH1 flag based on last yield's except depth

‎crates/codegen/src/snapshots/rustpython_codegen__compile__tests__nested_bool_op.snap‎

Lines changed: 23 additions & 14 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎crates/codegen/src/symboltable.rs‎

Lines changed: 38 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -297,7 +297,8 @@ fn drop_class_free(symbol_table: &mut SymbolTable, newfree: &mut IndexSet<String
297297
}
298298

299299
// Classes with function definitions need __classdict__ for PEP 649
300-
if !symbol_table.needs_classdict {
300+
// (but not when `from __future__ import annotations` is active)
301+
if !symbol_table.needs_classdict && !symbol_table.future_annotations {
301302
let has_functions = symbol_table.sub_tables.iter().any(|t| {
302303
matches!(
303304
t.typ,
@@ -571,6 +572,9 @@ impl SymbolTableAnalyzer {
571572
newfree.extend(child_free);
572573
}
573574
if let Some(ann_free) = annotation_free {
575+
// Propagate annotation-scope free names to this scope so
576+
// implicit class-scope cells (__classdict__/__conditional_annotations__)
577+
// can be materialized by drop_class_free when needed.
574578
newfree.extend(ann_free);
575579
}
576580

@@ -758,6 +762,12 @@ impl SymbolTableAnalyzer {
758762
if let Some(decl_depth) = decl_depth {
759763
// decl_depth is the number of tables between the current one and
760764
// the one that declared the cell var
765+
// For implicit class scope variables (__classdict__, __conditional_annotations__),
766+
// only propagate free to annotation/type-param scopes, not regular functions.
767+
// Regular method functions don't need these in their freevars.
768+
let is_class_implicit =
769+
name == "__classdict__" || name == "__conditional_annotations__";
770+
761771
for (table, typ) in self.tables.iter_mut().rev().take(decl_depth) {
762772
if let CompilerScope::Class = typ {
763773
if let Some(free_class) = table.get_mut(name) {
@@ -768,10 +778,19 @@ impl SymbolTableAnalyzer {
768778
symbol.scope = SymbolScope::Free;
769779
table.insert(name.to_owned(), symbol);
770780
}
781+
} else if is_class_implicit
782+
&& matches!(
783+
typ,
784+
CompilerScope::Function
785+
| CompilerScope::AsyncFunction
786+
| CompilerScope::Lambda
787+
)
788+
{
789+
// Skip: don't add __classdict__/__conditional_annotations__
790+
// as free vars in regular functions — only annotation/type scopes need them
771791
} else if !table.contains_key(name) {
772792
let mut symbol = Symbol::new(name);
773793
symbol.scope = SymbolScope::Free;
774-
// symbol.is_referenced = true;
775794
table.insert(name.to_owned(), symbol);
776795
}
777796
}
@@ -1226,20 +1245,30 @@ impl SymbolTableBuilder {
12261245
}
12271246

12281247
fn scan_annotation(&mut self, annotation: &ast::Expr) -> SymbolTableResult {
1248+
self.scan_annotation_inner(annotation, false)
1249+
}
1250+
1251+
/// Scan an annotation from an AnnAssign statement (can be conditional)
1252+
fn scan_ann_assign_annotation(&mut self, annotation: &ast::Expr) -> SymbolTableResult {
1253+
self.scan_annotation_inner(annotation, true)
1254+
}
1255+
1256+
fn scan_annotation_inner(
1257+
&mut self,
1258+
annotation: &ast::Expr,
1259+
is_ann_assign: bool,
1260+
) -> SymbolTableResult {
12291261
let current_scope = self.tables.last().map(|t| t.typ);
12301262

1231-
// PEP 649: Check if this is a conditional annotation
1232-
// Module-level: always conditional (module may be partially executed)
1233-
// Class-level: conditional only when inside if/for/while/etc.
1234-
if !self.future_annotations {
1263+
// PEP 649: Only AnnAssign annotations can be conditional.
1264+
// Function parameter/return annotations are never conditional.
1265+
if is_ann_assign && !self.future_annotations {
12351266
let is_conditional = matches!(current_scope, Some(CompilerScope::Module))
12361267
|| (matches!(current_scope, Some(CompilerScope::Class))
12371268
&& self.in_conditional_block);
12381269

12391270
if is_conditional && !self.tables.last().unwrap().has_conditional_annotations {
12401271
self.tables.last_mut().unwrap().has_conditional_annotations = true;
1241-
// Register __conditional_annotations__ as both Assigned and Used so that
1242-
// it becomes a Cell variable in class scope (children reference it as Free)
12431272
self.register_name(
12441273
"__conditional_annotations__",
12451274
SymbolUsage::Assigned,
@@ -1571,7 +1600,7 @@ impl SymbolTableBuilder {
15711600
// sub_tables that cause mismatch in the annotation scope's sub_table index.
15721601
let is_simple_name = *simple && matches!(&**target, Expr::Name(_));
15731602
if is_simple_name {
1574-
self.scan_annotation(annotation)?;
1603+
self.scan_ann_assign_annotation(annotation)?;
15751604
} else {
15761605
// Still validate annotation for forbidden expressions
15771606
// (yield, await, named) even for non-simple targets.

0 commit comments

Comments
 (0)