Merged
Conversation
ponylang-main
added
the
discuss during sync
Member
Author
|
Review surfaced two pre-existing leaks of the same class (function allocates a tree via |
SeanTAllen
force-pushed
the
fix-issue-5195-transitive-typealias-unfold
branch
from
addb505 to
dcfdd31
Compare
PR #5145 stopped expanding type aliases during name resolution, leaving them as TK_TYPEALIASREF in the AST. Every site that needs the underlying concrete type calls typealias_unfold on demand. The helper unfolded exactly one level, so chained aliases (`type A is B; type B is (U64, U64)`) returned another TK_TYPEALIASREF and any caller that inspected the head of the result crashed or miscompiled. PRs #5193 and #5196 fixed two crash sites with per-site unfolds, but the same bug class kept surfacing elsewhere. Make typealias_unfold transitive: if reify and apply_cap produce another TK_TYPEALIASREF, recurse until the head is concrete. The helper now guarantees its result has a non-alias head, so callers can rely on that postcondition. Termination is bounded by the alias chain length, which is finite because pass/names.c rejects cyclic alias defs via AST_FLAG_RECURSE_1 before any TK_TYPEALIASREF is emitted. A coupling comment in names.c documents this dependency for future maintainers. Also fixes a related #5145 regression in gentrace_needed's TRACE_TUPLE branch, which iterated tuple element children without unfolding the operand types and crashed on both single-level and chained tuple aliases (found during review via an asymmetric argument/parameter pattern). And removes the now-stale "known gap" comment in trace_dynamic_tuple's TRACE_TUPLE branch (added by #5196) since chained aliases now resolve to a concrete TK_TUPLETYPE through the transitive unfold. The new full-program-tests cover the affected codegen and expression paths at specific exit codes so they catch both assertion crashes and silent miscompilation. Closes #5195
ponylang-main
removed
the
discuss during sync
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
2 participants
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.
PR #5145 stopped expanding type aliases at name resolution, leaving
TK_TYPEALIASREFin the AST. The on-demandtypealias_unfoldhelper unfolded exactly one level, so chained aliases (type A is B; type B is (U64, U64)) returned another alias and any caller that inspected the head of the result crashed or miscompiled. PRs #5193 and #5196 fixed two crash sites with per-site unfolds; #5195 (this PR) makes the helper transitive at the source so the bug class is closed structurally.The helper now guarantees its result has a non-alias head. Termination is bounded by the alias chain length, which is finite because
pass/names.crejects cyclic alias defs viaAST_FLAG_RECURSE_1before anyTK_TYPEALIASREFis emitted. A coupling comment innames.cdocuments this dependency for future maintainers.Also fixes a related #5145 regression in
gentrace_needed'sTRACE_TUPLEbranch (iterated tuple element children without unfolding the operand types; crashed both single-level and chained tuple aliases; found during review via an asymmetric argument/parameter pattern) and removes the now-stale "known gap" comment fromtrace_dynamic_tuple(added in #5196 when the chained-alias case was untestable).New full-program-tests cover the affected codegen and expression paths. Each asserts a specific exit code so it catches both assertion crashes and silent miscompilation.
Closes #5195