Commit 9567ee7
Refactor EraseUnions: DU-based domain model for union lowering (#19518)
* Add UnionLayout taxonomy + assertions (no behavior change)
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Extract shared concerns (nullability, debug proxy)
Extract three helper functions from megafunctions in EraseUnions.fs:
- rewriteFieldsForStructFlattening: Nullable attribute rewriting for struct DU
field flattening, extracted from mkClassUnionDef
- emitDebugProxyType: Debug proxy type generation for union alternatives,
extracted from convAlternativeDef
- rootTypeNullableAttrs: Root type [Nullable(2)] attribute application,
extracted from mkClassUnionDef
Zero behavior change - code is moved verbatim into named functions.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Migrate instruction functions to exhaustive layout matching
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Decompose convAlternativeDef into focused sub-functions
- Wire TypeDefContext into mkClassUnionDef for parameter bundling
- Change return type from 6-element tuple to AlternativeDefResult record
- Extract emitMakerMethod for non-nullary case maker methods
- Extract emitTesterMethodAndProperty for Is* test methods/properties
- Extract emitNullaryCaseAccessor for nullary case getter properties
- Extract emitConstantAccessor for unique singleton object accessors
- Extract emitNullaryConstField for nullary case static field definitions
- Extract emitNestedAlternativeType for nested case type definitions
- Rename convAlternativeDef to processAlternative (~46 lines)
- Format with fantomas
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Decompose mkClassUnionDef into focused sub-functions
Extract emitRootClassFields, emitRootConstructors, emitConstFieldInitializers,
emitTagInfrastructure, assembleUnionTypeDef, computeSelfAndTagFields, and
computeEnumTypeDef from the monolithic mkClassUnionDef function. The main
function is now a 54-line orchestrator that delegates to these helpers.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Remove old UnionReprDecisions infrastructure
Delete DiscriminationTechnique DU, UnionReprDecisions generic class,
cuspecRepr/cudefRepr instances, NoTypesGeneratedViaThisReprDecider,
and layoutToTechnique bridge function.
All code paths now use UnionLayout type with exhaustive active patterns
and focused helper functions (altFoldsAsRootInstance, altOptimizesToRoot,
maintainConstantField, hasNullCase).
Simplify AlternativeDefResult.NullaryConstFields tuple by removing the
unused (ILTypeDef * IlxUnionInfo) element.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Refactor mkNewData: replace boolean deconstruction with pattern match
The NoHelpers branch extracted isStruct/isNull into booleans then used
if/elif chains. Replace with direct pattern matching on (layout, cidx)
and layout active patterns, making the logic readable at a glance.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Deduplicate tyForAlt by reusing altOptimizesToRoot
tyForAlt duplicated the entire altOptimizesToRoot classification logic
inline. Extract tyForAltIdx taking cidx for direct calls, and have
tyForAlt look up cidx. All internal callers that already have cidx
now call tyForAltIdx directly.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Simplify processAlternative NoHelpers branch with pattern match
Replace 'not alt.IsNullary && (match ctx.layout with ValueTypeLayout
-> true | ReferenceTypeLayout -> false)' with a direct match on
ctx.layout with a when guard, collapsing two match arms into one.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Simplify emitTesterMethodAndProperty nullness guard
Replace the boolean chain 'g.checkNullness && g.langFeatureNullness &&
(match layout ... -> true | ... -> false) && not alt.IsNullary' with a
direct pattern match on ctx.layout with when guard. Also merge the two
early-return conditions into one.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Simplify emitRootClassFields loop guard with layout match
Replace 'altFoldsAsRootInstance || (match layout ValueTypeLayout -> true
| ReferenceTypeLayout -> false)' with a clear match: value types always
put fields on root, reference types only when they fold as root instance.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Decompose emitRootConstructors complex guard into named conditions
Break the 5-line boolean expression into named conditions with comments
explaining when the root ctor is needed vs skipped.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Replace isStruct bool with layout match in rewriteFieldsForStructFlattening
The function took an isStruct bool and checked 'isStruct && cases > 1'
which is exactly TaggedStructUnion. Match on layout directly and remove
the now-unnecessary cud parameter.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Eliminate isSingleNonNullaryFoldedToRoot in favor of altFoldsAsRootInstance
isSingleNonNullaryFoldedToRoot duplicated logic already in
altFoldsAsRootInstance for SmallRefUnion. Replace all 4 call sites with
altFoldsAsRootInstance which encodes the same semantics. Also simplify
caseFoldsToRootClass which was a thin wrapper around the eliminated fn.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Eliminate caseFoldsToRootClass, inline via altFoldsAsRootInstance
caseFoldsToRootClass was a thin wrapper around altFoldsAsRootInstance
for SmallRefUnion. All 3 callers are already inside SmallRefUnion match
arms, so they can call altFoldsAsRootInstance directly.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Refactor classifyUnion: use match expression for readable classification
Replace nested if/elif chain with a match on (isList, alts.Length,
isStruct) tuple. Hoist allNullary computation. Each match arm maps
cleanly to one UnionLayout case.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Simplify maintainConstantField: replace chained AP-to-bool with match
Replace 'alt.IsNullary && (match ValueTypeLayout -> false | ...) &&
(match CaseIsNull -> false | ...)' with a nested match that reads
naturally: null-represented cases don't need a constant field,
value types don't need one, ref types do.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Simplify emitNullaryCaseAccessor: match on CaseIsNull directly
Replace 'g.checkNullness && g.langFeatureNullness && (match CaseIsNull
-> true | ...)' with a direct match on (layout, num) with a when guard,
eliminating the AP-to-bool intermediate.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Apply fantomas formatting
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Wave 1: Replace avoidHelpers bool with DataAccess DU
Introduce DataAccess = RawFields | ViaHelpers | ViaListHelpers that
collapses the per-call-site avoidHelpers:bool × per-union HasHelpers
enum into a single DU computed once at entry points.
- avoidHelpers parameter eliminated from 16 internal + 7 public functions
- doesRuntimeTypeDiscriminateUseHelper simplified from 3-way && to DU match
- emitLdDataTagPrim: 'match HasHelpers with AllHelpers when not avoidHelpers'
becomes clean 'match access with ViaHelpers | ViaListHelpers'
- adjustFieldName split: DataAccess version for access path,
adjustFieldNameForTypeDef for type-def path
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Wave 2: Rename nullCaseIdx→nullAsTrueValueIdx, ListTailOrNull→FSharpList
nullCaseIdx was confusable with 'nullary' (no data). It actually means
UseNullAsTrueValue — a case represented as null at runtime. Renamed to
nullAsTrueValueIdx throughout.
ListTailOrNull renamed to FSharpList for clarity — this layout is
exclusively for F# list<'T>.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Wave 3: Split allNullary bool into explicit TaggedRef/TaggedRefAllNullary DU cases
TaggedRefUnion(baseTy, allNullary:bool) → TaggedRef baseTy + TaggedRefAllNullary baseTy
TaggedStructUnion(baseTy, allNullary:bool) → TaggedStruct baseTy + TaggedStructAllNullary baseTy
Eliminates the hidden 3-way logic where TaggedRefUnion(_, true) was
enum-like (all on root), TaggedRefUnion(_, false) split nullary→root
vs non-nullary→nested. Now explicit DU cases, no boolean field.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Wave 3+4: Split allNullary and SmallRefUnion into explicit DU cases
Fix SpecialFSharpOptionHelpers mapping (ViaHelpers, not RawFields).
TaggedRefUnion(_, allNullary) → TaggedRef + TaggedRefAllNullary
TaggedStructUnion(_, allNullary) → TaggedStruct + TaggedStructAllNullary
SmallRefUnion(_, opt) → SmallRef + SmallRefWithNullAsTrueValue(_, idx)
UnionLayout now has 8 cases, zero boolean fields, zero option fields.
Every match arm is explicit — no hidden 3-way logic via bool destructuring.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Wave 5: Remove dead code, eliminate hasNullCase, explicit match arms
- Remove 3 dead SmallRef when-guards (altFoldsAsRootInstance always
returns false for SmallRef — dead code since the DU split)
- Replace wildcard in altFoldsAsRootInstance with explicit cases
for compiler-enforced exhaustiveness
- Eliminate hasNullCase function — callers match SmallRefWithNullAsTrueValue directly
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Wave 6: Extract nullnessCheckingEnabled helper, restructure classifyUnion
- Add nullnessCheckingEnabled helper replacing 5 scattered
'g.checkNullness && g.langFeatureNullness' conjunctions
- Restructure classifyUnion: replace 4 when-guarded arms with
nested match on (isStruct, allNullary) — exhaustive, no guards
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Wave 7: Inline doesRuntimeTypeDiscriminateUseHelper, match-based tester guard
- Inline doesRuntimeTypeDiscriminateUseHelper (1-line function, 2 call
sites) directly into mkRuntimeTypeDiscriminate and mkRuntimeTypeDiscriminateThen
- Replace 'cud.UnionCases.Length <= 1 || (match SmallRefWithNull -> true)'
with clean match on layout cases (SingleCaseRef, SingleCaseStruct,
SmallRefWithNullAsTrueValue → skip tester)
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Wave 8: Split TaggedStructAllNullary from when guard, remove dead fallthrough
TaggedStructAllNullary + when alt.IsNullary was redundant — all cases
are nullary by definition. Split into explicit arm without guard.
Remove TaggedStructAllNullary from default fallthrough (already handled).
Remove dead 'when cud.UnionCases.Length <= 1' guard (covered by layout).
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* F10+F11: Remove duplicate comment, clean unreachable match arm
Remove duplicate XML doc comment at line 1785.
Remove dead FSharpList arm from emitRawConstruction default (already
handled by when alt.IsNullary and non-nullary Cons arms above).
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Round 2: Domain model improvements (F1+F5+F7+F8+F4)
F1: Replace NullaryConstFields 5-tuple with NullaryConstFieldInfo record
— named fields instead of opaque (IlxUnionCase*ILType*int*ILFieldDef*bool)
F5: Introduce CaseStorage DU (Null|Singleton|OnRoot|InNestedType|StructTag)
— classifyCaseStorage computes once per case, used in emitCastToCase
and processAlternative to eliminate duplicated decision trees
F7: Rename helpers HOW→WHAT for domain clarity:
altFoldsAsRootInstance → caseFieldsOnRoot
altOptimizesToRoot → caseRepresentedOnRoot
maintainConstantField → needsSingletonField
convNewDataInstrInternal → emitRawNewData
F8: emitDebugProxyType: 11 positional params → TypeDefContext + 2 params
F4: Extract ILStamping record from TypeDefContext, separating domain
data (layout, cuspec, cud) from infrastructure callbacks
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Round 2b: CaseIdentity, CaseStorage broadened, renames, dedup
G1: Introduce CaseIdentity record + resolveCase — eliminates repeated
(alt, altTy, altName) computation in 4 emit functions
G3: Use CaseStorage in emitRawConstruction — replaces nested layout
match + 4 when guards with flat match on precomputed CaseStorage.
Fix: needsSingletonField fallback for nullary SmallRef cases.
G4: Merge duplicate Singleton/InNestedType branches (OR-pattern)
G5: Remove mkTagFieldFormalType (identical to mkTagFieldType)
G9: Rename self* → rootCase* for domain clarity
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Perf: struct CaseIdentity, inline CaseIsNull AP, eliminate allocations
- [<Struct>] on CaseIdentity record — allocated per-instruction-site,
immediately destructured, no need to heap-allocate
- inline on (|CaseIsNull|CaseIsAllocated|) AP — avoids tuple allocation
at 8 call sites (tuple was (layout, cidx))
- Array.filter |> Array.length = 1 → Array.existsOne in caseFieldsOnRoot
— eliminates intermediate array allocation per call
- cuspec.Alternatives → cuspec.AlternativesArray in emitLdDataTagPrim
— avoids Array.toList allocation
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Eliminate redundant classifyFromSpec/baseTyOfUnionSpec in EraseUnions
Add tyForAltIdxWith and resolveCaseWith that accept precomputed layout
and baseTy. Update emit functions (emitRawConstruction, emitIsCase,
emitBranchOnCase, emitCastToCase, emitCaseSwitch, emitLdDataTagPrim)
and type-def functions (emitNullaryConstField, emitNestedAlternativeType)
to use the precomputed values instead of recomputing per call.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Readability: extract nullable rewrite helper, improve comments
- Extract rewriteNullableAttrForFlattenedField from
rewriteFieldsForStructFlattening — reduces nesting from 3→1 level,
documents the byte semantics clearly
- Add comments: reverse iteration rationale in emitLdDataTagPrim,
minNullaryIdx ctor sharing logic, isAbstract derivation
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Phase 4: emitIsCase+emitBranchOnCase → CaseStorage × DiscriminationMethod
Replace inline re-derivation from raw UnionLayout with two-axis
classification: CaseStorage (WHERE) × DiscriminationMethod AP (HOW).
emitIsCase: match storage with Null→null-ceq, then match (storage,
layout) with OnRoot+RuntimeType→non-null-test, NoDiscrimination→
always-true, RuntimeType→isinst, TagField→tag-ceq, TailNull→tail-check.
emitBranchOnCase: same two-axis structure with branch instructions.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Phase 4: Architecture docs, InteropCapability, reduce mkMethodsAndProps params
- Add two-axis architecture comment documenting UnionLayout×CaseStorage model
- Add InteropCapability type + classifier for fslang-suggestions/1463 readiness
(ClosedHierarchy|UnionProtocol|UnionProtocolWithTupleBoxing|NoInterop)
- Reduce mkMethodsAndPropertiesForFields from 8 params to 3 (ctx, ilTy, fields)
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Remove speculative InteropCapability code
Dead code for a future feature that hasn't been approved.
Only code that is actually used belongs in the codebase.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Remove dead code: _validateActivePatterns, NonNullaryFoldsToRoot, FieldsOnRootType APs
All three were never called by any live code path:
- _validateActivePatterns: 'compile-time check' trick — real match sites enforce exhaustiveness
- NonNullaryFoldsToRoot AP: only consumed by the removed validation function
- FieldsOnRootType AP: same
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Enrich DiscriminationMethod AP to carry baseTy and nullAsTrueValueIdx
Active patterns can carry data. The DiscriminationMethod AP now returns:
- DiscriminateByTagField baseTy
- DiscriminateByRuntimeType(baseTy, nullAsTrueValueIdx: int option)
- DiscriminateByTailNull baseTy
- NoDiscrimination baseTy
This eliminates 3 separate baseTyOfUnionSpec calls and 2 re-matches
on SmallRefWithNullAsTrueValue to extract nullIdx in emit functions.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Split EraseUnions.fs into 3 files by semantic boundary
EraseUnions.Types.fs (421 lines): Domain model, classification, APs
UnionLayout, CaseStorage, DataAccess, CaseIdentity, all active
patterns, classification functions, layout-based helpers.
EraseUnions.Emit.fs (442 lines): IL instruction emission
Per-instruction-site IL: construct, discriminate, branch, cast,
switch, load field, load tag. Public API consumed by IlxGen.fs.
EraseUnions.fs (1039 lines): Type definition generation
Per-union-definition: generate ILTypeDef with nested types, methods,
properties, fields, debug proxies. The orchestrator.
Dependency flow: Types ← Emit ← TypeDef (one-directional).
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Fix bug + review findings: bounds check, stale comments, magic numbers
BUG FIX: emitRawConstruction for SingleCaseStruct nullary — was emitting
tag-ctor for CaseStorage.OnRoot+IsNullary but SingleCaseStruct has no
tag field (NoTagField). Now checks HasTagField|NoTagField before
choosing ctor shape.
Review fixes (3-model voted, 8/10 unanimous):
- H1: altOfUnionSpec blanket 'with _' catch → bounds check
- H2: tyForAlt Array.findIndex → tryFindIndex + meaningful error
- H3+H4: Fix stale case counts in architecture comment (8→9, 5→4)
- H5: Remove unused _cuspec param from mkTagFieldType (10 call sites)
- H7: List.ofArray|>List.mapi → Array.mapi|>Array.toList
- H8: Rename quadruple negation hasFieldsOrTagButNoMethods → onlyMethodsOnRoot
- H9: Named constants for DynamicDependency flags 0x660/0x7E0
Also revert unrelated FSharp.Core doc/test changes (per expert review).
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* DDD review: pure classification in Types.fs, architecture docs, section headers
P1: Move TypeDefContext/ILStamping/AlternativeDefResult/NullaryConstFieldInfo
from Types.fs to EraseUnions.fs — they're type-def scaffolding, not
classification algebra. Types.fs is now purely: DU types, classifiers, APs.
P2+P6: Add architecture overview at top of EraseUnions.fs with pipeline
description and concrete DU→UnionLayout→CaseStorage example mappings
(Option, Color, Result, Shape, Token).
P3: Add section header before nullable-attr rewriting functions.
P7: Fix duplicate comment in rewriteNullableAttrForFlattenedField.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Council fixes: private mkMethodsAndPropertiesForFields, shared adjustFieldNameForList
- mkMethodsAndPropertiesForFields → private (internal-only, flagged by Opus)
- Extract adjustFieldNameForList to Types.fs as shared naming core
(adjustFieldNameForTypeDef and adjustFieldName both delegate to it,
eliminating duplicated Head→HeadOrDefault / Tail→TailOrNull mapping)
- Flagged by 4/7 council models across 2 rounds
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Fix: preserve exact IL for Option — add DataAccess.ViaOptionHelpers
SpecialFSharpOptionHelpers needs helpers for FIELD access (private
fields) but raw discrimination for TAG access (inline isinst, not
GetTag call). The old code made these decisions independently.
DataAccess.ViaOptionHelpers: field access uses get_X helpers (like
ViaHelpers), tag access uses raw isinst chain (like RawFields).
This preserves exact IL output — zero .bsl changes.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Apply patch from /run fantomas
---------
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: GH Actions <actions@github.com>1 parent b74a9e1 commit 9567ee7
6 files changed
Lines changed: 1790 additions & 1533 deletions
File tree
- src/Compiler
- CodeGen
0 commit comments