Skip to content

Commit 3e9f825

Browse files
authored
Enable PEP 709 inlined comprehensions (#7412)
* Enable PEP 709 inlined comprehensions for function-like scopes Activate the existing compile_inlined_comprehension() implementation by fixing 6 bugs that prevented it from working: - LoadFastAndClear: push NULL (not None) when slot is empty so StoreFast can restore empty state after comprehension - StoreFast: accept NULL from stack for the restore path - sub_tables.remove(0) replaced with next_sub_table cursor to match the pattern used elsewhere in the compiler - in_inlined_comp flag moved from non-inlined to inlined path - is_inlined_comprehension_context() now checks comp_inlined flag and restricts inlining to function-like scopes - comp_inlined set only when parent scope uses fastlocals Symbol table analysis handles conflict detection: - Nested scopes in comprehension → skip inlining - Bound name conflicts with parent symbol → skip inlining - Cross-comprehension reference conflicts → skip inlining - Splice comprehension sub_tables into parent for nested scope tracking * Add localspluskinds, unify DEREF to localsplus index - Add CO_FAST_LOCAL/CELL/FREE/HIDDEN constants and localspluskinds field to CodeObject for per-slot metadata - Change DEREF instruction opargs from cell-relative indices (NameIdx) to localsplus absolute indices (oparg::VarNum) - Add fixup_deref_opargs pass in ir.rs to convert cell-relative indices to localsplus indices after finalization - Replace get_cell_name with get_localsplus_name in InstrDisplayContext trait - Update VM cell_ref/get_cell_contents/set_cell_contents to use localsplus indices directly (no nlocals offset) - Update function.rs cell2arg, super.rs __class__ lookup with explicit nlocals offsets * Fix clippy warnings, formatting, restore _opcode_metadata.py Fix cast_possible_truncation, nonminimal_bool, collapsible_if, manual_contains clippy lints. Restore _opcode_metadata.py to upstream/main version (3.14 aligned). Pre-copy closure cells in Frame::new for coroutine locals(). Handle raw values in merged cell slots during inlined comps. Exclude async comprehensions from inlining path. * Exclude async/await comprehensions from PEP 709 inlining in symboltable Async comprehensions and comprehensions with await in the element expression need their own coroutine scope and cannot be inlined. The symboltable builder was not checking these conditions, causing incorrect symbol scope resolution when an async comprehension was nested inside an inlined comprehension (e.g. [[x async for x in g] for j in items]).
1 parent 4abe4c5 commit 3e9f825

File tree

14 files changed

+813
-343
lines changed

14 files changed

+813
-343
lines changed

.cspell.dict/cpython.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -109,6 +109,7 @@ lineiterator
109109
linetable
110110
loadfast
111111
localsplus
112+
localspluskinds
112113
Lshift
113114
lsprof
114115
MAXBLOCKS

.cspell.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,7 @@
7070
"lossily",
7171
"mcache",
7272
"oparg",
73+
"opargs",
7374
"pyc",
7475
"significand",
7576
"summands",

crates/codegen/src/compile.rs

Lines changed: 204 additions & 87 deletions
Large diffs are not rendered by default.

crates/codegen/src/ir.rs

Lines changed: 115 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -8,9 +8,10 @@ use num_traits::ToPrimitive;
88
use rustpython_compiler_core::{
99
OneIndexed, SourceLocation,
1010
bytecode::{
11-
AnyInstruction, Arg, CodeFlags, CodeObject, CodeUnit, CodeUnits, ConstantData,
12-
ExceptionTableEntry, InstrDisplayContext, Instruction, InstructionMetadata, Label, OpArg,
13-
PseudoInstruction, PyCodeLocationInfoKind, encode_exception_table, oparg,
11+
AnyInstruction, Arg, CO_FAST_CELL, CO_FAST_FREE, CO_FAST_HIDDEN, CO_FAST_LOCAL, CodeFlags,
12+
CodeObject, CodeUnit, CodeUnits, ConstantData, ExceptionTableEntry, InstrDisplayContext,
13+
Instruction, InstructionMetadata, Label, OpArg, PseudoInstruction, PyCodeLocationInfoKind,
14+
encode_exception_table, oparg,
1415
},
1516
varint::{write_signed_varint, write_varint},
1617
};
@@ -210,7 +211,6 @@ impl CodeInfo {
210211
self.optimize_load_global_push_null();
211212

212213
let max_stackdepth = self.max_stackdepth()?;
213-
let cell2arg = self.cell2arg();
214214

215215
let Self {
216216
flags,
@@ -236,7 +236,7 @@ impl CodeInfo {
236236
varnames: varname_cache,
237237
cellvars: cellvar_cache,
238238
freevars: freevar_cache,
239-
fast_hidden: _,
239+
fast_hidden,
240240
argcount: arg_count,
241241
posonlyargcount: posonlyarg_count,
242242
kwonlyargcount: kwonlyarg_count,
@@ -247,8 +247,12 @@ impl CodeInfo {
247247
let mut locations = Vec::new();
248248
let mut linetable_locations: Vec<LineTableLocation> = Vec::new();
249249

250-
// Convert pseudo ops and remove resulting NOPs (keep line-marker NOPs)
251-
convert_pseudo_ops(&mut blocks, varname_cache.len() as u32);
250+
// Build cellfixedoffsets for cell-local merging
251+
let cellfixedoffsets =
252+
build_cellfixedoffsets(&varname_cache, &cellvar_cache, &freevar_cache);
253+
// Convert pseudo ops (LoadClosure uses cellfixedoffsets) and fixup DEREF opargs
254+
convert_pseudo_ops(&mut blocks, &cellfixedoffsets);
255+
fixup_deref_opargs(&mut blocks, &cellfixedoffsets);
252256
// Remove redundant NOPs, keeping line-marker NOPs only when
253257
// they are needed to preserve tracing.
254258
let mut block_order = Vec::new();
@@ -482,6 +486,41 @@ impl CodeInfo {
482486
// Generate exception table before moving source_path
483487
let exceptiontable = generate_exception_table(&blocks, &block_to_index);
484488

489+
// Build localspluskinds with cell-local merging
490+
let nlocals = varname_cache.len();
491+
let ncells = cellvar_cache.len();
492+
let nfrees = freevar_cache.len();
493+
let numdropped = cellvar_cache
494+
.iter()
495+
.filter(|cv| varname_cache.contains(cv.as_str()))
496+
.count();
497+
let nlocalsplus = nlocals + ncells - numdropped + nfrees;
498+
let mut localspluskinds = vec![0u8; nlocalsplus];
499+
// Mark locals
500+
for kind in localspluskinds.iter_mut().take(nlocals) {
501+
*kind = CO_FAST_LOCAL;
502+
}
503+
// Mark cells (merged and non-merged)
504+
for (i, cellvar) in cellvar_cache.iter().enumerate() {
505+
let idx = cellfixedoffsets[i] as usize;
506+
if varname_cache.contains(cellvar.as_str()) {
507+
localspluskinds[idx] |= CO_FAST_CELL; // merged: LOCAL | CELL
508+
} else {
509+
localspluskinds[idx] = CO_FAST_CELL;
510+
}
511+
}
512+
// Mark frees
513+
for i in 0..nfrees {
514+
let idx = cellfixedoffsets[ncells + i] as usize;
515+
localspluskinds[idx] = CO_FAST_FREE;
516+
}
517+
// Apply CO_FAST_HIDDEN for inlined comprehension variables
518+
for (name, &hidden) in &fast_hidden {
519+
if hidden && let Some(idx) = varname_cache.get_index_of(name.as_str()) {
520+
localspluskinds[idx] |= CO_FAST_HIDDEN;
521+
}
522+
}
523+
485524
Ok(CodeObject {
486525
flags,
487526
posonlyarg_count,
@@ -500,43 +539,12 @@ impl CodeInfo {
500539
varnames: varname_cache.into_iter().collect(),
501540
cellvars: cellvar_cache.into_iter().collect(),
502541
freevars: freevar_cache.into_iter().collect(),
503-
cell2arg,
542+
localspluskinds: localspluskinds.into_boxed_slice(),
504543
linetable,
505544
exceptiontable,
506545
})
507546
}
508547

509-
fn cell2arg(&self) -> Option<Box<[i32]>> {
510-
if self.metadata.cellvars.is_empty() {
511-
return None;
512-
}
513-
514-
let total_args = self.metadata.argcount
515-
+ self.metadata.kwonlyargcount
516-
+ self.flags.contains(CodeFlags::VARARGS) as u32
517-
+ self.flags.contains(CodeFlags::VARKEYWORDS) as u32;
518-
519-
let mut found_cellarg = false;
520-
let cell2arg = self
521-
.metadata
522-
.cellvars
523-
.iter()
524-
.map(|var| {
525-
self.metadata
526-
.varnames
527-
.get_index_of(var)
528-
// check that it's actually an arg
529-
.filter(|i| *i < total_args as usize)
530-
.map_or(-1, |i| {
531-
found_cellarg = true;
532-
i as i32
533-
})
534-
})
535-
.collect::<Box<[_]>>();
536-
537-
if found_cellarg { Some(cell2arg) } else { None }
538-
}
539-
540548
fn dce(&mut self) {
541549
for block in &mut self.blocks {
542550
let mut last_instr = None;
@@ -1107,12 +1115,19 @@ impl InstrDisplayContext for CodeInfo {
11071115
self.metadata.varnames[var_num.as_usize()].as_ref()
11081116
}
11091117

1110-
fn get_cell_name(&self, i: usize) -> &str {
1111-
self.metadata
1112-
.cellvars
1113-
.get_index(i)
1114-
.unwrap_or_else(|| &self.metadata.freevars[i - self.metadata.cellvars.len()])
1115-
.as_ref()
1118+
fn get_localsplus_name(&self, var_num: oparg::VarNum) -> &str {
1119+
let idx = var_num.as_usize();
1120+
let nlocals = self.metadata.varnames.len();
1121+
if idx < nlocals {
1122+
self.metadata.varnames[idx].as_ref()
1123+
} else {
1124+
let cell_idx = idx - nlocals;
1125+
self.metadata
1126+
.cellvars
1127+
.get_index(cell_idx)
1128+
.unwrap_or_else(|| &self.metadata.freevars[cell_idx - self.metadata.cellvars.len()])
1129+
.as_ref()
1130+
}
11161131
}
11171132
}
11181133

@@ -1768,7 +1783,7 @@ pub(crate) fn label_exception_targets(blocks: &mut [Block]) {
17681783

17691784
/// Convert remaining pseudo ops to real instructions or NOP.
17701785
/// flowgraph.c convert_pseudo_ops
1771-
pub(crate) fn convert_pseudo_ops(blocks: &mut [Block], varnames_len: u32) {
1786+
pub(crate) fn convert_pseudo_ops(blocks: &mut [Block], cellfixedoffsets: &[u32]) {
17721787
for block in blocks.iter_mut() {
17731788
for info in &mut block.instructions {
17741789
let Some(pseudo) = info.instr.pseudo() else {
@@ -1786,9 +1801,10 @@ pub(crate) fn convert_pseudo_ops(blocks: &mut [Block], varnames_len: u32) {
17861801
PseudoInstruction::PopBlock => {
17871802
info.instr = Instruction::Nop.into();
17881803
}
1789-
// LOAD_CLOSURE → LOAD_FAST (with varnames offset)
1804+
// LOAD_CLOSURE → LOAD_FAST (using cellfixedoffsets for merged layout)
17901805
PseudoInstruction::LoadClosure { i } => {
1791-
let new_idx = varnames_len + i.get(info.arg);
1806+
let cell_relative = i.get(info.arg) as usize;
1807+
let new_idx = cellfixedoffsets[cell_relative];
17921808
info.arg = OpArg::new(new_idx);
17931809
info.instr = Instruction::LoadFast {
17941810
var_num: Arg::marker(),
@@ -1808,3 +1824,54 @@ pub(crate) fn convert_pseudo_ops(blocks: &mut [Block], varnames_len: u32) {
18081824
}
18091825
}
18101826
}
1827+
1828+
/// Build cellfixedoffsets mapping: cell/free index -> localsplus index.
1829+
/// Merged cells (cellvar also in varnames) get the local slot index.
1830+
/// Non-merged cells get slots after nlocals. Free vars follow.
1831+
pub(crate) fn build_cellfixedoffsets(
1832+
varnames: &IndexSet<String>,
1833+
cellvars: &IndexSet<String>,
1834+
freevars: &IndexSet<String>,
1835+
) -> Vec<u32> {
1836+
let nlocals = varnames.len();
1837+
let ncells = cellvars.len();
1838+
let nfrees = freevars.len();
1839+
let mut fixed = Vec::with_capacity(ncells + nfrees);
1840+
let mut numdropped = 0usize;
1841+
for (i, cellvar) in cellvars.iter().enumerate() {
1842+
if let Some(local_idx) = varnames.get_index_of(cellvar) {
1843+
fixed.push(local_idx as u32);
1844+
numdropped += 1;
1845+
} else {
1846+
fixed.push((nlocals + i - numdropped) as u32);
1847+
}
1848+
}
1849+
for i in 0..nfrees {
1850+
fixed.push((nlocals + ncells - numdropped + i) as u32);
1851+
}
1852+
fixed
1853+
}
1854+
1855+
/// Convert DEREF instruction opargs from cell-relative indices to localsplus indices
1856+
/// using the cellfixedoffsets mapping.
1857+
pub(crate) fn fixup_deref_opargs(blocks: &mut [Block], cellfixedoffsets: &[u32]) {
1858+
for block in blocks.iter_mut() {
1859+
for info in &mut block.instructions {
1860+
let Some(instr) = info.instr.real() else {
1861+
continue;
1862+
};
1863+
let needs_fixup = matches!(
1864+
instr,
1865+
Instruction::LoadDeref { .. }
1866+
| Instruction::StoreDeref { .. }
1867+
| Instruction::DeleteDeref { .. }
1868+
| Instruction::LoadFromDictOrDeref { .. }
1869+
| Instruction::MakeCell { .. }
1870+
);
1871+
if needs_fixup {
1872+
let cell_relative = u32::from(info.arg) as usize;
1873+
info.arg = OpArg::new(cellfixedoffsets[cell_relative]);
1874+
}
1875+
}
1876+
}
1877+
}

0 commit comments

Comments
 (0)