Repository navigation
fix(cast): reject non-atom results in flat STR casts - #927
Merged
Merged
Conversation
Addresses #733. Keep flat element-wise casts and reject recursive vector results before string-atom access.
singaraiona
approved these changes
Oct 21, 2026
Collaborator
Author
|
Updated to 82a9840 after the release-cost pass, with accepted938 integrated normally. Your approval of9e was for the earlier small guard; this adds a real resolved-target STR kernel, so I am requesting fresh review rather than carrying that approval forward. Kind-check-only specialization did not show a reliable gain; eliminating the existing repeated target lookup/dispatch did (scoped figures/provenance and unchanged-format/null controls are in the revised body). Old full-suite evidence remains attributed9e, not this revision. |
singaraiona
approved these changes
Oct 22, 2026
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.
Landed normally
014b98a after the independently approved changed82a formatter and normal accepted-base imports. Fresh exact632f/current028base/APPROVED/CLEAN/MERGEABLE/resolved and all11 actual COMPLETED/SUCCESS gates, ordinary protected expected-SHA squash. Complete checked/landed tree 77857a32f5351dce66a27bfbc29b897ee5c39ce5 identical; remote dev verified. No master/tag/release action. Closing references checked empty: this preserves the documented historical-crash reproduction limits rather than auto-closing733.
Current-head scoped tests below retain their provenance; original9e full and prior matched formatter prices are not relabeled as final-head executions.
Current accepted-prefetch integration —
632f1adcNormal expected-head import of accepted #942 /02804c38. Exact ordered parents, computed complete merge tree and all4 unchanged feature blobs verified. Fresh non-root ALL-TU ClangASan/UBSan as4/text-null12/cast64/affine20, imported TIME3/metadata2/tiny chained-join pair1 passed; GCC builtins DEBUG/NDEBUG syntax2, whole relevant logs diagnostic0, exact clean head/both binaries checked independently. No new full, TSan, current native pricing or author-backtester claim. Earlier evidence retains its actual heads. Fresh current-head/base/review/discussion/required gates still precede normal landing.
Prior accepted-identity-LEFT integration —
3583c75dNormal expected-head import of accepted #943 /f7b23ec7, exact ordered parents and computed complete merge tree verified. All4feature blobs unchanged. Fresh non-root ALL-TU ClangASan/UBSan as4/text-null12/cast64/affine20, imported TIME3/metadata2 passed; GCC changed production TU DEBUG/NDEBUG syntax2, whole relevant logs diagnostic0, clean exact head/both binaries verified. No new local full, TSan, combined-head native pricing or author-backtester claim. Fresh exact-head/current-base/review/discussion/required CI checks remain needed before normal landing. Earlier evidence retains its proper heads.
Prior TIME32 integration —
5ec3df53Normal expected-head import of accepted #939 /5f6e619b. Ordered parents/computed whole merge tree verified; all4feature blobs unchanged. Fresh non-root ALL-TU/header-consistent ClangASan/UBSan as4, text-null12, cast64, affine20, imported TIME3 and metadata2 passed. GCC changed production TU DEBUG/NDEBUG syntax2, whole relevant logs diagnostic0, exact clean head/both binaries checked. No new local full/TSan/native/backtester claim; earlier evidence retains its named heads. Fresh exact-head CI/current-base/reviews/threads remain required before ordinary landing.
Prior B0 integration —
1bea85f1Normal expected-head import of accepted #936 / b0b0a1b. Ordered parents and computed complete merge tree verified; all4feature blobs unchanged. Fresh non-root ALL-TU/header-consistent ClangASan/UBSan as4, text-null12, cast64, affine20 and imported metadata2 passed; GCC changed production TU DEBUG/NDEBUG syntax2, whole relevant logs diagnostic0, exact clean head/both binaries verified. No new local full, TSan, native pricing or author-backtester claim. Earlier measurements/controls below retain their original heads. Fresh exact-head CI is still required.
Prior D715 integration —
23099bd3Anton approved the changed formatter at 82a9840, not only the earlier guard-only head. Normal import of accepted d7153e7 gives23099bd3: ordered parents/computed complete merge tree verified and all4feature blobs unchanged. Fresh non-root all-TU/header-consistent ClangASan/UBSan scopes: as4/4, text-null12/12, cast64/64, imported affine20/20; GCC builtins DEBUG/NDEBUG syntax2; whole relevant logs diagnostic0, exact clean head/both binaries checked. No new full/TSan/release-pricing claim. Waiting for this exact head's actual required checks.
Contract / issue733
STR collection conversion produces a flat STR vector, one string atom per source cell. Nested collection children recursively produce vectors, which must not be read through scalar string accessors. Reject that unsupported result with an owned type error, retaining the input and releasing partial output.
On accepted646 I reproduced erroneous empty strings for the grouped-parted/literal forms, not the historical reported misalignment abort. This addresses733's shape/contract defect without inventing flattening or nested-rank semantics; no historical-crash reproduction claim.
Reviewed feature: avoid per-flat-cell contract checks
82a9840, with accepted938/a587 integrated normally (no force-push):
The changed82a formatter subsequently received Anton’s independent approval. The current-base import and fresh scopes are documented above; fresh exact-head CI remains required.
Optimized evidence — not an alleged40% guard penalty
First I tested moving just the kind check out of the flat loop. It gave no robust win; I did not publish a refactor merely for counting fewer checks. The meaningful gain below removes pre-existing per-cell target redispatch, not only the new shape guard.
Matched accepted-a587+guard baseline versus resolved-target kernel, same Clang18 project O3/native flags, one pinned i7-6700 CPU, background helpers0. Private native cast entrypoint,500,000 inputs, input setup and every output cell's bytes/length validated outside timing, one discarded process per variant/case then five randomized fresh processes. Shared host/governor not isolated.
LIST sample ranges overlap; no LIST improvement is claimed. No whole-engine, fused-query, ingestion, cold-cache or cross-platform speedup claim. Both sides include938's append fix, so it is not a confounded comparison against927's older646 base.
Method/provenance: same common optimized archive, replace only the changed builtins TU with the actual captured project flags; hash changed source/archive/executable. Not two fresh whole-PR release builds. Complete private source baseline34412292 equals the normal9e+a587 merge tree; final82a differs from it only in the measured/tested
builtins.cblob. Source/whole-tree difference independently verified after publication.Correctness evidence and limits
Production-identical private checkout on current baseline, non-root Clang18 DEBUG ASan/UBSan:
rfl/type/as4/4,text_null12/12,cast64/64 (overlapping scopes), whole relevant logs diagnostic0. Existing nested/mixed/empty/parted rejection and source-preservation fixtures retained.Separate optimized baseline/candidate comparison: 88 scalar/vector records byte-identical, including null flags and unchanged source payload: signed widths, BOOL/U8, F32/F64 NaNs/infinities/subnormals/signedzero, GUID, DATE/TIME/TIMESTAMP/TIMESPAN, runtime SYM/STR, long and embedded-NUL text, slices. This is scoped differential evidence, not a universal correctness theorem or foreign-domain/exhaustive-error audit. All positive timing outputs also independently matched numeric/text expectations. GCC DEBUG and DEBUG+NDEBUG isolated syntax/warning checks passed.
The original full non-root 6413/6430,17skipped,0failed belongs to9e196613 only, not this new kernel. No new local full suite, full TSan/GCC runtime,32-bit or allocation-failure injection claim. The adjacent pre-existing append-error/formatter-error ownership caveats remain outside this repair; no new failure-atomicity promise. Fresh required CI and independent review remain necessary.