Skip to content

Commit dcfdd31

Browse files
committed
Make typealias_unfold transitive
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
1 parent 4b85f11 commit dcfdd31

File tree

30 files changed

+555
-15
lines changed

30 files changed

+555
-15
lines changed

src/libponyc/codegen/gentrace.c

Lines changed: 47 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -814,20 +814,14 @@ static void trace_dynamic_tuple(compile_t* c, LLVMValueRef ctx,
814814
LLVMPositionBuilderAtEnd(c->builder, trace_block);
815815

816816
// If we are (A, B), turn (_, _) into (A, _).
817-
// If the element is a type alias for a tuple, unfold it once so the
817+
// If the element is a type alias for a tuple, unfold it so the
818818
// recursive call sees the concrete TK_TUPLETYPE; iterating the alias
819819
// ref's children (TK_ID, typeargs, cap, eph) as tuple elements would
820820
// trip trace_type's default assertion. typealias_unfold is guaranteed
821821
// to succeed here because trace_type already unfolded the same child
822-
// to classify it as TRACE_TUPLE; reify is deterministic. Chained
823-
// aliases (alias of alias of tuple) are a known gap tracked in
824-
// ponylang/ponyc#5195 — typealias_unfold only unfolds one level, so
825-
// swap is still a TK_TYPEALIASREF and the recursive call hits the
826-
// same assertion. Constructing a test that reaches this site with a
827-
// chained alias is currently blocked by other PR #5145 regressions
828-
// elsewhere in codegen (the static trace_tuple path in this file and
829-
// gen_digestof_value in genreference.c), which fire first for every
830-
// configuration tried.
822+
// to classify it as TRACE_TUPLE; reify is deterministic. The unfold
823+
// is transitive, so chained aliases (alias of alias of tuple) also
824+
// resolve to a concrete TK_TUPLETYPE here.
831825
ast_t* swap;
832826

833827
if(ast_id(child) == TK_TYPEALIASREF)
@@ -1048,24 +1042,63 @@ bool gentrace_needed(compile_t* c, ast_t* src_type, ast_t* dst_type)
10481042

10491043
case TRACE_TUPLE:
10501044
{
1045+
// Unfold type aliases so the child iteration sees concrete tuple
1046+
// elements rather than the alias ref's children (TK_ID, typeargs,
1047+
// cap, eph). Without this, a tuple-aliased operand causes the loop
1048+
// to walk the wrong children and trip the trailing assertion.
1049+
// Note: although `trace_type` handles TK_TYPEALIASREF recursively
1050+
// and returned TRACE_TUPLE above, it takes src_type by value and
1051+
// doesn't mutate the caller's pointer, so src_type and dst_type
1052+
// here are still the original (possibly alias) ASTs.
1053+
ast_t* src_unfolded = NULL;
1054+
ast_t* dst_unfolded = NULL;
1055+
1056+
if(ast_id(src_type) == TK_TYPEALIASREF)
1057+
{
1058+
src_unfolded = typealias_unfold(src_type);
1059+
if(src_unfolded != NULL)
1060+
src_type = src_unfolded;
1061+
}
1062+
1063+
if(ast_id(dst_type) == TK_TYPEALIASREF)
1064+
{
1065+
dst_unfolded = typealias_unfold(dst_type);
1066+
if(dst_unfolded != NULL)
1067+
dst_type = dst_unfolded;
1068+
}
1069+
1070+
bool result;
10511071
if(ast_id(dst_type) == TK_TUPLETYPE)
10521072
{
1073+
result = false;
10531074
ast_t* src_child = ast_child(src_type);
10541075
ast_t* dst_child = ast_child(dst_type);
10551076
while((src_child != NULL) && (dst_child != NULL))
10561077
{
10571078
if(gentrace_needed(c, src_child, dst_child))
1058-
return true;
1079+
{
1080+
result = true;
1081+
break;
1082+
}
10591083
src_child = ast_sibling(src_child);
10601084
dst_child = ast_sibling(dst_child);
10611085
}
1062-
pony_assert(src_child == NULL && dst_child == NULL);
1086+
// The assert only holds when the loop ran to completion. If we
1087+
// broke early with `result = true`, src_child and dst_child
1088+
// point to the element that needs tracing, not NULL.
1089+
if(!result)
1090+
pony_assert(src_child == NULL && dst_child == NULL);
10631091
} else {
10641092
// This is a boxed tuple. We'll have to trace the box anyway.
1065-
return true;
1093+
result = true;
10661094
}
10671095

1068-
return false;
1096+
if(src_unfolded != NULL)
1097+
ast_free_unattached(src_unfolded);
1098+
if(dst_unfolded != NULL)
1099+
ast_free_unattached(dst_unfolded);
1100+
1101+
return result;
10691102
}
10701103

10711104
default:

src/libponyc/pass/names.c

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,12 @@
55
#include "../pkg/package.h"
66
#include "ponyassert.h"
77

8+
// Resolve a type alias definition, rejecting cyclic chains. Note that
9+
// `typealias_unfold` (in src/libponyc/type/typealias.c) relies on this
10+
// rejection: its transitive recursion is bounded by the assumption that
11+
// alias chains form a finite DAG, which only holds because cycles are
12+
// rejected here. If you change the cycle detection mechanism, also update
13+
// the safety argument in `typealias_unfold`.
814
static bool names_resolvealias(pass_opt_t* opt, ast_t* def, ast_t** type)
915
{
1016
int state = ast_checkflag(def,

src/libponyc/type/typealias.c

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -75,5 +75,19 @@ ast_t* typealias_unfold(ast_t* typealiasref)
7575
if((tcap != TK_NONE) || (teph != TK_NONE))
7676
apply_cap_to_type(r_alias, tcap, teph);
7777

78+
// If the result is itself a TK_TYPEALIASREF (a chained alias like
79+
// `type A is B; type B is (U64, U64)`), unfold transitively. This is
80+
// bounded by the length of the alias chain: pass/names.c's
81+
// names_resolvealias uses AST_FLAG_RECURSE_1 to detect and reject cyclic
82+
// alias defs before any TK_TYPEALIASREF is emitted, so the chain is a
83+
// finite DAG. Callers rely on the returned head not being a
84+
// TK_TYPEALIASREF (see typealias.h).
85+
if(ast_id(r_alias) == TK_TYPEALIASREF)
86+
{
87+
ast_t* next = typealias_unfold(r_alias);
88+
ast_free_unattached(r_alias);
89+
return next;
90+
}
91+
7892
return r_alias;
7993
}

src/libponyc/type/typealias.h

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,17 @@ PONY_EXTERN_C_BEGIN
99
/**
1010
* Unfold a TK_TYPEALIASREF to its underlying type. Reifies the alias
1111
* definition with the stored type arguments and applies the cap and
12-
* ephemeral modifiers.
12+
* ephemeral modifiers. If the result is itself a TK_TYPEALIASREF (a chained
13+
* alias), unfolds transitively until the head is a concrete type.
14+
*
15+
* Guarantees that a non-NULL result has a head that is not TK_TYPEALIASREF.
16+
* Callers dispatching on the head of the result can therefore rely on the
17+
* full set of non-alias heads (TK_NOMINAL, TK_UNIONTYPE, TK_ISECTTYPE,
18+
* TK_TUPLETYPE, TK_TYPEPARAMREF, TK_ARROW, etc.) without a TK_TYPEALIASREF
19+
* case. Nested aliases inside a compound type (a member of a TK_UNIONTYPE,
20+
* TK_ISECTTYPE, or TK_TUPLETYPE that is itself an alias) are not unfolded;
21+
* callers that recurse into compound types must still dispatch on
22+
* TK_TYPEALIASREF for interior positions.
1323
*
1424
* Returns a newly allocated AST node, or NULL if reification fails.
1525
* The caller must free a non-NULL result with ast_free_unattached when done.
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
1
Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
// Regression test for chained type aliases in behavior message argument
2+
// tracing.
3+
//
4+
// Sending a chained-alias-tuple argument to a behavior whose parameter
5+
// is also typed with the same alias triggers `gen_send_message` in
6+
// `gencall.c` to emit a GC-trace call for the argument. The trace
7+
// codegen runs via `gentrace` -> `trace_type` (TRACE_TUPLE) ->
8+
// `trace_tuple` at `gentrace.c:611`. The `if(ast_id(dst_type) ==
9+
// TK_TUPLETYPE)` check is FALSE for an alias-typed parameter, so
10+
// control falls into the else (boxed tuple) branch which iterates
11+
// `ast_child(src_type)`. For a single-level or chained TK_TYPEALIASREF
12+
// `src_type`, `ast_child` walks (TK_ID, typeargs, cap, eph); the
13+
// recursive `gentrace` call on TK_ID hits `trace_type`'s default
14+
// `pony_assert(0)` at `gentrace.c:383`.
15+
//
16+
// The transitive unfold in `typealias_unfold` (together with gentrace's
17+
// top-level unfold of src/dst) resolves the chain to a concrete
18+
// TK_TUPLETYPE before `trace_tuple` runs, so the iteration sees real
19+
// tuple element children.
20+
//
21+
// This test exercises the message-send codegen path with both src and
22+
// dst being alias types, distinct from the asymmetric pattern covered
23+
// by type-alias-tuple-gentrace-needed and from the actor-field path
24+
// covered by type-alias-chained-static-trace. See ponylang/ponyc#5195.
25+
26+
use @pony_exitcode[None](code: I32)
27+
28+
type _Pair is (U64, U64)
29+
type _Middle is _Pair
30+
31+
actor Helper
32+
be receive(x: _Middle) =>
33+
@pony_exitcode(1)
34+
35+
actor Main
36+
new create(env: Env) =>
37+
let h = Helper
38+
h.receive((U64(1), U64(2)))
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
1
Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
// Regression test for chained type aliases as array element types.
2+
//
3+
// Building an `Array[_Middle]` where `_Middle` is a chained alias to a
4+
// tuple triggers generation of the array's element trace function. The
5+
// trace codegen calls `gentrace` on the element type, which dispatches
6+
// via `trace_type` (`src/libponyc/codegen/gentrace.c:354`). For a chained
7+
// alias, the one-level unfold path reaches `trace_type`'s default
8+
// `pony_assert(0)` at line 383.
9+
//
10+
// This test exercises the array-element trace codegen path for chained
11+
// aliases, distinct from the actor-field path covered by
12+
// type-alias-chained-static-trace and the actor-message path covered by
13+
// type-alias-chained-actor-message. See ponylang/ponyc#5195.
14+
15+
use @pony_exitcode[None](code: I32)
16+
17+
type _Pair is (U64, U64)
18+
type _Middle is _Pair
19+
20+
actor Main
21+
new create(env: Env) =>
22+
let arr: Array[_Middle] = [(U64(1), U64(2)); (U64(3), U64(4))]
23+
try
24+
let first = arr(0)?
25+
let second = arr(1)?
26+
if ((first._1 + first._2) == 3) and ((second._1 + second._2) == 7) then
27+
@pony_exitcode(1)
28+
end
29+
else
30+
// Unreachable: arr(0) and arr(1) cannot fail on a 2-element array.
31+
// Use a distinct exit code so the test fails visibly if it ever does.
32+
@pony_exitcode(2)
33+
end
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
1
Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
// Positive-coverage test for chained type aliases over a class type with
2+
// cap-restricted method dispatch.
3+
//
4+
// **Note on counterfactual sensitivity:** this test passes both with and
5+
// without the transitive `typealias_unfold` fix, so it does not catch a
6+
// regression in this PR's specific change on its own. The reason is that
7+
// every code path that resolves chained class aliases for method
8+
// dispatch (lookup.c, subtype.c) recurses on TK_TYPEALIASREF results
9+
// from `typealias_unfold`, so the per-caller recursion handles a chain
10+
// of length N regardless of whether the helper itself is single-level or
11+
// transitive. Tuple-based tests in this PR exercise the actual crash
12+
// sites; this test exists to document that chained class aliases work
13+
// end-to-end (allocation through the chain, ref-cap method dispatch
14+
// through the chain, box-cap read through the chain) and to guard
15+
// against future regressions in that surface area. See ponylang/ponyc#5195.
16+
17+
use @pony_exitcode[None](code: I32)
18+
19+
class Counter
20+
var _count: U64 = 0
21+
22+
fun ref bump() =>
23+
_count = _count + 1
24+
25+
fun box current(): U64 =>
26+
_count
27+
28+
type _CounterAlias is Counter
29+
type _ChainedCounter is _CounterAlias
30+
31+
actor Main
32+
new create(env: Env) =>
33+
// Allocate via the chained alias type. Resolution must walk the chain
34+
// (_ChainedCounter -> _CounterAlias -> Counter) and produce a value
35+
// whose cap permits the ref-restricted bump() call below.
36+
let c: _ChainedCounter = Counter
37+
38+
// Calls a ref method. If cap propagation through the chain breaks,
39+
// this fails to compile (cap mismatch on the receiver).
40+
c.bump()
41+
c.bump()
42+
c.bump()
43+
44+
// Calls a box method. Verifies the receiver is also box-compatible
45+
// (ref <: box) and the mutation observed by bump() actually happened.
46+
if c.current() == 3 then
47+
@pony_exitcode(1)
48+
end

0 commit comments

Comments
 (0)