From 56491f1d01af001b990b8c21a94e1ae0cbda5b99 Mon Sep 17 00:00:00 2001 From: Tomas Grosup Date: Mon, 16 Feb 2026 16:12:47 +0100 Subject: [PATCH 1/3] Fix DU duplicate methods, event metadata Fixes #14321 Fixes #16565 Fixes #5834 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../.FSharp.Compiler.Service/10.0.300.md | 3 + .../Checking/Expressions/CheckExpressions.fs | 6 +- src/Compiler/CodeGen/IlxGen.fs | 28 ++- .../CodeGenRegressions_TypeDefs.fs | 192 ++++++++++++++++++ .../FSharp.Compiler.ComponentTests.fsproj | 1 + 5 files changed, 227 insertions(+), 3 deletions(-) create mode 100644 tests/FSharp.Compiler.ComponentTests/EmittedIL/CodeGenRegressions/CodeGenRegressions_TypeDefs.fs diff --git a/docs/release-notes/.FSharp.Compiler.Service/10.0.300.md b/docs/release-notes/.FSharp.Compiler.Service/10.0.300.md index ca425fb63c4..597bf78b5bb 100644 --- a/docs/release-notes/.FSharp.Compiler.Service/10.0.300.md +++ b/docs/release-notes/.FSharp.Compiler.Service/10.0.300.md @@ -21,6 +21,9 @@ * Fix FS3356 false positive for instance extension members with same name on different types, introduced by [#18821](https://github.com/dotnet/fsharp/pull/18821). ([PR #19260](https://github.com/dotnet/fsharp/pull/19260)) * Fix graph-based type checking incorrectly resolving dependencies when the same module name is defined across multiple files in the same namespace. ([PR #19280](https://github.com/dotnet/fsharp/pull/19280)) * F# Scripts: Fix default reference paths resolving when an SDK directory is specified. ([PR #19270](https://github.com/dotnet/fsharp/pull/19270)) +* Fix DU case names matching IWSAM member names no longer cause duplicate property entries. (Issue [#14321](https://github.com/dotnet/fsharp/issues/14321), [PR #19341](https://github.com/dotnet/fsharp/pull/19341)) +* Fix DefaultAugmentation(false) duplicate entry in method table. (Issue [#16565](https://github.com/dotnet/fsharp/issues/16565), [PR #19341](https://github.com/dotnet/fsharp/pull/19341)) +* Fix abstract event accessors now have SpecialName flag. (Issue [#5834](https://github.com/dotnet/fsharp/issues/5834), [PR #19341](https://github.com/dotnet/fsharp/pull/19341)) ### Added * FSharpType: add ImportILType ([PR #19300](https://github.com/dotnet/fsharp/pull/19300)) diff --git a/src/Compiler/Checking/Expressions/CheckExpressions.fs b/src/Compiler/Checking/Expressions/CheckExpressions.fs index be76ee77ac5..54e83ac44f4 100644 --- a/src/Compiler/Checking/Expressions/CheckExpressions.fs +++ b/src/Compiler/Checking/Expressions/CheckExpressions.fs @@ -13077,7 +13077,11 @@ let TcAndPublishValSpec (cenv: cenv, env, containerInfo: ContainerInfo, declKind let checkXmlDocs = cenv.diagnosticOptions.CheckXmlDocs let xmlDoc = xmlDoc.ToXmlDoc(checkXmlDocs, paramNames) - let vspec = MakeAndPublishVal cenv env (altActualParent, true, declKind, ValNotInRecScope, valscheme, attrs, xmlDoc, literalValue, false) + let isGeneratedEventVal = + CompileAsEvent g attrs + && (id.idText.StartsWithOrdinal("add_") || id.idText.StartsWithOrdinal("remove_")) + + let vspec = MakeAndPublishVal cenv env (altActualParent, true, declKind, ValNotInRecScope, valscheme, attrs, xmlDoc, literalValue, isGeneratedEventVal) PublishArguments cenv env vspec synValSig allDeclaredTypars.Length diff --git a/src/Compiler/CodeGen/IlxGen.fs b/src/Compiler/CodeGen/IlxGen.fs index 1e2f26b011e..9942993de8e 100644 --- a/src/Compiler/CodeGen/IlxGen.fs +++ b/src/Compiler/CodeGen/IlxGen.fs @@ -10750,6 +10750,13 @@ and GenAbstractBinding cenv eenv tref (vref: ValRef) = | SynMemberKind.Constructor | SynMemberKind.Member -> let mdef = mdef.With(customAttrs = mkILCustomAttrs ilAttrs) + + let mdef = + if vref.Deref.val_flags.IsGeneratedEventVal then + mdef.WithSpecialName + else + mdef + [ mdef ], [], [] | SynMemberKind.PropertyGetSet -> error (Error(FSComp.SR.ilUnexpectedGetSetAnnotation (), m)) | SynMemberKind.PropertySet @@ -11768,6 +11775,17 @@ and GenTypeDef cenv mgbuf lazyInitInfo eenv m (tycon: Tycon) : ILTypeRef option // // Also discard the F#-compiler supplied implementation of the Empty, IsEmpty, Value and None properties. + let nullaryCaseNames = + if cuinfo.HasHelpers = AllHelpers || cuinfo.HasHelpers = NoHelpers then + cuinfo.UnionCases + |> Array.choose (fun alt -> if alt.IsNullary then Some alt.Name else None) + |> Set.ofArray + else + Set.empty + + let isNullaryCaseClash name = + not nullaryCaseNames.IsEmpty && nullaryCaseNames.Contains name + let tdefDiscards = Some( (fun (md: ILMethodDef) -> @@ -11776,7 +11794,11 @@ and GenTypeDef cenv mgbuf lazyInitInfo eenv m (tycon: Tycon) : ILTypeRef option || (cuinfo.HasHelpers = SpecialFSharpOptionHelpers && (md.Name = "get_Value" || md.Name = "get_None" || md.Name = "Some")) || (cuinfo.HasHelpers = AllHelpers - && (md.Name.StartsWith("get_Is") && not (tdef2.Methods.FindByName(md.Name).IsEmpty)))), + && (md.Name.StartsWith("get_Is") && not (tdef2.Methods.FindByName(md.Name).IsEmpty))) + || (md.Name.StartsWith("get_") + && md.Name.Length > 4 + && isNullaryCaseClash (md.Name.Substring(4)) + && not (tdef2.Methods.FindByName(md.Name).IsEmpty))), (fun (pd: ILPropertyDef) -> (cuinfo.HasHelpers = SpecialFSharpListHelpers @@ -11784,7 +11806,9 @@ and GenTypeDef cenv mgbuf lazyInitInfo eenv m (tycon: Tycon) : ILTypeRef option || (cuinfo.HasHelpers = SpecialFSharpOptionHelpers && (pd.Name = "Value" || pd.Name = "None")) || (cuinfo.HasHelpers = AllHelpers - && (pd.Name.StartsWith("Is") && not (tdef2.Properties.LookupByName(pd.Name).IsEmpty)))) + && (pd.Name.StartsWith("Is") && not (tdef2.Properties.LookupByName(pd.Name).IsEmpty))) + || (isNullaryCaseClash pd.Name + && not (tdef2.Properties.LookupByName(pd.Name).IsEmpty))) ) tdef2, tdefDiscards diff --git a/tests/FSharp.Compiler.ComponentTests/EmittedIL/CodeGenRegressions/CodeGenRegressions_TypeDefs.fs b/tests/FSharp.Compiler.ComponentTests/EmittedIL/CodeGenRegressions/CodeGenRegressions_TypeDefs.fs new file mode 100644 index 00000000000..71f603dcd81 --- /dev/null +++ b/tests/FSharp.Compiler.ComponentTests/EmittedIL/CodeGenRegressions/CodeGenRegressions_TypeDefs.fs @@ -0,0 +1,192 @@ +// Copyright (c) Microsoft Corporation. All Rights Reserved. See License.txt in the project root for license information. + +namespace EmittedIL + +open Xunit +open FSharp.Test +open FSharp.Test.Compiler +open FSharp.Test.Utilities + +module CodeGenRegressions_TypeDefs = + + let private getActualIL (result: CompilationResult) = + match result with + | CompilationResult.Success s -> + match s.OutputPath with + | Some p -> + let (_, _, actualIL) = ILChecker.verifyILAndReturnActual [] p [ "// dummy" ] + actualIL + | None -> failwith "No output path" + | _ -> failwith "Compilation failed" + + // https://github.com/dotnet/fsharp/issues/16565 + [] + let ``Issue_16565_DefaultAugmentationFalseDuplicateEntry`` () = + let source = """ +module Test + +open System + +[] +type Option<'T> = + | Some of Value: 'T + | None + + member x.Value = + match x with + | Some x -> x + | None -> raise (new InvalidOperationException("Option.Value")) + + static member None : Option<'T> = None + +and 'T option = Option<'T> + +let v = Option.Some 42 +printfn "Value: %d" v.Value +let n = Option.None +printfn "None created successfully" +""" + FSharp source + |> asExe + |> compile + |> shouldSucceed + |> run + |> shouldSucceed + |> ignore + + // https://github.com/dotnet/fsharp/issues/14321 + [] + let ``Issue_14321_DuAndIWSAMNames`` () = + let source = """ +module Test + +#nowarn "3535" // IWSAM warning + +type EngineError<'e> = + static abstract Overheated : 'e + static abstract LowOil : 'e + +type CarError = + | Overheated + | LowOil + | DeviceNotPaired + + interface EngineError with + static member Overheated = Overheated + static member LowOil = LowOil +""" + FSharp source + |> asLibrary + |> compile + |> shouldSucceed + |> ignore + + // https://github.com/dotnet/fsharp/issues/14321 + // Runtime test: type must load without "duplicate entry in method table" + [] + let ``Issue_14321_DuAndIWSAMNames_Runtime`` () = + let source = """ +module Test + +#nowarn "3535" + +type EngineError<'e> = + static abstract Overheated : 'e + static abstract LowOil : 'e + +type CarError = + | Overheated + | LowOil + | DeviceNotPaired + + interface EngineError with + static member Overheated = Overheated + static member LowOil = LowOil + +[] +let main _ = + let err = CarError.Overheated + match err with + | Overheated -> printfn "Got Overheated" + | LowOil -> printfn "Got LowOil" + | DeviceNotPaired -> printfn "Got DeviceNotPaired" + 0 +""" + FSharp source + |> asExe + |> compile + |> shouldSucceed + |> run + |> shouldSucceed + |> ignore + + // https://github.com/dotnet/fsharp/issues/5834 + [] + let ``Issue_5834_EventSpecialname`` () = + let source = """ +module Test + +open System +open System.Reflection + +type IAbstract1 = + [] + abstract member Event : IEvent + +type IAbstract2 = + [] + abstract member Event : IDelegateEvent + +[] +type Abstract3() = + [] + abstract member Event : IDelegateEvent + +type Concrete1() = + let event = new Event() + [] + member this.Event = event.Publish + +type Concrete2() = + [] + member this.Event = { new IDelegateEvent with + member this.AddHandler _ = () + member this.RemoveHandler _ = () } + +type ConcreteWithObsolete() = + let evt = new Event() + [] + [] + member this.MyEvent = evt.Publish + +[] +let main _ = + let mutable failures = 0 + let check (t: Type) = + t.GetMethods(BindingFlags.Public ||| BindingFlags.Instance ||| BindingFlags.DeclaredOnly) + |> Array.filter (fun m -> m.Name.Contains("Event")) + |> Array.iter (fun m -> + if not m.IsSpecialName then + printfn "FAIL: %s.%s missing specialname" t.Name m.Name + failures <- failures + 1) + + check typeof + check typeof + check typeof + check typeof + check typeof + check typeof + + if failures > 0 then + failwithf "BUG: %d event accessors missing specialname" failures + printfn "SUCCESS: All event accessors have specialname" + 0 +""" + FSharp source + |> asExe + |> compile + |> shouldSucceed + |> run + |> shouldSucceed + |> ignore + diff --git a/tests/FSharp.Compiler.ComponentTests/FSharp.Compiler.ComponentTests.fsproj b/tests/FSharp.Compiler.ComponentTests/FSharp.Compiler.ComponentTests.fsproj index fd02dace6e1..9e57b5cd2a9 100644 --- a/tests/FSharp.Compiler.ComponentTests/FSharp.Compiler.ComponentTests.fsproj +++ b/tests/FSharp.Compiler.ComponentTests/FSharp.Compiler.ComponentTests.fsproj @@ -280,6 +280,7 @@ + From 14017783b65d530ffd16b59a869a401043e8cee1 Mon Sep 17 00:00:00 2001 From: Tomas Grosup Date: Thu, 26 Feb 2026 15:52:27 +0100 Subject: [PATCH 2/3] Fix property discard for NoHelpers nullary case dedup MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit For DefaultAugmentation(false) (NoHelpers), EraseUnions generates a get_ method but no corresponding property for nullary cases. The method discard correctly removes the user's get_ method, but the property discard only checked tdef2.Properties — which is empty for NoHelpers — leaving an orphaned property whose getter method ref points to the discarded method. Fix: also check tdef2.Methods for a corresponding get_ method when deciding whether to discard a user property. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/Compiler/CodeGen/IlxGen.fs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/Compiler/CodeGen/IlxGen.fs b/src/Compiler/CodeGen/IlxGen.fs index 9942993de8e..42cf07aecbd 100644 --- a/src/Compiler/CodeGen/IlxGen.fs +++ b/src/Compiler/CodeGen/IlxGen.fs @@ -11808,7 +11808,8 @@ and GenTypeDef cenv mgbuf lazyInitInfo eenv m (tycon: Tycon) : ILTypeRef option || (cuinfo.HasHelpers = AllHelpers && (pd.Name.StartsWith("Is") && not (tdef2.Properties.LookupByName(pd.Name).IsEmpty))) || (isNullaryCaseClash pd.Name - && not (tdef2.Properties.LookupByName(pd.Name).IsEmpty))) + && (not (tdef2.Properties.LookupByName(pd.Name).IsEmpty) + || not (tdef2.Methods.FindByName("get_" + pd.Name).IsEmpty)))) ) tdef2, tdefDiscards From cea4f9035e4881c891b045c8cbbf6b070c271fa5 Mon Sep 17 00:00:00 2001 From: Tomas Grosup Date: Mon, 2 Mar 2026 14:19:01 +0100 Subject: [PATCH 3/3] Fix flaky ModuleReaderCancellationTests on MacOS Add tolerance for async cancellation token cleanup on slower CI platforms. The Cancellable.HasCancellationToken assertion can fail when the AsyncLocal cleanup from previous async operations has not yet propagated. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../ModuleReaderCancellationTests.fs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tests/FSharp.Compiler.Service.Tests/ModuleReaderCancellationTests.fs b/tests/FSharp.Compiler.Service.Tests/ModuleReaderCancellationTests.fs index f307586cd5c..9759e63c900 100644 --- a/tests/FSharp.Compiler.Service.Tests/ModuleReaderCancellationTests.fs +++ b/tests/FSharp.Compiler.Service.Tests/ModuleReaderCancellationTests.fs @@ -150,6 +150,10 @@ let parseAndCheck path source options = | _, FSharpCheckFileAnswer.Aborted -> None | _, FSharpCheckFileAnswer.Succeeded results -> Some results + // Allow time for async cancellation token cleanup on slower platforms (e.g. MacOS CI) + if Cancellable.HasCancellationToken then + System.Threading.Thread.Sleep(100) + Cancellable.HasCancellationToken |> shouldEqual false result