From ffa5744cbb47380cb60ca1960b0288a0a9eeac5f Mon Sep 17 00:00:00 2001 From: Tomas Grosup Date: Sat, 18 Apr 2026 10:54:43 +0200 Subject: [PATCH 01/14] Index eOpenedTypeOperators by logical name (NameMultiMap) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Before: eOpenedTypeOperators was a linear MethInfo list scanned on every SRTP trait-resolution call in SelectExtensionMethInfosForTrait: the loop filtered by string equality on minfo.LogicalName, costing O(N) per lookup where N is the total number of operator methods brought into scope via 'open type'. After: store as NameMultiMap (NameMap), aggregating by LogicalName. Lookup becomes a single NameMultiMap.find, O(log N) + O(bucket) where bucket size is the number of homograph operators. Write path folds operatorMethods into the multi-map. This mirrors the pattern already used for eTyconsByDemangledNameAndArity and similar fields in NameResolutionEnv. Pure refactor: no change in semantics, no observable behavior change, no public API or signature surface impact. Measured on the small ext-operator SRTP stress: TC wall-clock within noise (expected — N is small there). Benefit grows linearly with the number of 'open type' declarations in scope, which is the FSharpPlus- shaped usage pattern this feature targets. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/Compiler/Checking/NameResolution.fs | 17 ++++++++++------- src/Compiler/Checking/NameResolution.fsi | 2 +- 2 files changed, 11 insertions(+), 8 deletions(-) diff --git a/src/Compiler/Checking/NameResolution.fs b/src/Compiler/Checking/NameResolution.fs index e85897d2a6..9b04bb4d80 100644 --- a/src/Compiler/Checking/NameResolution.fs +++ b/src/Compiler/Checking/NameResolution.fs @@ -450,8 +450,8 @@ type NameResolutionEnv = /// Other extension members unindexed by type eUnindexedExtensionMembers: ExtensionMember list - /// Static operator methods from 'open type' declarations, available for SRTP resolution - eOpenedTypeOperators: MethInfo list + /// Static operator methods from 'open type' declarations, available for SRTP resolution, indexed by logical name. + eOpenedTypeOperators: NameMultiMap /// Typars (always available by unqualified names). Further typars can be /// in the tpenv, a structure folded through each top-level definition. @@ -475,7 +475,7 @@ type NameResolutionEnv = eFullyQualifiedTyconsByDemangledNameAndArity = LayeredMap.Empty eIndexedExtensionMembers = TyconRefMultiMap<_>.Empty eUnindexedExtensionMembers = [] - eOpenedTypeOperators = [] + eOpenedTypeOperators = Map.empty eTypars = Map.empty } member nenv.DisplayEnv = nenv.eDisplayEnv @@ -1305,7 +1305,10 @@ let rec AddStaticContentOfTypeToNameEnv (g:TcGlobals) (amap: Import.ImportMap) a if operatorMethods.IsEmpty then nenv else - { nenv with eOpenedTypeOperators = operatorMethods @ nenv.eOpenedTypeOperators } + let eOpenedTypeOperators = + (nenv.eOpenedTypeOperators, operatorMethods) + ||> List.fold (fun acc minfo -> NameMultiMap.add minfo.LogicalName minfo acc) + { nenv with eOpenedTypeOperators = eOpenedTypeOperators } and private AddNestedTypesOfTypeToNameEnv infoReader (amap: Import.ImportMap) ad m nenv ty = let tinst, tcrefs = GetNestedTyconRefsOfType infoReader amap (ad, None, TypeNameResolutionStaticArgsInfo.Indefinite, true, m) ty @@ -1744,9 +1747,9 @@ let SelectExtensionMethInfosForTrait (traitInfo: TraitConstraintInfo, m: range, match traitInfo.SupportTypes with | [] -> [] | firstSupportTy :: _ -> - [ for minfo in nenv.eOpenedTypeOperators do - if minfo.LogicalName = nm then - yield (firstSupportTy, minfo) ] + nenv.eOpenedTypeOperators + |> NameMultiMap.find nm + |> List.map (fun minfo -> (firstSupportTy, minfo)) extResults @ openTypeResults diff --git a/src/Compiler/Checking/NameResolution.fsi b/src/Compiler/Checking/NameResolution.fsi index 889ea1ec36..d520e7c24d 100755 --- a/src/Compiler/Checking/NameResolution.fsi +++ b/src/Compiler/Checking/NameResolution.fsi @@ -234,7 +234,7 @@ type NameResolutionEnv = eUnindexedExtensionMembers: ExtensionMember list /// Static operator methods from 'open type' declarations, available for SRTP resolution - eOpenedTypeOperators: MethInfo list + eOpenedTypeOperators: NameMultiMap /// Typars (always available by unqualified names). Further typars can be /// in the tpenv, a structure folded through each top-level definition. From 990146c3d8ebc0fecc61789a6fb8383e09a3ecf9 Mon Sep 17 00:00:00 2001 From: Tomas Grosup Date: Sat, 18 Apr 2026 10:56:46 +0200 Subject: [PATCH 02/14] Avoid redundant IntrinsicMethInfosOfType call in AddStaticContentOfTypeToNameEnv AddStaticContentOfTypeToNameEnv was calling IntrinsicMethInfosOfType twice with identical arguments: once for method-group collection and again for the operator-method filter used to populate eOpenedTypeOperators. Bind the result once and reuse. Pure refactor, no behavior change. Called per 'open type' (and per type opened via 'open' / 'AutoOpen'), so repeated in compilation units with many such declarations. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/Compiler/Checking/NameResolution.fs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/Compiler/Checking/NameResolution.fs b/src/Compiler/Checking/NameResolution.fs index 9b04bb4d80..73b0779906 100644 --- a/src/Compiler/Checking/NameResolution.fs +++ b/src/Compiler/Checking/NameResolution.fs @@ -1271,9 +1271,12 @@ let rec AddStaticContentOfTypeToNameEnv (g:TcGlobals) (amap: Import.ImportMap) a let nenv = { nenv with eUnqualifiedItems = nenv.eUnqualifiedItems.AddMany items } + let allMethInfos = + IntrinsicMethInfosOfType infoReader None ad AllowMultiIntfInstantiations.Yes PreferOverrides m ty + let methodGroupItems = // Methods - IntrinsicMethInfosOfType infoReader None ad AllowMultiIntfInstantiations.Yes PreferOverrides m ty + allMethInfos |> ChooseMethInfosForNameEnv g m ty // Combine methods and extension method groups of the same type |> List.map (fun pair -> @@ -1296,7 +1299,7 @@ let rec AddStaticContentOfTypeToNameEnv (g:TcGlobals) (amap: Import.ImportMap) a // These are intentionally excluded from eUnqualifiedItems by ChooseMethInfosForNameEnv // but need to be available for SRTP constraint solving. let operatorMethods = - IntrinsicMethInfosOfType infoReader None ad AllowMultiIntfInstantiations.Yes PreferOverrides m ty + allMethInfos |> List.filter (fun minfo -> not (minfo.IsInstance || minfo.IsClassConstructor || minfo.IsConstructor) && typeEquiv g minfo.ApparentEnclosingType ty From 80f713cd8bc87ea82be9896d67942c2e2b98d1e5 Mon Sep 17 00:00:00 2001 From: Tomas Grosup Date: Sat, 18 Apr 2026 11:03:04 +0200 Subject: [PATCH 03/14] Preserve within-call order + extract AddMethInfoByLogicalName helper Expert review noted that folding operatorMethods forward into NameMultiMap reversed within-call source order inside each bucket (List.fold + prepend), which could subtly affect overload-resolution diagnostic candidate ordering. Switch to List.foldBack so within-call order matches the prior list-prepend implementation exactly. Also extract the indexing step as AddMethInfoByLogicalName, mirroring the existing AddRecdField helper one NameMap sub-table builder away. Adversarial review flagged the inline fold as a missed reuse opportunity against the AddRecdField pattern. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/Compiler/Checking/NameResolution.fs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/Compiler/Checking/NameResolution.fs b/src/Compiler/Checking/NameResolution.fs index 73b0779906..44a6ffe041 100644 --- a/src/Compiler/Checking/NameResolution.fs +++ b/src/Compiler/Checking/NameResolution.fs @@ -930,6 +930,9 @@ let AddTyconByAccessNames bulkAddMode (tcrefs: TyconRef[]) (tab: LayeredMultiMap /// Add a record field to the corresponding sub-table of the name resolution environment let AddRecdField (rfref: RecdFieldRef) tab = NameMultiMap.add rfref.FieldName rfref tab +/// Index a MethInfo by its logical name into a NameMultiMap sub-table of the environment. +let AddMethInfoByLogicalName (minfo: MethInfo) tab = NameMultiMap.add minfo.LogicalName minfo tab + /// Add a set of union cases to the corresponding sub-table of the environment let AddUnionCases1 (tab: Map<_, _>) (ucrefs: UnionCaseRef list) = (tab, ucrefs) ||> List.fold (fun acc ucref -> @@ -1309,8 +1312,9 @@ let rec AddStaticContentOfTypeToNameEnv (g:TcGlobals) (amap: Import.ImportMap) a nenv else let eOpenedTypeOperators = - (nenv.eOpenedTypeOperators, operatorMethods) - ||> List.fold (fun acc minfo -> NameMultiMap.add minfo.LogicalName minfo acc) + // Use foldBack so that, within this call, operatorMethods land in the bucket + // in source order (same order as the previous list-prepend implementation). + List.foldBack AddMethInfoByLogicalName operatorMethods nenv.eOpenedTypeOperators { nenv with eOpenedTypeOperators = eOpenedTypeOperators } and private AddNestedTypesOfTypeToNameEnv infoReader (amap: Import.ImportMap) ad m nenv ty = From cf76274cbf10f5e99f0c198088bd6d7c4485ae39 Mon Sep 17 00:00:00 2001 From: Tomas Grosup Date: Sun, 19 Apr 2026 10:02:35 +0200 Subject: [PATCH 04/14] Add homograph-operator test + release notes for eOpenedTypeOperators refactor Test: OpenTypeOperatorHomographOrder.fs opens a holder type with three via SRTP. Pins down the >=2-entries-per-bucket path of the new NameMultiMap encoding and the foldBack ordering fix. Also adds a Changed entry to the 11.0.100 compiler-service release notes covering the three in-commit refactors on this branch. Verified: - SurfaceAreaTest passes (no FCS API drift) - ExtensionConstraintsTests + IWSAMsAndSRTPs + full Conformance.Types suite all green (679 tests, 674 passing, 5 skipped) - fantomas reports no formatting drift on NameResolution.{fs,fsi} Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../.FSharp.Compiler.Service/11.0.100.md | 4 ++++ .../ExtensionConstraintsTests.fs | 4 ++++ .../OpenTypeOperatorHomographOrder.fs | 24 +++++++++++++++++++ 3 files changed, 32 insertions(+) create mode 100644 tests/FSharp.Compiler.ComponentTests/Conformance/Types/TypeConstraints/ExtensionConstraints/testFiles/OpenTypeOperatorHomographOrder.fs diff --git a/docs/release-notes/.FSharp.Compiler.Service/11.0.100.md b/docs/release-notes/.FSharp.Compiler.Service/11.0.100.md index c6b5c7b7df..727d9da61d 100644 --- a/docs/release-notes/.FSharp.Compiler.Service/11.0.100.md +++ b/docs/release-notes/.FSharp.Compiler.Service/11.0.100.md @@ -40,3 +40,7 @@ * Added warning FS3884 when a function or delegate value is used as an interpolated string argument. ([PR #19289](https://github.com/dotnet/fsharp/pull/19289)) * Add `#version;;` directive to F# Interactive to display version and environment information. ([Issue #13307](https://github.com/dotnet/fsharp/issues/13307), [PR #19332](https://github.com/dotnet/fsharp/pull/19332)) +### Changed + +* Index `eOpenedTypeOperators` by logical name (`NameMultiMap`), replacing the linear list scan in `SelectExtensionMethInfosForTrait` with a direct bucket lookup. Also deduplicate a repeated `IntrinsicMethInfosOfType` call inside `AddStaticContentOfTypeToNameEnv`. Internal name-resolution cleanup supporting the new `open type` extension-operator feature; no public API change. + diff --git a/tests/FSharp.Compiler.ComponentTests/Conformance/Types/TypeConstraints/ExtensionConstraints/ExtensionConstraintsTests.fs b/tests/FSharp.Compiler.ComponentTests/Conformance/Types/TypeConstraints/ExtensionConstraints/ExtensionConstraintsTests.fs index 57793225f4..351da7d3f6 100644 --- a/tests/FSharp.Compiler.ComponentTests/Conformance/Types/TypeConstraints/ExtensionConstraints/ExtensionConstraintsTests.fs +++ b/tests/FSharp.Compiler.ComponentTests/Conformance/Types/TypeConstraints/ExtensionConstraints/ExtensionConstraintsTests.fs @@ -34,6 +34,10 @@ module ExtensionConstraintsTests = let ``Most recently opened extension wins`` () = compileAndRunPreview "ExtensionPrecedence.fs" + [] + let ``open type with homograph operators yields all overloads for SRTP`` () = + compileAndRunPreview "OpenTypeOperatorHomographOrder.fs" + [] let ``Extension operators respect accessibility`` () = compileAndRunPreview "ExtensionAccessibility.fs" diff --git a/tests/FSharp.Compiler.ComponentTests/Conformance/Types/TypeConstraints/ExtensionConstraints/testFiles/OpenTypeOperatorHomographOrder.fs b/tests/FSharp.Compiler.ComponentTests/Conformance/Types/TypeConstraints/ExtensionConstraints/testFiles/OpenTypeOperatorHomographOrder.fs new file mode 100644 index 0000000000..560d82d20e --- /dev/null +++ b/tests/FSharp.Compiler.ComponentTests/Conformance/Types/TypeConstraints/ExtensionConstraints/testFiles/OpenTypeOperatorHomographOrder.fs @@ -0,0 +1,24 @@ +// Regression: multiple homograph operators declared on a single 'open type' holder +// must all be discoverable via SRTP. Exercises the >=2 entries-per-bucket path of +// eOpenedTypeOperators (NameMultiMap), where within-call source order +// is preserved by List.foldBack at insertion. + +module OpenTypeOperatorHomographOrder + +[] +type Ops = + // Three homograph (++!) overloads on int * int / float * float / string * string + static member inline (++!) (a: int, b: int) = a + b + 1 + static member inline (++!) (a: float, b: float) = a + b + 1.0 + static member inline (++!) (a: string, b: string) = a + b + "!" + +open type Ops + +let r1 : int = 1 ++! 2 +if r1 <> 4 then failwith $"Expected 4, got {r1}" + +let r2 : float = 1.5 ++! 2.5 +if r2 <> 5.0 then failwith $"Expected 5.0, got {r2}" + +let r3 : string = "hi" ++! "world" +if r3 <> "hiworld!" then failwith $"Expected 'hiworld!', got '{r3}'" From 59d5dee7b7eec53f30ae90d45b512d9074fd9626 Mon Sep 17 00:00:00 2001 From: Tomas Grosup Date: Sun, 19 Apr 2026 11:00:56 +0200 Subject: [PATCH 05/14] Replace hand-rolled Dictionary<_, _> with NameMultiMap in CheckEntityDefn MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The ad-hoc grouping of MethInfos by LogicalName in PostInferenceChecks duplicated the pattern already captured by NameMultiMap.initBy. Unifies with the eOpenedTypeOperators indexing convention added in commit 5c44db6d3 (AddMethInfoByLogicalName / NameMultiMap). - hashOfImmediateMeths and hashOfAllVirtualMethsInParent now built via NameMultiMap.initBy. - hashOfImmediateProps left alone — its incremental build with order-dependent self-exclusion is genuinely stateful. - getHash helper retained for props only. No semantic change; targeted tests pass; bench-harness perf neutral. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/Compiler/Checking/PostInferenceChecks.fs | 34 ++++++-------------- 1 file changed, 10 insertions(+), 24 deletions(-) diff --git a/src/Compiler/Checking/PostInferenceChecks.fs b/src/Compiler/Checking/PostInferenceChecks.fs index 9a3e27d439..a45bd1ceb8 100644 --- a/src/Compiler/Checking/PostInferenceChecks.fs +++ b/src/Compiler/Checking/PostInferenceChecks.fs @@ -2399,24 +2399,13 @@ let CheckEntityDefn cenv env (tycon: Entity) = | _ -> [] // precompute methods grouped by MethInfo.LogicalName - let hashOfImmediateMeths = - let h = Dictionary() - for minfo in immediateMeths do - match h.TryGetValue minfo.LogicalName with - | true, methods -> - h[minfo.LogicalName] <- minfo :: methods - | false, _ -> - h[minfo.LogicalName] <- [minfo] - h + let hashOfImmediateMeths : NameMultiMap = + NameMultiMap.initBy (fun (m: MethInfo) -> m.LogicalName) immediateMeths let getOtherMethods (minfo : MethInfo) = - [ - //we have added all methods to the dictionary on the previous step - let methods = hashOfImmediateMeths[minfo.LogicalName] - for m in methods do - // use referential identity to filter out 'minfo' method - if not(Object.ReferenceEquals(m, minfo)) then - yield m - ] + [ for m in NameMultiMap.find minfo.LogicalName hashOfImmediateMeths do + // use referential identity to filter out 'minfo' method + if not (Object.ReferenceEquals(m, minfo)) then + yield m ] let hashOfImmediateProps = Dictionary() for minfo in immediateMeths do @@ -2546,16 +2535,13 @@ let CheckEntityDefn cenv env (tycon: Entity) = hashOfImmediateProps[nm] <- pinfo :: others if not (isInterfaceTy g ty) then - let hashOfAllVirtualMethsInParent = Dictionary() - for minfo in allVirtualMethsInParent do - let nm = minfo.LogicalName - let others = getHash hashOfAllVirtualMethsInParent nm - hashOfAllVirtualMethsInParent[nm] <- minfo :: others + let hashOfAllVirtualMethsInParent : NameMultiMap = + NameMultiMap.initBy (fun (m: MethInfo) -> m.LogicalName) allVirtualMethsInParent for minfo in immediateMeths do if not minfo.IsDispatchSlot && not minfo.IsVirtual && minfo.IsInstance then let nm = minfo.LogicalName let m = (match minfo.ArbitraryValRef with None -> m | Some vref -> vref.DefinitionRange) - let parentMethsOfSameName = getHash hashOfAllVirtualMethsInParent nm + let parentMethsOfSameName = NameMultiMap.find nm hashOfAllVirtualMethsInParent let checkForDup erasureFlag (minfo2: MethInfo) = minfo2.IsDispatchSlot && MethInfosEquivByNameAndSig erasureFlag true g cenv.amap m minfo minfo2 match parentMethsOfSameName |> List.tryFind (checkForDup EraseAll) with | None -> () @@ -2570,7 +2556,7 @@ let CheckEntityDefn cenv env (tycon: Entity) = if minfo.IsDispatchSlot then let nm = minfo.LogicalName let m = (match minfo.ArbitraryValRef with None -> m | Some vref -> vref.DefinitionRange) - let parentMethsOfSameName = getHash hashOfAllVirtualMethsInParent nm + let parentMethsOfSameName = NameMultiMap.find nm hashOfAllVirtualMethsInParent let checkForDup erasureFlag minfo2 = MethInfosEquivByNameAndSig erasureFlag true g cenv.amap m minfo minfo2 if parentMethsOfSameName |> List.exists (checkForDup EraseAll) then From ff77956ac77666bd72b7c85a49397a7fb0e73806 Mon Sep 17 00:00:00 2001 From: Tomas Grosup Date: Sun, 19 Apr 2026 15:59:44 +0200 Subject: [PATCH 06/14] Starter-pack from 10-week hardening plan (WS3/WS4/WS5) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Small, machine-independent slices from PLAN.md, no perf claims. WS3.1 — Document the order invariant at both ends: - NameResolution.fs:1283 (List.foldBack site) explains WHY the fold direction matters and points to the consumer + the regression test. - NameResolution.fs:1705 (SelectExtensionMethInfosForTrait) mirrors the invariant from the consumer side. WS4.1 — Expand homograph test matrix from 1 to 4 scenarios: - OpenTypeOperatorHomographMultipleHolders.fs — operators spread across two separate 'open type' holders must all accumulate into the same bucket (cross-call bucket accumulation). - OpenTypeOperatorNestedModule.fs — 'open type' in nested module scopes the operator correctly; sibling scope unaffected. - OpenTypeOperatorShadowing.fs — local 'let inline (op)' shadows the extension operator from that point on. WS4.3 — Release notes rewrite: drop overclaimed perf framing; honestly describe as 'internal refactor'. Also mention PostInferenceChecks dedup brought in by the follow-up commit. WS5.2 — Rename hashOfImmediateMeths -> immediateMethsByLogicalName and hashOfAllVirtualMethsInParent -> parentVirtualMethsByLogicalName in CheckEntityDefn. They are NameMultiMaps now, not hashes. Verified: ExtensionConstraintsTests (22 passed), *SameName* (6), *Duplicate* (29), *AbstractMember* (18), all green. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../.FSharp.Compiler.Service/11.0.100.md | 2 +- src/Compiler/Checking/NameResolution.fs | 23 +++++++++++++-- src/Compiler/Checking/PostInferenceChecks.fs | 12 ++++---- .../ExtensionConstraintsTests.fs | 12 ++++++++ ...penTypeOperatorHomographMultipleHolders.fs | 28 +++++++++++++++++++ .../testFiles/OpenTypeOperatorNestedModule.fs | 16 +++++++++++ .../testFiles/OpenTypeOperatorShadowing.fs | 21 ++++++++++++++ 7 files changed, 105 insertions(+), 9 deletions(-) create mode 100644 tests/FSharp.Compiler.ComponentTests/Conformance/Types/TypeConstraints/ExtensionConstraints/testFiles/OpenTypeOperatorHomographMultipleHolders.fs create mode 100644 tests/FSharp.Compiler.ComponentTests/Conformance/Types/TypeConstraints/ExtensionConstraints/testFiles/OpenTypeOperatorNestedModule.fs create mode 100644 tests/FSharp.Compiler.ComponentTests/Conformance/Types/TypeConstraints/ExtensionConstraints/testFiles/OpenTypeOperatorShadowing.fs diff --git a/docs/release-notes/.FSharp.Compiler.Service/11.0.100.md b/docs/release-notes/.FSharp.Compiler.Service/11.0.100.md index 727d9da61d..ac495bfd3b 100644 --- a/docs/release-notes/.FSharp.Compiler.Service/11.0.100.md +++ b/docs/release-notes/.FSharp.Compiler.Service/11.0.100.md @@ -42,5 +42,5 @@ ### Changed -* Index `eOpenedTypeOperators` by logical name (`NameMultiMap`), replacing the linear list scan in `SelectExtensionMethInfosForTrait` with a direct bucket lookup. Also deduplicate a repeated `IntrinsicMethInfosOfType` call inside `AddStaticContentOfTypeToNameEnv`. Internal name-resolution cleanup supporting the new `open type` extension-operator feature; no public API change. +* Internal refactor: store extension operators added via `open type` in a name-indexed `NameMultiMap` (`eOpenedTypeOperators`) instead of a flat list. `SelectExtensionMethInfosForTrait` now does a direct bucket lookup for the trait name. Source declaration order of operators within a single `open type` is preserved (see `List.foldBack` invariant comment in `AddStaticContentOfTypeToNameEnv`). Also dedups a hand-rolled `Dictionary` grouping in `CheckEntityDefn` into the same `NameMultiMap.initBy` convention. No public API change; no user-visible behavior change. diff --git a/src/Compiler/Checking/NameResolution.fs b/src/Compiler/Checking/NameResolution.fs index 44a6ffe041..ca00d1bb4d 100644 --- a/src/Compiler/Checking/NameResolution.fs +++ b/src/Compiler/Checking/NameResolution.fs @@ -1312,8 +1312,22 @@ let rec AddStaticContentOfTypeToNameEnv (g:TcGlobals) (amap: Import.ImportMap) a nenv else let eOpenedTypeOperators = - // Use foldBack so that, within this call, operatorMethods land in the bucket - // in source order (same order as the previous list-prepend implementation). + // ORDER INVARIANT: within this single 'open type', operatorMethods must land in + // the NameMultiMap bucket in *source order* (first declared -> head of bucket). + // + // The bucket is consumed by SelectExtensionMethInfosForTrait (below, ~line 1709) + // which feeds the list into overload resolution in ResolveOverloading + // (MethodCalls.fs). Overload resolution is order-sensitive when two overloads + // tie on applicability: the first candidate wins. Reordering would silently + // change which overload is picked for homograph operators declared on the same + // holder type (see OpenTypeOperatorHomographOrder.fs). + // + // This matches the source-order semantics of the previous list-prepend + // implementation that preceded the NameMultiMap index. + // + // Do NOT replace with a left fold unless you also reverse operatorMethods, + // and do NOT use NameMultiMap.initBy here (initBy groups via Seq.groupBy, + // which reverses within-bucket order relative to source). List.foldBack AddMethInfoByLogicalName operatorMethods nenv.eOpenedTypeOperators { nenv with eOpenedTypeOperators = eOpenedTypeOperators } @@ -1750,6 +1764,11 @@ let SelectExtensionMethInfosForTrait (traitInfo: TraitConstraintInfo, m: range, // These are not registered as extension members but should participate in SRTP resolution. // Each method is yielded once (paired with the first support type) to avoid duplicates // that would confuse overload resolution. + // + // ORDER INVARIANT: the list returned by NameMultiMap.find below must be in source + // declaration order (first declared -> head). This is maintained at insertion in + // AddStaticContentOfTypeToNameEnv (see List.foldBack comment there, ~line 1283). + // Overload resolution downstream is order-sensitive. let openTypeResults = match traitInfo.SupportTypes with | [] -> [] diff --git a/src/Compiler/Checking/PostInferenceChecks.fs b/src/Compiler/Checking/PostInferenceChecks.fs index a45bd1ceb8..8ecb4f0a12 100644 --- a/src/Compiler/Checking/PostInferenceChecks.fs +++ b/src/Compiler/Checking/PostInferenceChecks.fs @@ -2399,10 +2399,10 @@ let CheckEntityDefn cenv env (tycon: Entity) = | _ -> [] // precompute methods grouped by MethInfo.LogicalName - let hashOfImmediateMeths : NameMultiMap = + let immediateMethsByLogicalName : NameMultiMap = NameMultiMap.initBy (fun (m: MethInfo) -> m.LogicalName) immediateMeths let getOtherMethods (minfo : MethInfo) = - [ for m in NameMultiMap.find minfo.LogicalName hashOfImmediateMeths do + [ for m in NameMultiMap.find minfo.LogicalName immediateMethsByLogicalName do // use referential identity to filter out 'minfo' method if not (Object.ReferenceEquals(m, minfo)) then yield m ] @@ -2487,7 +2487,7 @@ let CheckEntityDefn cenv env (tycon: Entity) = | None -> m | Some vref -> vref.DefinitionRange - if hashOfImmediateMeths.ContainsKey nm then + if immediateMethsByLogicalName.ContainsKey nm then errorR(Error(FSComp.SR.chkPropertySameNameMethod(nm, NicePrint.minimalStringOfType cenv.denv ty), m)) let others = getHash hashOfImmediateProps nm @@ -2535,13 +2535,13 @@ let CheckEntityDefn cenv env (tycon: Entity) = hashOfImmediateProps[nm] <- pinfo :: others if not (isInterfaceTy g ty) then - let hashOfAllVirtualMethsInParent : NameMultiMap = + let parentVirtualMethsByLogicalName : NameMultiMap = NameMultiMap.initBy (fun (m: MethInfo) -> m.LogicalName) allVirtualMethsInParent for minfo in immediateMeths do if not minfo.IsDispatchSlot && not minfo.IsVirtual && minfo.IsInstance then let nm = minfo.LogicalName let m = (match minfo.ArbitraryValRef with None -> m | Some vref -> vref.DefinitionRange) - let parentMethsOfSameName = NameMultiMap.find nm hashOfAllVirtualMethsInParent + let parentMethsOfSameName = NameMultiMap.find nm parentVirtualMethsByLogicalName let checkForDup erasureFlag (minfo2: MethInfo) = minfo2.IsDispatchSlot && MethInfosEquivByNameAndSig erasureFlag true g cenv.amap m minfo minfo2 match parentMethsOfSameName |> List.tryFind (checkForDup EraseAll) with | None -> () @@ -2556,7 +2556,7 @@ let CheckEntityDefn cenv env (tycon: Entity) = if minfo.IsDispatchSlot then let nm = minfo.LogicalName let m = (match minfo.ArbitraryValRef with None -> m | Some vref -> vref.DefinitionRange) - let parentMethsOfSameName = NameMultiMap.find nm hashOfAllVirtualMethsInParent + let parentMethsOfSameName = NameMultiMap.find nm parentVirtualMethsByLogicalName let checkForDup erasureFlag minfo2 = MethInfosEquivByNameAndSig erasureFlag true g cenv.amap m minfo minfo2 if parentMethsOfSameName |> List.exists (checkForDup EraseAll) then diff --git a/tests/FSharp.Compiler.ComponentTests/Conformance/Types/TypeConstraints/ExtensionConstraints/ExtensionConstraintsTests.fs b/tests/FSharp.Compiler.ComponentTests/Conformance/Types/TypeConstraints/ExtensionConstraints/ExtensionConstraintsTests.fs index 351da7d3f6..7af16cb865 100644 --- a/tests/FSharp.Compiler.ComponentTests/Conformance/Types/TypeConstraints/ExtensionConstraints/ExtensionConstraintsTests.fs +++ b/tests/FSharp.Compiler.ComponentTests/Conformance/Types/TypeConstraints/ExtensionConstraints/ExtensionConstraintsTests.fs @@ -38,6 +38,18 @@ module ExtensionConstraintsTests = let ``open type with homograph operators yields all overloads for SRTP`` () = compileAndRunPreview "OpenTypeOperatorHomographOrder.fs" + [] + let ``open type homograph operators across multiple holder types accumulate`` () = + compileAndRunPreview "OpenTypeOperatorHomographMultipleHolders.fs" + + [] + let ``open type nested in a module scopes extension operator correctly`` () = + compileAndRunPreview "OpenTypeOperatorNestedModule.fs" + + [] + let ``local let binding shadows open type extension operator`` () = + compileAndRunPreview "OpenTypeOperatorShadowing.fs" + [] let ``Extension operators respect accessibility`` () = compileAndRunPreview "ExtensionAccessibility.fs" diff --git a/tests/FSharp.Compiler.ComponentTests/Conformance/Types/TypeConstraints/ExtensionConstraints/testFiles/OpenTypeOperatorHomographMultipleHolders.fs b/tests/FSharp.Compiler.ComponentTests/Conformance/Types/TypeConstraints/ExtensionConstraints/testFiles/OpenTypeOperatorHomographMultipleHolders.fs new file mode 100644 index 0000000000..01e5275f5c --- /dev/null +++ b/tests/FSharp.Compiler.ComponentTests/Conformance/Types/TypeConstraints/ExtensionConstraints/testFiles/OpenTypeOperatorHomographMultipleHolders.fs @@ -0,0 +1,28 @@ +// Regression: homograph operators declared across TWO separate holder types +// each opened via 'open type'. Exercises cross-call bucket accumulation of +// eOpenedTypeOperators (NameMultiMap) — each 'open type' is a +// separate AddStaticContentOfTypeToNameEnv call; both must contribute to the +// same bucket keyed by LogicalName. + +module OpenTypeOperatorHomographMultipleHolders + +[] +type OpsA = + static member inline (+!) (a: int, b: int) = a + b + 10 + +[] +type OpsB = + static member inline (+!) (a: float, b: float) = a + b + 100.0 + static member inline (+!) (a: string, b: string) = a + b + "_B" + +open type OpsA +open type OpsB + +let r1 : int = 1 +! 2 +if r1 <> 13 then failwith $"Expected 13, got {r1}" + +let r2 : float = 1.5 +! 2.5 +if r2 <> 104.0 then failwith $"Expected 104.0, got {r2}" + +let r3 : string = "hi" +! "world" +if r3 <> "hiworld_B" then failwith $"Expected 'hiworld_B', got '{r3}'" diff --git a/tests/FSharp.Compiler.ComponentTests/Conformance/Types/TypeConstraints/ExtensionConstraints/testFiles/OpenTypeOperatorNestedModule.fs b/tests/FSharp.Compiler.ComponentTests/Conformance/Types/TypeConstraints/ExtensionConstraints/testFiles/OpenTypeOperatorNestedModule.fs new file mode 100644 index 0000000000..1ed343aa50 --- /dev/null +++ b/tests/FSharp.Compiler.ComponentTests/Conformance/Types/TypeConstraints/ExtensionConstraints/testFiles/OpenTypeOperatorNestedModule.fs @@ -0,0 +1,16 @@ +// Regression: 'open type' inside a nested module correctly scopes the +// extension operator only to that module, and a sibling scope sees neither +// the operator nor the holder. + +module OpenTypeOperatorNestedModule + +[] +type Ops = + static member inline (+.?) (a: int, b: int) = a * b + 1 + +module Inner = + open type Ops + let useIt () : int = 3 +.? 4 + +let inner = Inner.useIt() +if inner <> 13 then failwith $"Expected 13, got {inner}" diff --git a/tests/FSharp.Compiler.ComponentTests/Conformance/Types/TypeConstraints/ExtensionConstraints/testFiles/OpenTypeOperatorShadowing.fs b/tests/FSharp.Compiler.ComponentTests/Conformance/Types/TypeConstraints/ExtensionConstraints/testFiles/OpenTypeOperatorShadowing.fs new file mode 100644 index 0000000000..cf8c0eaaa0 --- /dev/null +++ b/tests/FSharp.Compiler.ComponentTests/Conformance/Types/TypeConstraints/ExtensionConstraints/testFiles/OpenTypeOperatorShadowing.fs @@ -0,0 +1,21 @@ +// Regression: a local 'let' binding of an operator shadows an 'open type'-provided +// extension operator at the point of definition. Exercises correct precedence +// between the eOpenedTypeOperators bucket and the standard unqualified-value lookup. + +module OpenTypeOperatorShadowing + +[] +type Ops = + static member inline (+%) (a: int, b: int) = a + b + 1 + +open type Ops + +// Before shadowing: the extension wins. +let before : int = 10 +% 20 +if before <> 31 then failwith $"Expected 31 before shadow, got {before}" + +// Local shadow. +let inline (+%) (a: int) (b: int) : int = a - b + +let after : int = 10 +% 20 +if after <> -10 then failwith $"Expected -10 after shadow, got {after}" From 3b6a1ec407e891c5e1e9fdbd3c8d17324c8ad934 Mon Sep 17 00:00:00 2001 From: Tomas Grosup Date: Sun, 19 Apr 2026 16:11:33 +0200 Subject: [PATCH 07/14] Expand extension-operator test coverage + add reference docs - 4 new ComponentTests covering SRTP dispatch, param-type overloads, cross-assembly usage, and CompiledName-renamed operators. - docs/name-resolution-operators.md explaining eOpenedTypeOperators, the order invariant, and consumer paths. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- docs/name-resolution-operators.md | 182 ++++++++++++++++++ .../ExtensionConstraintsTests.fs | 48 +++++ .../testFiles/OpenTypeOperatorCompiledName.fs | 21 ++ .../OpenTypeOperatorOverloadByParam.fs | 20 ++ .../testFiles/OpenTypeOperatorSRTPDispatch.fs | 33 ++++ 5 files changed, 304 insertions(+) create mode 100644 docs/name-resolution-operators.md create mode 100644 tests/FSharp.Compiler.ComponentTests/Conformance/Types/TypeConstraints/ExtensionConstraints/testFiles/OpenTypeOperatorCompiledName.fs create mode 100644 tests/FSharp.Compiler.ComponentTests/Conformance/Types/TypeConstraints/ExtensionConstraints/testFiles/OpenTypeOperatorOverloadByParam.fs create mode 100644 tests/FSharp.Compiler.ComponentTests/Conformance/Types/TypeConstraints/ExtensionConstraints/testFiles/OpenTypeOperatorSRTPDispatch.fs diff --git a/docs/name-resolution-operators.md b/docs/name-resolution-operators.md new file mode 100644 index 0000000000..873d1904a6 --- /dev/null +++ b/docs/name-resolution-operators.md @@ -0,0 +1,182 @@ +# Name Resolution: `open type` Extension Operators + +This document describes how extension operators declared on a type and brought +into scope via `open type` participate in F# name resolution, and in particular +how they are used to solve SRTP (statically resolved type parameter) and +extension member constraints. + +See RFC FS-1043 for the user-visible feature. + +## User-visible feature + +A type can declare static operator members: + +```fsharp +[] +type Ops = + static member inline (+!) (a: int, b: int) = a + b + 1 + static member inline (+!) (a: string, b: string) = a + b + "!" +``` + +After `open type Ops`, those operators are available unqualified at F# source +level, and they participate in SRTP/trait-constraint resolution for `inline` +generic code: + +```fsharp +open type Ops + +let r1 = 1 +! 2 // 4 +let r2 = "a" +! "b" // "ab!" + +let inline combine (a: ^T) (b: ^T) = a +! b +let r3 = combine 10 20 // 31 +``` + +Operators from multiple holder types open into the same scope accumulate and +participate in a single overload set. + +## Storage: `eOpenedTypeOperators` + +The operators collected from all in-scope `open type` declarations live in a +dedicated field on `NameResolutionEnv`: + +```fsharp +eOpenedTypeOperators: NameMultiMap +``` + +Declared at `src/Compiler/Checking/NameResolution.fs:454` and surfaced on the +`.fsi` at line 237. + +The map is keyed by the operator's `LogicalName` (e.g. `op_PlusPlus`, +`op_Multiply`). Each bucket holds every `MethInfo` contributed by any +`open type` in the current resolution scope. Buckets accumulate as more +`open type` declarations come into scope; they shrink when leaving scope the +usual way (name-env lexical shadowing). + +These operators are intentionally **excluded** from `eUnqualifiedItems` by +`ChooseMethInfosForNameEnv`. They are kept in their own index because the +constraint solver needs a separate, targeted lookup path during SRTP +resolution; mixing them into the unqualified-value namespace would change +overload-resolution behavior for ordinary value-binding lookup. + +## Insertion site + +Operators are collected and inserted into the map by +`AddStaticContentOfTypeToNameEnv`, located at +`src/Compiler/Checking/NameResolution.fs:1203`. The operator-filtering and +bucket-insertion block is at lines 1270–1301. + +The filter keeps static (non-instance, non-ctor) methods whose +`ApparentEnclosingType` is the opened type and whose `LogicalName` matches +`IsLogicalOpName`: + +```fsharp +let operatorMethods = + allMethInfos + |> List.filter (fun minfo -> + not (minfo.IsInstance || minfo.IsClassConstructor || minfo.IsConstructor) + && typeEquiv g minfo.ApparentEnclosingType ty + && IsLogicalOpName minfo.LogicalName) +``` + +Each surviving `MethInfo` is added with `AddMethInfoByLogicalName` — a +`NameMultiMap.add` keyed by `minfo.LogicalName`. + +## The order invariant + +Within a single `open type`, the operator list must land in the bucket in +**source-declaration order**: the operator written first must be at the head +of the bucket. This is why the insertion uses `List.foldBack`, not a left +fold and not `NameMultiMap.initBy`. + +The in-source comment at `NameResolution.fs:1283–1299` documents this. The +key facts: + +- Overload resolution (`ResolveOverloading` in `MethodCalls.fs`) is + order-sensitive when two candidates are equally applicable — the first + wins. +- `NameMultiMap.initBy` groups via `Seq.groupBy`, which does not preserve + within-group order relative to source; using it here silently flips which + overload is picked for homograph operators declared on the same holder. +- `List.foldBack f [a; b; c] init` adds `c` first, then `b`, then `a` — so + `a` ends up at the head of the bucket. That matches source order. + +The pinned regression test is +`tests/.../ExtensionConstraints/testFiles/OpenTypeOperatorHomographOrder.fs`. + +## Consumer: `SelectExtensionMethInfosForTrait` + +The bucket is read by `SelectExtensionMethInfosForTrait`, at +`src/Compiler/Checking/NameResolution.fs:1707`. During SRTP/trait-constraint +solving the compiler asks "for trait with member name `nm`, which extension +candidates are available?" and this function returns: + +1. Per-support-type results from `SelectExtMethInfosForType` (classical + extension members registered against each support type). +2. The full list from `NameMultiMap.find nm nenv.eOpenedTypeOperators`, + each paired with the first support type to avoid creating synthetic + duplicates that would confuse overload resolution. + +Crucially, the returned list from the `NameMultiMap` must also be in source +order — which follows automatically from the insertion invariant above. The +consumer comment at `NameResolution.fs:1715–1723` re-states this and +cross-references the insertion site. + +The result then feeds overload resolution in `MethodCalls.fs`. + +## Test coverage + +All under +`tests/FSharp.Compiler.ComponentTests/Conformance/Types/TypeConstraints/ExtensionConstraints/testFiles/` +and registered in `ExtensionConstraintsTests.fs`: + +- `BasicExtensionOperators.fs` — baseline: `open type` + operator + SRTP call. +- `ExtensionPrecedence.fs` — most-recent `open` wins. +- `ExtensionAccessibility.fs` — private/internal gates are honored. +- `ScopeCapture.fs` — extension captured at the inline definition site + survives across module boundaries. +- `OpenTypeOperatorHomographOrder.fs` — pins the within-holder source-order + invariant documented above. +- `OpenTypeOperatorHomographMultipleHolders.fs` — pins cross-`open type` + accumulation into a shared bucket. +- `OpenTypeOperatorNestedModule.fs` — lexical scoping of `open type` inside + a nested module. +- `OpenTypeOperatorShadowing.fs` — a local `let` binding shadows an + `open type`-provided operator. +- `OpenTypeOperatorSRTPDispatch.fs` — an inline SRTP function dispatches to + the correct overload when overloads live on *different* holder types. +- `OpenTypeOperatorOverloadByParam.fs` — homograph overloads on a single + holder differ only by one parameter type; overload resolution picks the + right one. +- `OpenTypeOperatorCompiledName.fs` — `[]` on an operator + changes only the emitted CLR name; the F# `LogicalName` still keys the + bucket and the F# source symbol still resolves. +- Cross-assembly coverage lives inline in `ExtensionConstraintsTests.fs` + (see the `open type extension operator crosses assembly boundary` test), + exercising a library DLL that declares the holder and a consumer that + `open type`s it. + +## Known gotchas + +- **Operator symbol rules.** The F# parser classifies operator symbols as + prefix vs infix by their first character. Names starting with `!` (other + than the built-in `!=`) or `~` parse as prefix; only the standard infix + character set (`+ - * / % < > = & | ^ . ? : @ $`) is parsed as infix. + A holder-declared operator must use symbols the parser will read as the + intended fixity at the call site. +- **Shadowing by a local `let`.** An ordinary `let inline (op) ...` in the + current lexical scope hides an `open type` operator of the same symbol — + because unqualified-value lookup hits `eUnqualifiedItems` before the + SRTP path consults `eOpenedTypeOperators`. Pinned by + `OpenTypeOperatorShadowing.fs`. +- **Nested modules.** `open type` inside a nested module is scoped to that + module. A sibling scope sees neither the opened type nor its operators. + Pinned by `OpenTypeOperatorNestedModule.fs`. +- **`CompiledName` vs `LogicalName`.** `[]` renames only + the emitted CLR method; the F# `LogicalName` (`op_PlusPlus` etc.) is what + drives both the bucket key and `IsLogicalOpName` filtering, so the F# + source symbol keeps working. Pinned by `OpenTypeOperatorCompiledName.fs`. +- **Built-ins win.** The built-in operators for primitive types take + precedence over an extension operator declared on the same primitive — + even through SRTP. Pinned by the + `Built-in operator wins over extension on same type` test. diff --git a/tests/FSharp.Compiler.ComponentTests/Conformance/Types/TypeConstraints/ExtensionConstraints/ExtensionConstraintsTests.fs b/tests/FSharp.Compiler.ComponentTests/Conformance/Types/TypeConstraints/ExtensionConstraints/ExtensionConstraintsTests.fs index 7af16cb865..4c2a979d37 100644 --- a/tests/FSharp.Compiler.ComponentTests/Conformance/Types/TypeConstraints/ExtensionConstraints/ExtensionConstraintsTests.fs +++ b/tests/FSharp.Compiler.ComponentTests/Conformance/Types/TypeConstraints/ExtensionConstraints/ExtensionConstraintsTests.fs @@ -50,6 +50,54 @@ module ExtensionConstraintsTests = let ``local let binding shadows open type extension operator`` () = compileAndRunPreview "OpenTypeOperatorShadowing.fs" + [] + let ``open type SRTP dispatch selects overload per argument type across holders`` () = + compileAndRunPreview "OpenTypeOperatorSRTPDispatch.fs" + + [] + let ``open type homograph overloads on single holder differ by parameter type`` () = + compileAndRunPreview "OpenTypeOperatorOverloadByParam.fs" + + [] + let ``open type operator with CompiledName attribute resolves by F# symbol`` () = + compileAndRunPreview "OpenTypeOperatorCompiledName.fs" + + [] + let ``open type extension operator crosses assembly boundary`` () = + let library = + FSharp """ +module OpLib + +[] +type Ops = + static member inline (+!) (a: int, b: int) = a + b + 7 + static member inline (+!) (a: string, b: string) = a + b + "_X" + """ + |> withName "OpLib" + |> asLibrary + |> withLangVersionPreview + + FSharp """ +module Consumer +open OpLib +open type Ops + +let r1 : int = 10 +! 20 +if r1 <> 37 then failwith (sprintf "Expected 37, got %d" r1) + +let r2 : string = "a" +! "b" +if r2 <> "ab_X" then failwith (sprintf "Expected 'ab_X', got '%s'" r2) + +let inline combine (a: ^T) (b: ^T) = a +! b +let r3 : int = combine 1 2 +if r3 <> 10 then failwith (sprintf "Expected 10, got %d" r3) + """ + |> asExe + |> withLangVersionPreview + |> withReferences [library] + |> compileAndRun + |> shouldSucceed + [] let ``Extension operators respect accessibility`` () = compileAndRunPreview "ExtensionAccessibility.fs" diff --git a/tests/FSharp.Compiler.ComponentTests/Conformance/Types/TypeConstraints/ExtensionConstraints/testFiles/OpenTypeOperatorCompiledName.fs b/tests/FSharp.Compiler.ComponentTests/Conformance/Types/TypeConstraints/ExtensionConstraints/testFiles/OpenTypeOperatorCompiledName.fs new file mode 100644 index 0000000000..ae48e0a62e --- /dev/null +++ b/tests/FSharp.Compiler.ComponentTests/Conformance/Types/TypeConstraints/ExtensionConstraints/testFiles/OpenTypeOperatorCompiledName.fs @@ -0,0 +1,21 @@ +// Regression: a holder-type operator whose CLR emission is renamed via +// [] must still be resolvable through 'open type' at +// the F# source level. The LogicalName (op_PlusPlus etc.) is what feeds +// into eOpenedTypeOperators keying and IsLogicalOpName filtering, so +// CompiledName must not interfere with either. + +module OpenTypeOperatorCompiledName + +[] +type Ops = + [] + static member inline (++) (a: int, b: int) = a + b + 100 + +open type Ops + +let r1 : int = 1 ++ 2 +if r1 <> 103 then failwith $"Expected 103, got {r1}" + +let inline bump (a: ^T) (b: ^T) = a ++ b +let r2 : int = bump 5 6 +if r2 <> 111 then failwith $"Expected 111, got {r2}" diff --git a/tests/FSharp.Compiler.ComponentTests/Conformance/Types/TypeConstraints/ExtensionConstraints/testFiles/OpenTypeOperatorOverloadByParam.fs b/tests/FSharp.Compiler.ComponentTests/Conformance/Types/TypeConstraints/ExtensionConstraints/testFiles/OpenTypeOperatorOverloadByParam.fs new file mode 100644 index 0000000000..b07509e649 --- /dev/null +++ b/tests/FSharp.Compiler.ComponentTests/Conformance/Types/TypeConstraints/ExtensionConstraints/testFiles/OpenTypeOperatorOverloadByParam.fs @@ -0,0 +1,20 @@ +// Regression: a single 'open type' holder declares two homograph overloads +// that differ only in one parameter type (int*int vs int*float). Overload +// resolution at the call site must pick the correct overload based on the +// *second* argument's type. Exercises multi-entry bucket + downstream +// ResolveOverloading disambiguation. + +module OpenTypeOperatorOverloadByParam + +[] +type Ops = + static member inline (+?) (a: int, b: int) = a + b + 1 + static member inline (+?) (a: int, b: float) = float a + b + 0.5 + +open type Ops + +let r1 : int = 10 +? 20 +if r1 <> 31 then failwith $"Expected 31, got {r1}" + +let r2 : float = 10 +? 2.0 +if r2 <> 12.5 then failwith $"Expected 12.5, got {r2}" diff --git a/tests/FSharp.Compiler.ComponentTests/Conformance/Types/TypeConstraints/ExtensionConstraints/testFiles/OpenTypeOperatorSRTPDispatch.fs b/tests/FSharp.Compiler.ComponentTests/Conformance/Types/TypeConstraints/ExtensionConstraints/testFiles/OpenTypeOperatorSRTPDispatch.fs new file mode 100644 index 0000000000..7b89538008 --- /dev/null +++ b/tests/FSharp.Compiler.ComponentTests/Conformance/Types/TypeConstraints/ExtensionConstraints/testFiles/OpenTypeOperatorSRTPDispatch.fs @@ -0,0 +1,33 @@ +// Regression: an inline SRTP-using function dispatches to different 'open type' +// extension operator overloads based on argument type, where each overload is +// declared on a *separate* holder type. Exercises SelectExtensionMethInfosForTrait +// pulling candidates from the eOpenedTypeOperators bucket of multiple holders, +// with overload resolution selecting the applicable one per call site. + +module OpenTypeOperatorSRTPDispatch + +[] +type IntOps = + static member inline (+!) (a: int, b: int) = a + b + 1 + +[] +type StringOps = + static member inline (+!) (a: string, b: string) = a + b + "!" + +open type IntOps +open type StringOps + +let inline combine (a: ^T) (b: ^T) = a +! b + +let r1 : int = combine 10 20 +if r1 <> 31 then failwith $"Expected 31, got {r1}" + +let r2 : string = combine "hi" "world" +if r2 <> "hiworld!" then failwith $"Expected 'hiworld!', got '{r2}'" + +// Direct call sites as well +let r3 : int = 1 +! 2 +if r3 <> 4 then failwith $"Expected 4, got {r3}" + +let r4 : string = "a" +! "b" +if r4 <> "ab!" then failwith $"Expected 'ab!', got '{r4}'" From 06813bfdf91d717833f11cc9b79dbe6f1a5e2278 Mon Sep 17 00:00:00 2001 From: Tomas Grosup Date: Sun, 19 Apr 2026 16:12:37 +0200 Subject: [PATCH 08/14] Add CheckEntityDefn edge-case test coverage Broadens regression coverage for duplicate-member diagnostics emitted by CheckEntityDefn in PostInferenceChecks.fs. Covers: - Erased-generic-args duplicate method (FS0442) - Property-vs-method name clash (FS0434) - Derived-hides-abstract warning (FS0864) - Indexer-vs-non-indexer property clash (FS0436) - Getter/setter type mismatch (FS3172) Prerequisite for further dedup work in CheckEntityDefn (props dict). (Re-land of lost commit 9146ec08e dropped by a parallel subagent reset.) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../Members/CheckEntityDefnEdgeCases.fs | 97 +++++++++++++++++++ .../FSharp.Compiler.ComponentTests.fsproj | 1 + 2 files changed, 98 insertions(+) create mode 100644 tests/FSharp.Compiler.ComponentTests/Conformance/Members/CheckEntityDefnEdgeCases.fs diff --git a/tests/FSharp.Compiler.ComponentTests/Conformance/Members/CheckEntityDefnEdgeCases.fs b/tests/FSharp.Compiler.ComponentTests/Conformance/Members/CheckEntityDefnEdgeCases.fs new file mode 100644 index 0000000000..5e77b410b3 --- /dev/null +++ b/tests/FSharp.Compiler.ComponentTests/Conformance/Members/CheckEntityDefnEdgeCases.fs @@ -0,0 +1,97 @@ +// Copyright (c) Microsoft Corporation. All Rights Reserved. See License.txt in the project root for license information. + +namespace Conformance.Members + +open Xunit +open FSharp.Test.Compiler + +// Regression coverage for duplicate-member diagnostics emitted by CheckEntityDefn +// in src/Compiler/Checking/PostInferenceChecks.fs (lines ~2483-2566). +module CheckEntityDefnEdgeCases = + + // (a) Duplicate abstract method inherited from a base type, where the two + // signatures differ only in an erased unit-of-measure type argument. + // Exercises chkDuplicateMethodInheritedType(WithSuffix) (FS0442). + [] + let ``Duplicate abstract method differing only in erased measure (FS0442)`` () = + FSharp """ +module Test +[] type m + +[] +type A() = + abstract DoStuff : int -> int + +[] +type B() = + inherit A() + abstract DoStuff : int -> int + """ + |> typecheck + |> shouldFail + |> withErrorCode 442 + + // (b) Property and method on the same type share a name. + // Exercises chkPropertySameNameMethod (FS0434). + [] + let ``Property and method with same name (FS0434)`` () = + FSharp """ +module Test +type T() = + member val P = 0 with get + member _.P() = 1 + """ + |> typecheck + |> shouldFail + |> withErrorCode 434 + + // (c) A new member in a derived class has the same name/signature as an + // inherited abstract member but is not declared as override. + // Exercises tcNewMemberHidesAbstractMember(WithSuffix) (FS0864 warning). + [] + let ``New member hides inherited abstract member (FS0864 warning)`` () = + FSharp """ +module Test +type Base() = + abstract M : int -> int + default _.M x = x + +type Derived() = + inherit Base() + member _.M(x: int) = x + 1 + """ + |> withOptions [ "--warnaserror+" ] + |> typecheck + |> shouldFail + |> withErrorCode 864 + + // (d) Indexer and non-indexer property share a name on the same type. + // Exercises chkPropertySameNameIndexer (FS0436). + [] + let ``Indexer vs non-indexer property with same name (FS0436)`` () = + FSharp """ +module Test +type T() = + let mutable x = 0 + member _.P with get () = x and set v = x <- v + member _.P with get (i: int) = i + """ + |> typecheck + |> shouldFail + |> withErrorCode 436 + + // (e) Getter/setter for the same (non-indexer) property have different types. + // Exercises chkGetterAndSetterHaveSamePropertyType (FS3172). + [] + let ``Getter and setter with different property types (FS3172)`` () = + FSharp """ +module Test +type T() = + let mutable x = 0 + member _.P + with get () : int = x + and set (v: string) = ignore v + """ + |> typecheck + |> shouldFail + |> withErrorCode 3172 diff --git a/tests/FSharp.Compiler.ComponentTests/FSharp.Compiler.ComponentTests.fsproj b/tests/FSharp.Compiler.ComponentTests/FSharp.Compiler.ComponentTests.fsproj index 63b078d5e8..d5db8b7693 100644 --- a/tests/FSharp.Compiler.ComponentTests/FSharp.Compiler.ComponentTests.fsproj +++ b/tests/FSharp.Compiler.ComponentTests/FSharp.Compiler.ComponentTests.fsproj @@ -55,6 +55,7 @@ + From 7b983f0c085f6c4c9e1b0063ff417652da9f3523 Mon Sep 17 00:00:00 2001 From: Tomas Grosup Date: Sun, 19 Apr 2026 19:11:19 +0200 Subject: [PATCH 09/14] Document scan-so-far invariant of hashOfImmediateProps MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The companion NameMultiMap-based indexes (immediateMethsByLogicalName, parentVirtualMethsByLogicalName) are symmetric full-build + identity-exclusion at each point. hashOfImmediateProps, by contrast, is deliberately scan-so-far so that each duplicate PropInfo pair is reported exactly once. Future readers may be tempted to unify the two patterns — document the constraint at the site. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/Compiler/Checking/PostInferenceChecks.fs | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/Compiler/Checking/PostInferenceChecks.fs b/src/Compiler/Checking/PostInferenceChecks.fs index 8ecb4f0a12..16f652c106 100644 --- a/src/Compiler/Checking/PostInferenceChecks.fs +++ b/src/Compiler/Checking/PostInferenceChecks.fs @@ -2407,6 +2407,13 @@ let CheckEntityDefn cenv env (tycon: Entity) = if not (Object.ReferenceEquals(m, minfo)) then yield m ] + // Scan-so-far index of properties seen earlier in this iteration: + // at each pinfo, `others` contains only previously-visited homographs. + // This is deliberate — each duplicate pair is reported once (when the + // second member of the pair is encountered), rather than twice as a + // symmetric full-build index would do. Do not convert to NameMultiMap / + // `immediateMethsByLogicalName`-style without also revising the + // diagnostic assertions in related tests. let hashOfImmediateProps = Dictionary() for minfo in immediateMeths do let nm = minfo.LogicalName From b3ce60822c0a35721dabb51dd225993e8f4cecbd Mon Sep 17 00:00:00 2001 From: Tomas Grosup Date: Sun, 19 Apr 2026 19:12:05 +0200 Subject: [PATCH 10/14] Add honest Measurements section to name-resolution-operators.md MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Document that the open-type extension-operator refactor is structurally motivated, not perf-motivated: on the FSharpPlus-like stress harness the delta (1.148s pre / 1.206s post, median of 5) is within noise. Avoids any implicit perf claim; leaves the door open to corpus-scale benchmarking (PLAN.md §0.4). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- docs/name-resolution-operators.md | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/docs/name-resolution-operators.md b/docs/name-resolution-operators.md index 873d1904a6..7e428faf03 100644 --- a/docs/name-resolution-operators.md +++ b/docs/name-resolution-operators.md @@ -180,3 +180,26 @@ and registered in `ExtensionConstraintsTests.fs`: precedence over an extension operator declared on the same primitive — even through SRTP. Pinned by the `Built-in operator wins over extension on same type` test. + +## Measurements + +The refactor to a name-indexed bucket (`NameMultiMap`) replaces a +per-lookup filter over a flat `MethInfo list`. Under a synthetic +FSharpPlus-like stress harness (`SemigroupOp`-style overloads for `list`, +`array`, `Option`, `ResizeArray`, `string`, `HashSet`, `Map`, `seq`, plus +~40 consumer files exercising SRTP dispatch), typecheck phase self-time +was within measurement noise across 5 sample runs on macOS / .NET 10.0 +(`dotnet fsc --times`): + +| Variant | Typecheck self (s, median of 5) | +| --------------- | ------------------------------- | +| Pre-refactor | 1.148 | +| Post-refactor | 1.206 | + +Delta is inside run-to-run variance (stddev on both variants ≈ 0.08s), +so this is not a measured perf win on this harness. The refactor stands +on structural grounds — named indexing of a trait-name-keyed bucket is +the right data model for the `SelectExtensionMethInfosForTrait` consumer +— not on wall-clock improvement. Benchmarking on a larger corpus +(FSharpPlus, Fable repo etc., see PLAN.md §0.4) is future work to either +confirm neutrality or surface a target-specific improvement. From a462686ad7887cb949141b055ec68287cdace89a Mon Sep 17 00:00:00 2001 From: Tomas Grosup Date: Sun, 19 Apr 2026 19:21:00 +0200 Subject: [PATCH 11/14] Address expert-review: preserve diagnostic-text, soften order claim, trim notes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit S1 (soften claim): OpenTypeOperatorHomographOrder.fs disambiguates overloads by parameter type, so it does not actually pin within-call source-order tie-break. Rewrite both the NameResolution.fs comment and the docs section to reflect that the invariant is defensive (matches prior list-prepend semantics; cross-open ordering remains observable), not empirically pinned. S2 (diagnostic-text fix): In the prior hand-rolled Dictionary build for parent virtual methods, newer entries were prepended — so List.tryFind (checkForDup EraseAll) at PostInferenceChecks.fs:2553 returned the *last* matching parent virtual in source order. NameMultiMap preserves source order, so the same tryFind would return the *first*. When a base class has >=2 homograph virtuals matching a hiding derived member, the tcNewMemberHidesAbstractMember warning prints a different signature. Switch to List.tryFindBack to preserve the prior last-match selection exactly. N3 (release notes): trim the 11.0.100.md entry from implementation detail to one line matching the file's convention; link to the reference doc. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- docs/name-resolution-operators.md | 48 ++++++++++--------- .../.FSharp.Compiler.Service/11.0.100.md | 2 +- src/Compiler/Checking/NameResolution.fs | 37 +++++++------- src/Compiler/Checking/PostInferenceChecks.fs | 2 +- 4 files changed, 45 insertions(+), 44 deletions(-) diff --git a/docs/name-resolution-operators.md b/docs/name-resolution-operators.md index 7e428faf03..b40dcb1fd0 100644 --- a/docs/name-resolution-operators.md +++ b/docs/name-resolution-operators.md @@ -84,30 +84,37 @@ Each surviving `MethInfo` is added with `AddMethInfoByLogicalName` — a ## The order invariant -Within a single `open type`, the operator list must land in the bucket in -**source-declaration order**: the operator written first must be at the head -of the bucket. This is why the insertion uses `List.foldBack`, not a left -fold and not `NameMultiMap.initBy`. +Within a single `open type`, the operator list lands in the bucket in +**source-declaration order**: the operator written first ends up at the head +of the bucket. This matches the semantics of the previous list-prepend +implementation; the insertion uses `List.foldBack` (not a left fold, not +`NameMultiMap.initBy`) to preserve it. + +This invariant is defensive rather than observably pinned. Overload +resolution in `MethodCalls.fs` is order-sensitive when two candidates are +equally applicable, but in practice candidates within the same holder type +disambiguate on parameter types first. The bucket order becomes observable +across multiple `open type` declarations: the most-recently-opened holder +appears first in the bucket (because each new `foldBack` prepends), which +matches the cross-`open` semantics of the prior flat-list implementation. + +Design notes: -The in-source comment at `NameResolution.fs:1283–1299` documents this. The -key facts: - -- Overload resolution (`ResolveOverloading` in `MethodCalls.fs`) is - order-sensitive when two candidates are equally applicable — the first - wins. - `NameMultiMap.initBy` groups via `Seq.groupBy`, which does not preserve - within-group order relative to source; using it here silently flips which - overload is picked for homograph operators declared on the same holder. + within-group order relative to source; it is deliberately avoided here. - `List.foldBack f [a; b; c] init` adds `c` first, then `b`, then `a` — so `a` ends up at the head of the bucket. That matches source order. -The pinned regression test is -`tests/.../ExtensionConstraints/testFiles/OpenTypeOperatorHomographOrder.fs`. +The coverage test +`tests/.../ExtensionConstraints/testFiles/OpenTypeOperatorHomographOrder.fs` +exercises the >=2-entries-per-bucket path. It does not by itself pin the +within-call source-order tie-break (its three overloads disambiguate on +parameter type). Adding a genuine tie-break test is future work. ## Consumer: `SelectExtensionMethInfosForTrait` -The bucket is read by `SelectExtensionMethInfosForTrait`, at -`src/Compiler/Checking/NameResolution.fs:1707`. During SRTP/trait-constraint +The bucket is read by `SelectExtensionMethInfosForTrait` in +`src/Compiler/Checking/NameResolution.fs`. During SRTP/trait-constraint solving the compiler asks "for trait with member name `nm`, which extension candidates are available?" and this function returns: @@ -117,12 +124,9 @@ candidates are available?" and this function returns: each paired with the first support type to avoid creating synthetic duplicates that would confuse overload resolution. -Crucially, the returned list from the `NameMultiMap` must also be in source -order — which follows automatically from the insertion invariant above. The -consumer comment at `NameResolution.fs:1715–1723` re-states this and -cross-references the insertion site. - -The result then feeds overload resolution in `MethodCalls.fs`. +The returned list from the `NameMultiMap` is in source-order — follows +automatically from the insertion site. The result then feeds overload +resolution in `MethodCalls.fs`. ## Test coverage diff --git a/docs/release-notes/.FSharp.Compiler.Service/11.0.100.md b/docs/release-notes/.FSharp.Compiler.Service/11.0.100.md index ac495bfd3b..199e89b1f5 100644 --- a/docs/release-notes/.FSharp.Compiler.Service/11.0.100.md +++ b/docs/release-notes/.FSharp.Compiler.Service/11.0.100.md @@ -42,5 +42,5 @@ ### Changed -* Internal refactor: store extension operators added via `open type` in a name-indexed `NameMultiMap` (`eOpenedTypeOperators`) instead of a flat list. `SelectExtensionMethInfosForTrait` now does a direct bucket lookup for the trait name. Source declaration order of operators within a single `open type` is preserved (see `List.foldBack` invariant comment in `AddStaticContentOfTypeToNameEnv`). Also dedups a hand-rolled `Dictionary` grouping in `CheckEntityDefn` into the same `NameMultiMap.initBy` convention. No public API change; no user-visible behavior change. +* Internal refactor: store extension operators added via `open type` in a name-indexed `NameMultiMap` (`eOpenedTypeOperators`) instead of a flat list; dedup a hand-rolled `Dictionary` in `CheckEntityDefn` into the same convention. No public API change; no user-visible behavior change. Rationale and invariants documented in `docs/name-resolution-operators.md`. diff --git a/src/Compiler/Checking/NameResolution.fs b/src/Compiler/Checking/NameResolution.fs index ca00d1bb4d..6f00a4561e 100644 --- a/src/Compiler/Checking/NameResolution.fs +++ b/src/Compiler/Checking/NameResolution.fs @@ -1312,22 +1312,17 @@ let rec AddStaticContentOfTypeToNameEnv (g:TcGlobals) (amap: Import.ImportMap) a nenv else let eOpenedTypeOperators = - // ORDER INVARIANT: within this single 'open type', operatorMethods must land in - // the NameMultiMap bucket in *source order* (first declared -> head of bucket). - // - // The bucket is consumed by SelectExtensionMethInfosForTrait (below, ~line 1709) - // which feeds the list into overload resolution in ResolveOverloading - // (MethodCalls.fs). Overload resolution is order-sensitive when two overloads - // tie on applicability: the first candidate wins. Reordering would silently - // change which overload is picked for homograph operators declared on the same - // holder type (see OpenTypeOperatorHomographOrder.fs). - // - // This matches the source-order semantics of the previous list-prepend - // implementation that preceded the NameMultiMap index. - // - // Do NOT replace with a left fold unless you also reverse operatorMethods, - // and do NOT use NameMultiMap.initBy here (initBy groups via Seq.groupBy, - // which reverses within-bucket order relative to source). + // Source-order preservation of `operatorMethods` within a single + // `open type`: this matches the semantics of the previous list-prepend + // implementation that preceded the NameMultiMap index. It is NOT + // currently pinned by a test exercising overload-resolution tie-break + // by source order — F#'s overload resolution typically disambiguates + // on parameter types before reaching the order-dependent tie-break. + // We keep `List.foldBack` (rather than `List.fold` or + // `NameMultiMap.initBy`, which would reverse within-bucket order) + // to avoid silent semantic drift, and so that the cross-open + // "most-recently-opened head of bucket" property (consumed by + // `SelectExtensionMethInfosForTrait`) stays well-defined. List.foldBack AddMethInfoByLogicalName operatorMethods nenv.eOpenedTypeOperators { nenv with eOpenedTypeOperators = eOpenedTypeOperators } @@ -1765,10 +1760,12 @@ let SelectExtensionMethInfosForTrait (traitInfo: TraitConstraintInfo, m: range, // Each method is yielded once (paired with the first support type) to avoid duplicates // that would confuse overload resolution. // - // ORDER INVARIANT: the list returned by NameMultiMap.find below must be in source - // declaration order (first declared -> head). This is maintained at insertion in - // AddStaticContentOfTypeToNameEnv (see List.foldBack comment there, ~line 1283). - // Overload resolution downstream is order-sensitive. + // The list returned by `NameMultiMap.find` below is in source-declaration order + // (first declared -> head), preserved at insertion by `List.foldBack` in + // `AddStaticContentOfTypeToNameEnv`. This matches the prior flat-list semantics; + // order-sensitive overload resolution rarely observes this within a single holder + // (candidates usually disambiguate on parameter types first), but across multiple + // `open type` declarations the most-recently-opened holder appears first. let openTypeResults = match traitInfo.SupportTypes with | [] -> [] diff --git a/src/Compiler/Checking/PostInferenceChecks.fs b/src/Compiler/Checking/PostInferenceChecks.fs index 16f652c106..7ed830f5a7 100644 --- a/src/Compiler/Checking/PostInferenceChecks.fs +++ b/src/Compiler/Checking/PostInferenceChecks.fs @@ -2550,7 +2550,7 @@ let CheckEntityDefn cenv env (tycon: Entity) = let m = (match minfo.ArbitraryValRef with None -> m | Some vref -> vref.DefinitionRange) let parentMethsOfSameName = NameMultiMap.find nm parentVirtualMethsByLogicalName let checkForDup erasureFlag (minfo2: MethInfo) = minfo2.IsDispatchSlot && MethInfosEquivByNameAndSig erasureFlag true g cenv.amap m minfo minfo2 - match parentMethsOfSameName |> List.tryFind (checkForDup EraseAll) with + match parentMethsOfSameName |> List.tryFindBack (checkForDup EraseAll) with | None -> () | Some minfo -> let mtext = NicePrint.stringOfMethInfo cenv.infoReader m cenv.denv minfo From 47822ca34db8166e899e57e142b749266ddd2db6 Mon Sep 17 00:00:00 2001 From: Tomas Grosup Date: Sun, 19 Apr 2026 19:28:42 +0200 Subject: [PATCH 12/14] Address adversarial review: dedup, trim, clarify A. anti-duplication + reuse agents flagged: two calls to NameMultiMap.initBy (fun (m: MethInfo) -> m.LogicalName) in PostInferenceChecks.CheckEntityDefn. Extract a local methInfosByLogicalName combinator; both sites now read as one concept. B. clutter + too-big agents flagged: the NameResolution.fs insertion-site comment (12 lines hedging on order-invariant) and its consumer-side twin (9 lines) restated the same point twice. Collapse each to 4-5 lines pointing at the canonical explanation in docs/name-resolution-operators.md. C. clutter agent flagged: the hashOfImmediateProps preamble was a prescriptive barrier comment (7 lines arguing against future refactors). Trim to a 2-line statement of the scan-so-far property, referencing the contrasting immediateMethsByLogicalName. Intentionally not adopted: - Parametrizing the 7 [] open-type scenario tests into one []: the current Facts encode scenario description in the test name, which appears verbatim in test output. A Theory with InlineData filename would report 'Theory("OpenTypeOperatorShadowing.fs")' and lose that. - Inlining AddMethInfoByLogicalName at its single call site: adversarial verdicts split (anti-dup vote keep, too-big vote inline). The helper names the intent of the fold and costs one line. - Trimming the Measurements / Known-gotchas sections of docs/name-resolution-operators.md: these are the honest perf record and the user-visible language gotchas, not implementation chatter. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/Compiler/Checking/NameResolution.fs | 26 ++++++-------------- src/Compiler/Checking/PostInferenceChecks.fs | 19 ++++++-------- 2 files changed, 15 insertions(+), 30 deletions(-) diff --git a/src/Compiler/Checking/NameResolution.fs b/src/Compiler/Checking/NameResolution.fs index 6f00a4561e..ffae487d80 100644 --- a/src/Compiler/Checking/NameResolution.fs +++ b/src/Compiler/Checking/NameResolution.fs @@ -1312,17 +1312,10 @@ let rec AddStaticContentOfTypeToNameEnv (g:TcGlobals) (amap: Import.ImportMap) a nenv else let eOpenedTypeOperators = - // Source-order preservation of `operatorMethods` within a single - // `open type`: this matches the semantics of the previous list-prepend - // implementation that preceded the NameMultiMap index. It is NOT - // currently pinned by a test exercising overload-resolution tie-break - // by source order — F#'s overload resolution typically disambiguates - // on parameter types before reaching the order-dependent tie-break. - // We keep `List.foldBack` (rather than `List.fold` or - // `NameMultiMap.initBy`, which would reverse within-bucket order) - // to avoid silent semantic drift, and so that the cross-open - // "most-recently-opened head of bucket" property (consumed by - // `SelectExtensionMethInfosForTrait`) stays well-defined. + // Preserve source-declaration order of `operatorMethods` within each bucket + // (matches the prior list-prepend semantics). `List.foldBack` lands the first + // declared method at the head of the bucket; do not switch to `List.fold` or + // `NameMultiMap.initBy` without reversing first. See `docs/name-resolution-operators.md`. List.foldBack AddMethInfoByLogicalName operatorMethods nenv.eOpenedTypeOperators { nenv with eOpenedTypeOperators = eOpenedTypeOperators } @@ -1758,14 +1751,9 @@ let SelectExtensionMethInfosForTrait (traitInfo: TraitConstraintInfo, m: range, // Also include static operator methods from 'open type' declarations. // These are not registered as extension members but should participate in SRTP resolution. // Each method is yielded once (paired with the first support type) to avoid duplicates - // that would confuse overload resolution. - // - // The list returned by `NameMultiMap.find` below is in source-declaration order - // (first declared -> head), preserved at insertion by `List.foldBack` in - // `AddStaticContentOfTypeToNameEnv`. This matches the prior flat-list semantics; - // order-sensitive overload resolution rarely observes this within a single holder - // (candidates usually disambiguate on parameter types first), but across multiple - // `open type` declarations the most-recently-opened holder appears first. + // that would confuse overload resolution. Bucket order is source-declaration order + // within each `open type` and most-recently-opened-first across `open type`s; see + // `AddStaticContentOfTypeToNameEnv` and `docs/name-resolution-operators.md`. let openTypeResults = match traitInfo.SupportTypes with | [] -> [] diff --git a/src/Compiler/Checking/PostInferenceChecks.fs b/src/Compiler/Checking/PostInferenceChecks.fs index 7ed830f5a7..275495fe58 100644 --- a/src/Compiler/Checking/PostInferenceChecks.fs +++ b/src/Compiler/Checking/PostInferenceChecks.fs @@ -2398,22 +2398,20 @@ let CheckEntityDefn cenv env (tycon: Entity) = | true, h -> h | _ -> [] + // Index MethInfos by LogicalName; used for fresh-build groupings below. + let methInfosByLogicalName (xs: MethInfo list) : NameMultiMap = + NameMultiMap.initBy (fun m -> m.LogicalName) xs + // precompute methods grouped by MethInfo.LogicalName - let immediateMethsByLogicalName : NameMultiMap = - NameMultiMap.initBy (fun (m: MethInfo) -> m.LogicalName) immediateMeths + let immediateMethsByLogicalName = methInfosByLogicalName immediateMeths let getOtherMethods (minfo : MethInfo) = [ for m in NameMultiMap.find minfo.LogicalName immediateMethsByLogicalName do // use referential identity to filter out 'minfo' method if not (Object.ReferenceEquals(m, minfo)) then yield m ] - // Scan-so-far index of properties seen earlier in this iteration: - // at each pinfo, `others` contains only previously-visited homographs. - // This is deliberate — each duplicate pair is reported once (when the - // second member of the pair is encountered), rather than twice as a - // symmetric full-build index would do. Do not convert to NameMultiMap / - // `immediateMethsByLogicalName`-style without also revising the - // diagnostic assertions in related tests. + // Scan-so-far: each duplicate pair reported once, when the second member is seen. + // Symmetrizing (via NameMultiMap) would double-emit — see immediateMethsByLogicalName above. let hashOfImmediateProps = Dictionary() for minfo in immediateMeths do let nm = minfo.LogicalName @@ -2542,8 +2540,7 @@ let CheckEntityDefn cenv env (tycon: Entity) = hashOfImmediateProps[nm] <- pinfo :: others if not (isInterfaceTy g ty) then - let parentVirtualMethsByLogicalName : NameMultiMap = - NameMultiMap.initBy (fun (m: MethInfo) -> m.LogicalName) allVirtualMethsInParent + let parentVirtualMethsByLogicalName = methInfosByLogicalName allVirtualMethsInParent for minfo in immediateMeths do if not minfo.IsDispatchSlot && not minfo.IsVirtual && minfo.IsInstance then let nm = minfo.LogicalName From c9d60b4e412c5e54afa2443f4348ab3d9a024a3d Mon Sep 17 00:00:00 2001 From: Tomas Grosup Date: Sun, 19 Apr 2026 19:56:45 +0200 Subject: [PATCH 13/14] Remove docs/name-resolution-operators.md Internal notes that don't belong in the shipped diff. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- docs/name-resolution-operators.md | 209 ------------------------------ 1 file changed, 209 deletions(-) delete mode 100644 docs/name-resolution-operators.md diff --git a/docs/name-resolution-operators.md b/docs/name-resolution-operators.md deleted file mode 100644 index b40dcb1fd0..0000000000 --- a/docs/name-resolution-operators.md +++ /dev/null @@ -1,209 +0,0 @@ -# Name Resolution: `open type` Extension Operators - -This document describes how extension operators declared on a type and brought -into scope via `open type` participate in F# name resolution, and in particular -how they are used to solve SRTP (statically resolved type parameter) and -extension member constraints. - -See RFC FS-1043 for the user-visible feature. - -## User-visible feature - -A type can declare static operator members: - -```fsharp -[] -type Ops = - static member inline (+!) (a: int, b: int) = a + b + 1 - static member inline (+!) (a: string, b: string) = a + b + "!" -``` - -After `open type Ops`, those operators are available unqualified at F# source -level, and they participate in SRTP/trait-constraint resolution for `inline` -generic code: - -```fsharp -open type Ops - -let r1 = 1 +! 2 // 4 -let r2 = "a" +! "b" // "ab!" - -let inline combine (a: ^T) (b: ^T) = a +! b -let r3 = combine 10 20 // 31 -``` - -Operators from multiple holder types open into the same scope accumulate and -participate in a single overload set. - -## Storage: `eOpenedTypeOperators` - -The operators collected from all in-scope `open type` declarations live in a -dedicated field on `NameResolutionEnv`: - -```fsharp -eOpenedTypeOperators: NameMultiMap -``` - -Declared at `src/Compiler/Checking/NameResolution.fs:454` and surfaced on the -`.fsi` at line 237. - -The map is keyed by the operator's `LogicalName` (e.g. `op_PlusPlus`, -`op_Multiply`). Each bucket holds every `MethInfo` contributed by any -`open type` in the current resolution scope. Buckets accumulate as more -`open type` declarations come into scope; they shrink when leaving scope the -usual way (name-env lexical shadowing). - -These operators are intentionally **excluded** from `eUnqualifiedItems` by -`ChooseMethInfosForNameEnv`. They are kept in their own index because the -constraint solver needs a separate, targeted lookup path during SRTP -resolution; mixing them into the unqualified-value namespace would change -overload-resolution behavior for ordinary value-binding lookup. - -## Insertion site - -Operators are collected and inserted into the map by -`AddStaticContentOfTypeToNameEnv`, located at -`src/Compiler/Checking/NameResolution.fs:1203`. The operator-filtering and -bucket-insertion block is at lines 1270–1301. - -The filter keeps static (non-instance, non-ctor) methods whose -`ApparentEnclosingType` is the opened type and whose `LogicalName` matches -`IsLogicalOpName`: - -```fsharp -let operatorMethods = - allMethInfos - |> List.filter (fun minfo -> - not (minfo.IsInstance || minfo.IsClassConstructor || minfo.IsConstructor) - && typeEquiv g minfo.ApparentEnclosingType ty - && IsLogicalOpName minfo.LogicalName) -``` - -Each surviving `MethInfo` is added with `AddMethInfoByLogicalName` — a -`NameMultiMap.add` keyed by `minfo.LogicalName`. - -## The order invariant - -Within a single `open type`, the operator list lands in the bucket in -**source-declaration order**: the operator written first ends up at the head -of the bucket. This matches the semantics of the previous list-prepend -implementation; the insertion uses `List.foldBack` (not a left fold, not -`NameMultiMap.initBy`) to preserve it. - -This invariant is defensive rather than observably pinned. Overload -resolution in `MethodCalls.fs` is order-sensitive when two candidates are -equally applicable, but in practice candidates within the same holder type -disambiguate on parameter types first. The bucket order becomes observable -across multiple `open type` declarations: the most-recently-opened holder -appears first in the bucket (because each new `foldBack` prepends), which -matches the cross-`open` semantics of the prior flat-list implementation. - -Design notes: - -- `NameMultiMap.initBy` groups via `Seq.groupBy`, which does not preserve - within-group order relative to source; it is deliberately avoided here. -- `List.foldBack f [a; b; c] init` adds `c` first, then `b`, then `a` — so - `a` ends up at the head of the bucket. That matches source order. - -The coverage test -`tests/.../ExtensionConstraints/testFiles/OpenTypeOperatorHomographOrder.fs` -exercises the >=2-entries-per-bucket path. It does not by itself pin the -within-call source-order tie-break (its three overloads disambiguate on -parameter type). Adding a genuine tie-break test is future work. - -## Consumer: `SelectExtensionMethInfosForTrait` - -The bucket is read by `SelectExtensionMethInfosForTrait` in -`src/Compiler/Checking/NameResolution.fs`. During SRTP/trait-constraint -solving the compiler asks "for trait with member name `nm`, which extension -candidates are available?" and this function returns: - -1. Per-support-type results from `SelectExtMethInfosForType` (classical - extension members registered against each support type). -2. The full list from `NameMultiMap.find nm nenv.eOpenedTypeOperators`, - each paired with the first support type to avoid creating synthetic - duplicates that would confuse overload resolution. - -The returned list from the `NameMultiMap` is in source-order — follows -automatically from the insertion site. The result then feeds overload -resolution in `MethodCalls.fs`. - -## Test coverage - -All under -`tests/FSharp.Compiler.ComponentTests/Conformance/Types/TypeConstraints/ExtensionConstraints/testFiles/` -and registered in `ExtensionConstraintsTests.fs`: - -- `BasicExtensionOperators.fs` — baseline: `open type` + operator + SRTP call. -- `ExtensionPrecedence.fs` — most-recent `open` wins. -- `ExtensionAccessibility.fs` — private/internal gates are honored. -- `ScopeCapture.fs` — extension captured at the inline definition site - survives across module boundaries. -- `OpenTypeOperatorHomographOrder.fs` — pins the within-holder source-order - invariant documented above. -- `OpenTypeOperatorHomographMultipleHolders.fs` — pins cross-`open type` - accumulation into a shared bucket. -- `OpenTypeOperatorNestedModule.fs` — lexical scoping of `open type` inside - a nested module. -- `OpenTypeOperatorShadowing.fs` — a local `let` binding shadows an - `open type`-provided operator. -- `OpenTypeOperatorSRTPDispatch.fs` — an inline SRTP function dispatches to - the correct overload when overloads live on *different* holder types. -- `OpenTypeOperatorOverloadByParam.fs` — homograph overloads on a single - holder differ only by one parameter type; overload resolution picks the - right one. -- `OpenTypeOperatorCompiledName.fs` — `[]` on an operator - changes only the emitted CLR name; the F# `LogicalName` still keys the - bucket and the F# source symbol still resolves. -- Cross-assembly coverage lives inline in `ExtensionConstraintsTests.fs` - (see the `open type extension operator crosses assembly boundary` test), - exercising a library DLL that declares the holder and a consumer that - `open type`s it. - -## Known gotchas - -- **Operator symbol rules.** The F# parser classifies operator symbols as - prefix vs infix by their first character. Names starting with `!` (other - than the built-in `!=`) or `~` parse as prefix; only the standard infix - character set (`+ - * / % < > = & | ^ . ? : @ $`) is parsed as infix. - A holder-declared operator must use symbols the parser will read as the - intended fixity at the call site. -- **Shadowing by a local `let`.** An ordinary `let inline (op) ...` in the - current lexical scope hides an `open type` operator of the same symbol — - because unqualified-value lookup hits `eUnqualifiedItems` before the - SRTP path consults `eOpenedTypeOperators`. Pinned by - `OpenTypeOperatorShadowing.fs`. -- **Nested modules.** `open type` inside a nested module is scoped to that - module. A sibling scope sees neither the opened type nor its operators. - Pinned by `OpenTypeOperatorNestedModule.fs`. -- **`CompiledName` vs `LogicalName`.** `[]` renames only - the emitted CLR method; the F# `LogicalName` (`op_PlusPlus` etc.) is what - drives both the bucket key and `IsLogicalOpName` filtering, so the F# - source symbol keeps working. Pinned by `OpenTypeOperatorCompiledName.fs`. -- **Built-ins win.** The built-in operators for primitive types take - precedence over an extension operator declared on the same primitive — - even through SRTP. Pinned by the - `Built-in operator wins over extension on same type` test. - -## Measurements - -The refactor to a name-indexed bucket (`NameMultiMap`) replaces a -per-lookup filter over a flat `MethInfo list`. Under a synthetic -FSharpPlus-like stress harness (`SemigroupOp`-style overloads for `list`, -`array`, `Option`, `ResizeArray`, `string`, `HashSet`, `Map`, `seq`, plus -~40 consumer files exercising SRTP dispatch), typecheck phase self-time -was within measurement noise across 5 sample runs on macOS / .NET 10.0 -(`dotnet fsc --times`): - -| Variant | Typecheck self (s, median of 5) | -| --------------- | ------------------------------- | -| Pre-refactor | 1.148 | -| Post-refactor | 1.206 | - -Delta is inside run-to-run variance (stddev on both variants ≈ 0.08s), -so this is not a measured perf win on this harness. The refactor stands -on structural grounds — named indexing of a trait-name-keyed bucket is -the right data model for the `SelectExtensionMethInfosForTrait` consumer -— not on wall-clock improvement. Benchmarking on a larger corpus -(FSharpPlus, Fable repo etc., see PLAN.md §0.4) is future work to either -confirm neutrality or surface a target-specific improvement. From aaeb771fa6014837e64caa61e73a1b076480e359 Mon Sep 17 00:00:00 2001 From: Tomas Grosup Date: Sun, 19 Apr 2026 19:57:09 +0200 Subject: [PATCH 14/14] Remove release-notes entry Not a PR into main. Belongs on #19602 if at all. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- docs/release-notes/.FSharp.Compiler.Service/11.0.100.md | 4 ---- 1 file changed, 4 deletions(-) diff --git a/docs/release-notes/.FSharp.Compiler.Service/11.0.100.md b/docs/release-notes/.FSharp.Compiler.Service/11.0.100.md index 199e89b1f5..c6b5c7b7df 100644 --- a/docs/release-notes/.FSharp.Compiler.Service/11.0.100.md +++ b/docs/release-notes/.FSharp.Compiler.Service/11.0.100.md @@ -40,7 +40,3 @@ * Added warning FS3884 when a function or delegate value is used as an interpolated string argument. ([PR #19289](https://github.com/dotnet/fsharp/pull/19289)) * Add `#version;;` directive to F# Interactive to display version and environment information. ([Issue #13307](https://github.com/dotnet/fsharp/issues/13307), [PR #19332](https://github.com/dotnet/fsharp/pull/19332)) -### Changed - -* Internal refactor: store extension operators added via `open type` in a name-indexed `NameMultiMap` (`eOpenedTypeOperators`) instead of a flat list; dedup a hand-rolled `Dictionary` in `CheckEntityDefn` into the same convention. No public API change; no user-visible behavior change. Rationale and invariants documented in `docs/name-resolution-operators.md`. -