From 6cddf2e35de5b83811d2160d83bd22b6153962e8 Mon Sep 17 00:00:00 2001 From: Francois Chagnon Date: Tue, 13 Oct 2026 16:37:45 -0400 Subject: [PATCH 1/2] Keep dependent lookup returns inside independent loop rejection guards --- src/decompiler.rs | 94 ++++++++++++++++++++++++++ tests/fixtures/loop_lookup_guards.py | 33 +++++++++ tests/integration.rs | 22 ++++++ tests/pyc/loop_lookup_guards.3.11.pyc | Bin 0 -> 1443 bytes tests/pyc/loop_lookup_guards.3.12.pyc | Bin 0 -> 1195 bytes tests/pyc/loop_lookup_guards.3.13.pyc | Bin 0 -> 1246 bytes tests/pyc/loop_lookup_guards.3.14.pyc | Bin 0 -> 1266 bytes 7 files changed, 149 insertions(+) create mode 100644 tests/fixtures/loop_lookup_guards.py create mode 100644 tests/pyc/loop_lookup_guards.3.11.pyc create mode 100644 tests/pyc/loop_lookup_guards.3.12.pyc create mode 100644 tests/pyc/loop_lookup_guards.3.13.pyc create mode 100644 tests/pyc/loop_lookup_guards.3.14.pyc diff --git a/src/decompiler.rs b/src/decompiler.rs index 33a3465d..4360b7a5 100644 --- a/src/decompiler.rs +++ b/src/decompiler.rs @@ -30799,6 +30799,100 @@ return None; return; } } + // A 3.12+ lookup rejection guard has exactly one fall-through back + // edge before its success target. Preserve it as `if: continue` only + // when the success span assigns the local checked by an independent + // None guard and returned after that guard. Other suites retain their + // existing handling; changing protected suites also changes their + // exception-region boundaries. + // Treating the success region as an If suite bounded by the NEXT + // back edge closes that suite at an independent later guard and + // ejects the return that relies on its assignments (a coordinator + // lookup then returns an unbound local for non-dictionary rows). + // Inline comprehension filters retain their own bookkeeping. A + // nested while loop is not this for loop's rejection trampoline. + if self.version.at_least(3, 12) && self.inline_comp.is_none() + && self.code.exception_entries().is_empty() + && raw_target == target && target > self.cur_next + { + let current_loop = self.blocks.iter().rev() + .find(|block| matches!(block.kind, BlockType::For | BlockType::While)) + .filter(|block| block.kind == BlockType::For) + .map(|block| (block.start, block.end)); + let trampoline = self.idx_of.get(&self.cur_next).map(|&index| { + self.instrs[index..].iter().take_while(|item| item.offset < target) + .filter(|item| !matches!(item.op, Op::NOP | Op::NOT_TAKEN | Op::CACHE)) + .collect::>() + }); + if let (Some((top, loop_end)), Some(steps)) = (current_loop, trampoline) { + let dependent_return = self.idx_of.get(&target).map_or(false, |&index| { + let body = self.instrs[index..].iter().take_while(|item| item.offset < loop_end) + .filter(|item| !matches!(item.op, Op::NOP | Op::NOT_TAKEN | Op::CACHE)) + .collect::>(); + let guard = match body.iter().position(|item| item.target.is_some() + || matches!(item.op, Op::RETURN_VALUE | Op::RETURN_CONST | Op::RAISE_VARARGS | Op::RERAISE)) { + Some(index) => index, None => return false, + }; + if guard < 2 || !matches!(body[guard].op, Op::POP_JUMP_IF_NONE | Op::POP_JUMP_IF_NOT_NONE) { + return false; + } + let stored = body[..guard].iter().rev().find_map(|item| match item.op { + Op::STORE_FAST => Some(item.arg), + Op::STORE_FAST_LOAD_FAST => Some(item.arg >> 4), + Op::STORE_FAST_STORE_FAST => Some(item.arg & 15), + _ => None, + }); + let local = match stored { Some(local) => local, None => return false }; + let tested = body[guard - 1]; + if !(matches!(tested.op, Op::LOAD_FAST | Op::LOAD_FAST_BORROW | Op::LOAD_FAST_CHECK) + && tested.arg == local + || tested.op == Op::STORE_FAST_LOAD_FAST && tested.arg & 15 == local) { + return false; + } + let back = match body.get(guard + 1) { Some(back) => back, None => return false }; + if !matches!(back.op, Op::JUMP_BACKWARD | Op::JUMP_BACKWARD_NO_INTERRUPT) + || back.target != Some(top) || body[guard].target != Some(back.end()) { + return false; + } + let returned = match body.get(guard + 2) { Some(value) => value, None => return false }; + if !matches!(returned.op, Op::LOAD_FAST | Op::LOAD_FAST_BORROW | Op::LOAD_FAST_CHECK) + || returned.arg != local { return false; } + let mut tail = guard + 3; + while tail + 1 < body.len() && body[tail].op == Op::SWAP + && body[tail].arg == 2 && body[tail + 1].op == Op::POP_TOP { + tail += 2; + } + body.get(tail).map_or(false, |item| item.op == Op::RETURN_VALUE) + }); + if steps.len() == 1 + && dependent_return + && matches!(steps[0].op, Op::JUMP_BACKWARD | Op::JUMP_BACKWARD_NO_INTERRUPT) + && steps[0].target == Some(top) && steps[0].end() == target + { + self.push_stmt(Stmt::If { + // This layout negates a comparison guard by its jump, not + // its operator. Folding `<` to `>=` changes NaN and + // user comparison hooks. None-identity guards use + // their dedicated jump spelling and can invert IS. + cond: if jump_if_true { + match &*cond { + Expr::Compare { operands, ops } + if ops.len() == 1 + && matches!(ops[0], CmpOp::Is | CmpOp::IsNot) + && operands.len() == 2 + && operands.iter().any(|value| matches!(&**value, + Expr::Const(object) if matches!(&**object, PyObject::None))) => + { negate_cond(cond) } + _ => simplify_not_or_wrap(cond), + } + } else { cond }, + body: vec![Stmt::Continue], orelse: Vec::new(), + }); + self.skip_until = Some(target); + return; + } + } + } if self.version.at_least(3, 12) { if let Some((merged, body_end, tramp_end)) = self.try_or_join(&cond, jump_if_true, target) diff --git a/tests/fixtures/loop_lookup_guards.py b/tests/fixtures/loop_lookup_guards.py new file mode 100644 index 00000000..ed3f08d6 --- /dev/null +++ b/tests/fixtures/loop_lookup_guards.py @@ -0,0 +1,33 @@ +def rating_lookup(values): + for value in values: + if not value.rating < 10: + continue + coordinator = value.coordinator + if coordinator is None: + continue + return coordinator + + +def dictionary_lookup(rows): + for row in rows: + if not isinstance(row, dict): + continue + coordinator = row.get("coordinator") + if coordinator is None: + continue + return coordinator + + +def none_lookup(rows): + for row in rows: + if row is None: + continue + coordinator = row.coordinator + if coordinator is None: + continue + return coordinator + + +def comprehension(rows): + return [row.get("coordinator") for row in rows if isinstance(row, dict)] + diff --git a/tests/integration.rs b/tests/integration.rs index f4af5082..30354ab8 100644 --- a/tests/integration.rs +++ b/tests/integration.rs @@ -169,3 +169,25 @@ fn py314_hello() { "py3.14 hello", ); } + +#[test] +fn independent_lookup_guards_keep_assignment_and_return_inside_the_loop() { + for version in ["3.11", "3.12", "3.13", "3.14"] { + let fixture = format!("loop_lookup_guards.{version}.pyc"); + let (src, clean) = decompile_fixture(&fixture); + assert!(clean, "{fixture}: output marked incomplete:\n{src}"); + // The existing None-jump printer uses either equivalent spelling. + let src = src.replace("not coordinator is not None", "coordinator is None"); + assert_contains( + &src, + &[ + "def rating_lookup(values):\n for value in values:\n if not value.rating < 10:\n continue\n coordinator = value.coordinator\n if coordinator is None:\n continue\n return coordinator", + "def dictionary_lookup(rows):\n for row in rows:\n if not isinstance(row, dict):\n continue\n coordinator = row.get('coordinator')\n if coordinator is None:\n continue\n return coordinator", + "def none_lookup(rows):\n for row in rows:\n if row is None:\n continue\n coordinator = row.coordinator\n if coordinator is None:\n continue\n return coordinator", + "return [row.get('coordinator') for row in rows if isinstance(row, dict)]", + ], + &fixture, + ); + assert!(!src.contains("value.rating >="), "{fixture}: comparison inverted"); + } +} diff --git a/tests/pyc/loop_lookup_guards.3.11.pyc b/tests/pyc/loop_lookup_guards.3.11.pyc new file mode 100644 index 0000000000000000000000000000000000000000..3c1bfc22827fdec62dcee33edfb685169056fd57 GIT binary patch literal 1443 zcmbtUO=uHA6rR~lVw+96_AmX3KWi;WptglV5iAu{sL&sTiU*;TZFY^@rn_NgqgE2^ zK_p1PAB6NG^i(Z9c=F=OgWeO#AuM?EtlTQVMn1Qi z$*V-8 zQbS5qr88_n9C z{=BWlMh)+QmjmCEN9^P=VmVj)q7yZ!VNzQ))4CJVjI`~D8Qq4B5V0OmM+6MeN;=>S zr<-XSmZ_4hHgmnOETloW7w-=2d2Avz8dE zC&vEDBVox9;E^%Fd7K2nHv$+S$p9a{pn z`#a*BvIkHUPT65}CTmbTZEfaeADv?Z(eLRd zg)|wT$y#YOOJ_l7t0>HT3Sb*mr3Lrfr61Nm^?uQ6)ARM|c~~o5dp`GU?!{s)Hd2p` z_5&7@>qI zb5oLJ63aR_!j73uOUq~VS#lcI*s=5`KvTpRH-!@z{u4g*`4V3{KE-$uLmm7U`H4RP DgoZQh literal 0 HcmV?d00001 diff --git a/tests/pyc/loop_lookup_guards.3.12.pyc b/tests/pyc/loop_lookup_guards.3.12.pyc new file mode 100644 index 0000000000000000000000000000000000000000..8230f1de4a4268097aa63c38700f8b8e66172057 GIT binary patch literal 1195 zcmbtT&rcIU7@e8jmda8aY$-yGHb{h31Hpp_Azn1$AoL)KCM1TG-F00kyUom2Y_cRA zG${!Qa)Lw;TnShI01n=~U=?jP-aK(rdXkgh%x;^u;b7t<`}SqOAG7bxH=pA1ZV4gu zIJS3d{_yF^PvFgkLsq{6WC!W!HJV2{2E;l6B)SABm1Q-eHF9|gZ^)ne6M!A$p@-<% zFhXnC!%Og`bLn`QAjI%y65_x**BbH2I^G=eh(<7)LozBOmE=ykAyFlMl;&N_Hho2N z9I9J(-gT%d`O%m8Ldjr$M7(JPk{SQ0Y&Lr~>>A8vT86mymA9>6-P_4c2lXe3jK)Do#JZ^PSNtR}Lxo>VQJ`JNv6oF1tt zH6?wdT;<;qgVosJ?>OYwp>>#eYrMwFo4%>n69$ZJO#~w`t5l$iT1^7{;qRmW~g? z%Fw#eR_pdjPPnlz`P~+?Z06=|&G2R2(p+CM4Hp73byk@#0k#=OW3ZFo%QG#<&eL+k zn-qLk;9-*h;7uYKcq9Et4D4Qi?|w}T3ugv)%x!abc5$%BuX&gW$^b+Su0LCrNW&`1I z%~>f@Vl_mMbSvFTZCYQ@n@1>-&)gt)jFNrB=$%of!i?`qFmo{Qnay!#OFgoAveY9p??;GL3_&j;=w=2ncLg7I6$L@5k-Gd{jqjd&>+Gw^rkWI_;%`dZrQJ#^Hgg=TQ&Gx~_XsR* zUckDAfe&lKYcSCyxS!)&t!F>pZ(k=_od{QS1dDbX#IvSYwqp`F z+`Q(SvA6Ai^)ZMQgpU^(cs}kECZ&K$cx5jB|Kq*yP^jucdgCmQ2^lc z{lJ^!$n@e*^iLM4d({DS3Xp_uupmj$k|ae-lL}fT=^BQpJJX03NkURQ)nzmVNI?K* zt^(vqirm95=hvd=W(PnHGcLhg2mqt#`54TmDY{&Xxq>m&rh_mHtuTOR!V}P69?VHZ zsnf*LmTu%6HDj7As~c&{WKqRwdYWFbHSW0Oo4GNXi$jit!%oD~xW&hp_2-t&G(Ns! zn)xL(Pwf0s&Q7x|kLA}KUro5a!CEm{RL!rWDZr2R?xMTm4{ok+tZzTu^M?<-!Ahuq zKQy`%8ZD|7HB?py#5aFmxuvh_gN~jmCc+tV;#88{3#lo|O$u@gCwx<6;e^g)-5V7` zxati1B}1WgZY5!|u5*g!#6>hEpF~MJ4dU{57L`MP@AmL}Yu_I!`y-N9@4MVqZhLh%GRQ=jx*X literal 0 HcmV?d00001 From a28cd4eabf4db799140d9f3c8b0d1a5dbb562489 Mon Sep 17 00:00:00 2001 From: Francois Chagnon Date: Tue, 13 Oct 2026 18:35:36 -0400 Subject: [PATCH 2/2] Add harness coverage for the reconstruction fix --- tests/behavior/cases/b_loop_lookup_guards.py | 37 ++++++++++++++++++++ 1 file changed, 37 insertions(+) create mode 100644 tests/behavior/cases/b_loop_lookup_guards.py diff --git a/tests/behavior/cases/b_loop_lookup_guards.py b/tests/behavior/cases/b_loop_lookup_guards.py new file mode 100644 index 00000000..558b76a2 --- /dev/null +++ b/tests/behavior/cases/b_loop_lookup_guards.py @@ -0,0 +1,37 @@ +# MIN_VERSION: 3.12 + + +def rating_lookup(values): + for value in values: + if not value.rating < 10: + continue + coordinator = value.coordinator + if coordinator is None: + continue + return coordinator + + +def dictionary_lookup(rows): + for row in rows: + if not isinstance(row, dict): + continue + coordinator = row.get("coordinator") + if coordinator is None: + continue + return coordinator + + +class Rating: + def __init__(self, value): + self.value = value + def __lt__(self, limit): + print("lt", self.value, limit) + return self.value < limit + def __ge__(self, limit): + raise AssertionError("comparison must not be inverted") +class Row: + def __init__(self, rating, coordinator): + self.rating = Rating(rating) + self.coordinator = coordinator +print(rating_lookup([Row(float("nan"), 8), Row(3, None), Row(4, 0)])) +print(dictionary_lookup([1, {}, {"coordinator": False}]))