#49 Enhance bytecode decompiler with slices and tuples - #50

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@pycrefine.py`:
- Around line 2332-2335: The dispatch maps "CALL_FUNCTION_KW" to _op_call but
_op_call only checks for "CALL_KW"/"kwnames" in argval, so update _op_call to
recognize CALL_FUNCTION_KW: detect instr.opname == "CALL_FUNCTION_KW" (or that
argval is an int opcode arg) and handle it by treating argval as oparg and
popping the kwnames tuple from the stack (same logic used for keyword calls), or
alternatively add a small branch in _op_call to reconstruct keyword args when
instr.opname == "CALL_FUNCTION_KW"; ensure the keyword-detection logic
references the instruction's opname/arg and not just argval contents so Python
3.10 keyword calls are handled correctly.
- Around line 5287-5296: The fallback for the STORE_FAST_STORE_FAST case
currently emits hardcoded "var1, var2 = ..." which produces incorrect
reconstructed code when instr.argval isn't a tuple; change the else branch in
the STORE_FAST_STORE_FAST handler to use instr.argrepr (which contains the
original target text like "(foo, bar)") instead of literal placeholders and emit
a commented or noted fallback via self._append_reconstructed (e.g., something
like f"# fallback targets {instr.argrepr} = {expr}" or include instr.argrepr
directly in the assignment) so the decompiler shows the real target names when
argval cannot be unpacked; update the code path that reads instr.argval and
calls self._append_reconstructed to reference instr.argrepr rather than
hardcoded names.

In `@tests/test_pycrefine.py`:
- Around line 4588-4625: The assertions in tests test_api_21_ternary_in_modulo,
test_api_22_ternary_modulo_is_none, and test_api_23_tuple_unpack_and_ternary are
brittle: stop globally stripping parentheses and either assert on stable,
smaller substrings or use regex to tolerate optional surrounding
parentheses/whitespace. Update the checks that currently call out.replace("(",
"").replace(")", "") to instead use either assertRegex/out contains a targeted
regex (e.g. match the ternary pattern "not found' if in_.* is None else 'reset"
or "in_a if 'value:' not in in_a else in_a[6:]") or assertIn on a shorter stable
fragment like "if 'value:' not in in_a else in_a[6:]" and "'not found' if in_b
is None else 'reset'"; keep the other assertions (e.g. "'post: %s'", "in_a, in_b
= api_21(None)", "return in_a is not None") unchanged. Ensure changes are made
in the tests named test_api_21_ternary_in_modulo,
test_api_22_ternary_modulo_is_none, and test_api_23_tuple_unpack_and_ternary and
reference the decompile(...) output variable `out`.

---

Nitpick comments:
In `@pycrefine.py`:
- Around line 1467-1471: The if/else branch around building the index expression
is redundant because both branches do the same append; in the block that handles
slice-notation strings (related to BUILD_SLICE) simplify by removing the
conditional and always do mini_stack.append(f"{obj}[{key}]")—locate the code
using the variables/key names key, obj, mini_stack (and the mention of
BUILD_SLICE) and delete the entire if/else, replacing it with the single append
statement.

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@pycrefine.py`:
- Around line 4501-4507: The code currently only parenthesizes the right operand
(r_str) when it contains a ternary before forming the binary expression with
_bin39[opname]; update the logic to also detect and parenthesize the left
operand (the variable left used in the final f-string) when it contains ' if '
and ' else ' so that both sides are protected from conditional-expression
precedence; apply the same change at the other analogous spot (the block around
the other stack append where left/right and r_str are used) so both occurrences
handle ternary on left and right consistently.
- Around line 770-789: The ternary matcher loop currently checks ins.opname
against _TERNARY_CJUMPS using startswith(), which misses Python 3.11 directed
opcodes (e.g. POP_JUMP_FORWARD_IF_NONE); replace that ad-hoc check with the
existing helper _is_compound_cjump to detect compound/ternary conditional jumps
reliably across versions (call self._is_compound_cjump with the
instruction/opname in the loop where the current any(... _TERNARY_CJUMPS ...)
check occurs, keeping _STACKSINK_OPS and the rest of the logic unchanged).
- Around line 2618-2620: The decompiler currently falls through when handling
the UNPACK_SEQUENCE 1 case (where len(store_targets) == n == 1 and self.stack)
and emits a plain assignment, losing the exact-length validation; update the
branch that handles this case so it emits the singleton-unpacking form by
formatting the target and expression as "{store_targets[0]}, = {expr}" (i.e.,
add the trailing comma) and then advance self.pc to look just like the
multi-target branch does; modify the code path that checks len(store_targets) ==
n and n == 1 to produce that unpacking string and increment self.pc to look
while leaving the rest of the STORE_* handling unchanged.

Verify each finding against the current code and only fix it if needed.

Duplicate comments:
In `@pycrefine.py`:
- Around line 5126-5131: The emitted code currently only parenthesizes the right
operand's ternary expression (r_str) before pushing f"({left} {bin_op}
{r_str})", leaving the left operand vulnerable; update the logic in the
BINARY_OP handling to detect a conditional expression in the left operand (e.g.,
check if ' if ' and ' else ' appear in str(left), similar to the right-side
check) and wrap left in parentheses when present so you push f"({left_wrapped}
{bin_op} {r_str})" (ensure you reuse the same ternary-detection logic and
variable names like left, right, r_str, bin_op, and self.stack).

Verify each finding against the current code and only fix it if needed.

Outside diff comments:
In `@pycrefine.py`:
- Around line 3266-3321: The _op_call implementation must special-case
CALL_FUNCTION_EX before the generic arg-count logic: detect if instr.opname ==
"CALL_FUNCTION_EX", treat instr.arg as a flags bitmask (bit 0 signals presence
of **kwargs) rather than a count, then pop items from self.stack in the
CALL_FUNCTION_EX order (if flags&1: pop kwargs dict first, then args tuple, then
func; else pop args tuple then func), convert the args tuple elements and kwargs
dict items into the same string format used elsewhere (handling ("func","...")
or ("class","...") tuple placeholders), and construct final_args as "func(*args,
**kwargs)" / "func(*args)" accordingly before pushing the reconstructed call; do
not use num_args for this opcode. Ensure this logic lives in _op_call and runs
before the existing CALL_KW / generic argument-handling branches.

---

Duplicate comments:
In `@pycrefine.py`:
- Around line 5221-5226: The left-hand side of binary ops in the 3.11+ path
isn't parenthesized when it's a ternary, so create a string for the left (e.g.,
l_str = str(left)), mirror the right-side check (if ' if ' in l_str and ' else '
in l_str: l_str = f'({l_str})'), and then use l_str and r_str in the
self.stack.append call instead of raw left; i.e., update the block that
currently builds r_str and calls self.stack.append(f"({left} {bin_op} {r_str})")
to parenthesize left ternaries too.

In `@tests/test_pycrefine.py`:
- Line 4594: Replace the brittle exact-string assertion
self.assertIn("print(in_a if 'value:' not in in_a else in_a[6:])", out) with a
regex-based check (e.g., self.assertRegex or re.search) against out so the test
tolerates harmless whitespace and single/double-quote normalization; craft the
regex to match the print(...) call semantics (allow optional spaces, accept
either ' or " around value:, and the conditional form in_a if ... else in_a[6:])
and update imports if necessary.

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@pycrefine.py`:
- Around line 3287-3318: The CALL_FUNCTION_EX branch currently returns after
building the call string and thus bypasses the later tuple-placeholder handling
that converts internal ("func", ...) or ("class", ...) tuples into their source
rendering; update the CALL_FUNCTION_EX handling (the code that processes
kwargs_dict, args_tuple and func_val) to reuse the existing tuple-call rendering
logic used elsewhere by invoking that same conversion on func_val, args_tuple
and kwargs_dict (i.e., normalize tuple placeholders into their source form
before forming call_args) so that calls like (lambda...)(*args) or comprehension
helpers produce the correct source instead of leaking raw tuples; ensure you
apply this reuse where func_val, args and kwargs are assembled before the final
self.stack.append and return.
- Around line 2621-2664: The lookahead in _op_unpack_sequence can fail and
currently returns without fixing the decompiler stack, leaving the original
iterable (or nothing) and corrupting subsequent STORE_* handling; update
_op_unpack_sequence so that when len(store_targets) != n (i.e. the collapse
fails) you consume the original iterable value from self.stack (pop it) and push
n placeholder/unpacked entries onto self.stack so downstream
STORE_FAST/STORE_NAME/STORE_DEREF handlers will pop the correct number of
values; modify the failure path in _op_unpack_sequence to adjust self.stack
accordingly (refer to the local variables n, store_targets, self.stack and
method name _append_reconstructed for context).
- Around line 2357-2360: The CALL-family handlers (e.g., _op_call referenced in
the mapping) currently ignore Python 3.11's separate KW_NAMES opcode; implement
a KW_NAMES handler that saves the tuple of keyword names into an instance field
(e.g., self._pending_kw_names), then update _op_call (and other call handlers:
CALL_FUNCTION, CALL_FUNCTION_KW, CALL_FUNCTION_EX, CALL_METHOD) to check for and
consume that stored kw name tuple when reconstructing call arguments (associate
names with the trailing positional/keyword values and then clear
self._pending_kw_names), and apply the same change where calls are handled in
the other region noted (around the code referenced at lines ~3320-3343) so
keyword calls like f(x=1) are reconstructed correctly.

---

Nitpick comments:
In `@pycrefine.py`:
- Around line 663-665: The current condition adds a blank separator before any
block keyword even when nested; change it to only add the blank when the block
is top-level by checking indentation/leading whitespace. Update the condition
that uses line.startswith(("def ", "class ", "if ", "for ", "while ", "try:"))
to also verify the line has no leading spaces/tabs (e.g., compare line to
line.lstrip() or check first character is not whitespace) before appending "" to
self.reconstructed so nested blocks won't get the top-level separator.