Plan v2: Fix #4088 — Default Arguments as Type-Level Method Overloads #4977
SeanTAllen
started this conversation in
ponyc
Replies: 0 comments
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Uh oh!
There was an error while loading. Please reload this page.
-
Change classification: Fixed (bug fix) and Changed (breaking subtyping change) for issue #4088. Requires a release notes file and both
changelog - fixedandchangelog - changedPR labels. Since this is multi-type, the CHANGELOG is updated manually after merge.Supersedes: Plan v1, which scoped out interfaces/traits.
Mental Model
Default arguments are not call-site syntactic sugar. They are type-level method overloads: a method declaration
f(x: A = expr)on a type is equivalent to that type providing two methods:Each type carries its own reduced-arity methods with its own defaults. At runtime, vtable dispatch picks the concrete type's overload, ensuring the correct defaults are applied regardless of how the call site sees the receiver — union, intersection, interface, or trait.
Problem
The current compiler treats defaults as call-site sugar:
apply_default_arg()extracts the default from whichever methodlookup()finds and splices it into the call AST. For abstract receivers:lookup_union()/lookup_isect()picks a "representative" method. All members silently get the representative's defaults at runtime.lookup_nominal()finds the interface's own method. The interface's default is used, even when the concrete type declares a different one.Both are wrong under the new model. Each concrete type should use its own defaults.
Approach: Shim Functions via Selector Coloring
For each concrete type with defaulted parameters, create per-concrete-type "shim" methods with reduced arity. These shims:
The shims participate in selector coloring (
paint.c) and get their own vtable slots. At the call site, a reduced-arity call dispatches through the shim's vtable slot, ensuring each concrete type uses its own defaults at runtime.Scope
Subtyping Change (Breaking)
Under the new model, if an interface declares
fun foo(x: U32 = 1), it means callers of the interface may callfoo()with no arguments. Any type claiming to implement this interface must therefore be able to handle a zero-argumentfoo()call — which means it must have its own default forx.Current behavior (broken):
New behavior:
Quietdoesn't satisfyConfigurablebecauseConfigurablerequires bothconfig(Bool): Stringandconfig(): String, butQuietonly provides the first.Rule: When checking if method
subis a subtype of methodsuper, ifsuperhas a default at parameter positioni, thensubmust also have a default at positioni. The default expressions need not be identical — each type provides its own. What matters is that the sub method can handle the same reduced arities as the super method.This check lives in
is_reified_fun_sub_fun(), which is called from multiple places:is_nominal_sub_structural()callsis_fun_sub_fun()→is_reified_fun_sub_fun().lookup_union()/lookup_isect()callis_subtype_fun()→is_reified_fun_sub_fun()when selecting the representative method. The effect is beneficial: when one member hasfoo(x: U32 = 1)and another hasfoo(x: U32), the defaultless method becomes the representative (it's the supertype). This meansapply_default_arg()correctly fails with "not enough arguments" rather than silently using the wrong default. The new check doesn't create problems here — it refines representative selection in the right direction.The expr pass validation (
check_union_defaults()in Step 2) provides better error messages for union/intersection cases. Since it runs BEFOREcheck_arg_types()(which callsapply_default_arg()), the custom error fires first. But even without it, the subtyping-influenced representative selection would produce a correct (if less descriptive) error.Implementation Steps
Step 1: Subtyping — Require Matching Defaults (subtype.c)
Goal: When checking method subtyping, verify that the sub method has defaults wherever the super method has defaults.
Changes in
is_reified_fun_sub_fun()(subtype.c:243-397):After the existing contravariant parameter type check loop (lines 348-381), add a second pass over the parameter lists to check default argument compatibility:
This is the minimal, correct check. It answers: "can every call that works on the super type also work on the sub type?" If the super declares a default, callers may omit that argument, so the sub must be able to handle that omission too.
Reification preserves defaults:
is_reified_fun_sub_fun()works on reified copies created byreify_method_def()(reify.c:239). That function'sdup_childarray marks params (index 3) astrue, so default argument nodes (child 2 of each param) are preserved in the reified copy. Confirmed by reading the code.No changes to
is_fun_sub_fun(): The parameter count check at line 466 remains the same. Both methods have the same total parameter count; what changes is which parameters have defaults.Step 2: Annotate Call Sites (expr pass —
call.c)Goal: Detect when default arguments are applied on an abstract receiver, validate compatibility, and annotate the call AST for codegen.
Changes in
method_application()(call.c:610-668):After
extend_positional_args()andapply_named_args()but BEFOREcheck_arg_types():Guard: only proceed if
!partial. For partial application,TK_NONEargs represent unfilled positions, not default-needing positions.Count how many positional args are still
TK_NONE(need defaults).If there are
TK_NONEargs, determine whether the receiver is abstract:method_receiver_type(lhs). Note: this function only digs throughTK_FUNREF/TK_FUNAPP/TK_FUNCHAIN, but it works forTK_BEREFcalls too — the receiver is the direct first child for behavior references, so no dig-through is needed.TK_UNIONTYPEorTK_ISECTTYPE: abstract.TK_NOMINAL: checkast_id((ast_t*)ast_data(r_type))— abstract ifTK_INTERFACEorTK_TRAIT.If the receiver is abstract and there are
TK_NONEargs:a. For TRAILING defaults only (all
TK_NONEargs at the end): record the explicit arg count for shim dispatch.b. For NON-TRAILING defaults (gaps from named args): check whether all members have the SAME default expression for each gap. If they differ, emit an error. If they're the same, proceed normally.
After
check_arg_types()succeeds, annotate theTK_CALLnode:ast_setdata(ast, (void*)(uintptr_t)(explicit_count + 1)). The+1distinguishes "0 explicit args" from "no annotation" (NULL). Note:treecheckdef.hmust be updated to addHAS_DATAto theTK_CALLrule so the tree checker doesn't flag this annotation as an error.Interaction with
check_arg_types(): By the time codegen runs,apply_default_arg()(called fromcheck_arg_types()) has already spliced the representative's default expressions into the positional args AST — all N args are present. The annotation tells codegen to generate only the firstexplicit_countargs and dispatch through the shim, which fills in the correct per-type defaults. The representative's defaults remain in the AST for type-checking purposes only.Validation for unions/intersections: New helper
check_union_defaults()iterates members, looks up the method on each vialookup(), and validates defaults at the specified positions. Error conditions:"cannot omit argument '%s': not all members of the union/intersection provide a default""cannot omit argument '%s': members of the union/intersection have different defaults for this parameter"Validation for interfaces/traits: The subtyping check (Step 1) already ensures all implementors have defaults wherever the interface does. No additional validation needed at the call site — if the code type-checks, all concrete types that could be dispatched have compatible defaults.
Step 3: Create Shim Methods (reach pass —
reach.c)Goal: For each concrete type that participates in abstract type dispatch and has defaulted parameters, create shim
reach_method_tentries with reduced arity.Two creation points:
a. Concrete subtype shims: New function
add_default_shim_to_subtype()— called fromadd_rmethod_to_subtype()after creating the normal forwarding method. For each concrete subtype:lookup().TK_PARAM:TK_SEQif default exists,TK_NONEif not).TK_LOCATION(__loc). Since__locexpands to the call-site location, all members produce the same value — no shim is needed.k(fromtotal_params - 1down to first non-defaulted param, excludingTK_LOCATIONdefaults):reach_method_twithparam_count = k.kentries from the parent method'sparamsarray (abstract type's param types, for consistent mangled names).mangled_nameviamake_mangled_name().name(genname) viagenname_fun_shim():"{cap}_{method_name}[_TypeArgs]$d{k}".forwarding = trueanddefault_shim = true.fun = deferred_reify_dup(m->fun)(required formake_prototype()/make_signature()).result = m->result(the abstract type's result type, matching the pattern used for forwarding methods — ensures consistent mangled names across all concrete types).r_methods(for lookup) andr_mangled(for painting).b. Abstract type shims: New function
add_default_shims_to_abstract()— called fromadd_rmethod()afteradd_rmethod_to_subtypes()completes, for all abstract types (TK_UNIONTYPE,TK_ISECTTYPE,TK_INTERFACE,TK_TRAIT). For each valid trailing reduced arityk:reach_method_twithoutforwardingordefault_shimflags (abstract types don't get bodies).fun = deferred_reify_dup(m->fun)(required formake_prototype()).r_methodsandr_mangled.mangled_nameas concrete subtypes' shims → same vtable slot.New field on
reach_method_t:bool default_shim. Initialized tofalseby existingmemset(0, ...). Set totrueonly on concrete subtype shim methods.New function in
genname.c:genname_fun_shim(cap, name, typeargs, arity)→"{cap}_{name}[_TypeArgs]$d{arity}".New lookup function:
reach_method_shim()— mirrors cap normalization fromreach_method(), computes shim genname viagenname_fun_shim(), looks up inr_methods.Step 4: Paint Phase — No Changes Needed
Shims have different mangled names from full methods → separate vtable slots. All concrete types' shims for a given arity share the same mangled name → same vtable slot. The painting algorithm handles this automatically.
Step 5: Codegen — Function Signatures (genfun.c) — No Changes Needed
make_signature()andmake_prototype()usem->param_countandm->params. Shims with reduced values get correct LLVM signatures automatically.Step 6: Codegen — Shim Body Generation (genfun.c)
New function:
genfun_default_shim()— modeled ongenfun_forward(). Called fromgenfun_method()whenm->default_shimis true (checked BEFOREm->forwarding).Generates a shim body on a concrete type:
reach_method(t, m->cap, n->name, m->typeargs)→ full-arity method.m_real->fun->ast→ast_childidx(method_ast, 3).codegen_startfun(c, c_m->func, ..., m_real->fun, ...). Pass the REAL method'sdeferred_reification_tfor correct type parameter mappings. Note:name_params()(genfun.c:58-83) iterates the params AST usingm->param_countas the loop bound. Sincem->fun->asthas the full-arity param list butm->param_count = k, the loop safely walks only the firstkchildren — this works becauseast_sibling()traversal doesn't depend on total child count.m_real->param_count + 1):args[0]= receiver (pass through)args[1..m->param_count]= explicitly-provided params (pass through with casts)args[m->param_count+1..m_real->param_count]= defaulted params: applydeferred_reify()on default expression,gen_expr(), cast, free reified AST.Step 7: Codegen — Call Site Dispatch (gencall.c)
After resolving the method via
reach_method(), check the TK_CALL annotation:Ordering is critical: The shim check and method switch must happen BEFORE the argument buffer allocation (line 772:
size_t count = m->param_count + 1) and before the first argument generation loop (lines 776-792). The implementation places the shim check between lines 766 (method resolution) and 772 (buffer allocation), so thatm->param_countis already the shim's reduced arity when the buffer is sized and loops are bounded.m->param_count. After the expr pass, the positional args AST has ALL N arguments (defaults were spliced in byapply_default_arg()). With a shim, the buffer is sized fork+1entries. Bothwhile(arg != NULL)loops (lines 779-792 and 881-888) must be changed to also check(size_t)(i - 1) < m->param_count, otherwise they iterate all N AST children and write past the buffer end.shim_data > 0, forceis_message = false(shim handles calling the real sender).can_inline_message_send()defensively: whenm->default_shim, usereach_method_shim()for subtype lookup.Step 8: Handle Behaviors
Same as v1 — shims for behaviors are simple functions that fill in defaults and call the real behavior's sender. No
func_handleroradd_dispatch_case()needed. Behavior shims automatically become senders through the existingmake_prototypemechanism: whenast_id(m->fun->ast) == TK_BE,make_prototypegenerates a sender prototype (with_sendsuffix). Sincem->forwarding = true, handler creation is suppressed — this follows the same pattern asgenfun_forward()for behavior forwarding methods.Step 9: Partial Application
Unaffected. The
!partialguard in Step 2 prevents annotation. Partial application on abstract types with differing defaults remains out of scope.Step 10: Tests
Good Pony (should compile):
fun foo[A: Stringable val](x: (A | None) = None)) — exercises the deferred reification path in shim codegenBad Pony (should error):
Behavior tests:
Step 11: Build and Run Tests
Step 12: Release Notes
Create a release notes file. This is both a bug fix (union/intersection defaults) and a breaking change (subtyping now requires matching defaults). Since both
changelog - fixedandchangelog - changedapply, the CHANGELOG is updated manually after merge.The release note should describe:
Files Modified
src/libponyc/type/subtype.cis_reified_fun_sub_fun()src/libponyc/expr/call.ccheck_union_defaults()src/libponyc/ast/treecheckdef.hHAS_DATAtoTK_CALLrulesrc/libponyc/reach/reach.hbool default_shim; declarereach_method_shim()src/libponyc/reach/reach.csrc/libponyc/codegen/genname.hgenname_fun_shim()src/libponyc/codegen/genname.cgenname_fun_shim()src/libponyc/codegen/genfun.cgenfun_default_shim()body generatorsrc/libponyc/codegen/gencall.cgen_call(); updatecan_inline_message_send()test/libponyc/badpony.cctest/libponyc/goodpony.ccAppendix: Non-Trailing Default Gaps
Shims work by reduced arity — a shim for arity
kaccepts the firstkpositional arguments and fills in defaults for the rest. This naturally handles trailing defaults (omitting arguments from the end), but not non-trailing gaps where named arguments skip over middle defaulted parameters.Example
After
apply_named_args(), the positional argument list is[42, TK_NONE, true]— there's a gap at position 1 (y), not at the trailing end. No single arity value represents "providedxandzbut noty":(x)and fills in bothyandz(x, y)and fills in justzxandz, fill iny"Why shims don't cover this
Handling arbitrary subsets of defaulted parameters would require either:
ddefaulted parameters, that's2^d - d - 1additional shims beyond the trailing ones (exponential growth).Neither is justified for what is an uncommon calling pattern.
Current behavior
When a non-trailing gap is detected on an abstract type call:
This is an improvement over the current silent wrong-default behavior.
Beta Was this translation helpful? Give feedback.
All reactions