From 05d7dfebd0cfaf8e86ae3343e2ddbad170f9d815 Mon Sep 17 00:00:00 2001 From: Petr Pokorny Date: Wed, 29 Jun 2022 17:40:33 +0200 Subject: [PATCH 01/13] Getters on struct records get readonly attribute --- src/Compiler/CodeGen/IlxGen.fs | 21 ++- .../EmittedIL/StructGettersReadOnly.fs | 142 ++++++++++++++++++ .../FSharp.Compiler.ComponentTests.fsproj | 2 + tests/FSharp.Test.Utilities/Compiler.fs | 22 ++- .../FSharp.Test.Utilities.fsproj | 1 + .../FSharp.Test.Utilities/ReflectionHelper.fs | 63 ++++++++ 6 files changed, 236 insertions(+), 15 deletions(-) create mode 100644 tests/FSharp.Compiler.ComponentTests/EmittedIL/StructGettersReadOnly.fs create mode 100644 tests/FSharp.Test.Utilities/ReflectionHelper.fs diff --git a/src/Compiler/CodeGen/IlxGen.fs b/src/Compiler/CodeGen/IlxGen.fs index 802583aeaa0..470c4898ed3 100644 --- a/src/Compiler/CodeGen/IlxGen.fs +++ b/src/Compiler/CodeGen/IlxGen.fs @@ -68,7 +68,7 @@ let iLdcDouble i = AI_ldc(DT_R8, ILConst.R8 i) let iLdcSingle i = AI_ldc(DT_R4, ILConst.R4 i) /// Make a method that simply loads a field -let mkLdfldMethodDef (ilMethName, reprAccess, isStatic, ilTy, ilFieldName, ilPropType) = +let mkLdfldMethodDef (ilMethName, reprAccess, isStatic, ilTy, ilFieldName, ilPropType, customAttrs) = let ilFieldSpec = mkILFieldSpecInTy (ilTy, ilFieldName, ilPropType) let ilReturn = mkILReturn ilPropType @@ -84,7 +84,9 @@ let mkLdfldMethodDef (ilMethName, reprAccess, isStatic, ilTy, ilFieldName, ilPro mkILNonGenericInstanceMethod (ilMethName, reprAccess, [], ilReturn, body) - ilMethodDef.WithSpecialName + ilMethodDef + .With(customAttrs = mkILCustomAttrs customAttrs) + .WithSpecialName /// Choose the constructor parameter names for fields let ChooseParamNames fieldNamesAndTypes = @@ -598,11 +600,13 @@ type PtrsOK = | PtrTypesOK | PtrTypesNotOK +let GenReadOnlyAttribute (g: TcGlobals) = mkILCustomAttribute (g.attrib_IsReadOnlyAttribute.TypeRef, [], [], []) + let GenReadOnlyAttributeIfNecessary (g: TcGlobals) ty = let add = isInByrefTy g ty && g.attrib_IsReadOnlyAttribute.TyconRef.CanDeref if add then - let attr = mkILCustomAttribute (g.attrib_IsReadOnlyAttribute.TypeRef, [], [], []) + let attr = GenReadOnlyAttribute g Some attr else None @@ -2087,7 +2091,8 @@ type AssemblyBuilder(cenv: cenv, anonTypeTable: AnonTypeGenerationTable) as mgbu let ilMethods = [ for propName, fldName, fldTy in flds -> - mkLdfldMethodDef ("get_" + propName, ILMemberAccess.Public, false, ilTy, fldName, fldTy) + let attrs = if isStruct then [GenReadOnlyAttribute g] else [] + mkLdfldMethodDef ("get_" + propName, ILMemberAccess.Public, false, ilTy, fldName, fldTy, attrs) yield! genToStringMethod ilTy ] @@ -9089,7 +9094,7 @@ and GenMethodForBinding // Check if we're compiling the property as a .NET event assert not (CompileAsEvent cenv.g v.Attribs) - // Emit the property, but not if its a private method impl + // Emit the property, but not if it's a private method impl if mdef.Access <> ILMemberAccess.Private then let vtyp = ReturnTypeOfPropertyVal g v let ilPropTy = GenType cenv m eenvUnderMethTypeTypars.tyenv vtyp @@ -10692,7 +10697,9 @@ and GenTypeDef cenv mgbuf lazyInitInfo eenv m (tycon: Tycon) = let ilPropName = fspec.LogicalName let ilMethName = "get_" + ilPropName let access = ComputeMemberAccess isPropHidden - yield mkLdfldMethodDef (ilMethName, access, isStatic, ilThisTy, ilFieldName, ilPropType) + let isStruct = tcref.IsFSharpStructOrEnumTycon && not tcref.IsEnumTycon + let attrs = if isStruct && not isStatic then [GenReadOnlyAttribute g] else [] + yield mkLdfldMethodDef (ilMethName, access, isStatic, ilThisTy, ilFieldName, ilPropType, attrs) // Generate property setter methods for the mutable fields for useGenuineField, ilFieldName, isFSharpMutable, isStatic, _, ilPropType, isPropHidden, fspec in fieldSummaries do @@ -11216,7 +11223,7 @@ and GenExnDef cenv mgbuf eenv m (exnc: Tycon) = let ilFieldName = ComputeFieldName exnc fld let ilMethodDef = - mkLdfldMethodDef (ilMethName, reprAccess, false, ilThisTy, ilFieldName, ilPropType) + mkLdfldMethodDef (ilMethName, reprAccess, false, ilThisTy, ilFieldName, ilPropType, []) let ilFieldDef = mkILInstanceField (ilFieldName, ilPropType, None, ILMemberAccess.Assembly) diff --git a/tests/FSharp.Compiler.ComponentTests/EmittedIL/StructGettersReadOnly.fs b/tests/FSharp.Compiler.ComponentTests/EmittedIL/StructGettersReadOnly.fs new file mode 100644 index 00000000000..29b41a6c1ba --- /dev/null +++ b/tests/FSharp.Compiler.ComponentTests/EmittedIL/StructGettersReadOnly.fs @@ -0,0 +1,142 @@ +namespace FSharp.Compiler.ComponentTests.EmittedIL + +open Microsoft.FSharp.Core +open Xunit +open FSharp.Test.Compiler +open FSharp.Test.ReflectionHelper + +module ``Struct getters readonly`` = + + let structRecord = + FSharp + """ + module TestStructRecord + + [] type MyRecord = { MyField : int } + """ + + let nonStructRecord = + FSharp + """ + module TestNonStructRecord + + type MyRecord = { MyField : int } + """ + + let structAnonRecord = + FSharp + """ + module TestStructAnonRecord + + let myRecord = struct {| MyField = 3 |} + """ + + let nonStructAnonRecord = + FSharp + """ + module TestNonStructAnonRecord + + let myRecord = {| MyField = 3 |} + """ + + let structNonRecord = + FSharp + """ + module TestStructNonRecord + + [] + type MyStruct(myField: int) = + member _.MyField = myField + """ + + let structWithCustomGetter = + FSharp + """ + module TestStructWithCustomGetter + + [] + type MyStruct = + val mutable x: int + member this.MyField with get () = this.x <- 4 + """ + + [] + let ``Struct record has readonly attribute on getter`` () = + structRecord + |> compileAssembly + |> getType "TestStructRecord+MyRecord" + |> getMethod "get_MyField" + |> should haveAttribute "IsReadOnlyAttribute" + + [] + let ``Struct anonymous record has readonly attribute on getter`` () = + structAnonRecord + |> compileAssembly + |> getFirstAnonymousType + |> getMethod "get_MyField" + |> should haveAttribute "IsReadOnlyAttribute" + + [] + let ``Non-struct anonymous record doesn't have readonly attribute on getter`` () = + nonStructAnonRecord + |> compileAssembly + |> getFirstAnonymousType + |> getMethod "get_MyField" + |> shouldn't haveAttribute "IsReadOnlyAttribute" + + [] + let ``Non-struct record doesn't have readonly getters`` () = + nonStructRecord + |> compileAssembly + |> getType "TestNonStructRecord+MyRecord" + |> getMethod "get_MyField" + |> shouldn't haveAttribute "IsReadOnlyAttribute" + + [] + let ``Struct non-record has readonly getters`` () = + structNonRecord + |> compileAssembly + |> getType "TestStructNonRecord+MyStruct" + |> getMethod "get_MyField" + |> should haveAttribute "IsReadOnlyAttribute" + + [] + let ``Custom getter on a struct doesn't have readonly attribute`` () = + structWithCustomGetter + |> compileAssembly + |> getType "TestStructWithCustomGetter+MyStruct" + |> getMethod "get_MyField" + |> shouldn't haveAttribute "IsReadOnlyAttribute" + + [] + let ``Struct record has readonly attribute on getter in IL`` () = + structRecord + |> compile + |> shouldSucceed + |> verifyIL [ """ + .method public hidebysig specialname + instance int32 get_MyField() cil managed + { + .custom instance void [runtime]System.Runtime.CompilerServices.IsReadOnlyAttribute::.ctor() = ( 01 00 00 00 ) + + .maxstack 8 + IL_0000: ldarg.0 + IL_0001: ldfld int32 TestStructRecord/MyRecord::MyField@ + IL_0006: ret + }""" ] + + [] + let ``Non-struct record doesn't have readonly getters in IL`` () = + nonStructRecord + |> compile + |> shouldSucceed + |> verifyIL [ """ + .method public hidebysig specialname + instance int32 get_MyField() cil managed + { + + .maxstack 8 + IL_0000: ldarg.0 + IL_0001: ldfld int32 TestNonStructRecord/MyRecord::MyField@ + IL_0006: ret + } """ ] diff --git a/tests/FSharp.Compiler.ComponentTests/FSharp.Compiler.ComponentTests.fsproj b/tests/FSharp.Compiler.ComponentTests/FSharp.Compiler.ComponentTests.fsproj index 562fbb40a88..12a18baf33e 100644 --- a/tests/FSharp.Compiler.ComponentTests/FSharp.Compiler.ComponentTests.fsproj +++ b/tests/FSharp.Compiler.ComponentTests/FSharp.Compiler.ComponentTests.fsproj @@ -92,6 +92,7 @@ + @@ -122,6 +123,7 @@ + diff --git a/tests/FSharp.Test.Utilities/Compiler.fs b/tests/FSharp.Test.Utilities/Compiler.fs index a374f3d4475..2c0452ae08d 100644 --- a/tests/FSharp.Test.Utilities/Compiler.fs +++ b/tests/FSharp.Test.Utilities/Compiler.fs @@ -631,16 +631,22 @@ module rec Compiler = | _ -> failwith "Compilation has errors." - let compileGuid (cUnit: CompilationUnit) : Guid = - let bytes = - compile cUnit - |> shouldSucceed - |> getAssemblyInBytes + let getAssembly = getAssemblyInBytes >> Assembly.Load - use reader1 = new PEReader(bytes.ToImmutableArray()) - let reader1 = reader1.GetMetadataReader() + let withPeReader func compilationResult = + let bytes = getAssemblyInBytes compilationResult + use reader = new PEReader(bytes.ToImmutableArray()) + func reader - reader1.GetModuleDefinition().Mvid |> reader1.GetGuid + let withMetadataReader func = + withPeReader (fun reader -> reader.GetMetadataReader() |> func) + + let compileGuid = + compile + >> shouldSucceed + >> withMetadataReader (fun reader -> reader.GetModuleDefinition().Mvid |> reader.GetGuid) + + let compileAssembly = compile >> shouldSucceed >> getAssembly let private parseFSharp (fsSource: FSharpCompilationSource) : CompilationResult = let source = fsSource.Source.GetSourceText |> Option.defaultValue "" diff --git a/tests/FSharp.Test.Utilities/FSharp.Test.Utilities.fsproj b/tests/FSharp.Test.Utilities/FSharp.Test.Utilities.fsproj index c9852f0fb8c..a20052b3f66 100644 --- a/tests/FSharp.Test.Utilities/FSharp.Test.Utilities.fsproj +++ b/tests/FSharp.Test.Utilities/FSharp.Test.Utilities.fsproj @@ -26,6 +26,7 @@ + diff --git a/tests/FSharp.Test.Utilities/ReflectionHelper.fs b/tests/FSharp.Test.Utilities/ReflectionHelper.fs new file mode 100644 index 00000000000..5759752c706 --- /dev/null +++ b/tests/FSharp.Test.Utilities/ReflectionHelper.fs @@ -0,0 +1,63 @@ +module FSharp.Test.ReflectionHelper + +open System +open System.Reflection + +/// Gets the given type from the assembly (otherwise throws) +let getType typeName (asm: Assembly) = + match asm.GetType(typeName, false) with + | null -> + let allTypes = + asm.GetTypes() + |> Array.map (fun ty -> ty.Name) + |> Array.reduce (fun x y -> $"%s{x}\r%s{y}") + + failwith $"Error: Assembly did not contain type %s{typeName}.\nAll types in asm:\n%s{allTypes}" + | ty -> ty + +/// Gets all anonymous types from the assembly +let getAnonymousTypes (asm: Assembly) = + [ for ty in asm.GetTypes() do + if ty.FullName.StartsWith "<>f__AnonymousType" then ty ] + +/// Gets the first anonymous type from the assembly +let getFirstAnonymousType asm = + match getAnonymousTypes asm with + | ty :: _ -> ty + | [] -> failwith "Error: No anonymous types found in the assembly" + +/// Gets a type's method +let getMethod methodName (ty: Type) = + match ty.GetMethod(methodName) with + | null -> failwith $"Error: Type did not contain member %s{methodName}" + | methodInfo -> methodInfo + +/// Assert that function f returns Ok for given input +let should f x y = + match f x y with + | Ok _ -> () + | Error message -> failwith $"%s{message} but it should" + +/// Assert that function f doesn't return Ok for given input +let shouldn't f x y = + match f x y with + | Ok message -> failwith $"%s{message} but it shouldn't" + | Error _ -> () + +/// Verify the object contains a custom attribute with the given name. E.g. "ObsoleteAttribute" +let haveAttribute attrName thingy = + let attrs = + match box thingy with + | :? Type as ty -> ty.GetCustomAttributes(false) + | :? MethodInfo as mi -> mi.GetCustomAttributes(false) + | :? PropertyInfo as pi -> pi.GetCustomAttributes(false) + | :? EventInfo as ei -> ei.GetCustomAttributes(false) + | _ -> failwith "Error: Unsupported primitive type, unable to get custom attributes." + + let hasAttribute = + attrs |> Array.exists (fun att -> att.GetType().Name = attrName) + + if hasAttribute then + Ok $"'{thingy}' has attribute '{attrName}'" + else + Error $"'{thingy}' doesn't have attribute '{attrName}'" From 859d150b25bf777469edbe18ffcc5503703c6122 Mon Sep 17 00:00:00 2001 From: Petr Pokorny Date: Wed, 29 Jun 2022 18:51:27 +0200 Subject: [PATCH 02/13] Fantomas --- src/Compiler/CodeGen/IlxGen.fs | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/src/Compiler/CodeGen/IlxGen.fs b/src/Compiler/CodeGen/IlxGen.fs index 470c4898ed3..d39dd1fa238 100644 --- a/src/Compiler/CodeGen/IlxGen.fs +++ b/src/Compiler/CodeGen/IlxGen.fs @@ -84,9 +84,7 @@ let mkLdfldMethodDef (ilMethName, reprAccess, isStatic, ilTy, ilFieldName, ilPro mkILNonGenericInstanceMethod (ilMethName, reprAccess, [], ilReturn, body) - ilMethodDef - .With(customAttrs = mkILCustomAttrs customAttrs) - .WithSpecialName + ilMethodDef.With(customAttrs = mkILCustomAttrs customAttrs).WithSpecialName /// Choose the constructor parameter names for fields let ChooseParamNames fieldNamesAndTypes = @@ -600,7 +598,8 @@ type PtrsOK = | PtrTypesOK | PtrTypesNotOK -let GenReadOnlyAttribute (g: TcGlobals) = mkILCustomAttribute (g.attrib_IsReadOnlyAttribute.TypeRef, [], [], []) +let GenReadOnlyAttribute (g: TcGlobals) = + mkILCustomAttribute (g.attrib_IsReadOnlyAttribute.TypeRef, [], [], []) let GenReadOnlyAttributeIfNecessary (g: TcGlobals) ty = let add = isInByrefTy g ty && g.attrib_IsReadOnlyAttribute.TyconRef.CanDeref @@ -2091,7 +2090,7 @@ type AssemblyBuilder(cenv: cenv, anonTypeTable: AnonTypeGenerationTable) as mgbu let ilMethods = [ for propName, fldName, fldTy in flds -> - let attrs = if isStruct then [GenReadOnlyAttribute g] else [] + let attrs = if isStruct then [ GenReadOnlyAttribute g ] else [] mkLdfldMethodDef ("get_" + propName, ILMemberAccess.Public, false, ilTy, fldName, fldTy, attrs) yield! genToStringMethod ilTy ] @@ -10698,7 +10697,13 @@ and GenTypeDef cenv mgbuf lazyInitInfo eenv m (tycon: Tycon) = let ilMethName = "get_" + ilPropName let access = ComputeMemberAccess isPropHidden let isStruct = tcref.IsFSharpStructOrEnumTycon && not tcref.IsEnumTycon - let attrs = if isStruct && not isStatic then [GenReadOnlyAttribute g] else [] + + let attrs = + if isStruct && not isStatic then + [ GenReadOnlyAttribute g ] + else + [] + yield mkLdfldMethodDef (ilMethName, access, isStatic, ilThisTy, ilFieldName, ilPropType, attrs) // Generate property setter methods for the mutable fields From 41035ab58740a46a3d18d69b510bfacf8b667754 Mon Sep 17 00:00:00 2001 From: Petr Pokorny Date: Wed, 29 Jun 2022 19:31:06 +0200 Subject: [PATCH 03/13] Explicit argument --- tests/FSharp.Test.Utilities/Compiler.fs | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/tests/FSharp.Test.Utilities/Compiler.fs b/tests/FSharp.Test.Utilities/Compiler.fs index 2c0452ae08d..54822b93d80 100644 --- a/tests/FSharp.Test.Utilities/Compiler.fs +++ b/tests/FSharp.Test.Utilities/Compiler.fs @@ -641,12 +641,17 @@ module rec Compiler = let withMetadataReader func = withPeReader (fun reader -> reader.GetMetadataReader() |> func) - let compileGuid = - compile - >> shouldSucceed - >> withMetadataReader (fun reader -> reader.GetModuleDefinition().Mvid |> reader.GetGuid) + let compileGuid cUnit = + cUnit + |> compile + |> shouldSucceed + |> withMetadataReader (fun reader -> reader.GetModuleDefinition().Mvid |> reader.GetGuid) - let compileAssembly = compile >> shouldSucceed >> getAssembly + let compileAssembly cUnit = + cUnit + |> compile + |> shouldSucceed + |> getAssembly let private parseFSharp (fsSource: FSharpCompilationSource) : CompilationResult = let source = fsSource.Source.GetSourceText |> Option.defaultValue "" From a19aa4a4d31dc8c36accd492f8040885f902887c Mon Sep 17 00:00:00 2001 From: Petr Pokorny Date: Thu, 30 Jun 2022 14:03:12 +0200 Subject: [PATCH 04/13] Tests update --- .../EmittedIL/Misc/Structs02.fs.il.debug.bsl | 1 + .../Misc/Structs02.fs.il.release.bsl | 1 + .../EmittedIL/StructGettersReadOnly.fs | 20 ++++++++++++++++++- .../FloatsAndDoubles.fs.il.debug.bsl | 2 ++ .../FloatsAndDoubles.fs.il.release.bsl | 2 ++ 5 files changed, 25 insertions(+), 1 deletion(-) diff --git a/tests/FSharp.Compiler.ComponentTests/EmittedIL/Misc/Structs02.fs.il.debug.bsl b/tests/FSharp.Compiler.ComponentTests/EmittedIL/Misc/Structs02.fs.il.debug.bsl index 11721f6522a..cc8f56cd993 100644 --- a/tests/FSharp.Compiler.ComponentTests/EmittedIL/Misc/Structs02.fs.il.debug.bsl +++ b/tests/FSharp.Compiler.ComponentTests/EmittedIL/Misc/Structs02.fs.il.debug.bsl @@ -66,6 +66,7 @@ .method public hidebysig specialname instance int32 get_hash() cil managed { + .custom instance void [runtime]System.Runtime.CompilerServices.IsReadOnlyAttribute::.ctor() = ( 01 00 00 00 ) // Code size 7 (0x7) .maxstack 8 IL_0000: ldarg.0 diff --git a/tests/FSharp.Compiler.ComponentTests/EmittedIL/Misc/Structs02.fs.il.release.bsl b/tests/FSharp.Compiler.ComponentTests/EmittedIL/Misc/Structs02.fs.il.release.bsl index 5c412d430d3..ed4eefc32ed 100644 --- a/tests/FSharp.Compiler.ComponentTests/EmittedIL/Misc/Structs02.fs.il.release.bsl +++ b/tests/FSharp.Compiler.ComponentTests/EmittedIL/Misc/Structs02.fs.il.release.bsl @@ -66,6 +66,7 @@ .method public hidebysig specialname instance int32 get_hash() cil managed { + .custom instance void [runtime]System.Runtime.CompilerServices.IsReadOnlyAttribute::.ctor() = ( 01 00 00 00 ) // Code size 7 (0x7) .maxstack 8 IL_0000: ldarg.0 diff --git a/tests/FSharp.Compiler.ComponentTests/EmittedIL/StructGettersReadOnly.fs b/tests/FSharp.Compiler.ComponentTests/EmittedIL/StructGettersReadOnly.fs index 29b41a6c1ba..720b02f0f02 100644 --- a/tests/FSharp.Compiler.ComponentTests/EmittedIL/StructGettersReadOnly.fs +++ b/tests/FSharp.Compiler.ComponentTests/EmittedIL/StructGettersReadOnly.fs @@ -49,6 +49,16 @@ module ``Struct getters readonly`` = member _.MyField = myField """ + let structNonRecordVal = + FSharp + """ + module TestStructNonRecordVal + + [] + type MyStruct = + val MyField: int + """ + let structWithCustomGetter = FSharp """ @@ -93,13 +103,21 @@ module ``Struct getters readonly`` = |> shouldn't haveAttribute "IsReadOnlyAttribute" [] - let ``Struct non-record has readonly getters`` () = + let ``Struct has readonly getters`` () = structNonRecord |> compileAssembly |> getType "TestStructNonRecord+MyStruct" |> getMethod "get_MyField" |> should haveAttribute "IsReadOnlyAttribute" + [] + let ``Struct val has readonly getter`` () = + structNonRecordVal + |> compileAssembly + |> getType "TestStructNonRecordVal+MyStruct" + |> getMethod "get_MyField" + |> should haveAttribute "IsReadOnlyAttribute" + [] let ``Custom getter on a struct doesn't have readonly attribute`` () = structWithCustomGetter diff --git a/tests/FSharp.Compiler.ComponentTests/EmittedIL/Structure/FloatsAndDoubles.fs.il.debug.bsl b/tests/FSharp.Compiler.ComponentTests/EmittedIL/Structure/FloatsAndDoubles.fs.il.debug.bsl index 1e35a8e9c26..0e84741e970 100644 --- a/tests/FSharp.Compiler.ComponentTests/EmittedIL/Structure/FloatsAndDoubles.fs.il.debug.bsl +++ b/tests/FSharp.Compiler.ComponentTests/EmittedIL/Structure/FloatsAndDoubles.fs.il.debug.bsl @@ -65,6 +65,7 @@ .method public hidebysig specialname instance float64 get_F() cil managed { + .custom instance void [runtime]System.Runtime.CompilerServices.IsReadOnlyAttribute::.ctor() = ( 01 00 00 00 ) // Code size 7 (0x7) .maxstack 8 IL_0000: ldarg.0 @@ -409,6 +410,7 @@ .method public hidebysig specialname instance float64 get_D() cil managed { + .custom instance void [runtime]System.Runtime.CompilerServices.IsReadOnlyAttribute::.ctor() = ( 01 00 00 00 ) // Code size 7 (0x7) .maxstack 8 IL_0000: ldarg.0 diff --git a/tests/FSharp.Compiler.ComponentTests/EmittedIL/Structure/FloatsAndDoubles.fs.il.release.bsl b/tests/FSharp.Compiler.ComponentTests/EmittedIL/Structure/FloatsAndDoubles.fs.il.release.bsl index caa632f0fa0..9d6c3245635 100644 --- a/tests/FSharp.Compiler.ComponentTests/EmittedIL/Structure/FloatsAndDoubles.fs.il.release.bsl +++ b/tests/FSharp.Compiler.ComponentTests/EmittedIL/Structure/FloatsAndDoubles.fs.il.release.bsl @@ -65,6 +65,7 @@ .method public hidebysig specialname instance float64 get_F() cil managed { + .custom instance void [runtime]System.Runtime.CompilerServices.IsReadOnlyAttribute::.ctor() = ( 01 00 00 00 ) // Code size 7 (0x7) .maxstack 8 IL_0000: ldarg.0 @@ -376,6 +377,7 @@ .method public hidebysig specialname instance float64 get_D() cil managed { + .custom instance void [runtime]System.Runtime.CompilerServices.IsReadOnlyAttribute::.ctor() = ( 01 00 00 00 ) // Code size 7 (0x7) .maxstack 8 IL_0000: ldarg.0 From 6415f1481cbb18d61261345cc0a623f9a95f2e6a Mon Sep 17 00:00:00 2001 From: Petr Pokorny Date: Wed, 29 Jun 2022 17:40:33 +0200 Subject: [PATCH 05/13] Getters on struct records get readonly attribute --- src/Compiler/CodeGen/IlxGen.fs | 21 ++- .../EmittedIL/StructGettersReadOnly.fs | 142 ++++++++++++++++++ .../FSharp.Compiler.ComponentTests.fsproj | 2 + tests/FSharp.Test.Utilities/Compiler.fs | 22 ++- .../FSharp.Test.Utilities.fsproj | 1 + .../FSharp.Test.Utilities/ReflectionHelper.fs | 63 ++++++++ 6 files changed, 236 insertions(+), 15 deletions(-) create mode 100644 tests/FSharp.Compiler.ComponentTests/EmittedIL/StructGettersReadOnly.fs create mode 100644 tests/FSharp.Test.Utilities/ReflectionHelper.fs diff --git a/src/Compiler/CodeGen/IlxGen.fs b/src/Compiler/CodeGen/IlxGen.fs index 802583aeaa0..470c4898ed3 100644 --- a/src/Compiler/CodeGen/IlxGen.fs +++ b/src/Compiler/CodeGen/IlxGen.fs @@ -68,7 +68,7 @@ let iLdcDouble i = AI_ldc(DT_R8, ILConst.R8 i) let iLdcSingle i = AI_ldc(DT_R4, ILConst.R4 i) /// Make a method that simply loads a field -let mkLdfldMethodDef (ilMethName, reprAccess, isStatic, ilTy, ilFieldName, ilPropType) = +let mkLdfldMethodDef (ilMethName, reprAccess, isStatic, ilTy, ilFieldName, ilPropType, customAttrs) = let ilFieldSpec = mkILFieldSpecInTy (ilTy, ilFieldName, ilPropType) let ilReturn = mkILReturn ilPropType @@ -84,7 +84,9 @@ let mkLdfldMethodDef (ilMethName, reprAccess, isStatic, ilTy, ilFieldName, ilPro mkILNonGenericInstanceMethod (ilMethName, reprAccess, [], ilReturn, body) - ilMethodDef.WithSpecialName + ilMethodDef + .With(customAttrs = mkILCustomAttrs customAttrs) + .WithSpecialName /// Choose the constructor parameter names for fields let ChooseParamNames fieldNamesAndTypes = @@ -598,11 +600,13 @@ type PtrsOK = | PtrTypesOK | PtrTypesNotOK +let GenReadOnlyAttribute (g: TcGlobals) = mkILCustomAttribute (g.attrib_IsReadOnlyAttribute.TypeRef, [], [], []) + let GenReadOnlyAttributeIfNecessary (g: TcGlobals) ty = let add = isInByrefTy g ty && g.attrib_IsReadOnlyAttribute.TyconRef.CanDeref if add then - let attr = mkILCustomAttribute (g.attrib_IsReadOnlyAttribute.TypeRef, [], [], []) + let attr = GenReadOnlyAttribute g Some attr else None @@ -2087,7 +2091,8 @@ type AssemblyBuilder(cenv: cenv, anonTypeTable: AnonTypeGenerationTable) as mgbu let ilMethods = [ for propName, fldName, fldTy in flds -> - mkLdfldMethodDef ("get_" + propName, ILMemberAccess.Public, false, ilTy, fldName, fldTy) + let attrs = if isStruct then [GenReadOnlyAttribute g] else [] + mkLdfldMethodDef ("get_" + propName, ILMemberAccess.Public, false, ilTy, fldName, fldTy, attrs) yield! genToStringMethod ilTy ] @@ -9089,7 +9094,7 @@ and GenMethodForBinding // Check if we're compiling the property as a .NET event assert not (CompileAsEvent cenv.g v.Attribs) - // Emit the property, but not if its a private method impl + // Emit the property, but not if it's a private method impl if mdef.Access <> ILMemberAccess.Private then let vtyp = ReturnTypeOfPropertyVal g v let ilPropTy = GenType cenv m eenvUnderMethTypeTypars.tyenv vtyp @@ -10692,7 +10697,9 @@ and GenTypeDef cenv mgbuf lazyInitInfo eenv m (tycon: Tycon) = let ilPropName = fspec.LogicalName let ilMethName = "get_" + ilPropName let access = ComputeMemberAccess isPropHidden - yield mkLdfldMethodDef (ilMethName, access, isStatic, ilThisTy, ilFieldName, ilPropType) + let isStruct = tcref.IsFSharpStructOrEnumTycon && not tcref.IsEnumTycon + let attrs = if isStruct && not isStatic then [GenReadOnlyAttribute g] else [] + yield mkLdfldMethodDef (ilMethName, access, isStatic, ilThisTy, ilFieldName, ilPropType, attrs) // Generate property setter methods for the mutable fields for useGenuineField, ilFieldName, isFSharpMutable, isStatic, _, ilPropType, isPropHidden, fspec in fieldSummaries do @@ -11216,7 +11223,7 @@ and GenExnDef cenv mgbuf eenv m (exnc: Tycon) = let ilFieldName = ComputeFieldName exnc fld let ilMethodDef = - mkLdfldMethodDef (ilMethName, reprAccess, false, ilThisTy, ilFieldName, ilPropType) + mkLdfldMethodDef (ilMethName, reprAccess, false, ilThisTy, ilFieldName, ilPropType, []) let ilFieldDef = mkILInstanceField (ilFieldName, ilPropType, None, ILMemberAccess.Assembly) diff --git a/tests/FSharp.Compiler.ComponentTests/EmittedIL/StructGettersReadOnly.fs b/tests/FSharp.Compiler.ComponentTests/EmittedIL/StructGettersReadOnly.fs new file mode 100644 index 00000000000..29b41a6c1ba --- /dev/null +++ b/tests/FSharp.Compiler.ComponentTests/EmittedIL/StructGettersReadOnly.fs @@ -0,0 +1,142 @@ +namespace FSharp.Compiler.ComponentTests.EmittedIL + +open Microsoft.FSharp.Core +open Xunit +open FSharp.Test.Compiler +open FSharp.Test.ReflectionHelper + +module ``Struct getters readonly`` = + + let structRecord = + FSharp + """ + module TestStructRecord + + [] type MyRecord = { MyField : int } + """ + + let nonStructRecord = + FSharp + """ + module TestNonStructRecord + + type MyRecord = { MyField : int } + """ + + let structAnonRecord = + FSharp + """ + module TestStructAnonRecord + + let myRecord = struct {| MyField = 3 |} + """ + + let nonStructAnonRecord = + FSharp + """ + module TestNonStructAnonRecord + + let myRecord = {| MyField = 3 |} + """ + + let structNonRecord = + FSharp + """ + module TestStructNonRecord + + [] + type MyStruct(myField: int) = + member _.MyField = myField + """ + + let structWithCustomGetter = + FSharp + """ + module TestStructWithCustomGetter + + [] + type MyStruct = + val mutable x: int + member this.MyField with get () = this.x <- 4 + """ + + [] + let ``Struct record has readonly attribute on getter`` () = + structRecord + |> compileAssembly + |> getType "TestStructRecord+MyRecord" + |> getMethod "get_MyField" + |> should haveAttribute "IsReadOnlyAttribute" + + [] + let ``Struct anonymous record has readonly attribute on getter`` () = + structAnonRecord + |> compileAssembly + |> getFirstAnonymousType + |> getMethod "get_MyField" + |> should haveAttribute "IsReadOnlyAttribute" + + [] + let ``Non-struct anonymous record doesn't have readonly attribute on getter`` () = + nonStructAnonRecord + |> compileAssembly + |> getFirstAnonymousType + |> getMethod "get_MyField" + |> shouldn't haveAttribute "IsReadOnlyAttribute" + + [] + let ``Non-struct record doesn't have readonly getters`` () = + nonStructRecord + |> compileAssembly + |> getType "TestNonStructRecord+MyRecord" + |> getMethod "get_MyField" + |> shouldn't haveAttribute "IsReadOnlyAttribute" + + [] + let ``Struct non-record has readonly getters`` () = + structNonRecord + |> compileAssembly + |> getType "TestStructNonRecord+MyStruct" + |> getMethod "get_MyField" + |> should haveAttribute "IsReadOnlyAttribute" + + [] + let ``Custom getter on a struct doesn't have readonly attribute`` () = + structWithCustomGetter + |> compileAssembly + |> getType "TestStructWithCustomGetter+MyStruct" + |> getMethod "get_MyField" + |> shouldn't haveAttribute "IsReadOnlyAttribute" + + [] + let ``Struct record has readonly attribute on getter in IL`` () = + structRecord + |> compile + |> shouldSucceed + |> verifyIL [ """ + .method public hidebysig specialname + instance int32 get_MyField() cil managed + { + .custom instance void [runtime]System.Runtime.CompilerServices.IsReadOnlyAttribute::.ctor() = ( 01 00 00 00 ) + + .maxstack 8 + IL_0000: ldarg.0 + IL_0001: ldfld int32 TestStructRecord/MyRecord::MyField@ + IL_0006: ret + }""" ] + + [] + let ``Non-struct record doesn't have readonly getters in IL`` () = + nonStructRecord + |> compile + |> shouldSucceed + |> verifyIL [ """ + .method public hidebysig specialname + instance int32 get_MyField() cil managed + { + + .maxstack 8 + IL_0000: ldarg.0 + IL_0001: ldfld int32 TestNonStructRecord/MyRecord::MyField@ + IL_0006: ret + } """ ] diff --git a/tests/FSharp.Compiler.ComponentTests/FSharp.Compiler.ComponentTests.fsproj b/tests/FSharp.Compiler.ComponentTests/FSharp.Compiler.ComponentTests.fsproj index 562fbb40a88..12a18baf33e 100644 --- a/tests/FSharp.Compiler.ComponentTests/FSharp.Compiler.ComponentTests.fsproj +++ b/tests/FSharp.Compiler.ComponentTests/FSharp.Compiler.ComponentTests.fsproj @@ -92,6 +92,7 @@ + @@ -122,6 +123,7 @@ + diff --git a/tests/FSharp.Test.Utilities/Compiler.fs b/tests/FSharp.Test.Utilities/Compiler.fs index a374f3d4475..2c0452ae08d 100644 --- a/tests/FSharp.Test.Utilities/Compiler.fs +++ b/tests/FSharp.Test.Utilities/Compiler.fs @@ -631,16 +631,22 @@ module rec Compiler = | _ -> failwith "Compilation has errors." - let compileGuid (cUnit: CompilationUnit) : Guid = - let bytes = - compile cUnit - |> shouldSucceed - |> getAssemblyInBytes + let getAssembly = getAssemblyInBytes >> Assembly.Load - use reader1 = new PEReader(bytes.ToImmutableArray()) - let reader1 = reader1.GetMetadataReader() + let withPeReader func compilationResult = + let bytes = getAssemblyInBytes compilationResult + use reader = new PEReader(bytes.ToImmutableArray()) + func reader - reader1.GetModuleDefinition().Mvid |> reader1.GetGuid + let withMetadataReader func = + withPeReader (fun reader -> reader.GetMetadataReader() |> func) + + let compileGuid = + compile + >> shouldSucceed + >> withMetadataReader (fun reader -> reader.GetModuleDefinition().Mvid |> reader.GetGuid) + + let compileAssembly = compile >> shouldSucceed >> getAssembly let private parseFSharp (fsSource: FSharpCompilationSource) : CompilationResult = let source = fsSource.Source.GetSourceText |> Option.defaultValue "" diff --git a/tests/FSharp.Test.Utilities/FSharp.Test.Utilities.fsproj b/tests/FSharp.Test.Utilities/FSharp.Test.Utilities.fsproj index c9852f0fb8c..a20052b3f66 100644 --- a/tests/FSharp.Test.Utilities/FSharp.Test.Utilities.fsproj +++ b/tests/FSharp.Test.Utilities/FSharp.Test.Utilities.fsproj @@ -26,6 +26,7 @@ + diff --git a/tests/FSharp.Test.Utilities/ReflectionHelper.fs b/tests/FSharp.Test.Utilities/ReflectionHelper.fs new file mode 100644 index 00000000000..5759752c706 --- /dev/null +++ b/tests/FSharp.Test.Utilities/ReflectionHelper.fs @@ -0,0 +1,63 @@ +module FSharp.Test.ReflectionHelper + +open System +open System.Reflection + +/// Gets the given type from the assembly (otherwise throws) +let getType typeName (asm: Assembly) = + match asm.GetType(typeName, false) with + | null -> + let allTypes = + asm.GetTypes() + |> Array.map (fun ty -> ty.Name) + |> Array.reduce (fun x y -> $"%s{x}\r%s{y}") + + failwith $"Error: Assembly did not contain type %s{typeName}.\nAll types in asm:\n%s{allTypes}" + | ty -> ty + +/// Gets all anonymous types from the assembly +let getAnonymousTypes (asm: Assembly) = + [ for ty in asm.GetTypes() do + if ty.FullName.StartsWith "<>f__AnonymousType" then ty ] + +/// Gets the first anonymous type from the assembly +let getFirstAnonymousType asm = + match getAnonymousTypes asm with + | ty :: _ -> ty + | [] -> failwith "Error: No anonymous types found in the assembly" + +/// Gets a type's method +let getMethod methodName (ty: Type) = + match ty.GetMethod(methodName) with + | null -> failwith $"Error: Type did not contain member %s{methodName}" + | methodInfo -> methodInfo + +/// Assert that function f returns Ok for given input +let should f x y = + match f x y with + | Ok _ -> () + | Error message -> failwith $"%s{message} but it should" + +/// Assert that function f doesn't return Ok for given input +let shouldn't f x y = + match f x y with + | Ok message -> failwith $"%s{message} but it shouldn't" + | Error _ -> () + +/// Verify the object contains a custom attribute with the given name. E.g. "ObsoleteAttribute" +let haveAttribute attrName thingy = + let attrs = + match box thingy with + | :? Type as ty -> ty.GetCustomAttributes(false) + | :? MethodInfo as mi -> mi.GetCustomAttributes(false) + | :? PropertyInfo as pi -> pi.GetCustomAttributes(false) + | :? EventInfo as ei -> ei.GetCustomAttributes(false) + | _ -> failwith "Error: Unsupported primitive type, unable to get custom attributes." + + let hasAttribute = + attrs |> Array.exists (fun att -> att.GetType().Name = attrName) + + if hasAttribute then + Ok $"'{thingy}' has attribute '{attrName}'" + else + Error $"'{thingy}' doesn't have attribute '{attrName}'" From 259c8aaab7271245a9faf288f5e5abc3cb82ae35 Mon Sep 17 00:00:00 2001 From: Petr Pokorny Date: Wed, 29 Jun 2022 18:51:27 +0200 Subject: [PATCH 06/13] Fantomas --- src/Compiler/CodeGen/IlxGen.fs | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/src/Compiler/CodeGen/IlxGen.fs b/src/Compiler/CodeGen/IlxGen.fs index 470c4898ed3..d39dd1fa238 100644 --- a/src/Compiler/CodeGen/IlxGen.fs +++ b/src/Compiler/CodeGen/IlxGen.fs @@ -84,9 +84,7 @@ let mkLdfldMethodDef (ilMethName, reprAccess, isStatic, ilTy, ilFieldName, ilPro mkILNonGenericInstanceMethod (ilMethName, reprAccess, [], ilReturn, body) - ilMethodDef - .With(customAttrs = mkILCustomAttrs customAttrs) - .WithSpecialName + ilMethodDef.With(customAttrs = mkILCustomAttrs customAttrs).WithSpecialName /// Choose the constructor parameter names for fields let ChooseParamNames fieldNamesAndTypes = @@ -600,7 +598,8 @@ type PtrsOK = | PtrTypesOK | PtrTypesNotOK -let GenReadOnlyAttribute (g: TcGlobals) = mkILCustomAttribute (g.attrib_IsReadOnlyAttribute.TypeRef, [], [], []) +let GenReadOnlyAttribute (g: TcGlobals) = + mkILCustomAttribute (g.attrib_IsReadOnlyAttribute.TypeRef, [], [], []) let GenReadOnlyAttributeIfNecessary (g: TcGlobals) ty = let add = isInByrefTy g ty && g.attrib_IsReadOnlyAttribute.TyconRef.CanDeref @@ -2091,7 +2090,7 @@ type AssemblyBuilder(cenv: cenv, anonTypeTable: AnonTypeGenerationTable) as mgbu let ilMethods = [ for propName, fldName, fldTy in flds -> - let attrs = if isStruct then [GenReadOnlyAttribute g] else [] + let attrs = if isStruct then [ GenReadOnlyAttribute g ] else [] mkLdfldMethodDef ("get_" + propName, ILMemberAccess.Public, false, ilTy, fldName, fldTy, attrs) yield! genToStringMethod ilTy ] @@ -10698,7 +10697,13 @@ and GenTypeDef cenv mgbuf lazyInitInfo eenv m (tycon: Tycon) = let ilMethName = "get_" + ilPropName let access = ComputeMemberAccess isPropHidden let isStruct = tcref.IsFSharpStructOrEnumTycon && not tcref.IsEnumTycon - let attrs = if isStruct && not isStatic then [GenReadOnlyAttribute g] else [] + + let attrs = + if isStruct && not isStatic then + [ GenReadOnlyAttribute g ] + else + [] + yield mkLdfldMethodDef (ilMethName, access, isStatic, ilThisTy, ilFieldName, ilPropType, attrs) // Generate property setter methods for the mutable fields From 91d29a6a9360c2d2c5350f469a6642330bcbd465 Mon Sep 17 00:00:00 2001 From: Petr Pokorny Date: Wed, 29 Jun 2022 19:31:06 +0200 Subject: [PATCH 07/13] Explicit argument --- tests/FSharp.Test.Utilities/Compiler.fs | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/tests/FSharp.Test.Utilities/Compiler.fs b/tests/FSharp.Test.Utilities/Compiler.fs index 2c0452ae08d..54822b93d80 100644 --- a/tests/FSharp.Test.Utilities/Compiler.fs +++ b/tests/FSharp.Test.Utilities/Compiler.fs @@ -641,12 +641,17 @@ module rec Compiler = let withMetadataReader func = withPeReader (fun reader -> reader.GetMetadataReader() |> func) - let compileGuid = - compile - >> shouldSucceed - >> withMetadataReader (fun reader -> reader.GetModuleDefinition().Mvid |> reader.GetGuid) + let compileGuid cUnit = + cUnit + |> compile + |> shouldSucceed + |> withMetadataReader (fun reader -> reader.GetModuleDefinition().Mvid |> reader.GetGuid) - let compileAssembly = compile >> shouldSucceed >> getAssembly + let compileAssembly cUnit = + cUnit + |> compile + |> shouldSucceed + |> getAssembly let private parseFSharp (fsSource: FSharpCompilationSource) : CompilationResult = let source = fsSource.Source.GetSourceText |> Option.defaultValue "" From 738184ccbc5a8afd3b44cb2bb59b78322c4c1373 Mon Sep 17 00:00:00 2001 From: Petr Pokorny Date: Thu, 30 Jun 2022 14:03:12 +0200 Subject: [PATCH 08/13] Tests update --- .../EmittedIL/Misc/Structs02.fs.il.debug.bsl | 1 + .../Misc/Structs02.fs.il.release.bsl | 1 + .../EmittedIL/StructGettersReadOnly.fs | 20 ++++++++++++++++++- .../FloatsAndDoubles.fs.il.debug.bsl | 2 ++ .../FloatsAndDoubles.fs.il.release.bsl | 2 ++ 5 files changed, 25 insertions(+), 1 deletion(-) diff --git a/tests/FSharp.Compiler.ComponentTests/EmittedIL/Misc/Structs02.fs.il.debug.bsl b/tests/FSharp.Compiler.ComponentTests/EmittedIL/Misc/Structs02.fs.il.debug.bsl index 11721f6522a..cc8f56cd993 100644 --- a/tests/FSharp.Compiler.ComponentTests/EmittedIL/Misc/Structs02.fs.il.debug.bsl +++ b/tests/FSharp.Compiler.ComponentTests/EmittedIL/Misc/Structs02.fs.il.debug.bsl @@ -66,6 +66,7 @@ .method public hidebysig specialname instance int32 get_hash() cil managed { + .custom instance void [runtime]System.Runtime.CompilerServices.IsReadOnlyAttribute::.ctor() = ( 01 00 00 00 ) // Code size 7 (0x7) .maxstack 8 IL_0000: ldarg.0 diff --git a/tests/FSharp.Compiler.ComponentTests/EmittedIL/Misc/Structs02.fs.il.release.bsl b/tests/FSharp.Compiler.ComponentTests/EmittedIL/Misc/Structs02.fs.il.release.bsl index 5c412d430d3..ed4eefc32ed 100644 --- a/tests/FSharp.Compiler.ComponentTests/EmittedIL/Misc/Structs02.fs.il.release.bsl +++ b/tests/FSharp.Compiler.ComponentTests/EmittedIL/Misc/Structs02.fs.il.release.bsl @@ -66,6 +66,7 @@ .method public hidebysig specialname instance int32 get_hash() cil managed { + .custom instance void [runtime]System.Runtime.CompilerServices.IsReadOnlyAttribute::.ctor() = ( 01 00 00 00 ) // Code size 7 (0x7) .maxstack 8 IL_0000: ldarg.0 diff --git a/tests/FSharp.Compiler.ComponentTests/EmittedIL/StructGettersReadOnly.fs b/tests/FSharp.Compiler.ComponentTests/EmittedIL/StructGettersReadOnly.fs index 29b41a6c1ba..720b02f0f02 100644 --- a/tests/FSharp.Compiler.ComponentTests/EmittedIL/StructGettersReadOnly.fs +++ b/tests/FSharp.Compiler.ComponentTests/EmittedIL/StructGettersReadOnly.fs @@ -49,6 +49,16 @@ module ``Struct getters readonly`` = member _.MyField = myField """ + let structNonRecordVal = + FSharp + """ + module TestStructNonRecordVal + + [] + type MyStruct = + val MyField: int + """ + let structWithCustomGetter = FSharp """ @@ -93,13 +103,21 @@ module ``Struct getters readonly`` = |> shouldn't haveAttribute "IsReadOnlyAttribute" [] - let ``Struct non-record has readonly getters`` () = + let ``Struct has readonly getters`` () = structNonRecord |> compileAssembly |> getType "TestStructNonRecord+MyStruct" |> getMethod "get_MyField" |> should haveAttribute "IsReadOnlyAttribute" + [] + let ``Struct val has readonly getter`` () = + structNonRecordVal + |> compileAssembly + |> getType "TestStructNonRecordVal+MyStruct" + |> getMethod "get_MyField" + |> should haveAttribute "IsReadOnlyAttribute" + [] let ``Custom getter on a struct doesn't have readonly attribute`` () = structWithCustomGetter diff --git a/tests/FSharp.Compiler.ComponentTests/EmittedIL/Structure/FloatsAndDoubles.fs.il.debug.bsl b/tests/FSharp.Compiler.ComponentTests/EmittedIL/Structure/FloatsAndDoubles.fs.il.debug.bsl index 1e35a8e9c26..0e84741e970 100644 --- a/tests/FSharp.Compiler.ComponentTests/EmittedIL/Structure/FloatsAndDoubles.fs.il.debug.bsl +++ b/tests/FSharp.Compiler.ComponentTests/EmittedIL/Structure/FloatsAndDoubles.fs.il.debug.bsl @@ -65,6 +65,7 @@ .method public hidebysig specialname instance float64 get_F() cil managed { + .custom instance void [runtime]System.Runtime.CompilerServices.IsReadOnlyAttribute::.ctor() = ( 01 00 00 00 ) // Code size 7 (0x7) .maxstack 8 IL_0000: ldarg.0 @@ -409,6 +410,7 @@ .method public hidebysig specialname instance float64 get_D() cil managed { + .custom instance void [runtime]System.Runtime.CompilerServices.IsReadOnlyAttribute::.ctor() = ( 01 00 00 00 ) // Code size 7 (0x7) .maxstack 8 IL_0000: ldarg.0 diff --git a/tests/FSharp.Compiler.ComponentTests/EmittedIL/Structure/FloatsAndDoubles.fs.il.release.bsl b/tests/FSharp.Compiler.ComponentTests/EmittedIL/Structure/FloatsAndDoubles.fs.il.release.bsl index caa632f0fa0..9d6c3245635 100644 --- a/tests/FSharp.Compiler.ComponentTests/EmittedIL/Structure/FloatsAndDoubles.fs.il.release.bsl +++ b/tests/FSharp.Compiler.ComponentTests/EmittedIL/Structure/FloatsAndDoubles.fs.il.release.bsl @@ -65,6 +65,7 @@ .method public hidebysig specialname instance float64 get_F() cil managed { + .custom instance void [runtime]System.Runtime.CompilerServices.IsReadOnlyAttribute::.ctor() = ( 01 00 00 00 ) // Code size 7 (0x7) .maxstack 8 IL_0000: ldarg.0 @@ -376,6 +377,7 @@ .method public hidebysig specialname instance float64 get_D() cil managed { + .custom instance void [runtime]System.Runtime.CompilerServices.IsReadOnlyAttribute::.ctor() = ( 01 00 00 00 ) // Code size 7 (0x7) .maxstack 8 IL_0000: ldarg.0 From bf6063ae851aa95e3f754066aca18cb933b5512a Mon Sep 17 00:00:00 2001 From: Petr Pokorny Date: Thu, 30 Jun 2022 17:19:11 +0200 Subject: [PATCH 09/13] Tests update --- .../EmittedIL/StructGettersReadOnly.fs | 42 ++++++------------- 1 file changed, 12 insertions(+), 30 deletions(-) diff --git a/tests/FSharp.Compiler.ComponentTests/EmittedIL/StructGettersReadOnly.fs b/tests/FSharp.Compiler.ComponentTests/EmittedIL/StructGettersReadOnly.fs index 720b02f0f02..77feb7f0260 100644 --- a/tests/FSharp.Compiler.ComponentTests/EmittedIL/StructGettersReadOnly.fs +++ b/tests/FSharp.Compiler.ComponentTests/EmittedIL/StructGettersReadOnly.fs @@ -10,7 +10,7 @@ module ``Struct getters readonly`` = let structRecord = FSharp """ - module TestStructRecord + module Test [] type MyRecord = { MyField : int } """ @@ -18,7 +18,7 @@ module ``Struct getters readonly`` = let nonStructRecord = FSharp """ - module TestNonStructRecord + module Test type MyRecord = { MyField : int } """ @@ -26,7 +26,7 @@ module ``Struct getters readonly`` = let structAnonRecord = FSharp """ - module TestStructAnonRecord + module Test let myRecord = struct {| MyField = 3 |} """ @@ -34,7 +34,7 @@ module ``Struct getters readonly`` = let nonStructAnonRecord = FSharp """ - module TestNonStructAnonRecord + module Test let myRecord = {| MyField = 3 |} """ @@ -42,17 +42,7 @@ module ``Struct getters readonly`` = let structNonRecord = FSharp """ - module TestStructNonRecord - - [] - type MyStruct(myField: int) = - member _.MyField = myField - """ - - let structNonRecordVal = - FSharp - """ - module TestStructNonRecordVal + module Test [] type MyStruct = @@ -62,7 +52,7 @@ module ``Struct getters readonly`` = let structWithCustomGetter = FSharp """ - module TestStructWithCustomGetter + module Test [] type MyStruct = @@ -74,7 +64,7 @@ module ``Struct getters readonly`` = let ``Struct record has readonly attribute on getter`` () = structRecord |> compileAssembly - |> getType "TestStructRecord+MyRecord" + |> getType "Test+MyRecord" |> getMethod "get_MyField" |> should haveAttribute "IsReadOnlyAttribute" @@ -98,7 +88,7 @@ module ``Struct getters readonly`` = let ``Non-struct record doesn't have readonly getters`` () = nonStructRecord |> compileAssembly - |> getType "TestNonStructRecord+MyRecord" + |> getType "Test+MyRecord" |> getMethod "get_MyField" |> shouldn't haveAttribute "IsReadOnlyAttribute" @@ -106,15 +96,7 @@ module ``Struct getters readonly`` = let ``Struct has readonly getters`` () = structNonRecord |> compileAssembly - |> getType "TestStructNonRecord+MyStruct" - |> getMethod "get_MyField" - |> should haveAttribute "IsReadOnlyAttribute" - - [] - let ``Struct val has readonly getter`` () = - structNonRecordVal - |> compileAssembly - |> getType "TestStructNonRecordVal+MyStruct" + |> getType "Test+MyStruct" |> getMethod "get_MyField" |> should haveAttribute "IsReadOnlyAttribute" @@ -122,7 +104,7 @@ module ``Struct getters readonly`` = let ``Custom getter on a struct doesn't have readonly attribute`` () = structWithCustomGetter |> compileAssembly - |> getType "TestStructWithCustomGetter+MyStruct" + |> getType "Test+MyStruct" |> getMethod "get_MyField" |> shouldn't haveAttribute "IsReadOnlyAttribute" @@ -139,7 +121,7 @@ module ``Struct getters readonly`` = .maxstack 8 IL_0000: ldarg.0 - IL_0001: ldfld int32 TestStructRecord/MyRecord::MyField@ + IL_0001: ldfld int32 Test/MyRecord::MyField@ IL_0006: ret }""" ] @@ -155,6 +137,6 @@ module ``Struct getters readonly`` = .maxstack 8 IL_0000: ldarg.0 - IL_0001: ldfld int32 TestNonStructRecord/MyRecord::MyField@ + IL_0001: ldfld int32 Test/MyRecord::MyField@ IL_0006: ret } """ ] From f6e8644847bda97e70cb67b3cf61930af88955ae Mon Sep 17 00:00:00 2001 From: Petr Pokorny Date: Thu, 7 Jul 2022 13:24:30 +0200 Subject: [PATCH 10/13] Small optimization --- src/Compiler/CodeGen/IlxGen.fs | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/src/Compiler/CodeGen/IlxGen.fs b/src/Compiler/CodeGen/IlxGen.fs index d39dd1fa238..f0d7b2b27fd 100644 --- a/src/Compiler/CodeGen/IlxGen.fs +++ b/src/Compiler/CodeGen/IlxGen.fs @@ -10696,13 +10696,12 @@ and GenTypeDef cenv mgbuf lazyInitInfo eenv m (tycon: Tycon) = let ilPropName = fspec.LogicalName let ilMethName = "get_" + ilPropName let access = ComputeMemberAccess isPropHidden - let isStruct = tcref.IsFSharpStructOrEnumTycon && not tcref.IsEnumTycon + let isStruct = tycon.IsFSharpStructOrEnumTycon && not tycon.IsEnumTycon let attrs = - if isStruct && not isStatic then - [ GenReadOnlyAttribute g ] - else - [] + [ + if isStruct && not isStatic then GenReadOnlyAttribute g + ] yield mkLdfldMethodDef (ilMethName, access, isStatic, ilThisTy, ilFieldName, ilPropType, attrs) From e8eca37ea91782b85386117c7c4c7dd03fa616d1 Mon Sep 17 00:00:00 2001 From: Petr Pokorny Date: Thu, 7 Jul 2022 13:24:53 +0200 Subject: [PATCH 11/13] Reorganized tests --- .../EmittedIL/StructGettersReadOnly.fs | 150 ++++++++---------- 1 file changed, 69 insertions(+), 81 deletions(-) diff --git a/tests/FSharp.Compiler.ComponentTests/EmittedIL/StructGettersReadOnly.fs b/tests/FSharp.Compiler.ComponentTests/EmittedIL/StructGettersReadOnly.fs index 77feb7f0260..ccb94a97e50 100644 --- a/tests/FSharp.Compiler.ComponentTests/EmittedIL/StructGettersReadOnly.fs +++ b/tests/FSharp.Compiler.ComponentTests/EmittedIL/StructGettersReadOnly.fs @@ -15,6 +15,31 @@ module ``Struct getters readonly`` = [] type MyRecord = { MyField : int } """ + [] + let ``Struct record has readonly attribute on getter`` () = + structRecord + |> compileAssembly + |> getType "Test+MyRecord" + |> getMethod "get_MyField" + |> should haveAttribute "IsReadOnlyAttribute" + + [] + let ``Struct record has readonly attribute on getter in IL`` () = + structRecord + |> compile + |> shouldSucceed + |> verifyIL [ """ + .method public hidebysig specialname + instance int32 get_MyField() cil managed + { + .custom instance void [runtime]System.Runtime.CompilerServices.IsReadOnlyAttribute::.ctor() = ( 01 00 00 00 ) + + .maxstack 8 + IL_0000: ldarg.0 + IL_0001: ldfld int32 Test/MyRecord::MyField@ + IL_0006: ret + }""" ] + let nonStructRecord = FSharp """ @@ -23,23 +48,58 @@ module ``Struct getters readonly`` = type MyRecord = { MyField : int } """ - let structAnonRecord = + [] + let ``Non-struct record doesn't have readonly getters`` () = + nonStructRecord + |> compileAssembly + |> getType "Test+MyRecord" + |> getMethod "get_MyField" + |> shouldn't haveAttribute "IsReadOnlyAttribute" + + [] + let ``Non-struct record doesn't have readonly getters in IL`` () = + nonStructRecord + |> compile + |> shouldSucceed + |> verifyIL [ """ + .method public hidebysig specialname + instance int32 get_MyField() cil managed + { + + .maxstack 8 + IL_0000: ldarg.0 + IL_0001: ldfld int32 Test/MyRecord::MyField@ + IL_0006: ret + } """ ] + + [] + let ``Struct anonymous record has readonly attribute on getter`` () = FSharp """ module Test let myRecord = struct {| MyField = 3 |} """ + |> compileAssembly + |> getFirstAnonymousType + |> getMethod "get_MyField" + |> should haveAttribute "IsReadOnlyAttribute" - let nonStructAnonRecord = + [] + let ``Non-struct anonymous record doesn't have readonly attribute on getter`` () = FSharp """ module Test let myRecord = {| MyField = 3 |} """ + |> compileAssembly + |> getFirstAnonymousType + |> getMethod "get_MyField" + |> shouldn't haveAttribute "IsReadOnlyAttribute" - let structNonRecord = + [] + let ``Struct has readonly getters`` () = FSharp """ module Test @@ -48,8 +108,13 @@ module ``Struct getters readonly`` = type MyStruct = val MyField: int """ + |> compileAssembly + |> getType "Test+MyStruct" + |> getMethod "get_MyField" + |> should haveAttribute "IsReadOnlyAttribute" - let structWithCustomGetter = + [] + let ``Custom getter on a struct doesn't have readonly attribute`` () = FSharp """ module Test @@ -59,84 +124,7 @@ module ``Struct getters readonly`` = val mutable x: int member this.MyField with get () = this.x <- 4 """ - - [] - let ``Struct record has readonly attribute on getter`` () = - structRecord - |> compileAssembly - |> getType "Test+MyRecord" - |> getMethod "get_MyField" - |> should haveAttribute "IsReadOnlyAttribute" - - [] - let ``Struct anonymous record has readonly attribute on getter`` () = - structAnonRecord - |> compileAssembly - |> getFirstAnonymousType - |> getMethod "get_MyField" - |> should haveAttribute "IsReadOnlyAttribute" - - [] - let ``Non-struct anonymous record doesn't have readonly attribute on getter`` () = - nonStructAnonRecord - |> compileAssembly - |> getFirstAnonymousType - |> getMethod "get_MyField" - |> shouldn't haveAttribute "IsReadOnlyAttribute" - - [] - let ``Non-struct record doesn't have readonly getters`` () = - nonStructRecord - |> compileAssembly - |> getType "Test+MyRecord" - |> getMethod "get_MyField" - |> shouldn't haveAttribute "IsReadOnlyAttribute" - - [] - let ``Struct has readonly getters`` () = - structNonRecord - |> compileAssembly - |> getType "Test+MyStruct" - |> getMethod "get_MyField" - |> should haveAttribute "IsReadOnlyAttribute" - - [] - let ``Custom getter on a struct doesn't have readonly attribute`` () = - structWithCustomGetter |> compileAssembly |> getType "Test+MyStruct" |> getMethod "get_MyField" |> shouldn't haveAttribute "IsReadOnlyAttribute" - - [] - let ``Struct record has readonly attribute on getter in IL`` () = - structRecord - |> compile - |> shouldSucceed - |> verifyIL [ """ - .method public hidebysig specialname - instance int32 get_MyField() cil managed - { - .custom instance void [runtime]System.Runtime.CompilerServices.IsReadOnlyAttribute::.ctor() = ( 01 00 00 00 ) - - .maxstack 8 - IL_0000: ldarg.0 - IL_0001: ldfld int32 Test/MyRecord::MyField@ - IL_0006: ret - }""" ] - - [] - let ``Non-struct record doesn't have readonly getters in IL`` () = - nonStructRecord - |> compile - |> shouldSucceed - |> verifyIL [ """ - .method public hidebysig specialname - instance int32 get_MyField() cil managed - { - - .maxstack 8 - IL_0000: ldarg.0 - IL_0001: ldfld int32 Test/MyRecord::MyField@ - IL_0006: ret - } """ ] From 9572852ae0dd3f90a71b562e9fa304e69a8de3a9 Mon Sep 17 00:00:00 2001 From: Petr Pokorny Date: Thu, 7 Jul 2022 14:09:28 +0200 Subject: [PATCH 12/13] Revert list construction --- src/Compiler/CodeGen/IlxGen.fs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/Compiler/CodeGen/IlxGen.fs b/src/Compiler/CodeGen/IlxGen.fs index f0d7b2b27fd..96631c19b69 100644 --- a/src/Compiler/CodeGen/IlxGen.fs +++ b/src/Compiler/CodeGen/IlxGen.fs @@ -10699,9 +10699,10 @@ and GenTypeDef cenv mgbuf lazyInitInfo eenv m (tycon: Tycon) = let isStruct = tycon.IsFSharpStructOrEnumTycon && not tycon.IsEnumTycon let attrs = - [ - if isStruct && not isStatic then GenReadOnlyAttribute g - ] + if isStruct && not isStatic then + [ GenReadOnlyAttribute g ] + else + [] yield mkLdfldMethodDef (ilMethName, access, isStatic, ilThisTy, ilFieldName, ilPropType, attrs) From b71a62cc1b214858874ac8dff67c238e08334cb2 Mon Sep 17 00:00:00 2001 From: 0101 <0101@innit.cz> Date: Fri, 8 Jul 2022 11:19:39 +0200 Subject: [PATCH 13/13] Unified way of determining a struct --- src/Compiler/CodeGen/IlxGen.fs | 2 +- src/Compiler/TypedTree/TypedTreeOps.fsi | 3 +++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/src/Compiler/CodeGen/IlxGen.fs b/src/Compiler/CodeGen/IlxGen.fs index 96631c19b69..a246f176612 100644 --- a/src/Compiler/CodeGen/IlxGen.fs +++ b/src/Compiler/CodeGen/IlxGen.fs @@ -10696,7 +10696,7 @@ and GenTypeDef cenv mgbuf lazyInitInfo eenv m (tycon: Tycon) = let ilPropName = fspec.LogicalName let ilMethName = "get_" + ilPropName let access = ComputeMemberAccess isPropHidden - let isStruct = tycon.IsFSharpStructOrEnumTycon && not tycon.IsEnumTycon + let isStruct = isStructTyconRef tcref let attrs = if isStruct && not isStatic then diff --git a/src/Compiler/TypedTree/TypedTreeOps.fsi b/src/Compiler/TypedTree/TypedTreeOps.fsi index a7ad47779dc..c0a2dd51211 100755 --- a/src/Compiler/TypedTree/TypedTreeOps.fsi +++ b/src/Compiler/TypedTree/TypedTreeOps.fsi @@ -1662,6 +1662,9 @@ val underlyingTypeOfEnumTy: TcGlobals -> TType -> TType /// If the input type is an enum type, then convert to its underlying type, otherwise return the input type val normalizeEnumTy: TcGlobals -> TType -> TType +/// Determine if TyconRef is to a struct type +val isStructTyconRef: TyconRef -> bool + /// Determine if a type is a struct type val isStructTy: TcGlobals -> TType -> bool