Skip to content

linq_fold: migrate to shared ast_match helpers (PR 1c)#2795

Merged
borisbat merged 1 commit into
masterfrom
bbatkin/linq-fold-shared-helpers
May 21, 2026
Merged

linq_fold: migrate to shared ast_match helpers (PR 1c)#2795
borisbat merged 1 commit into
masterfrom
bbatkin/linq-fold-shared-helpers

Conversation

@borisbat
Copy link
Copy Markdown
Collaborator

Third slice of the linq_fold / sqlite_linq / decs_boost dedup ladder (~/.claude/plans/inherited-strolling-lollipop.md). PR 1a (#2790) landed the shared helpers; PR 1b (#2794) migrated sqlite_linq; this PR retires the same backlog in linq_fold — the largest hand-rolled-probe inventory in the macro layer.

Net: -102 LOC (51 insertions, 153 deletions).

Summary

Mechanical 1:1 renames replacing seven local probes with their shared daslib/ast_match counterparts (all byte-identical implementations):

Local (deleted) Shared Callsites
fold_linq_cond peel_lambda_rename_var 39
fold_linq_cond_peel peel_lambda_replace_var 1
fold_linq_cond2 peel_lambda_rename_2vars 1
is_bind_field_access peel_tuple_field_read 7
is_each_call(call) match_call_in_module(call, "each", "builtin") != null 1 (in peel_each)

Plus dead-code cleanup — is_call_or_generic and is_call_or_generic_arg0 were leftover from the pre-cascade architecture, with zero callers anywhere in the repo. Deleted both.

Plan deviation

The plan listed is_each_call as "replaced by match_call_in_linq(expr, \"each\")", but the canonical each(array<T>) is defined in daslib/builtin.das:1548, not daslib/linq.das. The original is_each_call was module-agnostic; my first pass mechanically used match_call_in_linq which then never matched, turning peel_each into a permanent no-op and breaking test_take_skip_array_lane. Corrected to match_call_in_module(top, "each", "builtin"). This is module-gated (stricter than the original) but structurally safer — peel_each only unwraps in shapes the planner can reason about.

Why byte-identical

peel_lambda_rename_var uses the same renameVariable + apply_template recipe as fold_linq_cond; peel_tuple_field_read is the if-peel form of is_bind_field_access (per Boris's invariant: ExprRef2Value never nests, so the existing while-peel loop was over-defended; codebase-wide cleanup of these queues for PR 2). Same source at, same argName, same rename rules → same output AST.

Test plan

  • mcp lint clean on daslib/linq_fold.das
  • mcp format clean on daslib/linq_fold.das
  • Full matrix interp + AOT + JIT × {linq, decs, sqlite} — 9 lanes × 2359 tests = 21,231 test runs, all green
    • tests/linq: 1332/1332 × 3
    • tests/decs: 245/245 × 3
    • tests/dasSQLITE: 782/782 × 3
  • benchmarks/sql perf gate (7 representative benches × 5 reps × 100K rows, covering ARRAY / COUNTER / ACCUMULATOR / EARLY_EXIT / BufferGroupBy lanes):
bench m3f (5/5)
count_aggregate 4 ns/op
sum_aggregate 2 ns/op
average_aggregate 5 ns/op
first_match 0 ns/op (early-exit)
aggregate_match 6 ns/op
groupby_count 36–38 ns/op
groupby_sum 35 ns/op

All match shipping baselines (cf. recent slice PRs in master). Codegen invariant as the byte-identical migration predicts.

🤖 Generated with Claude Code

Replace seven local probes with their shared counterparts harvested
in PR 1a, retiring the largest in-file helper backlog of the dedup
plan ladder. Net effect: -102 LOC, full convergence of linq_fold's
AST-match surface on `daslib/ast_match`.

  fold_linq_cond       → peel_lambda_rename_var      (39 callsites)
  fold_linq_cond_peel  → peel_lambda_replace_var     (1 callsite)
  fold_linq_cond2      → peel_lambda_rename_2vars    (1 callsite)
  is_bind_field_access → peel_tuple_field_read       (7 callsites)
  is_each_call(call)   → match_call_in_module(call, "each", "builtin")
                                                     (1 callsite, in peel_each)

Plus dead-code removal:
  is_call_or_generic    — only consumed by is_call_or_generic_arg0
  is_call_or_generic_arg0 — no callers anywhere in the repo

Plan deviation: the dropped sites for is_each_call had been described
in the plan as "replaced by match_call_in_linq(expr, \"each\")", but
the canonical `each(array<T>)` is actually defined in `builtin`, not
`linq` — gating on the wrong module made peel_each a no-op and broke
test_take_skip_array_lane. Fixed to `match_call_in_module(..., "builtin")`.

All migrations are byte-identical: peel_lambda_* uses the same
renameVariable + apply_template recipe as fold_linq_cond*;
peel_tuple_field_read is the if-peel form of is_bind_field_access
(ExprRef2Value never nests, so the existing while-peel loop was
over-defended — the if-peel is equivalent and is the documented
shared form).

Validation:
  • full matrix interp + AOT + JIT × {linq, decs, sqlite}
    = 9 lanes × {1332 + 245 + 782 = 2359} = 21231 test runs, all green
  • benchmarks/sql perf gate (7 representative benches × 5 reps,
    covering ARRAY / COUNTER / ACCUMULATOR / EARLY_EXIT / BufferGroupBy):
    - count_aggregate    m3f 4 ns/op (5/5)
    - sum_aggregate      m3f 2 ns/op (5/5)
    - average_aggregate  m3f 5 ns/op (5/5)
    - first_match        m3f 0 ns/op (5/5, early-exit)
    - aggregate_match    m3f 6 ns/op (5/5)
    - groupby_count      m3f 36-38 ns/op
    - groupby_sum        m3f 35 ns/op (5/5)
    all match shipping baselines — codegen invariant.
  • mcp lint: clean

Plan reference: ~/.claude/plans/inherited-strolling-lollipop.md (PR 1c)

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings May 21, 2026 17: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

Migrates daslib/linq_fold.das off its locally duplicated AST-probe helpers and onto the shared implementations in daslib/ast_match, reducing duplicated macro-layer logic and keeping the linq folding planner consistent with other migrated consumers.

Changes:

  • Replaced local lambda-peel helpers with shared peel_lambda_* equivalents from daslib/ast_match.
  • Replaced local tuple-field probe with shared peel_tuple_field_read.
  • Replaced local is_each_call with match_call_in_module(..., "each", "builtin") in peel_each, and removed dead helper code.

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

@borisbat borisbat merged commit cf7ead7 into master May 21, 2026
33 checks passed
pull Bot pushed a commit to forksnd/daScript that referenced this pull request May 21, 2026
… H3 in plan_zip (PR 2a)

Slice 2a of the linq_fold dedup ladder (PR 2 reshaped):
- extend_specs_for_missing_having_reducers + rewrite_having_pred: hand-rolled while-peel loops replaced with calls to ast_match's qm_peel_ref2value, the same helper qmatch emits at every generate_match dispatch. Single source of truth for ExprRef2Value peel semantics across the macro layer.
- plan_zip projection elementType extraction: 11-line manual ExprMakeBlock → ExprBlock → ExprReturn → subexpr unwrap collapsed to one call to peel_lambda_single_return (H3, already shared via PR 1a).

The plan's "qmatch adoption" goal (items 1-3 in PR 2) reality-checked into noise after PR 1c — the shared helpers (peel_tuple_field_read, peel_lambda_single_return) are already shorter than the qmatch equivalents for the specific shapes linq_fold matches, and the recursive RTTI descents (extend_specs_for_missing_having_reducers walking ExprCall/Field/Op1/Op2/Op3) aren't a qmatch shape at all. Item 5 (ExprRef2Value while→if in qm_peel_ref2value itself) is deferred: tests/ast_match/test_ref2value_skip.das:test_nested_wrappers asserts triple-wrap peel as a future-proofing guard, and block-folding in ast_block_folding.cpp:44 could synthesize a nested wrapper. Keep the loop, route linq_fold through it.

Net: -15 LOC. Lint clean.

Validation:
- ast_match 371/371, linq 1332/1332, decs 245/245, dasSQLITE 782/782 (interp).
- ast_match 371/371, linq 1332/1332, decs 245/245, dasSQLITE 782/782 (AOT).
- ast_match 371/371, linq 1332/1332, decs 245/245 (JIT). test_capture_cfb.das pre-existing JIT failure on master (unrelated to this PR).
- 7 representative SQL benches (count/sum/average/aggregate_match/groupby_count/zip_dot_product/distinct_count): all m3f within rounding noise of PR GaijinEntertainment#2795 baselines.

Next: PR 3 (mechanical wins — qname sweep, push_block_list adoption, finalize_emission collapse, inline-collapse of $b(localVec) sites).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@borisbat borisbat deleted the bbatkin/linq-fold-shared-helpers branch May 30, 2026 15:21
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