Fix non-commutative union reduction for 3+ types involving modules#16952
Fix non-commutative union reduction for 3+ types involving modules#16952stakach wants to merge 2 commits into
Conversation
|
Going from a linear scan to a quadratic one is a huge leap, and we would prefer not doing so unless it is known (or better yet, proven by a human) to be optimal, because quadratic scans over union types have in the past led to some drastic build time explosions. |
60fa343 to
2430b38
Compare
|
Reworked to address concerns about asymptotic regression. The original The refined algorithm:
Net effect: identical work to the original on non-absorbing additions and on |
Summary
Resolves #10788 and #15752 — both symptoms of the same root cause in
Crystal::Program#type_combine.#10788example:#15752example:Pointer(Bar0 | Bar1 | Bar2 | Foo)(where eachBarNincludesFoo) was failing to reduce toPointer(Foo)for certain orderings, and an unrelated cast through that pointer corrupted instance variables on the read value.Root cause
type_combinewalks the input left-to-right and, for each newt2, scans existing accumulated types looking for a least-common-ancestor with eacht1. When it finds one, it absorbst1into the ancestor — but uses.all?, which short-circuits on the first absorption. Any latert1that also implements the same ancestor is left inall_types, so:B | C | A→ afterB | C→[B, C]→ seeingA, absorbsBonly → final[C, A]instead of[A].Fix
Iterate over a snapshot of
all_types(so we may keep mutating the live list) and absorb every matchingt1, deduping the ancestor on insertion. One localized change insrc/compiler/crystal/semantic/type_merge.cr#type_combine.After the fix, all 12 permutations from the issue collapse to
A, and thePointer(Bar0 | Bar1 | Bar2 | Foo)case correctly reduces toPointer(Foo).Test changes
spec/compiler/semantic/union_spec.crwith a helperassert_all_permutations_merge_tothat verifies all permutations of a 3- or 4-type list merge to the same canonical type. Two new specs cover the module-and-sibling-classes shape from both issues.spec/compiler/semantic/pointer_spec.cr's#15742test was asserting the buggy element type (Pointer(Bar2 | Foo)instead ofPointer(Foo)) — its own inline comment even saidshould most certainly be just Foo*. Updated the assertion to the now-correct value and added#15752to the spec title.Verification
#10788reproducer: all 12 macro orderings →A✓#15752reproducer:Pointer(Foo), and the through-cast no longer corrupts@a/@b✓