Plan: Selector Painting Fix for Issue #4102

[SeanTAllen]

Implementation plan for fixing #4102 — memory leak when a val parameter implements an iso parameter trait.

Problem Summary

When a behavior is called through a trait, and the concrete actor's parameter has a
different trace-significant capability (e.g., trait has iso, concrete has val), the
ORCA GC reference counting is broken. The sender traces as MUTABLE (iso) but the
receiver's dispatch case traces as IMMUTABLE (val). This mismatch causes objects to
never reach RC=0, leaking memory.

Root cause: The mangled name for methods (used by the paint/selector-coloring
algorithm to assign vtable indices) doesn't include parameter capabilities. String iso
and String val share the same reach_type_t (for String) and thus the same mangle
("o"). This means:

1. The forwarding method is never created (same mangled name → found in r_mangled →
   early return in add_rmethod_to_subtype)
2. The trait's method and concrete method share the same vtable index (same mangled name
   → same paint color)
3. The inline message send at the call site traces with the trait's params (iso →
   MUTABLE)
4. The dispatch case traces with the concrete method's params (val → IMMUTABLE)

Approach

jemc's selector painting proposal: give distinct message IDs (vtable indices) to methods
whose parameters differ in trace-significant ways. This means the trait's iso version
and the concrete's val version get separate vtable indices and separate dispatch cases,
each tracing correctly.

Trace-Significant Capability Buckets

Only three distinctions matter (matching the three PONY_TRACE_* values):

• mutable (m): iso, trn, ref, box → PONY_TRACE_MUTABLE
• immutable (i): val → PONY_TRACE_IMMUTABLE
• opaque (t): tag → PONY_TRACE_OPAQUE

For behavior parameters, only sendable caps are allowed: iso, val, tag. So effectively:
iso→m, val→i, tag→t.

Actors are always traced as opaque regardless of cap. Primitives don't need tracing.

Why Only the Inline Send Path Is Broken

When can_inline_message_send() returns false, the call goes through the vtable to the
forwarding sender function. The forwarding sender delegates entirely to the concrete
sender (calling it directly), which traces with the concrete method's params and sends a
message using the concrete vtable index. The dispatch case for the concrete method then
traces with the concrete params. Both sides match — no ORCA mismatch.

The bug only manifests on the inline send path: the call site directly allocates the
message and traces using the trait's method params (e.g., iso → MUTABLE), then sends with
the trait's vtable index. But the dispatch case at that index was generated from the
concrete method's params (val → IMMUTABLE). The fix ensures the forwarding method gets its
own vtable index and its own dispatch case that traces consistently with the sender.

Changes

1. make_mangled_name() — Include trace-significant capability

File: src/libponyc/reach/reach.c

Currently generates: cap_name_<param1_type_mangle><param2_type_mangle>...<result_mangle>

Change to include a trace-kind character for each parameter:
cap_name_<trace_kind1><param1_type_mangle><trace_kind2><param2_type_mangle>...<result_mangle>

Where trace_kind is computed from the parameter's AST:

static char trace_kind_char(ast_t* type)
{
  switch(ast_id(type))
  {
    case TK_NOMINAL:
    {
      ast_t* def = (ast_t*)ast_data(type);
      switch(ast_id(def))
      {
        case TK_ACTOR:
          return 't';  // actors always opaque
        case TK_PRIMITIVE:
          return 'p';  // primitives not traced
        default:
          switch(ast_id(cap_fetch(type)))
          {
            case TK_VAL: return 'i';
            case TK_TAG: return 't';
            default:     return 'm';
          }
      }
    }
    case TK_UNIONTYPE:
    case TK_ISECTTYPE:
    case TK_TUPLETYPE:
      // See "Edge Cases" section below
    default:
      return 'x';  // unknown/default
  }
}

Then in make_mangled_name:

for(size_t i = 0; i < m->param_count; i++)
{
  printbuf(buf, "%c%s", trace_kind_char(m->params[i].ast),
    m->params[i].type->mangle);
}

Impact: mangled_name is purely internal (used only for r_mangled hash lookup and
the paint algorithm). It does NOT appear in LLVM IR or symbol names. full_name (which
does appear in symbols) is derived from mangled_name via make_full_name(), so symbol
names will change for methods whose params have non-default trace kinds. This is harmless
and actually helpful for debugging.

When caps match (the common case), the forwarding method's mangled name still collides
with the concrete method's, so no extra vtable entries are created. Extra entries only
appear when there's an actual trace-significant mismatch.

2. add_rmethod_to_subtype() — No code changes needed

File: src/libponyc/reach/reach.c

With the new mangled names, the reach_mangled_get will naturally NOT find
the concrete method when the caps differ (different mangled names). The forwarding method
will be created with the trait's params (iso caps) and its own vtable index.

When caps match, the mangled names still collide, and the early return still
fires. No behavioral change for the common case.

3. genfun_forward() — Add dispatch cases for forwarding behaviors

File: src/libponyc/codegen/genfun.c

Currently genfun_forward generates only a forwarding sender function. For forwarding
behaviors (TK_BE) and actor constructors (TK_NEW on actors), we also need to add a
dispatch case.

After the existing sender generation code, add:

// For forwarding behaviors/constructors on actors, add a dispatch case that
// traces with the forwarding method's params (the trait's capabilities) but
// calls the concrete method's handler.
if(t->underlying == TK_ACTOR)
{
  token_id fun_id = ast_id(m->fun->ast);
  if((fun_id == TK_BE) || (fun_id == TK_NEW))
  {
    pony_assert(c_m2->func_handler != NULL);
    add_dispatch_case(c, t, m->params, m->vtable_index,
      c_m2->func_handler, c_m2->func_type, c_m->msg_type);

#if defined(USE_RUNTIME_TRACING)
    add_get_behavior_name_case(c, t, m->name, m->vtable_index);
#endif
  }
}

Key parameter choices for add_dispatch_case:

• params = m->params: the forwarding method's params (trait's caps) — so tracing
  matches what the sender did
• index = m->vtable_index: the forwarding method's vtable index — matches the message
  ID the sender used
• handler = c_m2->func_handler: the concrete method's handler — the actual behavior
  body to execute
• fun_type = c_m2->func_type: the concrete handler's LLVM function type — for
  parameter extraction in dispatch
• msg_type = c_m->msg_type: the forwarding method's message type — matches the
  message structure the sender allocated

The LLVM types for String iso and String val are identical (both pointers), so the
fun_type and msg_type are interchangeable between the two methods. Using the forwarding
method's msg_type is conceptually correct (it matches the sender's allocation).

4. genfun_method_bodies() — Two-pass iteration for ordering

File: src/libponyc/codegen/genfun.c

Problem: genfun_forward needs the concrete method's func_handler to exist.
Technically, LLVM function declarations are created during genfun_method_sigs() (which
runs before genfun_method_bodies()), so the declaration exists regardless of processing
order. However, genfun_method_bodies iterates r_mangled in hashmap order, which is
arbitrary. As a defensive measure, we split into two passes to guarantee concrete handlers
are fully generated before forwarding methods reference them.

Fix: Split the inner loop into two passes — non-forwarding methods first, forwarding
methods second:

while((n = reach_method_names_next(&t->methods, &i)) != NULL)
{
  size_t j = HASHMAP_BEGIN;
  reach_method_t* m;

  // First pass: non-forwarding methods (generates handlers)
  while((m = reach_mangled_next(&n->r_mangled, &j)) != NULL)
  {
    if(!m->forwarding)
    {
      if(!genfun_method(c, t, n, m))
        // ... error handling ...
    }
  }

  // Second pass: forwarding methods (may need handlers from first pass)
  j = HASHMAP_BEGIN;
  while((m = reach_mangled_next(&n->r_mangled, &j)) != NULL)
  {
    if(m->forwarding)
    {
      if(!genfun_method(c, t, n, m))
        // ... error handling ...
    }
  }
}

This ensures all concrete handlers are generated before any forwarding dispatch cases
that reference them.

5. Test: Full-program trace verification

Directory: test/full-program-tests/codegen-trace-iso-to-val-through-trait/

Create a test following the existing trace test pattern
(codegen-trace-object-different-cap as reference). The test:

1. Defines a class B (with a field, so it's traceable)
2. Defines trait tag Receiver with be receive(b: B iso)
3. Defines actor Main is Receiver with be receive(b: B val)
4. In create, calls the behavior through a trait-typed variable to exercise the
   forwarding path: let r: Receiver = this; r.receive(recover B end). Calling directly
   on this would use the concrete method and bypass the vtable dispatch entirely.
5. Takes a GC snapshot before the behavior runs
6. In the behavior, checks RC values using objectmap_has_object_rc
7. Sets exit code 1 on success, 0 on failure

expected-exit-code.txt: 1

Uses the shared FFI library at test/full-program-tests/codegen-trace/additional.cc.
The test source needs use "lib:codegen-trace-additional" to link against it.

6. Verify with the issue's minimal repro

Compile and run the program from the issue body (the Timer-based one) and verify memory
stabilizes instead of growing unboundedly. This is a manual verification step, not an
automated test.

7. Run existing test suite

make test-libponyc    # Compiler unit tests (includes paint tests)
make test-full-programs-release  # All full-program integration tests

Edge Cases

Unions

jemc's example:

be example(value: (Array[T] iso | String val))  // concrete
// subtype of:
be example(value: (Array[T] iso | String iso))  // trait

Both union types get mangle = "o" (unions always have mangle "o"). With the simple
nominal trace_kind_char, both would get 'x' (default for non-nominal types),
producing the same mangled name. The forwarding method would NOT be created, and the bug
would persist for this case.

Fix for unions: trace_kind_char must recursively walk union members. For a union,
compute a sorted string of member trace kinds:

case TK_UNIONTYPE:
{
  // Recursively compute trace kinds for all members, sorted
  // e.g., "Uimt" for a union with immutable, mutable, and tag members
  // The exact encoding doesn't matter as long as it's deterministic
  // and distinguishes trace-significant differences
}

This is more complex but necessary for correctness. The encoding must be deterministic
(sorted by some canonical order) so that equivalent unions produce the same string.

Recommendation: Implement the simple nominal case first, then add union/tuple
handling. The simple case covers the reported bug and the most common pattern. Union/tuple
parameter mismatches are valid but much rarer.

Tuples

Similar to unions. Tuple mangling already includes member type mangles
(<cardinality><mangle1><mangle2>...), but not member capabilities. The
trace_kind_char function would need to handle TK_TUPLETYPE by recursively encoding
each element's trace kind.

Intersections

Same pattern as unions. Walk all members and encode their trace kinds.

Type-parameterized parameters (TK_TYPEPARAMREF)

set_method_types() can produce TK_TYPEPARAMREF nodes in params[i].ast. The
trace_kind_char function returns 'x' for these (the default case). This is safe: both
the trait and concrete method would get 'x' for the same type-parameterized parameter,
so mangled names still match and no spurious forwarding methods are created. However, it
means this fix does NOT cover trace-kind mismatches on type-parameterized parameters. In
practice, methods are monomorphized during reach analysis, so TK_TYPEPARAMREF nodes
should not survive to this point for fully-specialized methods. This is an acknowledged
limitation — if a real case surfaces, trace_kind_char can be extended to handle it.

Actor constructors through interfaces

The same issue can apply to new on actors called through interfaces. The fix in
genfun_forward handles this by checking for both TK_BE and TK_NEW on actor types.

Non-behavior methods

Functions (TK_FUN) don't have dispatch cases or message tracing. The mangled name
change technically affects them too (they'd include trace kind characters), but since no
forwarding dispatch cases are added for functions, this has no behavioral impact — just
slightly different internal names.

To minimize the change, we could limit the trace kind encoding to behaviors only:

// In make_mangled_name:
bool is_behavior = (m->fun != NULL) &&
  ((ast_id(m->fun->ast) == TK_BE) ||
   ((ast_id(m->fun->ast) == TK_NEW) && /* is actor */));

for(size_t i = 0; i < m->param_count; i++)
{
  if(is_behavior)
    printbuf(buf, "%c", trace_kind_char(m->params[i].ast));
  printbuf(buf, "%s", m->params[i].type->mangle);
}

However, including it for all methods is simpler and harmless. The only cost is slightly
larger vtables when a non-behavior function has forwarding methods with different caps —
which is extremely rare and has no correctness impact.

Files Modified

File	Change
src/libponyc/reach/reach.c	Add trace_kind_char(), modify make_mangled_name()
src/libponyc/codegen/genfun.c	Modify genfun_forward() to add dispatch cases; modify genfun_method_bodies() for two-pass iteration
test/full-program-tests/codegen-trace-iso-to-val-through-trait/main.pony	New test
test/full-program-tests/codegen-trace-iso-to-val-through-trait/expected-exit-code.txt	New file: 1

[SeanTAllen]

Update: Test Verification Strategy

The full-program test from step 5 checks that ORCA reference counts balance correctly when a behavior is called through a trait with different parameter capabilities. However, PR #4256 ("Fix segfault caused by unsafe garbage collection optimization") is currently masking the bug: make_might_reference_actor() in gendesc.c is hard-coded to return true, which forces full tracing of immutable objects regardless of trace kind. This means the test would pass with or without our fix.

To verify the fix works independently of #4256's safety net, we use this workflow:

1. Temporarily disable Fix segfault caused by unsafe garbage collection optimization #4256: Change make_might_reference_actor() in src/libponyc/codegen/gendesc.c to return 0 instead of 1. This restores the old (unsafe) immutable optimization and exposes the trace-kind mismatch bug.

2. Build and run the test: The test should fail — confirming the bug exists without Fix segfault caused by unsafe garbage collection optimization #4256's safety net.

3. Apply the selector painting fix (steps 1-4 from the plan above).

4. Build and run the test again: The test should now pass — confirming our fix resolves the trace-kind mismatch independently.

5. Re-enable Fix segfault caused by unsafe garbage collection optimization #4256: Change make_might_reference_actor() back to return 1.

6. Build and run the test again: The test should still pass — our fix plus the safety net.

7. Run full test suite: make test-libponyc and make test-full-programs-release.

The make_might_reference_actor revert is never committed — it's purely for local verification. The PR description will document this test strategy.

Also removed step 6 ("Verify with the issue's minimal repro") from the original plan — the above strategy provides a more rigorous and reproducible verification than manual memory observation.

[SeanTAllen]

Implemented in PR #4944.