Auto-generated getters on structs get readonly attribute

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,7 @@ 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 +598,14 @@ 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 +2090,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 +9093,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 +10696,15 @@ 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 = isStructTyconRef tcref
+
+                            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 +11228,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)

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
 

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

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

tests/FSharp.Compiler.ComponentTests/EmittedIL/StructGettersReadOnly.fs

@@ -0,0 +1,130 @@
+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 Test
+
+            [<Struct>] type MyRecord = { MyField : int }
+            """
+
+    [<Fact>]
+    let ``Struct record has readonly attribute on getter`` () =
+        structRecord
+        |> compileAssembly
+        |> getType "Test+MyRecord"
+        |> getMethod "get_MyField"
+        |> should haveAttribute "IsReadOnlyAttribute"
+
+    [<Fact>]
+    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
+            """
+            module Test
+
+            type MyRecord = { MyField : int }
+            """
+
+    [<Fact>]
+    let ``Non-struct record doesn't have readonly getters`` () =
+        nonStructRecord
+        |> compileAssembly
+        |> getType "Test+MyRecord"
+        |> getMethod "get_MyField"
+        |> shouldn't haveAttribute "IsReadOnlyAttribute"
+
+    [<Fact>]
+    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
+            } """ ]
+
+    [<Fact>]
+    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"
+
+    [<Fact>]
+    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"
+
+    [<Fact>]
+    let ``Struct has readonly getters`` () =
+        FSharp
+            """
+            module Test
+
+            [<Struct>]
+            type MyStruct =
+                val MyField: int
+            """
+        |> compileAssembly
+        |> getType "Test+MyStruct"
+        |> getMethod "get_MyField"
+        |> should haveAttribute "IsReadOnlyAttribute"
+
+    [<Fact>]
+    let ``Custom getter on a struct doesn't have readonly attribute`` () =
+        FSharp
+            """
+            module Test
+
+            [<Struct>]
+            type MyStruct =
+                val mutable x: int
+                member this.MyField with get () = this.x <- 4
+            """
+        |> compileAssembly
+        |> getType "Test+MyStruct"
+        |> getMethod "get_MyField"
+        |> shouldn't haveAttribute "IsReadOnlyAttribute"

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

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

tests/FSharp.Compiler.ComponentTests/FSharp.Compiler.ComponentTests.fsproj

@@ -92,6 +92,7 @@
     <Compile Include="EmittedIL\Literals.fs" />
     <Compile Include="EmittedIL\SkipLocalsInit.fs" />
     <Compile Include="EmittedIL\StringFormatAndInterpolation.fs" />
+    <Compile Include="EmittedIL\StructGettersReadOnly.fs" />
     <Compile Include="EmittedIL\TailCalls.fs" />
     <Compile Include="EmittedIL\TupleElimination.fs" />
     <Compile Include="EmittedIL\TypeTestsInPatternMatching.fs" />
@@ -122,6 +123,7 @@
     <Compile Include="EmittedIL\Structure\Structure.fs" />
     <Compile Include="EmittedIL\TestFunctions\TestFunctions.fs" />
     <Compile Include="EmittedIL\Tuples\Tuples.fs" />
+
     <Compile Include="ErrorMessages\UnsupportedAttributes.fs" />
     <Compile Include="ErrorMessages\TypeEqualsMissingTests.fs" />
     <Compile Include="ErrorMessages\AccessOfTypeAbbreviationTests.fs" />

tests/FSharp.Test.Utilities/Compiler.fs

@@ -631,16 +631,27 @@ 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 cUnit =
+        cUnit
+        |> compile
+        |> shouldSucceed
+        |> withMetadataReader (fun reader -> reader.GetModuleDefinition().Mvid |> reader.GetGuid)
+
+    let compileAssembly cUnit =
+        cUnit
+        |> compile
+        |> shouldSucceed
+        |> getAssembly
 
     let private parseFSharp (fsSource: FSharpCompilationSource) : CompilationResult =
         let source = fsSource.Source.GetSourceText |> Option.defaultValue ""

tests/FSharp.Test.Utilities/FSharp.Test.Utilities.fsproj

@@ -26,6 +26,7 @@
     <Compile Include="Compiler.fs" />
     <Compile Include="Peverifier.fs" />
     <Compile Include="DirectoryAttribute.fs" />
+    <Compile Include="ReflectionHelper.fs" />
   </ItemGroup>
 
   <ItemGroup>

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}'"