From 50a5b90b5da8d0b8a867634158ea2e58d33a6ddd Mon Sep 17 00:00:00 2001 From: "Kevin Ransom (msft)" Date: Tue, 6 Jun 2023 03:10:31 -0700 Subject: [PATCH 1/3] Fix #15313 - The compiler automatically generates an IsReadOnlyAttribute on downlevel frameworks even if we have already defined it. (#15316) * Fix #15313 * fantomas --- src/Compiler/Driver/CreateILModule.fs | 10 ++- .../EmittedIL/Misc/byrefTests.fs | 80 +++++++++++++++++++ 2 files changed, 89 insertions(+), 1 deletion(-) diff --git a/src/Compiler/Driver/CreateILModule.fs b/src/Compiler/Driver/CreateILModule.fs index efcf212675b..45d9791ffd9 100644 --- a/src/Compiler/Driver/CreateILModule.fs +++ b/src/Compiler/Driver/CreateILModule.fs @@ -303,8 +303,16 @@ module MainModuleBuilder = RequireCompilationThread ctok + let isEmbeddableTypeWithLocalSourceImplementation (td: ILTypeDef) = + (TcGlobals.IsInEmbeddableKnownSet td.Name) + && not (codegenResults.ilTypeDefs |> List.exists (fun r -> r.Name = td.Name)) + let ilTypeDefs = - mkILTypeDefs (codegenResults.ilTypeDefs @ tcGlobals.tryRemoveEmbeddedILTypeDefs ()) + mkILTypeDefs ( + codegenResults.ilTypeDefs + @ (tcGlobals.tryRemoveEmbeddedILTypeDefs () + |> List.filter isEmbeddableTypeWithLocalSourceImplementation) + ) let mainModule = let hashAlg = diff --git a/tests/FSharp.Compiler.ComponentTests/EmittedIL/Misc/byrefTests.fs b/tests/FSharp.Compiler.ComponentTests/EmittedIL/Misc/byrefTests.fs index d4dd00a7c8d..fce4ebeb856 100644 --- a/tests/FSharp.Compiler.ComponentTests/EmittedIL/Misc/byrefTests.fs +++ b/tests/FSharp.Compiler.ComponentTests/EmittedIL/Misc/byrefTests.fs @@ -344,6 +344,58 @@ type C() = |> verifyIL [verifyProperty;verifyMethod;verifyIsReadOnlyAttribute] |> ignore + [] + let ``Returning an 'inref<_>' from a property should emit System.Runtime.CompilerServices.IsReadOnlyAttribute on the return type of the signature and generate the - Source contains ReadOnlyAttribute`` () = + let src = + """ +namespace System.Runtime.CompilerServices + +type IsReadOnlyAttribute() = + inherit System.Attribute() + +type C() = + let x = 59 + member _.X: inref<_> = &x + """ + + let verifyProperty = """.property instance int32& modreq([netstandard]System.Runtime.InteropServices.InAttribute) + X() + { + .custom instance void System.Runtime.CompilerServices.IsReadOnlyAttribute::.ctor() = ( 01 00 00 00 ) + .get instance int32& modreq([netstandard]System.Runtime.InteropServices.InAttribute) System.Runtime.CompilerServices.C::get_X() + }""" + + let verifyMethod = """.method public hidebysig specialname instance int32& modreq([netstandard]System.Runtime.InteropServices.InAttribute) + get_X() cil managed + { + .param [0] + .custom instance void System.Runtime.CompilerServices.IsReadOnlyAttribute::.ctor() = ( 01 00 00 00 )""" + + let verifyIsReadOnlyAttribute = """ +.class public auto ansi serializable System.Runtime.CompilerServices.IsReadOnlyAttribute + extends [netstandard]System.Attribute +{ + .custom instance void [FSharp.Core]Microsoft.FSharp.Core.CompilationMappingAttribute::.ctor(valuetype [FSharp.Core]Microsoft.FSharp.Core.SourceConstructFlags) = ( 01 00 03 00 00 00 00 00 ) + .method public specialname rtspecialname + instance void .ctor() cil managed + { + + .maxstack 8 + IL_0000: ldarg.0 + IL_0001: callvirt instance void [netstandard]System.Attribute::.ctor() + IL_0006: ldarg.0 + IL_0007: pop + IL_0008: ret + } + +}""" + + FSharp src + |> asNetStandard20 + |> compile + |> verifyIL [verifyProperty;verifyMethod;verifyIsReadOnlyAttribute] + |> ignore + [] let ``Returning an 'inref<_>' from a generic method should emit System.Runtime.CompilerServices.IsReadOnlyAttribute on the return type of the signature`` () = let src = @@ -366,6 +418,34 @@ type C<'T>() = |> verifyIL [verifyMethod] |> ignore + [] + let ``Returning an 'inref<_>' from a generic method should emit System.Runtime.CompilerServices.IsReadOnlyAttribute on the return type of the signature - Source contains ReadOnlyAttribute`` () = + let src = + """ +namespace System.Runtime.CompilerServices + +type IsReadOnlyAttribute() = + inherit System.Attribute() + +module Test = + + type C<'T>() = + let x = Unchecked.defaultof<'T> + member _.X<'U>(): inref<'T> = &x + """ + + let verifyMethod = """.method public hidebysig instance !T& modreq([netstandard]System.Runtime.InteropServices.InAttribute) + X() cil managed + { + .param [0] + .custom instance void System.Runtime.CompilerServices.IsReadOnlyAttribute::.ctor() = ( 01 00 00 00 )""" + FSharp src + |> asLibrary + |> asNetStandard20 + |> compile + |> verifyIL [verifyMethod] + + [] let ``Returning an 'inref<_>' from an abstract generic method should emit System.Runtime.CompilerServices.IsReadOnlyAttribute on the return type of the signature`` () = let src = From 47f4ff3f1abe3e8f946271accb835be3a6c9b828 Mon Sep 17 00:00:00 2001 From: Vlad Zarytovskii Date: Tue, 6 Jun 2023 12:43:37 +0200 Subject: [PATCH 2/3] Update INTERNAL.md --- INTERNAL.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/INTERNAL.md b/INTERNAL.md index ec64caa69de..21c0f28b798 100644 --- a/INTERNAL.md +++ b/INTERNAL.md @@ -81,6 +81,8 @@ Update the `insertTargetBranch` value at the bottom of `azure-pipelines.yml` in 6. Ensure the subscription was added by repeating step 3 above. 7. Note, the help in the `darc` tool is really good. E.g., you can simply run `darc` to see a list of all commands available, and if you run `darc ` with no arguments, you'll be given a list of arguments you can use. 8. Ensure that version numbers are bumped for a new branch. + 9. Change needed subscriptions for arcade and SDK: + 1. `darc get-subscriptions --target-repo fsharp`, and then use `darc update-subscription --id ` for corresponding channels (e.g. target new VS channel to specific SDK channel, or set up arcade auto-merges to release/* or main branch, depending on the timeline of release/upgrade cycle). ## Labeling issues on GitHub From 5fcabc9c049dd453ca99b27467e37e00b62f84a4 Mon Sep 17 00:00:00 2001 From: Vlad Zarytovskii Date: Tue, 6 Jun 2023 12:47:39 +0200 Subject: [PATCH 3/3] Update INTERNAL.md --- INTERNAL.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/INTERNAL.md b/INTERNAL.md index 21c0f28b798..b964f22d442 100644 --- a/INTERNAL.md +++ b/INTERNAL.md @@ -83,6 +83,8 @@ Update the `insertTargetBranch` value at the bottom of `azure-pipelines.yml` in 8. Ensure that version numbers are bumped for a new branch. 9. Change needed subscriptions for arcade and SDK: 1. `darc get-subscriptions --target-repo fsharp`, and then use `darc update-subscription --id ` for corresponding channels (e.g. target new VS channel to specific SDK channel, or set up arcade auto-merges to release/* or main branch, depending on the timeline of release/upgrade cycle). + 2. If new subscription needs to be added, the following command should be used `darc add-subscription --source-repo https://github.com/dotnet/arcade --target-repo https://github.com/dotnet/fsharp --target-branch --channel "" --update-frequency everyDay --standard-automerge +` ## Labeling issues on GitHub