From f423862350b81ef5f1d5c5ed6ed0b4d5b6d66b75 Mon Sep 17 00:00:00 2001 From: Vlad Zarytovskii Date: Mon, 6 Nov 2023 12:45:15 +0100 Subject: [PATCH 1/3] Test should fail --- .../FSharp.Compiler.ComponentTests.fsproj | 11 +++++----- .../Interop/ByrefTests.fs | 21 +++++++++++++++++++ 2 files changed, 27 insertions(+), 5 deletions(-) create mode 100644 tests/FSharp.Compiler.ComponentTests/Interop/ByrefTests.fs diff --git a/tests/FSharp.Compiler.ComponentTests/FSharp.Compiler.ComponentTests.fsproj b/tests/FSharp.Compiler.ComponentTests/FSharp.Compiler.ComponentTests.fsproj index aefeb44cc50..4c7500db8bb 100644 --- a/tests/FSharp.Compiler.ComponentTests/FSharp.Compiler.ComponentTests.fsproj +++ b/tests/FSharp.Compiler.ComponentTests/FSharp.Compiler.ComponentTests.fsproj @@ -29,7 +29,7 @@ FsUnit.fs - + @@ -112,7 +112,7 @@ - + @@ -175,7 +175,7 @@ - + @@ -191,7 +191,7 @@ - + @@ -225,6 +225,7 @@ + @@ -323,5 +324,5 @@ - + diff --git a/tests/FSharp.Compiler.ComponentTests/Interop/ByrefTests.fs b/tests/FSharp.Compiler.ComponentTests/Interop/ByrefTests.fs new file mode 100644 index 00000000000..e4451a821ff --- /dev/null +++ b/tests/FSharp.Compiler.ComponentTests/Interop/ByrefTests.fs @@ -0,0 +1,21 @@ +// Copyright (c) Microsoft Corporation. All Rights Reserved. See License.txt in the project root for license information. + +namespace Interop + +open Xunit +open FSharp.Test +open FSharp.Test.Compiler + +module ``Byref interop verification tests`` = + + [] + let ``Test that ref readonly is treated as ref`` () = + + FSharp """ + namespace ByrefTest + open System.Runtime.CompilerServices + type MyRecord = { Value : int } with + member this.SetValue(v: int) = (Unsafe.AsRef &this.Value) <- v + """ + |> compile + |> shouldSucceed From 908345ce6c8654f2e4d2307d1e24e3df71871668 Mon Sep 17 00:00:00 2001 From: Vlad Zarytovskii Date: Mon, 6 Nov 2023 14:27:07 +0100 Subject: [PATCH 2/3] Threat ref readonly as inref --- src/Compiler/Checking/TypeHierarchy.fs | 10 +++-- src/Compiler/TypedTree/TcGlobals.fs | 1 + .../Interop/ByrefTests.fs | 42 +++++++++++++++++-- 3 files changed, 46 insertions(+), 7 deletions(-) diff --git a/src/Compiler/Checking/TypeHierarchy.fs b/src/Compiler/Checking/TypeHierarchy.fs index f6949c707d8..a778b516d99 100644 --- a/src/Compiler/Checking/TypeHierarchy.fs +++ b/src/Compiler/Checking/TypeHierarchy.fs @@ -355,8 +355,13 @@ let ImportILTypeFromMetadata amap m scoref tinst minst ilTy = /// Read an Abstract IL type from metadata, including any attributes that may affect the type itself, and convert to an F# type. let ImportILTypeFromMetadataWithAttributes amap m scoref tinst minst ilTy getCattrs = let ty = RescopeAndImportILType scoref amap m (tinst@minst) ilTy - // If the type is a byref and one of attributes from a return or parameter has IsReadOnly, then it's a inref. - if isByrefTy amap.g ty && TryFindILAttribute amap.g.attrib_IsReadOnlyAttribute (getCattrs ()) then + // If the type is a byref and one of attributes from a return or parameter has + // - a `IsReadOnlyAttribute` - it's an inref + // - a `RequiresLocationAttribute` (in which case it's a `ref readonly`) which we treat as inref, + // latter is an ad-hoc fix for https://github.com/dotnet/runtime/issues/94317. + if isByrefTy amap.g ty + && (TryFindILAttribute amap.g.attrib_IsReadOnlyAttribute (getCattrs ()) + || TryFindILAttribute amap.g.attrib_RequiresLocationAttribute (getCattrs ())) then mkInByrefTy amap.g (destByrefTy amap.g ty) else ty @@ -428,4 +433,3 @@ let FixupNewTypars m (formalEnclosingTypars: Typars) (tinst: TType list) (tpsori let tprefInst = mkTyparInst formalEnclosingTypars tinst @ renaming (tpsorig, tps) ||> List.iter2 (fun tporig tp -> tp.SetConstraints (CopyTyparConstraints m tprefInst tporig)) renaming, tptys - diff --git a/src/Compiler/TypedTree/TcGlobals.fs b/src/Compiler/TypedTree/TcGlobals.fs index 01bd1a1283c..301096d092c 100755 --- a/src/Compiler/TypedTree/TcGlobals.fs +++ b/src/Compiler/TypedTree/TcGlobals.fs @@ -1423,6 +1423,7 @@ type TcGlobals( member val attrib_ParamArrayAttribute = findSysAttrib "System.ParamArrayAttribute" member val attrib_IDispatchConstantAttribute = tryFindSysAttrib "System.Runtime.CompilerServices.IDispatchConstantAttribute" member val attrib_IUnknownConstantAttribute = tryFindSysAttrib "System.Runtime.CompilerServices.IUnknownConstantAttribute" + member val attrib_RequiresLocationAttribute = findSysAttrib "System.Runtime.CompilerServices.RequiresLocationAttribute" // We use 'findSysAttrib' here because lookup on attribute is done by name comparison, and can proceed // even if the type is not found in a system assembly. diff --git a/tests/FSharp.Compiler.ComponentTests/Interop/ByrefTests.fs b/tests/FSharp.Compiler.ComponentTests/Interop/ByrefTests.fs index e4451a821ff..e813bd817cd 100644 --- a/tests/FSharp.Compiler.ComponentTests/Interop/ByrefTests.fs +++ b/tests/FSharp.Compiler.ComponentTests/Interop/ByrefTests.fs @@ -2,20 +2,54 @@ namespace Interop -open Xunit open FSharp.Test open FSharp.Test.Compiler module ``Byref interop verification tests`` = [] - let ``Test that ref readonly is treated as ref`` () = + let ``Test that ref readonly is treated as inref`` () = FSharp """ - namespace ByrefTest + module ByrefTest open System.Runtime.CompilerServices type MyRecord = { Value : int } with member this.SetValue(v: int) = (Unsafe.AsRef &this.Value) <- v + + let check mr = + if mr.Value <> 1 then + failwith "Value should be 1" + + mr.SetValue(42) + + if mr.Value <> 42 then + failwith $"Value should be 42, but is {mr.Value}" + 0 + + [] + let main _ = + let mr = { Value = 1 } + check mr """ - |> compile + |> asExe + |> compileAndRun |> shouldSucceed + + [] + let ``Test that ref readonly is treated as inref for ROS .ctor`` () = + FSharp """ + module Foo + open System + + [] + let main _ = + let mutable bt: int = 42 + let ros = ReadOnlySpan(&bt) + + if ros.Length <> 0 && ros[0] <> 42 then + failwith "Unexpected result" + 0 + """ + |> asExe + |> compileAndRun + |> shouldSucceed \ No newline at end of file From 9e5e6c589b31bf92537c98a4a7a80e6cdd331adf Mon Sep 17 00:00:00 2001 From: Vlad Zarytovskii Date: Mon, 6 Nov 2023 14:53:27 +0100 Subject: [PATCH 3/3] Fixed condition in test --- tests/FSharp.Compiler.ComponentTests/Interop/ByrefTests.fs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/FSharp.Compiler.ComponentTests/Interop/ByrefTests.fs b/tests/FSharp.Compiler.ComponentTests/Interop/ByrefTests.fs index e813bd817cd..dca7589c903 100644 --- a/tests/FSharp.Compiler.ComponentTests/Interop/ByrefTests.fs +++ b/tests/FSharp.Compiler.ComponentTests/Interop/ByrefTests.fs @@ -41,12 +41,13 @@ module ``Byref interop verification tests`` = module Foo open System + [] let main _ = let mutable bt: int = 42 let ros = ReadOnlySpan(&bt) - if ros.Length <> 0 && ros[0] <> 42 then + if ros.Length <> 1 || ros[0] <> 42 then failwith "Unexpected result" 0 """