Bug triage: nine audit rulings fixed red-first, the temp-string AOT desync, and the gen1_to_gen2 rename - #3662
Conversation
The grammar has accepted `def operator []<-` since the index-operator family landed, but no infer path ever constructed the name - `a[i] <- v` on a user type could never reach it. visit(ExprMove) now tries []<- first and falls back to promoting a plain `operator []` ref, mirroring the []= path in promoteAssignmentToProperty; the LHS ExprAt is marked underClone in preVisit so it survives to the dispatch. Docs already promised the operator (functions.rst, tutorial 32); functions.rst additionally gains the signature example showing the `var` RHS parameter that takes the move. Tests: free form, method form, and the operator-[]-ref fallback in tests/language/operators.das. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…rface In-place string mutation has no place in the land of immutable strings - and the pair was registered SideEffects::none while mutating its argument, so the optimizer was licensed to fold calls over literals into no-ops. Zero users in tree, daslib, or external checkouts; the C++ helpers stay (uriparser and ast_parse call them internally). to_lower/to_upper (the allocating forms) are the surface. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… to a pointer LambdaIterator hands the lambda the iteration-slot address, so only a reference parameter can write the element address into the slot. The declared signature took the pointer BY VALUE - the user's `a = addr(elem)` wrote a dead local, the slot stayed memset-zero, and the ref-iterator dereferenced null (AV in SimNode_ForWithIterator). Ref generators always worked because their lowering builds the yield argument as pointer-with-ref (ast_infer_type_make.cpp) - the exact shape the signature now demands. Signature is now `(var arg : auto(argT)? &)`; a `==&` trap overload turns the old crash shape into concept_assert error 31400 naming the required signature. Tests cover the user-lambda path and the generator lowering (both proving genuine refs by writing through the iterated element) plus the rejection. Also ASCII-fixed two em-dashes in each_kv assert messages (STYLE039). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
PRAGMA table_info omits generated columns, so every @sql_computed table failed its own check_schema on the column-count gate - the startup defense panicked on tables create_table itself had just made - and schema_from silently dropped the columns. The reader now uses table_xinfo (virtual-table hidden columns filtered out), carries `generated` through SchemaFromCol, and _sql_column_info carries `is_computed`. try_check_schema: counts compare against the ordinary-column floor and all-column ceiling (a struct may omit generated columns - the schema_from shape); computed-ness is cross-checked both ways; nullability/PK checks skip computed pairs (DDL forbids NOT NULL and PK on generated columns); uncovered ordinary columns are reported by name. For generated-free schemas the new gates are equivalent to the old ones - existing diagnostics preserved verbatim (test_98 untouched and green). schema_from: generated columns get no synthesized field (the DB does not store the expression; a plain field would break INSERTs); hand-declaring one without @sql_computed is a macro error. Live-consumer audit: dasllama-server has no sql usage (main.das compiles clean); the dictation bot's six startup check_schema calls are all-ordinary schemas (behavior identical, its suite green). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…uts untouched Every capture kind ($e $v $i $t $c $f $b $a) wrote the user's variable the moment the generated matcher walk reached it, so a failing guard later in the ladder left an unpredictable subset of bindings overwritten - and a failed qmatch could clobber bindings an earlier successful match had established. das_macros.md documented the hazard; now the promise "filled only on success" is the implementation. Captures stage into generated temps (declared at the matcher top, written during matching - including inside the qm_scan inner closure) and commit into the user's variables only after the last guard has passed. One staged slot per variable via qm_stage: the folded/unfolded const-constructor dual arm reuses it, and copy-init makes an arm that never writes its temp commit the user's own value back. The direct-match early-success path emits cloned commits of its own. tests/ast_match/test_qmatch_no_bind_on_fail.das pins one leak shape per capture kind (10 were red before the fix) plus success-through-scan and direct-path commits. Consumers green: ast_match 392, linq 2007, flatten 277, dasSQLITE 914, sql_conformance 94, language 1539, apply 27. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The staged temps for $b/$a captures are real das-heap array buffers; a guard failure returns before any commit, so without cleanup they sat until the next gc. Capture state now travels as one QmCaptures struct (decls/commits/cleanups/temps) and the generated matcher wraps body+commits in a finally that clears (elements are gc-owned clones) and deletes each array temp - running on guard-failure early returns and as a no-op after a success commit moves the buffer out. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
select's declared return type is dependent - array<typedecl(result_selector(type<TT>))> - and only collapses once its fresh generic instance finishes inferring. LinqFold gated on _type != null alone, so the wrapping form fired inside that window and baked the unresolved typedecl into the emitted buffer decl (error[30341] result_selector at the call site); the substitution is permanent, so no later pass could repair it. The dot form chain._fold() dodged it by accident: the ExprField->ExprCallMacro promotion returns the new node, so the macro first runs a pass later, after the instance resolved. Fix is the standard call-macro deferral idiom (same as _where/_sql): macro_verify !isAutoOrAlias, a transient error that evaporates on rerun. tests/linq/test_linq_fold_wrap_defer.das pins array-head and iterator-head wrap forms. It is deliberately a minimal standalone file: the window opens only for a FRESH select instantiation, so any other chain in the program warming the same instance masks the regression (in test_linq_fold.das the same test passes even without the fix). Suites: linq 2010, flatten 277, dasSQLITE 914, sql_conformance 94. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Only the file-path rails (./x.das, %/root.das, project module_get) set ModuleInfo::importName, and ast_requireModule gated alias registration on it — so the daslib / builtin / same-directory spellings parsed the `as` clause and silently dropped it, dying later with a misleading 30341 at the alias use site. An explicit `as` now registers unconditionally; the importName gate keeps guarding only the implicit path-stem registration. This also makes the 20510 duplicate-alias check reachable for non-path forms. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…t discarded
InferTypes never marked string-builder elements as consumed, so any
[nodiscard] call inside "{...}" tripped a false error[30166] - in call
arguments (print("{f()}")) and let-inits alike. The new
preVisitStringBuilderElement override runs markNoDiscard on every
element; a genuinely discarded statement-level call still errors
(pinned by failed_nodiscard.das).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Two different tools shared one name: the cmake das-fmt target (utils/dasFormatter, the gen1->gen2 syntax converter) and the source formatter utils/das-fmt/dasfmt.das (also compiled to bin/das-fmt.exe by CI). An SDK user typing das-fmt to format a file got their syntax converted instead. The converter is now gen1_to_gen2 everywhere: cmake target + install, run_utils_tests, extended_checks build targets, bundle smoke EXE presence, shipped-skills exe regex, usage text, MCP convert_to_gen2 exe path, and the skills/CLAUDE.md name-trap notes. The formatter keeps the das-fmt name. Also drops three dead get_target_property(DAS_FMT_*) lines nothing consumed. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…exe-rail DLL path Four fixes surfaced by the full preflight run of this branch: - daslib/ast_match.das: the 16 qmatch template-line nolints take the both-worlds ,LINT019 spelling - LINT004/PERF030 fire only in downstream instantiations (cloned template exprs carry this file's LineInfo), so standalone lint saw them as stale. - utils/mcp/registry_das.das: nolint:STYLE038 on build_das_tools (flat tool-registration table, the canonical irreducible shape). - each_ref docs: the signature change minted a new handmade-doc hash; fill the new stub, drop the orphaned old-hash file, and give the qm_tmp_* capture-staging helpers a das2rst group. - utils/preflight/main.das: the exe rail's temp-path das-lint is dynamically linked against libDaScriptDyn, which the loader cannot resolve from the system temp dir - prepend the daslang bin dir to the loader's environment (PATH / LD_LIBRARY_PATH / DYLD_LIBRARY_PATH) before spawning. Broke on any DLL-flavor local build since the rail artifacts moved to a unique temp path. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…o longer depends on the driving program The temp-string wrapper pass (MarkTempStrings / WrapLetTempStrings) fired only when the driving program had persistent_heap on and intern_strings/disable_temp_string_reclaim off. The pass mutates function bodies - shared-module ASTs included - so the same daslib function compiled to different trees (and different AOT hashes) depending on who compiled it first: macro-context compiles run with macro_context_persistent_heap and wrapped shared daslib functions (dastest links json under the macro module json_boost), while AOT stub generation compiled them bare. Every interpolation-heavy daslib module then failed AOT linking with error 50101 - 162 failures across json/jsonrpc/clargs/logger/sql in the full AOT sweep, invisible to per-PR CI (which only builds test_aot_subset). The wrapper is now always inserted; the heap modes move to runtime, where they belong per the entry-program-options model: freeTempString already no-ops for interned heaps, linear-heap frees are safe bump-retreat no-ops, and disable_temp_string_reclaim becomes a runtime flag on the string heap (set at simulate from the entry program) instead of a compile-side gate that poisoned hashes. The red evidence is the AOT sweep itself (cross-process by nature); the new tests-cpp case pins the in-process half of the invariant - the same module function hashes identically under differently-optioned drivers. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR is a bug-triage follow-up that fixes multiple compiler/daslib/runtime issues found during the docs arc, adds red-first regression coverage, resolves an AOT hash determinism bug tied to temp-string wrapping, and removes tooling ambiguity by renaming the gen1→gen2 converter binary.
Changes:
- Fixes compiler/operator/require-aliasing and
[nodiscard]interpolation handling, with new language regression tests. - Fixes daslib behaviors (qmatch transactional captures, linq
_fold(chain)inference deferral) and SQLite schema introspection for GENERATED/@sql_computed columns, with targeted tests. - Makes temp-string wrapping driver-independent (AOT hash determinism), and renames the converter
das-fmt→gen1_to_gen2across tooling/CI/docs/MCP.
Reviewed changes
Copilot reviewed 61 out of 61 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| utils/preflight/main.das | Prepends daslang bin dir to loader search path before running temp-path das-lint -exe. |
| utils/mcp/tools/convert_to_gen2.das | Updates MCP convert tool to call gen1_to_gen2 and renames helper. |
| utils/mcp/registry_das.das | Updates tool registry text and adds STYLE038 nolint to tool table. |
| utils/mcp/README.md | Documents convert_to_gen2 as using gen1_to_gen2. |
| utils/dasFormatter/Readme.md | Updates converter usage examples to gen1_to_gen2. |
| utils/dasFormatter/main.cpp | Updates converter help text to gen1_to_gen2. |
| utils/CMakeLists.txt | Updates utils tests to depend on/run gen1_to_gen2 --tests. |
| tests/strings/test_cpp_functions.das | Removes in-place case conversion from covered C++ string functions list; adds STYLE038 nolint. |
| tests/README.md | Updates tests index text for test_cpp_functions.das. |
| tests/linq/test_linq_fold_wrap_defer.das | Adds regression test for _fold(chain) deferral until type resolves. |
| tests/language/require_as.das | Adds coverage that require X as Y aliases work for all require forms. |
| tests/language/require_as_fixture.das | Adds same-dir fixture module for require-as tests. |
| tests/language/operators.das | Adds tests for []<- move-store operator dispatch (free-form + method + ref-return). |
| tests/language/nodiscard.das | Adds regression test that [nodiscard] results inside interpolation are “consumed”. |
| tests/language/failed_require_as.das | Negative test for alias collision (20510). |
| tests/language/failed_nodiscard.das | Negative test for genuinely-discarded [nodiscard] call. |
| tests/language/failed_each_ref.das | Negative test that wrong each_ref lambda shape is rejected with 31400. |
| tests/language/each_ref.das | Positive tests for corrected each_ref signature and generator lowering. |
| tests/dasSQLITE/test_check_schema_computed.das | Adds SQLite tests covering GENERATED columns and struct/DB computed-ness rules. |
| tests/ast_match/test_qmatch_no_bind_on_fail.das | Adds extensive regression coverage for transactional qmatch captures across all tags. |
| tests-cpp/small/test_temp_wrap_hash_persistent.das | Persistent-heap entry program for hash determinism test. |
| tests-cpp/small/test_temp_wrap_hash_default.das | Default-heap entry program for hash determinism test. |
| tests-cpp/small/test_temp_wrap_fixture.das | Fixture module whose function hash must be driver-independent. |
| tests-cpp/small/test_temp_string_wrap_determinism.cpp | New doctest ensuring function hash is driver-independent across driver options. |
| src/parser/parser_impl.cpp | Fixes require ... as ... alias registration to apply to all require forms. |
| src/builtin/module_builtin_string.cpp | Removes to_lower_in_place / to_upper_in_place from the das surface. |
| src/ast/ast_simulate.cpp | Moves disable_temp_string_reclaim handling to runtime heap configuration. |
| src/ast/ast_infer_type.cpp | Marks string-builder interpolation elements as “consumed” for [nodiscard]. |
| src/ast/ast_infer_type_op.cpp | Adds []<- operator dispatch path to ExprMove on ExprAt LHS. |
| src/ast/ast_allocate_stack.cpp | Makes temp-string wrapping pass unconditional to avoid AOT hash desync. |
| skills/strings.md | Updates guidance to drop in-place case conversion API. |
| skills/sql.md | Updates schema checking guidance to include GENERATED/@sql_computed behavior. |
| skills/preflight.md | Updates “name trap” documentation for converter vs formatter. |
| skills/mcp_tools.md | Updates MCP tool docs to reference gen1_to_gen2. |
| skills/make_pr.md | Updates converter vs formatter naming note to gen1_to_gen2. |
| skills/daslang/references/macros.md | Documents transactional qmatch capture semantics. |
| skills/das_macros.md | Updates qmatch capture semantics (transactional on failure). |
| skills/das_formatting.md | Updates converter vs formatter warning to reflect gen1_to_gen2. |
| modules/dasSQLITE/PROVIDER_CONTRACT.md | Updates provider contract to specify table_xinfo behavior. |
| modules/dasSQLITE/daslib/sqlite_provider.das | Switches schema introspection to PRAGMA table_xinfo, tracks GENERATED columns. |
| modules/dasSQLITE/daslib/sqlite_boost.das | Updates try_check_schema to handle/generated columns and computed-ness mismatches. |
| include/daScript/simulate/simulate.h | Makes freeTempString respect runtime reclaim-disabled flag. |
| include/daScript/simulate/heap.h | Adds reclaim-disabled flag plumbing to string heap. |
| include/daScript/ast/ast_infer_type.h | Declares preVisitStringBuilderElement override in infer visitor. |
| doc/source/stdlib/handmade/function-builtin-each_ref-0xd5c96473551e8fec.rst | Adds updated handmade doc for new each_ref signature hash. |
| doc/source/stdlib/handmade/function-builtin-each_ref-0x518c9960a2242c9.rst | Removes old handmade doc for prior each_ref signature hash. |
| doc/source/reference/utils/mcp.rst | Updates MCP docs to reference gen1_to_gen2. |
| doc/source/reference/tutorials/sql_02_insert_data.rst | Updates tutorial text for computed-vs-generated schema checks. |
| doc/source/reference/language/functions.rst | Documents []<- semantics and provides an example. |
| doc/reflections/das2rst.das | Updates reflection grouping to drop removed in-place case funcs; adds qmatch tmp staging group. |
| daslib/sql.das | Adds is_computed field to column info. |
| daslib/sql_provider.das | Adds generated field to SchemaFromCol. |
| daslib/sql_boost.das | Validates computed-ness vs GENERATED in schema_from; avoids synthesizing fields for GENERATED cols; carries is_computed. |
| daslib/linq_fold.das | Defers _fold(chain) macro until chain type is fully inferred. |
| daslib/builtin.das | Fixes each_ref signature; adds trap overload for wrong lambda shape; tweaks error text punctuation. |
| daslib/ast_match.das | Implements transactional qmatch captures (staging temps + commit-on-success) and adds cleanup discipline. |
| CMakeLists.txt | Renames converter target to gen1_to_gen2 and removes unused target-property queries. |
| CLAUDE.md | Updates formatter reminder text to reference gen1_to_gen2. |
| ci/smoke_test_bundle.sh | Updates bundle exe presence checks from das-fmt to gen1_to_gen2. |
| ci/check_shipped_skills.py | Extends shipped-skill exe regex to include gen1_to_gen2.exe. |
| .github/workflows/extended_checks.yml | Updates extended_checks build targets/comments to use gen1_to_gen2. |
Suppressed comments (1)
utils/mcp/tools/convert_to_gen2.das:34
do_convert_to_gen2builds a shell command string that embedsfileand executes it viapopen(cmd). This is unsafe for paths containing spaces/quotes and can become command-injection if a caller passes a crafted filename (the MCP tool is externally callable). Preferpopen_argvwith an argv array so the converter path and the file path are passed as literal arguments.
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
The coverage pass re-infers after instrumenting; on this deliberately- failing compile the re-infer surfaces the expected 31400 wrapped in an extra 31207 (macro failed to infer), breaking the exact expect count in the extended_checks Coverage step. options no_coverage is the established opt-out (aot and ast_match suites already use it). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 61 out of 61 changed files in this pull request and generated no new comments.
Suppressed comments (1)
utils/mcp/tools/convert_to_gen2.das:34
do_convert_to_gen2builds a shell command string and runs it viapopen(cmd). That makes conversion fail for file paths with spaces, and it also opens the door to shell injection if an MCP client passes a path containing shell metacharacters. Preferpopen_argvwith an argv array (and usepath_join+ a Windows.exesuffix) so the executable and file path are passed verbatim.
Bug-triage follow-up to the docs arc (#3661): the audit surfaced 9 code-bug candidates, kept out of the docs PR by design. Every one is now ruled on and fixed here — each with a red-first regression test — plus the converter-binary rename that fell out of candidate 6.
Compiler
operator []<-dispatch wired in ExprMove (f970b71) — the grammar accepted the operator butExprMovenever dispatched it, so a user-defined[]<-was silently unreachable. Mirrors the existing[]=promoter. Tests intests/language/operators.das.require X as Yregisters the alias for every require form (b04c0f2) —importNamewas only set for path-form requires (fs_file_info.cpp), sorequire daslib/strings_boost as sbwas a silent no-op. Explicitasnow registers unconditionally;importNameguards only the implicit path-stem registration. Tests pin builtin/daslib/same-dir/path forms,with-module honor, additive alias, and the newly-reachable 20510 collision (require_as.das+ fixture +failed_require_as.das).[nodiscard]results consumed by string interpolation are not discarded (7c3928e) —InferTypesnever marked string-builder elements as consumed, so any[nodiscard]call inside"{...}"tripped a falseerror[30166], in call arguments (print("{f()}")) and let-inits alike. NewpreVisitStringBuilderElementoverride runsmarkNoDiscardon every element; a genuinely discarded statement-level call still errors (failed_nodiscard.das).daslib
each_refcrash fixed (2b247c6) — the ref-generator lowering builds a lambda whose parameter is a reference to a pointer;each_ref's signature didn't match that shape, and the mismatch went uncaught all the way to an AV inSimNode_ForWithIterator. Signature is now? &with an==&trap overload that turns the wrong shape intoconcept_assert31400 at compile time. Tests:each_ref.das+failed_each_ref.das.$e $v $i $t $c $f $b $a) leaked bindings when a match FAILED partway ($v/$twere half-guarded, the rest not at all), so a later alternative saw stale captures. Captures now stage into temps and commit only on full success; dual-arm const-constructor forms dedup via a temps table with copy-init identity commits. Follow-up: array-capture temp buffers free deterministically (QmCapturesstruct,finallycleanup). 12 new tests pin every kind — 10 were red before the fix; linq/flatten/sql consumers all green._fold(chain)wrapping form defers until the chain type resolves (c785e2f) —LinqFoldfired before instance resolution whileselect's dependent typedecl return type was still auto/alias, failing infer. One-lineisAutoOrAliasdefer. The standalone red/green test matters: the window needs a FRESHselectinstance, so an in-file test was masked by cache warmth.check_schema/schema_fromsee GENERATED columns (6a57d61) —PRAGMA table_infohides generated columns; switched totable_xinfo+ the generated flag, with floor/ceiling count gates, a computed cross-check against@sql_computed, andschema_fromskipping generated-column synthesis. TDD red→green; dasllama-server and the dictation bot verified equivalent-behavior.builtin
to_lower_in_place/to_upper_in_placeremoved from the das surface (b696734) — registeredSideEffects::nonewhile mutating their argument, and silently no-oping on literals. The C++ helpers stay (internal callers); das code uses the value-returning forms.Tooling
das-fmt→gen1_to_gen2(e1818f3) — two different tools shared one name: the cmakedas-fmttarget (utils/dasFormatter, the gen1→gen2 syntax converter) and the source formatterutils/das-fmt/dasfmt.das(also compiled tobin/das-fmt.exeby CI). An SDK user typingdas-fmtto format a file got their syntax converted instead. Renamed everywhere: cmake target + install,run_utils_tests, extended_checks build targets, bundle smoke EXE presence, shipped-skills exe regex, usage text, MCPconvert_to_gen2exe path, and the skills/CLAUDE.md name-trap notes. The formatter keeps thedas-fmtname; the windows-ninja lane collision (both tools landing at./bin/das-fmt) is gone as a side effect. Also drops three deadget_target_property(DAS_FMT_*)lines nothing consumed.Found by preflight: temp-string AOT desync (fae23af)
The full AOT sweep for this branch turned up 162 ×
error[50101]across every interpolation-heavy daslib module (json, jsonrpc, clargs, logger, sql) — a pre-existing master bug shipped with the temp-string conversions (#3657), invisible to per-PR CI (which only buildstest_aot_subset). The temp-string wrapper pass fired only when the driving program hadpersistent_heapon, and it mutates function bodies — shared-module ASTs included — so the same daslib function compiled to different trees (and AOT hashes) depending on who compiled it first: macro-context compiles (which run withmacro_context_persistent_heap) wrapped shared daslib functions that AOT stub generation left bare.Fix: the wrapper is now always inserted — a function's tree never depends on the driver — and the heap modes move to runtime where they belong:
freeTempStringalready no-ops for interned heaps, linear-heap frees are safe bump-retreat no-ops, anddisable_temp_string_reclaimbecomes a runtime flag on the string heap (set at simulate from the entry program) instead of a hash-poisoning compile gate. New tests-cpp case pins the in-process invariant; the AOT sweep is the cross-process proof (162 → 0).Also in the branch (e8dbd47): preflight-surfaced fixes — both-worlds
LINT019spellings on the qmatch template-line nolints, theeach_refhandmade doc for its new signature hash (orphaned old-hash file removed), a das2rst group for theqm_tmp_*staging helpers, and a preflight exe-rail fix (the temp-pathdas-lintbinary couldn't resolvelibDaScriptDyn.dll; the daslang bin dir now rides the loader's environment).Validation
-W, ctest, interp/JIT/AOT suites, sequence smoke).tests/sweep: 13300 passed / 0 failed;tests/language: 1548/1548.gen1_to_gen2 --testsgreen; MCP convert + tools tests green.🤖 Generated with Claude Code