Simplify Tfun_modifiers: array → value class, unify joins#1187
Open
Simplify Tfun_modifiers: array → value class, unify joins#1187
Conversation
Add doc comments explaining variance-aware modifier encoding: - Tfun_modifiers type alias in skipNamedAst.sk (array states table) - join_modifiers and its four dispatch variants in skipTypes.sk - promote_tfun_modifiers subtyping expansion - join_purity_modifiers, join_tracking_modifiers, join_tfun_modifiers Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Remove the tracking expansion [Ftracked()] → [Ftracked(), Funtracked()] from promote_tfun_modifiers. The variant form for tracking conveyed no information beyond [Ftracked()]: - All consumers (check_tracking, fun_is_mutable_is_untracked, is_frozen_tfun_modifiers, PP, OuterIST) only ever read tracking[0]. - The join machinery already handles variance positions explicitly via its variance arguments; it doesn't need subtyping baked into the array shape. - Tracking has only two values, so a size-2 array can only ever be [Ftracked(), Funtracked()] — redundant with [Ftracked()] at [0]. Purity keeps its promotion because [Fpure(), Fmutable()] at [0] reports pure while [Fpure()] also reports pure — but for variance-aware joins, that encoding matters (covariant side sees Fpure, contravariant side sees Fmutable). Tracking can't take advantage of that since both positions reduce to the same [0] read. After this commit, tracking stays in canonical size-1 form outside the transient size-0 error state. The doc comment on Tfun_modifiers is updated to reflect this. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Implements the tracking lattice (Ftracked ≤ Funtracked) directly on single values instead of going through the generic array-based join_modifiers machinery. Nothing calls it yet — wiring comes next. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Replace Array<Tracking_modifier> with Tracking_modifier in the Tfun_modifiers type alias. The array encoding was overkill for tracking since there are only two variants and the subtyping relationship (tracked ≤ untracked) is now handled directly by join_tracking. Updates all 8 files that interact with the tracking component: - skipNamedAst.sk: type alias and doc comment - skipTypes.sk: join_tfun_modifiers uses join_tracking, promote passes tracking through, remove tracking.isEmpty() guards from join sites, simplify fun_is_mutable_is_untracked and is_frozen_tfun_modifiers - skipTyping.sk: check_tracking takes single value, pipe operator - skipNaming.sk: make_tfun_tracking and lambda_modifiers return bare values - skipMakeOuterIst.sk: remove tracking.size() == 1 guard - skipNamedAstPp.sk: print single value instead of array - skipTypedAstPp.sk: direct pattern match - skipTypedAstExpand.sk: tracking part of tfun_mods becomes identity Note on skipMakeOuterIst.sk: the original guard 'purity.size() == 1 && tracking.size() == 1' was a defensive assertion rather than handling an actual size-2 case. Earlier in the pipeline skipTypedAstExpand.sk runs prune_tfun_mods which reduces any non-empty modifier array to a single element, so both purity and tracking were always size 1 at this stage — the guard's fallthrough (NAst.Tfun _ -> invariant_violation) would only fire on a broken pipeline invariant. With the previous commit ensuring tracking is never promoted to size 2, and this commit making tracking a single value in the type itself, the tracking.size() == 1 check becomes tautologically true and unexpressible. Purity's guard remains because it can legitimately be size 2 pre-expansion. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
These array-based functions are no longer needed now that join_tfun_modifiers uses join_tracking (single-value) directly. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Replace the purity expansion [Fpure()] → [Fpure(), Fmutable()] with the identity. promote_tfun_modifiers now returns its input unchanged for both components. The variant form [Fpure(), Fmutable()] carried no information beyond [Fpure()] at position [0]: every consumer reads only [0], and the join machinery handles variance explicitly via its variance arguments. Tracking stopped being promoted for the same reason in an earlier commit; purity now follows. Call sites and the function itself are kept for this commit so the change stays small; subsequent commits remove the calls and delete the function. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
promote_tfun_modifiers is now the identity, so its call sites are no-ops. Remove them from the promote pass (Tfun/Tlambda covariant contexts) and from the lambda typing path. The function itself remains defined for now; the next commit deletes it. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
All call sites were removed in the previous commit. Drop the function. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Mirrors join_tracking: implements the purity lattice (Fpure ≤ Fmutable) directly on single values with explicit variance dispatch (LUB/GLB/compatibility/equality). Nothing calls it yet — wiring comes when Tfun_modifiers switches to a single Purity_modifier. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Replace Array<Purity_modifier> with Purity_modifier in the Tfun_modifiers type alias. Purity now mirrors tracking: a single value with subtyping handled directly by join_purity. Updates all files that interact with the purity component: - skipNamedAst.sk: type alias, doc comment, is_pure reads i0 directly - skipTypes.sk: join_tfun_modifiers uses join_purity, purity_modifiers_for_tfrozen takes/returns a single value, drop purity.isEmpty() guards from join sites, simplify fun_is_mutable_is_untracked and is_frozen_tfun_modifiers, convert freeze_modifiers/to_readonly_modifiers/chill_modifiers signatures - skipTyping.sk: literal Array[N.Fpure()] / Array[N.Fmutable()] become bare values; fix_fdt_tfun_purity_modifiers simplified - skipNaming.sk: make_met_tfun_purity and lambda_modifiers return bare values; lambda type construction drops Array wrapper; pattern-match guard simplified - skipMakeOuterIst.sk: drop purity.size() == 1 guard - skipNamedAstPp.sk: print single value instead of array - skipTypedAstPp.sk: direct pattern match - skipTypedAstExpand.sk: tfun_mods becomes identity - skipTypingUtils.sk: pattern-match guard simplified Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
With both purity and tracking now using single values, the entire array-based join_modifiers family is unreferenced. Remove: - join_modifiers (generic variance dispatch) - join_modifiers_inv / _plus_plus / _minus_minus / _minus_plus - uniq_modifiers - compare_purity_modifiers wrapper in skipTypes.sk (join_purity uses N.compare_purity_modifiers directly) - join_purity_modifiers Also remove prune_tfun_mods and tfun_mods from skipTypedAstExpand.sk — modifiers are already in canonical form, nothing to prune or rebuild. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Replace the (Purity_modifier, Tracking_modifier) tuple type alias with
a value class with named fields. Call sites now read mods.purity /
mods.tracking instead of destructuring anonymous tuple elements, which
reads better and eliminates all (purity, tracking) = mods unpacking.
- skipNamedAst.sk: replace alias with `value class Tfun_modifiers{
purity: Purity_modifier, tracking: Tracking_modifier} uses Equality,
Hashable`; update Tfun/Tlambda == and hash impls to compare the class
directly rather than destructuring into (pl, tl) pairs; update
is_pure signature
- Construction sites switch to Tfun_modifiers{purity, tracking}
- Destructure sites use .purity / .tracking field access
- modifiers.i0 at the freeze/chill/readonly call sites becomes
modifiers.purity
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Move the helper's logic onto Tfun_modifiers as two methods. The single caller (to_string_) now calls mods.isMutable() and mods.isUntracked() directly, dropping the 2-tuple return value. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Move the helper's logic onto Tfun_modifiers as a method. The single caller (lambda frozen-level in skipTyping.sk) now calls mods.isPure(). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Move the helper's logic onto Tfun_modifiers as a method. Callers in skipTypes.sk and skipTypingUtils.sk now use mods.isFrozen() instead of Types.is_frozen_tfun_modifiers(mods). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Derive a total order on each lattice that matches its subtyping relation: the strict element compares less than the lax one. Purity: Fpure < Fmutable Tracking: Ftracked < Funtracked Both swap `uses Equality` for `uses Orderable` (Orderable extends Equality) and implement compare via a private typeOrder helper, following the Mutability pattern in types.sk. This enables join_modifier to drop its is_lax predicate in the next commit and use <=/>= directly. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Replace the two near-identical join functions with a single generic join_modifier<T: Orderable> that uses <=/>=/== directly. The variance dispatch (LUB/GLB/compatibility/equality) is identical for both lattices — only the concrete "strict" vs "lax" elements differ, and those are already encoded as the ordering on each modifier type. Callers (join_tfun_modifiers, purity_modifiers_for_tfrozen) drop to a single line. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
With join_modifier using == via the Equality trait, the explicit compare_purity_modifiers and compare_tracking_modifiers wrappers (and the shared compare_modifiers helper they depended on) are unreferenced. Drop all three. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
mbouaziz
force-pushed
the
simplify-purity-modifier
branch
from
f257573 to
da4d370
Compare
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
1 participant
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.
Summary
Rework
Tfun_modifiers— the purity × tracking pair attached toTfun/Tlambdatypes — from a pair of variance-encoded arrays to a small value class with single-value fields, and collapse the variance-aware join machinery onto a singleOrderable-based function.Tfun_modifiersgoes from(Array<Purity_modifier>, Array<Tracking_modifier>)tovalue class Tfun_modifiers{purity: Purity_modifier, tracking: Tracking_modifier}. The array encoding was only needed to represent the promoted[Fpure(), Fmutable()]/[Ftracked(), Funtracked()]form used for covariant subtyping, but every consumer only ever read position[0]— the variant form carried no information beyond its first element. Removing it deletespromote_tfun_modifiersand the entireprune_tfun_mods/tfun_modspipeline.join_modifiers<Ta>family (_inv,_plus_plus,_minus_minus,_minus_plus, plusuniq_modifiers) goes away. In its place is a singlejoin_modifier<T: Orderable>that dispatches on(v1, v2)variance pairs using<=/>=/==directly. Both purity (Fpure < Fmutable) and tracking (Ftracked < Funtracked) implementOrderablewithtypeOrderfollowing theMutabilitypattern in types.sk.fun_is_mutable_is_untracked,is_pure, andis_frozen_tfun_modifiersbecomeisMutable()/isUntracked()/isPure()/isFrozen()methods on the value class.skargo check-clean.The work is layered: (1) tracking gets simplified first (5 commits), (2) then the same treatment is applied to purity (6 commits), (3) then
Tfun_modifiersbecomes a named value class (4 commits + later formatter/refactor commits), and (4) the two remainingjoin_*functions merge underjoin_modifierwithOrderable(3 commits at the tip).Test plan
skargo checkpasses at each commit (verified locally per commit)skargo testpasses for the compilerjoin_modifier's mixed-variance paths)AllowTrackedCall.skand other tracking tests still pass🤖 Generated with Claude Code