Skip to content

linq_fold: Theme 8 — three specialized fusion arms (closes audit)#2875

Merged
borisbat merged 2 commits into
masterfrom
bbatkin/linq-fold-theme8-fusion-arms
May 25, 2026
Merged

linq_fold: Theme 8 — three specialized fusion arms (closes audit)#2875
borisbat merged 2 commits into
masterfrom
bbatkin/linq-fold-theme8-fusion-arms

Conversation

@borisbat
Copy link
Copy Markdown
Collaborator

Summary

Closes Theme 8 of benchmarks/sql/linq_fold_chain_audit.md and with it the entire audit (all 8 themes shipped between 2026-05-24 and 2026-05-25 — PRs #2851, #2852, #2857, #2861, #2862, #2865, #2866, #2874, and this one).

Three small splice arms, bundled because they're independent and each is self-contained. Audit explicitly called these "low priority / cheap follow-ups."

2a — each(arr).reverse()._distinct[_by](K).to_array() (plan_reverse)

Array source only. New emission walks source backward via index (arr[len-1-k]), maintains var rev_dset : table<...> and gates push by set-insert on the dedup key. Saves cascade's reverse_to_array allocation AND the distinct_by_inplace second pass. LAST-per-key semantics preserved: backward walk picks first-seen-in-reversed-order = last-in-source occurrence, matching tier-2 reverse.distinct_by. v1 scope tight: array source required (non-array sources can't be backward-indexed); pre-reverse where_/select/take all bail to cascade.

3b — each(arr)._distinct[_by](K1)._order_by[_descending](K2).to_array() without take (plan_order_family)

The bail (distinctName != "" && takeExpr == null) relaxes to permit when firstName == "" (first/first_or_default + distinct still cascades — streaming-min path has no dset hook, deferred). The where_+order fused-loop path generalizes: when distinctName != "", declares var order_dset : table<typedecl(...)> and wraps pushExpr with a set-gated if (!key_exists(...)) block (mirrors Theme 3 Phase 3's bounded-heap distinct gate). Composes with WHERE (filter→distinct→sort) and Theme 1 terminal _select (project at return). Saves cascade's distinct_by_to_array intermediate iterator setup.

C4 — each(a) |> zip(each(b)) [|> chain_ops] |> reverse() |> <terminator> (plan_zip)

Trailing reverse as the last chain op between zip's chain and the terminator. Array lane emits _::reverse_inplace(bufName) before return; counter / accumulator (sum/min/max/avg) / any/all/contains lanes treat reverse as a no-op (mathematical identity). Bails on first / first_or_default (NOT identity under reverse). Constraint: reverse must be the last chain op — anything after would see the reversed stream and change semantics vs cascade.

What this changes

File Change
daslib/linq_fold.das +124 / -9 — three planner extensions
tests/linq/test_linq_fold_theme8_fusion_arms.das +394 — 16 tests / 16 sub-runs (positive + parity + anti-tests)
benchmarks/sql/linq_fold_chain_audit.md Theme 8 row + Status entry; classification flips on probes 2a / 3b / C4 from FALLS-OFF → SPLICE-FIRES; audit declared fully closed
doc/source/reference/linq_fold_patterns.rst +3 rows (2a in plan_reverse, 3b in plan_order_family, C4 in zip section)

Notes

  • No bench refresh. No existing bench in benchmarks/sql/ covers the 2a / 3b / C4 chain shapes (those were tier-2 cascade until this PR). Per the living-doc policy for results.md, planner-touching PRs only refresh when an existing bench exercises the shape — none does here. Follow-up TODO: add 3 benches for these shapes in a future PR.
  • Audit fully closed. This is the last theme. Future linq_fold work should come from new user-observed regressions or new probe shapes, not the existing audit.

Test plan

  • mcp__daslang__compile_check daslib/linq_fold.das — clean
  • mcp__daslang__lint daslib/linq_fold.das — no issues
  • mcp__daslang__lint tests/linq/test_linq_fold_theme8_fusion_arms.das — no issues
  • mcp__daslang__format_file tests/linq/test_linq_fold_theme8_fusion_arms.das — formatted
  • New Theme 8 test file: 36/36 pass INTERP (including parity tests vs handwritten cascades and 4 anti-tests for out-of-scope shapes)
  • All linq_fold tests pass (1144 across test_linq_fold*.das)
  • All linq-side tests pass (test_linq_sorting, test_linq_set, test_linq_querying, test_linq_join, test_linq_from_decs, test_linq_aggregation, test_linq_group_by, test_linq_transform, test_linq_bugs, test_linq_concat, test_linq_element, test_linq_generation, test_linq_partition)
  • Full tests/linq + tests/decs matrix via CI (local env has pre-existing dasbind.das missing in modules/dasLLVM/bindings/ blocking dastest.exe — unrelated to this PR; verified by reproducing on clean master)
  • Full AOT + JIT matrix via CI

🤖 Generated with Claude Code

Three small splice arms, bundled because they're independent. Closes
Theme 8 of benchmarks/sql/linq_fold_chain_audit.md and with it the
entire audit (all 8 themes shipped between 2026-05-24 and 2026-05-25).

* 2a (plan_reverse): each(arr).reverse()._distinct[_by](K).to_array()
  on array source. Backward index walk + table<K> set-gate. Saves
  cascade's reverse_to_array allocation AND distinct_by_inplace pass.

* 3b (plan_order_family): _distinct[_by] + _order_by[_descending]
  WITHOUT take. Existing where_+order fused-loop path generalized with
  var order_dset table + set-gated pushExpr (mirrors Theme 3 Phase 3
  bounded-heap gate). Composes with WHERE and terminal _select.

* C4 (plan_zip): trailing reverse as last chain op. Array lane emits
  reverse_inplace before return; counter/accumulator/any/all/contains
  lanes treat reverse as identity. Bails on first/first_or_default.

+16 tests / 16 sub-runs in test_linq_fold_theme8_fusion_arms.das,
including parity tests vs handwritten cascades and anti-tests for
each out-of-scope shape (2a-take, 3b-first, C4-first, C4-not-last).

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings May 25, 2026 19:52
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR closes Theme 8 of the linq_fold chain audit by extending _fold’s tier-1 planners with three additional specialized fusion arms, reducing fallbacks to the tier-2 cascade for a few previously uncovered chain shapes.

Changes:

  • Extend plan_reverse to splice each(arr).reverse()._distinct[_by](K).to_array() for array sources via a backward index walk and dedup set gate.
  • Extend plan_order_family to splice _distinct[_by](K1)._order_by[_descending](K2).to_array() (no take) by adding a dedup set gate to the fused-loop + in-place sort path.
  • Extend plan_zip to accept a trailing reverse() as the last chain op before the terminator, emitting reverse_inplace only for the array lane and treating it as identity for count/accumulator/any/all/contains lanes.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File Description
daslib/linq_fold.das Adds three new splice surfaces across plan_reverse, plan_order_family, and plan_zip to avoid tier-2 cascade allocations/passes for Theme 8 chain shapes.
tests/linq/test_linq_fold_theme8_fusion_arms.das Adds Theme 8 regression/parity tests plus anti-tests for out-of-scope shapes.
benchmarks/sql/linq_fold_chain_audit.md Marks Theme 8 probes as splicing and declares the audit fully closed.
doc/source/reference/linq_fold_patterns.rst Documents the newly supported Theme 8 splice patterns for reverse/distinct, distinct/order (no take), and zip+reverse.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread daslib/linq_fold.das Outdated
Comment thread daslib/linq_fold.das Outdated
Copilot R1 (2 comments): both `var rev_dset` (2a) and `var order_dset`
(3b) should be `var inscope` to match the existing bounded-heap distinct
gate (linq_fold.das:1558/1562) and the rest of this file (plan_distinct,
plan_group_by_core). Without `inscope`, the dedup table's backing
storage isn't deterministically finalized on scope exit.

Post-fix AST dump confirms both emissions now include
`var inscope <name>/*used_in_finally*/` + a `finally __::builtin\`finalize\`
block, matching the bounded-heap path's emission shape.

CI failure (`build` job, Sphinx LaTeX): `linq_fold_patterns.rst:206`
flagged 'Unknown target name: "where"' — my 2a row had `where_/select/take`
as plain text and Sphinx parses trailing-underscore as a reference
target. Wrapped each token in backticks so it parses as code, not a
reference.

Verified: local sphinx-build -W --keep-going -b latex passes clean;
36/36 Theme 8 tests still green.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.

@borisbat borisbat merged commit 53cd0f4 into master May 25, 2026
31 of 32 checks passed
pull Bot pushed a commit to forksnd/daScript that referenced this pull request May 28, 2026
…order, zip+reverse)

Adds bench files for the three splice arms PR GaijinEntertainment#2875 closed that the bench
rollup PR (GaijinEntertainment#2870) did not pick up:

- reverse_distinct_by    — audit 2a, array-only via emit_reverse_backward_walk_dset_gate
- distinct_by_order_to_array — audit 3b, array + decs via emit_fused_prefilter (no-take
                               mirror of distinct_by_order_take)
- zip_reverse_to_array   — audit C4, zip array lane (trailing reverse_inplace)

Each bench file documents the missing SQL/Decs cells inline. No daslib changes;
splice paths already verified by tests/linq/test_linq_fold_theme8_fusion_arms.das.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
pull Bot pushed a commit to forksnd/daScript that referenced this pull request May 28, 2026
New `reverse` analyze_chain branch at PHASE_DISTINCT. When wrapped by
`_distinct_by(_.K)` it flips the bare-aggregate inner subquery from
MIN(pk) to MAX(pk), picking the LAST row per K by source/PK order —
mirrors linq's `each(arr).reverse()._distinct_by(K)` "last per group"
splice from PR GaijinEntertainment#2875.

Strict gating: `reverse()` only valid when `q.distinctByCol` is already
set (i.e. the immediately-outer chain op was `_distinct_by`). Bare
`reverse() |> to_array` rejects loudly with a fixit pointing to
`_order_by_descending(...)` (SQL has no inherent row order to reverse).
Double-reverse rejects as a no-op.

3 new positive tests in test_32 (emit shape + runtime "last per K"
correctness + composition with outer ORDER BY). New rejection probe
failed_reverse_without_distinct_by.das.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@borisbat borisbat deleted the bbatkin/linq-fold-theme8-fusion-arms branch May 30, 2026 15:20
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.

2 participants