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..42cf07aecbd 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,10 @@ 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) + || not (tdef2.Methods.FindByName("get_" + 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 @@ + 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