Skip to content

Commit 93c31cb

Browse files
committed
Fix FS3356 false positive: only check static extension members for duplicate type name conflict
Instance extension members compile with the extended type as the first IL parameter, so they can never produce duplicate IL method signatures even when the simple type name matches. Only static extension members lack this distinguishing parameter. This fixes a regression where libraries like IcedTasks that define instance (inline) extension members on builder types from different namespaces (e.g. Microsoft.FSharp.Control.TaskBuilderBase and IcedTasks.TaskBase.TaskBuilderBase) were incorrectly rejected. Regression introduced by PR #18821 (commit e948a68).
1 parent bb026d4 commit 93c31cb

2 files changed

Lines changed: 62 additions & 12 deletions

File tree

src/Compiler/Checking/PostInferenceChecks.fs

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2680,21 +2680,27 @@ let CheckEntityDefns cenv env tycons =
26802680
// check modules
26812681
//--------------------------------------------------------------------------
26822682

2683-
/// Check for duplicate extension member names that would cause IL conflicts.
2684-
/// Extension members for types with the same simple name but different fully qualified names
2685-
/// will be emitted into the same IL container type, causing a duplicate member error.
2683+
/// Check for duplicate static extension member names that would cause IL conflicts.
2684+
/// Static extension members for types with the same simple name but different fully qualified names
2685+
/// compile to static methods in the same module IL type without a distinguishing first parameter,
2686+
/// so they can produce duplicate IL method signatures.
2687+
/// Instance extension members are safe because they compile with the extended type as the first
2688+
/// parameter, which differentiates the IL signatures.
26862689
let CheckForDuplicateExtensionMemberNames (cenv: cenv) (vals: Val seq) =
26872690
if cenv.reportErrors then
2688-
let extensionMembers =
2691+
let staticExtensionMembers =
26892692
vals
2690-
|> Seq.filter (fun v -> v.IsExtensionMember && v.IsMember)
2693+
|> Seq.filter (fun v ->
2694+
v.IsExtensionMember
2695+
&& v.IsMember
2696+
&& not (v.IsInstanceMember))
26912697
|> Seq.toList
26922698

2693-
if not extensionMembers.IsEmpty then
2699+
if not staticExtensionMembers.IsEmpty then
26942700
// Group by LogicalName which includes generic arity suffix (e.g., Expr`1 for Expr<'T>)
26952701
// This matches how types are compiled to IL, so Expr and Expr<'T> are separate groups
26962702
let groupedByLogicalName =
2697-
extensionMembers
2703+
staticExtensionMembers
26982704
|> List.groupBy (fun v -> v.MemberApparentEntity.LogicalName)
26992705

27002706
for (logicalName, members) in groupedByLogicalName do

tests/FSharp.Compiler.ComponentTests/Language/ExtensionMethodTests.fs

Lines changed: 49 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -697,7 +697,9 @@ module CompiledExtensions =
697697
]
698698

699699
[<Fact>]
700-
let ``Instance extension members for types with same simple name should error`` () =
700+
let ``Instance extension members for types with same simple name should succeed`` () =
701+
// Instance extension members compile with the extended type as the first IL parameter,
702+
// so they can never produce duplicate IL signatures even with same simple type name.
701703
Fsx
702704
"""
703705
module Compiled
@@ -712,10 +714,7 @@ module CompiledExtensions =
712714
member _.InstanceExtension() = ()
713715
"""
714716
|> compile
715-
|> shouldFail
716-
|> withDiagnostics [
717-
(Error 3356, Line 11, Col 18, Line 11, Col 35, "Extension members extending types with the same simple name 'Task' but different fully qualified names cannot be defined in the same module. Consider defining these extensions in separate modules.")
718-
]
717+
|> shouldSucceed
719718

720719
[<Fact>]
721720
let ``Extension members on generic types with same simple name should error`` () =
@@ -806,3 +805,48 @@ module CompiledExtensions =
806805
(Error 3356, Line 12, Col 23, Line 12, Col 33, "Extension members extending types with the same simple name 'Task' but different fully qualified names cannot be defined in the same module. Consider defining these extensions in separate modules.")
807806
(Error 3356, Line 13, Col 23, Line 13, Col 33, "Extension members extending types with the same simple name 'Task' but different fully qualified names cannot be defined in the same module. Consider defining these extensions in separate modules.")
808807
]
808+
809+
[<Fact>]
810+
let ``Instance inline extension members on builder types with same simple name should succeed`` () =
811+
// Regression test for IcedTasks-like pattern: instance (inline) extension members on
812+
// computation expression builder types with the same simple name from different namespaces.
813+
// Instance extension members compile with the extended type as the first IL parameter,
814+
// so signatures never collide even when the simple type name is the same.
815+
Fsx
816+
"""
817+
namespace NsA
818+
type BuilderBase() = class end
819+
820+
namespace NsB
821+
type BuilderBase() = class end
822+
823+
namespace Extensions
824+
module M =
825+
type NsA.BuilderBase with
826+
member inline this.Bind(x: int, f) = f x
827+
member inline this.ReturnFrom(x: int) = x
828+
829+
type NsB.BuilderBase with
830+
member inline this.Source(x: int) = x
831+
member inline this.Bind(x: string, f) = f x
832+
"""
833+
|> compile
834+
|> shouldSucceed
835+
836+
[<Fact>]
837+
let ``Mixed static and instance extension members - only static should error`` () =
838+
Fsx
839+
"""
840+
module Compiled
841+
842+
type Task = { F: int }
843+
844+
module CompiledExtensions =
845+
type System.Threading.Tasks.Task with
846+
static member StaticExt() = ()
847+
848+
type Task with
849+
member _.InstanceExt() = ()
850+
"""
851+
|> compile
852+
|> shouldSucceed

0 commit comments

Comments
 (0)