Skip to content

lowering: accept a destructuring element parameter in the fold-loop pass - #90

Merged
simontreanor merged 1 commit into
mainfrom
fix/fold-destructuring-elem
Aug 30, 2026
Merged

lowering: accept a destructuring element parameter in the fold-loop pass#90
simontreanor merged 1 commit into
mainfrom
fix/fold-destructuring-elem

Conversation

@simontreanor

Copy link
Copy Markdown
Owner

What

The in-place fold pass (DESIGN.md §5.1) rejected any folder whose element parameter was a pattern rather than a name, so List.fold (fun m (p, l) -> Map.add p l m) Map.empty steps fell back to _pf_fold (a fresh copy per step) and went quadratic, while the fst/snd spelling of the same folder was linear.

Only the accumulator parameter is substituted by name; the element parameter is just the loop target. This PR accepts any irrefutable element pattern:

  • PyStmt::For now carries a PyForTarget (a name or a nested tuple of names), so a tuple element emits Python's own header: for (p, l) in steps: / for (a, (b, c)) in xs: / for (p, _) in steps:.
  • Any other irrefutable shape binds a fresh temp and unpacks on the first line of the loop body through the existing unpack_into_as (the same machinery a destructuring def parameter uses). Not reachable from the parser today, which admits only names, wildcards and tuples as parameters, but the emit path is covered by a unit test.
  • The pattern's bound names count as body binders for the P8 collision check, so let clash p = … List.fold (fun m (p, l) -> …) still falls back.
  • The accumulator parameter (params[0]) keeps its existing name-only rule; PYFUN_NO_FOLD_OPT and the rejected path are byte-identical.

Top-level and block-local named folders (let add m (p, l) = …) get the same treatment, and it composes with the tuple accumulator.

Tests

tests/compile.rs: lambda, top-level named, block-local named, nested tuple, wildcard, tuple-accumulator composition, the P8 clash fallback, a destructuring accumulator still falling back, and an e2e check that the destructuring and fst/snd spellings both take the loop path and agree. src/lowering/fold_loop.rs: unit tests for loop_target.

cargo test, cargo clippy --all-targets, cargo fmt --check all clean.

Closes #85

A folder that names its element by destructuring (`fun m (p, l) -> Map.add
p l m`) was rejected by the in-place fold pass and fell back to `_pf_fold`,
which copies the accumulator every step and goes quadratic. Only the
accumulator parameter is substituted by name; the element parameter is the
loop target, so any irrefutable pattern works there.

`PyStmt::For` now takes a `PyForTarget` (a name or a nested tuple of names),
so a tuple element emits Python's own `for (p, l) in steps:` header. Any
other irrefutable shape binds a temp and unpacks on the first body line via
the existing `unpack_into_as`. The pattern's bound names feed the P8
collision check as body binders. The accumulator parameter keeps its rules.
simontreanor added a commit that referenced this pull request Aug 30, 2026
…after #90, #91 and #93

Resolves the overlap between the lookup peephole (#93) and the Option/Result
ladder (#94): both hooks run in the match lowering, lookup first since it
needs no Option at all; the two new list helpers return the _None_ singleton;
tests whose scrutinee both passes rewrote now expect the combined output.
@simontreanor
simontreanor merged commit 97c3594 into main Aug 30, 2026
16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

A fold whose folder destructures its element is quadratic: the in-place pass rejects it

1 participant