linq_fold: route ExprRef2Value peels through qm_peel_ref2value + adopt H3 in plan_zip (PR 2a)#2796
Merged
Merged
Conversation
… 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 #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>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR continues the linq_fold deduplication work by replacing ad-hoc AST unwrapping logic with shared helpers from daslib/ast_match, reducing duplicated “peel” semantics and keeping shape-handling consistent across the macro layer.
Changes:
- Replaced local
ExprRef2Valuepeeling loops withqm_peel_ref2valuein HAVING-related walkers/rewrites. - Simplified
plan_zipprojection element type extraction by usingpeel_lambda_single_returninstead of manual block/return unwrapping.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+4683
to
+4685
| var projRet = peel_lambda_single_return(cll._0.arguments[1]) | ||
| if (projRet == null || projRet._type == null) return null | ||
| var projElemType = clone_type(projRet._type) |
6 tasks
pull Bot
pushed a commit
to forksnd/daScript
that referenced
this pull request
May 22, 2026
…inalize_decs_emission extract, qn sweep
- A1: collapse `finalize_emission` into `finalize_emission_stmts`. Single
caller (`plan_reverse`) now extracts its qmacro_block body via
`push_block_list` before delegating. -8 LOC.
- A2: extract `finalize_decs_emission(emission, at, wrapToIter)` helper.
Three callers (`plan_decs_order_family`, `plan_decs_reverse`,
`plan_decs_distinct`) consolidate the `force_at + force_generated +
conditional iter wrap` tail into a single call. The two wrap conditions
(`needIterWrap && returnsArray` for order_family, `needIterWrap &&
needBuffer` for distinct) merge into a pre-multiplied `wrapToIter`
boolean at the call site. -9 LOC net.
- qname sweep: 124 of 132 `"`{prefix}`{at.line}`{at.column}"` sites
collapse to `qn("prefix", at)`. The 8 remaining sites carry an extra
backtick-segment after `{at.column}` (e.g. `{length(preCondStmts)}`,
`{spec.slot}`) that doesn't fit `qn`'s signature; left as-is.
- Category A inline-collapse / Category B push_block_list adoption:
SKIPPED after fresh audit. The plan's targeted sites have shifted to
conditional-push shapes (after prior slices) that no longer fit clean
inline-collapse. Defer to a future cleanup if it surfaces again.
Codegen invariant: `qn()` returns the byte-identical string as the inline
form, so all AOT baselines pass without refresh. Bench smoke 7 reps × 7
benches: m3f figures byte-identical to PR GaijinEntertainment#2796 baselines (count 4 / sum 2
/ average 5 / aggregate_match 6 / groupby_count 36 / zip_dot_product 7 /
distinct_count 15 ns/op).
Validation matrix (9 lanes):
- Interp: linq 1332/1332, decs 245/245, ast_match 371/371, dasSQLITE 782/782
- AOT: same
- JIT: same modulo test_capture_cfb.das pre-existing failure on master
Net: -16 LOC (139 ins / 155 del). MCP lint + CI lint + format all clean.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Slice 2a of the linq_fold dedup ladder (PR 2 reshaped after grounding). Two small dedups in
daslib/linq_fold.das, noast_match.daschanges:extend_specs_for_missing_having_reducers+rewrite_having_pred— hand-rolledwhile (expr is ExprRef2Value) { expr = (expr as ExprRef2Value).subexpr }loops replaced with calls toqm_peel_ref2valuefromdaslib/ast_match, the same helperqmatchemits at everygenerate_matchdispatch. Single source of truth forExprRef2Valuepeel semantics across the macro layer.plan_zipprojection elementType extraction — 11-line manualExprMakeBlock→ExprBlock→ExprReturn→subexprunwrap collapsed to one call topeel_lambda_single_return(H3, already shared via PR 1a).Net: -15 LOC (5 ins / 20 del). Lint clean.
Plan deviation: qmatch adoption scope shrank
The plan envisioned qmatch conversion of
peel_tuple_field_readinternals + HAVING-clause probes +extract_decs_bridgecascades +plan_zipprojection. Reality after grounding:peel_tuple_field_readlives inast_match.daswhich defines qmatch — circular reference prevents qmatch usage in the same file.peel_tuple_field_read(H9 from PR 1c), which is shorter than the qmatch equivalent (qmatch(c.args[0], $e(rec)._1).matched && rec is ExprVar && (rec as ExprVar).name == hbName).extend_specs_for_missing_having_reducersrecursive descent is generic RTTI dispatch on ExprCall/ExprField/ExprOp1/ExprOp2/ExprOp3 — qmatch is for shape matching, not for "is this any ExprOp2" recursion.extract_decs_bridgeis block-shape matching with semantic cross-statement constraints (3 statements with specific types,pushtarget var name matchesresdeclared in stmt 0,recordNamescount matchesfor.sourcescount) — beyond qmatch's pattern grammar.The genuine wins from PR 2 reduce to (a) routing the two
linq_foldwhile-peels throughqm_peel_ref2valuefor single source of truth, and (b)plan_zipprojection extraction via the existing H3 helper.Item 5 (ExprRef2Value while→if in
qm_peel_ref2valueitself) is intentionally deferred.tests/ast_match/test_ref2value_skip.das:test_nested_wrappersasserts triple-wrap peel as a future-proofing guard, andsrc/ast/ast_block_folding.cpp:44could synthesize a nested wrapper (it rewraps after moving throughExprCast, and the inner subexpr could itself be anExprRef2Value). The "never nests" claim in the plan is unverified for the synthetic case — keep the loop, route consumers through it.Test plan
daslib/linq_fold.daslint clean (MCP).tests/ast_match371/371 ·tests/linq1332/1332 ·tests/decs245/245 ·tests/dasSQLITE782/782.tests/ast_match371/371 ·tests/linq1332/1332 ·tests/decs245/245 ·tests/dasSQLITE782/782. (test_extract_const_string.das.cppwas missing from baseline → regenerated, committed via cmaketest_aot_ast_matchrebuild — all other AOT stubs "Content is same", confirming no codegen drift.)tests/ast_match371/371 (modulotest_capture_cfb.daspre-existing JIT failure on master, verified by stash+revert) ·tests/linq1332/1332 ·tests/decs245/245.plan_zipprojection path — invariant codegen confirmed.Next: PR 3 (mechanical wins — qname sweep,
push_block_listadoption,finalize_emissioncollapse, inline-collapse of$b(localVec)sites).🤖 Generated with Claude Code