Skip to content

Commit f0c3e7d

Browse files
youknowoneyouknowone
authored
Remove SUBSCRIPT, JUMP_IF_{TRUE,FALSE}_OR_POP (#6810)
* Align ForIter behavior (incomplete) * Romove JUMP_IF_{TRUE,FALSE}_OR_POP, subscript * Remove LoadAssetionError * Update snapshot for ForIter behavior change
1 parent 52a854a commit f0c3e7d

6 files changed

Lines changed: 17 additions & 94 deletions

File tree

‎Lib/_opcode_metadata.py‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -218,13 +218,9 @@
218218
'BUILD_CONST_KEY_MAP': 215,
219219
'BREAK': 216,
220220
'CONTINUE': 217,
221-
'JUMP_IF_FALSE_OR_POP': 218,
222-
'JUMP_IF_TRUE_OR_POP': 219,
223221
'JUMP_IF_NOT_EXC_MATCH': 220,
224-
'LOAD_ASSERTION_ERROR': 221,
225222
'RETURN_CONST': 222,
226223
'SET_EXC_INFO': 223,
227-
'SUBSCRIPT': 224,
228224
'INSTRUMENTED_END_FOR': 234,
229225
'INSTRUMENTED_POP_ITER': 235,
230226
'INSTRUMENTED_END_SEND': 236,

‎crates/codegen/src/compile.rs‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -489,7 +489,7 @@ impl Compiler {
489489
match ctx {
490490
ast::ExprContext::Load => {
491491
emit!(self, Instruction::BuildSlice { argc });
492-
emit!(self, Instruction::Subscript);
492+
emit!(self, Instruction::BinarySubscr);
493493
}
494494
ast::ExprContext::Store => {
495495
emit!(self, Instruction::BuildSlice { argc });
@@ -503,7 +503,7 @@ impl Compiler {
503503

504504
// Emit appropriate instruction based on context
505505
match ctx {
506-
ast::ExprContext::Load => emit!(self, Instruction::Subscript),
506+
ast::ExprContext::Load => emit!(self, Instruction::BinarySubscr),
507507
ast::ExprContext::Store => emit!(self, Instruction::StoreSubscr),
508508
ast::ExprContext::Del => emit!(self, Instruction::DeleteSubscr),
509509
ast::ExprContext::Invalid => {
@@ -4929,6 +4929,9 @@ impl Compiler {
49294929

49304930
if is_async {
49314931
emit!(self, Instruction::EndAsyncFor);
4932+
} else {
4933+
// Pop the iterator after loop ends
4934+
emit!(self, Instruction::PopTop);
49324935
}
49334936
self.compile_statements(orelse)?;
49344937

@@ -5987,14 +5990,9 @@ impl Compiler {
59875990
self.compile_addcompare(op);
59885991

59895992
// if comparison result is false, we break with this value; if true, try the next one.
5990-
/*
59915993
emit!(self, Instruction::Copy { index: 1 });
5992-
// emit!(self, Instruction::ToBool); // TODO: Uncomment this
59935994
emit!(self, Instruction::PopJumpIfFalse { target: cleanup });
59945995
emit!(self, Instruction::PopTop);
5995-
*/
5996-
5997-
emit!(self, Instruction::JumpIfFalseOrPop { target: cleanup });
59985996
}
59995997

60005998
self.compile_expression(last_comparator)?;
@@ -6205,7 +6203,7 @@ impl Compiler {
62056203
self.compile_expression(slice)?;
62066204
emit!(self, Instruction::Copy { index: 2_u32 });
62076205
emit!(self, Instruction::Copy { index: 2_u32 });
6208-
emit!(self, Instruction::Subscript);
6206+
emit!(self, Instruction::BinarySubscr);
62096207
AugAssignKind::Subscript
62106208
}
62116209
ast::Expr::Attribute(ast::ExprAttribute { value, attr, .. }) => {
@@ -7367,6 +7365,8 @@ impl Compiler {
73677365
if is_async {
73687366
emit!(self, Instruction::EndAsyncFor);
73697367
emit!(self, Instruction::PopTop);
7368+
} else {
7369+
emit!(self, Instruction::PopTop);
73707370
}
73717371
}
73727372

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

Lines changed: 3 additions & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎crates/compiler-core/src/bytecode/instruction.rs‎

Lines changed: 3 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -272,19 +272,11 @@ pub enum Instruction {
272272
Continue {
273273
target: Arg<Label>,
274274
} = 217,
275-
JumpIfFalseOrPop {
276-
target: Arg<Label>,
277-
} = 218,
278-
JumpIfTrueOrPop {
279-
target: Arg<Label>,
280-
} = 219,
281275
JumpIfNotExcMatch(Arg<Label>) = 220,
282-
LoadAssertionError = 221, // Placeholder
283276
ReturnConst {
284277
idx: Arg<u32>,
285278
} = 222,
286279
SetExcInfo = 223,
287-
Subscript = 224,
288280
// CPython 3.14 RESUME (128)
289281
Resume {
290282
arg: Arg<u32>,
@@ -441,17 +433,9 @@ impl TryFrom<u8> for Instruction {
441433
u8::from(Self::Continue {
442434
target: Arg::marker(),
443435
}),
444-
u8::from(Self::JumpIfFalseOrPop {
445-
target: Arg::marker(),
446-
}),
447-
u8::from(Self::JumpIfTrueOrPop {
448-
target: Arg::marker(),
449-
}),
450436
u8::from(Self::JumpIfNotExcMatch(Arg::marker())),
451-
u8::from(Self::LoadAssertionError),
452437
u8::from(Self::ReturnConst { idx: Arg::marker() }),
453438
u8::from(Self::SetExcInfo),
454-
u8::from(Self::Subscript),
455439
];
456440

457441
if (cpython_start..=cpython_end).contains(&value)
@@ -478,8 +462,6 @@ impl InstructionMetadata for Instruction {
478462
| Self::JumpIfNotExcMatch(l)
479463
| Self::PopJumpIfTrue { target: l }
480464
| Self::PopJumpIfFalse { target: l }
481-
| Self::JumpIfTrueOrPop { target: l }
482-
| Self::JumpIfFalseOrPop { target: l }
483465
| Self::ForIter { target: l }
484466
| Self::Break { target: l }
485467
| Self::Continue { target: l }
@@ -525,7 +507,6 @@ impl InstructionMetadata for Instruction {
525507
Self::DeleteGlobal(_) => 0,
526508
Self::DeleteDeref(_) => 0,
527509
Self::LoadFromDictOrDeref(_) => 1,
528-
Self::Subscript => -1,
529510
Self::StoreSubscr => -3,
530511
Self::DeleteSubscr => -2,
531512
Self::LoadAttr { .. } => 0,
@@ -551,20 +532,6 @@ impl InstructionMetadata for Instruction {
551532
Self::Break { .. } => 0,
552533
Self::PopJumpIfTrue { .. } => -1,
553534
Self::PopJumpIfFalse { .. } => -1,
554-
Self::JumpIfTrueOrPop { .. } => {
555-
if jump {
556-
0
557-
} else {
558-
-1
559-
}
560-
}
561-
Self::JumpIfFalseOrPop { .. } => {
562-
if jump {
563-
0
564-
} else {
565-
-1
566-
}
567-
}
568535
Self::MakeFunction => {
569536
// CPython 3.14 style: MakeFunction only pops code object
570537
-1 + 1 // pop code, push function
@@ -584,11 +551,9 @@ impl InstructionMetadata for Instruction {
584551
Self::FormatSimple => 0,
585552
Self::FormatWithSpec => -1,
586553
Self::ForIter { .. } => {
587-
if jump {
588-
-1
589-
} else {
590-
1
591-
}
554+
// jump=False: push next value (+1)
555+
// jump=True: iterator stays on stack, no change (0)
556+
if jump { 0 } else { 1 }
592557
}
593558
Self::IsOp(_) => -1,
594559
Self::ContainsOp(_) => -1,
@@ -684,7 +649,6 @@ impl InstructionMetadata for Instruction {
684649
Self::EndFor => 0,
685650
Self::ExitInitCheck => 0,
686651
Self::InterpreterExit => 0,
687-
Self::LoadAssertionError => 0,
688652
Self::LoadLocals => 0,
689653
Self::ReturnGenerator => 0,
690654
Self::StoreSlice => 0,
@@ -920,9 +884,7 @@ impl InstructionMetadata for Instruction {
920884
Self::JumpBackward { target } => w!(JUMP_BACKWARD, target),
921885
Self::JumpBackwardNoInterrupt { target } => w!(JUMP_BACKWARD_NO_INTERRUPT, target),
922886
Self::JumpForward { target } => w!(JUMP_FORWARD, target),
923-
Self::JumpIfFalseOrPop { target } => w!(JUMP_IF_FALSE_OR_POP, target),
924887
Self::JumpIfNotExcMatch(target) => w!(JUMP_IF_NOT_EXC_MATCH, target),
925-
Self::JumpIfTrueOrPop { target } => w!(JUMP_IF_TRUE_OR_POP, target),
926888
Self::ListAppend { i } => w!(LIST_APPEND, i),
927889
Self::ListExtend { i } => w!(LIST_EXTEND, i),
928890
Self::LoadAttr { idx } => {
@@ -994,7 +956,6 @@ impl InstructionMetadata for Instruction {
994956
Self::StoreGlobal(idx) => w!(STORE_GLOBAL, name = idx),
995957
Self::StoreName(idx) => w!(STORE_NAME, name = idx),
996958
Self::StoreSubscr => w!(STORE_SUBSCR),
997-
Self::Subscript => w!(SUBSCRIPT),
998959
Self::Swap { index } => w!(SWAP, index),
999960
Self::ToBool => w!(TO_BOOL),
1000961
Self::UnpackEx { args } => w!(UNPACK_EX, args),

‎crates/stdlib/src/opcode.rs‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -107,9 +107,7 @@ mod opcode {
107107
Instruction::Break { .. }
108108
| Instruction::Continue { .. }
109109
| Instruction::ForIter { .. }
110-
| Instruction::JumpIfFalseOrPop { .. }
111110
| Instruction::JumpIfNotExcMatch(_)
112-
| Instruction::JumpIfTrueOrPop { .. }
113111
| Instruction::PopJumpIfFalse { .. }
114112
| Instruction::PopJumpIfTrue { .. }
115113
| Instruction::Send { .. }

‎crates/vm/src/frame.rs‎

Lines changed: 3 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -1139,9 +1139,6 @@ impl ExecutingFrame<'_> {
11391139
self.push_value(vm.ctx.new_bool(value).into());
11401140
Ok(None)
11411141
}
1142-
Instruction::JumpIfFalseOrPop { target } => {
1143-
self.jump_if_or_pop(vm, target.get(arg), false)
1144-
}
11451142
Instruction::JumpIfNotExcMatch(target) => {
11461143
let b = self.pop_value();
11471144
let a = self.pop_value();
@@ -1165,9 +1162,6 @@ impl ExecutingFrame<'_> {
11651162
self.push_value(vm.ctx.new_bool(value).into());
11661163
self.pop_jump_if(vm, target.get(arg), false)
11671164
}
1168-
Instruction::JumpIfTrueOrPop { target } => {
1169-
self.jump_if_or_pop(vm, target.get(arg), true)
1170-
}
11711165
Instruction::JumpForward { target } => {
11721166
self.jump(target.get(arg));
11731167
Ok(None)
@@ -1734,7 +1728,6 @@ impl ExecutingFrame<'_> {
17341728
Ok(None)
17351729
}
17361730
Instruction::StoreSubscr => self.execute_store_subscript(vm),
1737-
Instruction::Subscript => self.execute_subscript(vm),
17381731
Instruction::Swap { index } => {
17391732
let len = self.state.stack.len();
17401733
debug_assert!(len > 0, "stack underflow in SWAP");
@@ -2074,14 +2067,6 @@ impl ExecutingFrame<'_> {
20742067
}
20752068
}
20762069

2077-
fn execute_subscript(&mut self, vm: &VirtualMachine) -> FrameResult {
2078-
let b_ref = self.pop_value();
2079-
let a_ref = self.pop_value();
2080-
let value = a_ref.get_item(&*b_ref, vm)?;
2081-
self.push_value(value);
2082-
Ok(None)
2083-
}
2084-
20852070
fn execute_store_subscript(&mut self, vm: &VirtualMachine) -> FrameResult {
20862071
let idx = self.pop_value();
20872072
let obj = self.pop_value();
@@ -2351,23 +2336,6 @@ impl ExecutingFrame<'_> {
23512336
Ok(None)
23522337
}
23532338

2354-
#[inline]
2355-
fn jump_if_or_pop(
2356-
&mut self,
2357-
vm: &VirtualMachine,
2358-
target: bytecode::Label,
2359-
flag: bool,
2360-
) -> FrameResult {
2361-
let obj = self.top_value();
2362-
let value = obj.to_owned().try_to_bool(vm)?;
2363-
if value == flag {
2364-
self.jump(target);
2365-
} else {
2366-
self.pop_value();
2367-
}
2368-
Ok(None)
2369-
}
2370-
23712339
/// The top of stack contains the iterator, lets push it forward
23722340
fn execute_for_iter(&mut self, vm: &VirtualMachine, target: bytecode::Label) -> FrameResult {
23732341
let top_of_stack = PyIter::new(self.top_value());
@@ -2380,15 +2348,13 @@ impl ExecutingFrame<'_> {
23802348
Ok(None)
23812349
}
23822350
Ok(PyIterReturn::StopIteration(_)) => {
2383-
// Pop iterator from stack:
2384-
self.pop_value();
2385-
2386-
// End of for loop
2351+
// CPython 3.14: Do NOT pop iterator here
2352+
// POP_ITER instruction will handle cleanup after the loop
23872353
self.jump(target);
23882354
Ok(None)
23892355
}
23902356
Err(next_error) => {
2391-
// Pop iterator from stack:
2357+
// On error, pop iterator and propagate
23922358
self.pop_value();
23932359
Err(next_error)
23942360
}

0 commit comments

Comments
 (0)