Preserve enum type in custom attribute argument of type obj (#995)

docs/release-notes/.FSharp.Compiler.Service/11.0.100.md

@@ -17,6 +17,7 @@
 * Fix `=` adjacent to an interpolated string (e.g. `C(Name=$"value")`) being lexed as the invalid operator `=$` instead of an assignment followed by an interpolated string. ([Issue #16696](https://github.com/dotnet/fsharp/issues/16696))
 * Preserve type abbreviations (`string`, user-defined aliases) in the refined type of bindings introduced after a `| null` pattern in a `match` expression. ([Issue #19646](https://github.com/dotnet/fsharp/issues/19646), [PR #19745](https://github.com/dotnet/fsharp/pull/19745))
 * Fix attributes on return type of unparenthesized tuple methods being silently dropped from IL. ([Issue #462](https://github.com/dotnet/fsharp/issues/462), [PR #19714](https://github.com/dotnet/fsharp/pull/19714))
+* Fix enum values losing their type when used in a custom attribute argument of type `obj` (they were stored as the underlying integer instead of the enum). ([Issue #995](https://github.com/dotnet/fsharp/issues/995), [PR #19975](https://github.com/dotnet/fsharp/pull/19975))
 * Fix false-positive nullness warning (FS3261) when pattern matching narrows nullness inside seq/list/array comprehensions. ([Issue #19644](https://github.com/dotnet/fsharp/issues/19644), [PR #19743](https://github.com/dotnet/fsharp/pull/19743))
 * Fix internal error FS0073 "Undefined or unsolved type variable" in IlxGen when nested inline SRTP functions with multiple overloads leave unsolved typars in the non-witness codegen path. ([Issue #19709](https://github.com/dotnet/fsharp/issues/19709), [PR #19710](https://github.com/dotnet/fsharp/pull/19710))
 * Fix NRE when calling virtual Object methods on value types through inline SRTP functions. ([Issue #8098](https://github.com/dotnet/fsharp/issues/8098), [PR #19511](https://github.com/dotnet/fsharp/pull/19511))

src/Compiler/AbstractIL/il.fs

@@ -1192,6 +1192,7 @@ type ILAttribElem =
     | Type of ILType option
     | TypeRef of ILTypeRef option
     | Array of ILType * ILAttribElem list
+    | Enum of enumType: ILType * value: ILAttribElem
 
 type ILAttributeNamedArg = string * ILType * bool * ILAttribElem
 
@@ -4897,6 +4898,8 @@ let rec encodeCustomAttrElemTypeForObject x =
     | ILAttribElem.Single _ -> [| et_R4 |]
     | ILAttribElem.Double _ -> [| et_R8 |]
     | ILAttribElem.Array(elemTy, _) -> [| yield et_SZARRAY; yield! encodeCustomAttrElemType elemTy |]
+    // An enum boxed in 'object' is encoded as 0x55 followed by the enum type's qualified name.
+    | ILAttribElem.Enum(enumTy, _) -> encodeCustomAttrElemType enumTy
 
 let parseILVersion (vstr: string) =
     // matches "v1.2.3.4" or "1.2.3.4". Note, if numbers are missing, returns -1 (not 0).
@@ -4994,6 +4997,25 @@ let rec decodeCustomAttrElemType bytes sigptr x =
         mkILArr1DTy elemTy, sigptr
     | x when x = 0x50uy -> PrimaryAssemblyILGlobals.typ_Type, sigptr
     | x when x = 0x51uy -> PrimaryAssemblyILGlobals.typ_Object, sigptr // SERIALIZATION_TYPE_TAGGED_OBJECT (ECMA-335 II.23.3)
+    | x when x = 0x55uy ->
+        // SERIALIZATION_TYPE_ENUM (ECMA-335 II.23.3): the enum type's qualified name follows.
+        // Occurs e.g. when an enum is boxed into an 'object'-typed argument.
+        let qualifiedName, sigptr = sigptr_get_serstring bytes sigptr
+
+        let unqualifiedName, rest =
+            let pieces = qualifiedName.Split ','
+
+            if pieces.Length > 1 then
+                pieces[0], Some(String.concat "," pieces[1..])
+            else
+                pieces[0], None
+
+        let scoref =
+            match rest with
+            | Some aname -> ILScopeRef.Assembly(ILAssemblyRef.FromAssemblyName(AssemblyName aname))
+            | None -> PrimaryAssemblyILGlobals.primaryAssemblyScopeRef
+
+        ILType.Value(mkILNonGenericTySpec (mkILTyRef (scoref, unqualifiedName))), sigptr
     | _ -> failwithf "decodeCustomAttrElemType ilg: unrecognized custom element type: %A" x
 
 /// Given a custom attribute element, encode it to a binary representation according to the rules in Ecma 335 Partition II.
@@ -5024,6 +5046,8 @@ let rec encodeCustomAttrPrimValue c =
             for elem in elems do
                 yield! encodeCustomAttrPrimValue elem
         |]
+    // The enum type is captured separately (in the type tag); the value is the underlying integer.
+    | ILAttribElem.Enum(_, value) -> encodeCustomAttrPrimValue value
 
 and encodeCustomAttrValue ty c =
     match ty, c with
@@ -5370,7 +5394,13 @@ let decodeILAttribData (ca: ILAttribute) =
                     ILAttribElem.Null, sigptr
                 else
                     let ty, sigptr = decodeCustomAttrElemType bytes sigptr et
-                    parseVal ty sigptr
+                    let v, sigptr = parseVal ty sigptr
+                    // Preserve the enum type of a boxed enum so it re-encodes with its 0x55 enum tag
+                    // (rather than the bare underlying integer) when the attribute is round-tripped,
+                    // e.g. during static linking. See https://github.com/dotnet/fsharp/issues/995.
+                    match ty with
+                    | ILType.Value _ -> ILAttribElem.Enum(ty, v), sigptr
+                    | _ -> v, sigptr
             | ILType.Array(shape, elemTy) when shape = ILArrayShape.SingleDimensional ->
                 let n, sigptr = sigptr_get_i32 bytes sigptr
 
@@ -5386,7 +5416,11 @@ let decodeILAttribData (ca: ILAttribute) =
 
                     let elems, sigptr = parseElems [] n sigptr
                     ILAttribElem.Array(elemTy, elems), sigptr
-            | ILType.Value _ -> (* assume it is an enumeration *)
+            | ILType.Value _ ->
+                // Assume an enumeration. Note: the underlying integer width is not present in the
+                // blob, so this reads it as int32. Enums with a non-int32 underlying type (byte,
+                // int16, int64, ...) are therefore not read correctly here; resolving that would
+                // require materializing the enum type, which this blob parser does not do.
                 let n, sigptr = sigptr_get_i32 bytes sigptr
                 ILAttribElem.Int32 n, sigptr
             | _ -> failwith "decodeILAttribData: attribute data involves an enum or System.Type value"
@@ -5409,29 +5443,9 @@ let decodeILAttribData (ca: ILAttribute) =
                 let isPropByte, sigptr = sigptr_get_u8 bytes sigptr
                 let isProp = (int isPropByte = 0x54)
                 let et, sigptr = sigptr_get_u8 bytes sigptr
-                // We have a named value
-                let ty, sigptr =
-                    if ( (* 0x50 = (int et) || *) 0x55 = (int et)) then
-                        let qualified_tname, sigptr = sigptr_get_serstring bytes sigptr
-
-                        let unqualified_tname, rest =
-                            let pieces = qualified_tname.Split ','
-
-                            if pieces.Length > 1 then
-                                pieces[0], Some(String.concat "," pieces[1..])
-                            else
-                                pieces[0], None
-
-                        let scoref =
-                            match rest with
-                            | Some aname -> ILScopeRef.Assembly(ILAssemblyRef.FromAssemblyName(AssemblyName aname))
-                            | None -> PrimaryAssemblyILGlobals.primaryAssemblyScopeRef
-
-                        let tref = mkILTyRef (scoref, unqualified_tname)
-                        let tspec = mkILNonGenericTySpec tref
-                        ILType.Value tspec, sigptr
-                    else
-                        decodeCustomAttrElemType bytes sigptr et
+                // We have a named value. The type tag (including the 0x55 enum form) is decoded by
+                // decodeCustomAttrElemType.
+                let ty, sigptr = decodeCustomAttrElemType bytes sigptr et
 
                 let nm, sigptr = sigptr_get_serstring bytes sigptr
                 let v, sigptr = parseVal ty sigptr

src/Compiler/AbstractIL/il.fsi

@@ -849,6 +849,11 @@ type ILAttribElem =
     | Type of ILType option
     | TypeRef of ILTypeRef option
     | Array of ILType * ILAttribElem list
+    /// Represents an enum value together with its enum type. Used when an enum is stored in a
+    /// custom-attribute argument of type 'object', so the enum type is preserved in the encoded
+    /// blob (ECMA-335 II.23.3) instead of being collapsed to its underlying integer. The second
+    /// element is the underlying integer value (e.g. ILAttribElem.Int32).
+    | Enum of enumType: ILType * value: ILAttribElem
 
 /// Named args: values and flags indicating if they are fields or properties.
 type ILAttributeNamedArg = string * ILType * bool * ILAttribElem

src/Compiler/AbstractIL/ilmorph.fs

@@ -166,6 +166,7 @@ let rec celem_ty2ty f celem =
     | ILAttribElem.Type(Some ty) -> ILAttribElem.Type(Some(f ty))
     | ILAttribElem.TypeRef(Some tref) -> ILAttribElem.TypeRef(Some (f (mkILBoxedType (mkILNonGenericTySpec tref))).TypeRef)
     | ILAttribElem.Array(elemTy, elems) -> ILAttribElem.Array(f elemTy, List.map (celem_ty2ty f) elems)
+    | ILAttribElem.Enum(enumTy, value) -> ILAttribElem.Enum(f enumTy, celem_ty2ty f value)
     | _ -> celem
 
 let cnamedarg_ty2ty f ((nm, ty, isProp, elem): ILAttributeNamedArg) = (nm, f ty, isProp, celem_ty2ty f elem)

src/Compiler/Checking/AttributeChecking.fs

@@ -44,6 +44,8 @@ let rec private evalILAttribElem elem =
     | ILAttribElem.Double x -> box x
     | ILAttribElem.Null -> null
     | ILAttribElem.Array (_, a) -> box [| for i in a -> evalILAttribElem i |]
+    // An enum value: evaluate to its underlying integer value (the enum type itself is not materialized here).
+    | ILAttribElem.Enum (_, value) -> evalILAttribElem value
     // TODO: typeof<..> in attribute values
     | ILAttribElem.Type (Some _t) -> fail() 
     | ILAttribElem.Type None -> null

src/Compiler/Checking/NicePrint.fs

@@ -647,6 +647,7 @@ module PrintTypes =
         | ILAttribElem.TypeRef (Some ty) -> 
             LeftL.keywordTypedefof ^^ SepL.leftAngle ^^ PrintIL.layoutILTypeRef denv ty ^^ RightL.rightAngle
         | ILAttribElem.TypeRef None -> emptyL
+        | ILAttribElem.Enum (_, value) -> layoutILAttribElement denv value
 
     and layoutILAttrib denv (ty, args) = 
         let argsL = bracketL (sepListL RightL.comma (List.map (layoutILAttribElement denv) args))

src/Compiler/CodeGen/IlxGen.fs

@@ -10387,6 +10387,21 @@ and GenAttribArg amap (g: TcGlobals) eenv x (ilArgTy: ILType) =
     // Detect 'null' used for an array argument
     | Expr.Const(Const.Zero, _, _), ILType.Array _ -> ILAttribElem.Null
 
+    // An enum value stored into an 'object'-typed argument must keep its enum type in the
+    // custom-attribute blob (ECMA-335 II.23.3), otherwise it round-trips as the underlying
+    // integer (e.g. 'Prop = MyEnum.B' surfaces as boxed int32). See
+    // https://github.com/dotnet/fsharp/issues/995. The enum type is carried alongside the
+    // underlying integer value, which is computed by recursing with the underlying IL type.
+    | Expr.Const(c, m, ty), _ when ilArgTy.TypeSpec.Name = "System.Object" && isEnumTy g ty ->
+        let enumIlTy = GenType amap m eenv.tyenv ty
+        let underlyingTy = underlyingTypeOfEnumTy g ty
+        let underlyingIlTy = GenType amap m eenv.tyenv underlyingTy
+
+        let underlyingElem =
+            GenAttribArg amap g eenv (Expr.Const(c, m, underlyingTy)) underlyingIlTy
+
+        ILAttribElem.Enum(enumIlTy, underlyingElem)
+
     // Detect standard constants
     | Expr.Const(c, m, ty), _ ->
         let tynm = ilArgTy.TypeSpec.Name

tests/FSharp.Compiler.ComponentTests/Conformance/BasicGrammarElements/CustomAttributes/Basic/Basic.fs

@@ -34,6 +34,48 @@ module CustomAttributes_Basic =
         |> verifyCompileAndRun
         |> shouldSucceed
 
+    // Regression for https://github.com/dotnet/fsharp/issues/995
+    [<Theory; Directory(__SOURCE_DIRECTORY__, Includes=[|"EnumValueAsObjectArg01.fs"|])>]
+    let ``EnumValueAsObjectArg01_fs`` compilation =
+        compilation
+        |> verifyCompileAndRun
+        |> shouldSucceed
+
+    // Cross-language: the same scenario as EnumValueAsObjectArg01.fs, but with the enum and the
+    // attribute defined in C#. See https://github.com/dotnet/fsharp/issues/995.
+    [<Fact>]
+    let ``Enum defined in C# used in an F# attribute arg of type obj keeps its type`` () =
+        let csLib =
+            CSharp """
+namespace CSharpLib
+{
+    public enum MyEnum { A = 1, B = 2 }
+
+    [System.AttributeUsage(System.AttributeTargets.All)]
+    public class MyAttribute : System.Attribute
+    {
+        public object Prop { get; set; }
+    }
+}
+"""
+            |> withName "CSharpLib"
+
+        FSharp """
+module Test
+open System
+open CSharpLib
+
+[<My(Prop = MyEnum.B)>]
+type MyClass = class end
+
+let prop = (typeof<MyClass>.GetCustomAttributes(false)[0] :?> MyAttribute).Prop
+if prop.GetType() <> typeof<MyEnum> then failwith "enum type was lost"
+if Convert.ToString(prop, Globalization.CultureInfo.InvariantCulture) <> "B" then failwith "expected \"B\""
+"""
+        |> withReferences [csLib]
+        |> compileExeAndRun
+        |> shouldSucceed
+
     // SOURCE=E_AttributeApplication01.fs					# E_AttributeApplication01.fs
     [<Theory; Directory(__SOURCE_DIRECTORY__, Includes=[|"E_AttributeApplication01.fs"|])>]
     let ``E_AttributeApplication01_fs`` compilation =

tests/FSharp.Compiler.ComponentTests/Conformance/BasicGrammarElements/CustomAttributes/Basic/EnumValueAsObjectArg01.fs

@@ -0,0 +1,38 @@
+// #Conformance #DeclarationElements #Attributes
+// Regression test for https://github.com/dotnet/fsharp/issues/995
+// An enum assigned to an attribute argument of type 'obj' must keep its enum type in the
+// emitted metadata, instead of being stored as the underlying int32.
+
+open System
+
+type MyAttribute() =
+    inherit Attribute()
+    let mutable prop : obj = null
+    member _.Prop
+        with get () : obj = prop
+        and  set (value: obj) = prop <- value
+
+type MyEnum =
+    | A = 1
+    | B = 2
+
+// An enum with a non-int32 underlying type, to exercise the encoded value width.
+type LongEnum =
+    | P = 1L
+    | Q = 2L
+
+[<My(Prop = MyEnum.B)>]
+type MyClass = class end
+
+[<My(Prop = LongEnum.Q)>]
+type MyClassLong = class end
+
+let propOf<'T> () = (typeof<'T>.GetCustomAttributes(false)[0] :?> MyAttribute).Prop
+
+let intProp = propOf<MyClass> ()
+if intProp.GetType() <> typeof<MyEnum> then failwith "MyEnum type was lost"
+if Convert.ToString(intProp, Globalization.CultureInfo.InvariantCulture) <> "B" then failwith "expected \"B\""
+
+let longProp = propOf<MyClassLong> ()
+if longProp.GetType() <> typeof<LongEnum> then failwith "LongEnum type was lost"
+if Convert.ToString(longProp, Globalization.CultureInfo.InvariantCulture) <> "Q" then failwith "expected \"Q\""

tests/FSharp.Compiler.Service.Tests/FSharp.Compiler.Service.SurfaceArea.netstandard20.bsl

@@ -144,6 +144,10 @@ FSharp.Compiler.AbstractIL.IL+ILAttribElem+Char: Char Item
 FSharp.Compiler.AbstractIL.IL+ILAttribElem+Char: Char get_Item()
 FSharp.Compiler.AbstractIL.IL+ILAttribElem+Double: Double Item
 FSharp.Compiler.AbstractIL.IL+ILAttribElem+Double: Double get_Item()
+FSharp.Compiler.AbstractIL.IL+ILAttribElem+Enum: ILAttribElem get_value()
+FSharp.Compiler.AbstractIL.IL+ILAttribElem+Enum: ILAttribElem value
+FSharp.Compiler.AbstractIL.IL+ILAttribElem+Enum: ILType enumType
+FSharp.Compiler.AbstractIL.IL+ILAttribElem+Enum: ILType get_enumType()
 FSharp.Compiler.AbstractIL.IL+ILAttribElem+Int16: Int16 Item
 FSharp.Compiler.AbstractIL.IL+ILAttribElem+Int16: Int16 get_Item()
 FSharp.Compiler.AbstractIL.IL+ILAttribElem+Int32: Int32 Item
@@ -161,6 +165,7 @@ FSharp.Compiler.AbstractIL.IL+ILAttribElem+Tags: Int32 Bool
 FSharp.Compiler.AbstractIL.IL+ILAttribElem+Tags: Int32 Byte
 FSharp.Compiler.AbstractIL.IL+ILAttribElem+Tags: Int32 Char
 FSharp.Compiler.AbstractIL.IL+ILAttribElem+Tags: Int32 Double
+FSharp.Compiler.AbstractIL.IL+ILAttribElem+Tags: Int32 Enum
 FSharp.Compiler.AbstractIL.IL+ILAttribElem+Tags: Int32 Int16
 FSharp.Compiler.AbstractIL.IL+ILAttribElem+Tags: Int32 Int32
 FSharp.Compiler.AbstractIL.IL+ILAttribElem+Tags: Int32 Int64
@@ -192,6 +197,7 @@ FSharp.Compiler.AbstractIL.IL+ILAttribElem: Boolean IsBool
 FSharp.Compiler.AbstractIL.IL+ILAttribElem: Boolean IsByte
 FSharp.Compiler.AbstractIL.IL+ILAttribElem: Boolean IsChar
 FSharp.Compiler.AbstractIL.IL+ILAttribElem: Boolean IsDouble
+FSharp.Compiler.AbstractIL.IL+ILAttribElem: Boolean IsEnum
 FSharp.Compiler.AbstractIL.IL+ILAttribElem: Boolean IsInt16
 FSharp.Compiler.AbstractIL.IL+ILAttribElem: Boolean IsInt32
 FSharp.Compiler.AbstractIL.IL+ILAttribElem: Boolean IsInt64
@@ -209,6 +215,7 @@ FSharp.Compiler.AbstractIL.IL+ILAttribElem: Boolean get_IsBool()
 FSharp.Compiler.AbstractIL.IL+ILAttribElem: Boolean get_IsByte()
 FSharp.Compiler.AbstractIL.IL+ILAttribElem: Boolean get_IsChar()
 FSharp.Compiler.AbstractIL.IL+ILAttribElem: Boolean get_IsDouble()
+FSharp.Compiler.AbstractIL.IL+ILAttribElem: Boolean get_IsEnum()
 FSharp.Compiler.AbstractIL.IL+ILAttribElem: Boolean get_IsInt16()
 FSharp.Compiler.AbstractIL.IL+ILAttribElem: Boolean get_IsInt32()
 FSharp.Compiler.AbstractIL.IL+ILAttribElem: Boolean get_IsInt64()
@@ -226,6 +233,7 @@ FSharp.Compiler.AbstractIL.IL+ILAttribElem: FSharp.Compiler.AbstractIL.IL+ILAttr
 FSharp.Compiler.AbstractIL.IL+ILAttribElem: FSharp.Compiler.AbstractIL.IL+ILAttribElem+Byte
 FSharp.Compiler.AbstractIL.IL+ILAttribElem: FSharp.Compiler.AbstractIL.IL+ILAttribElem+Char
 FSharp.Compiler.AbstractIL.IL+ILAttribElem: FSharp.Compiler.AbstractIL.IL+ILAttribElem+Double
+FSharp.Compiler.AbstractIL.IL+ILAttribElem: FSharp.Compiler.AbstractIL.IL+ILAttribElem+Enum
 FSharp.Compiler.AbstractIL.IL+ILAttribElem: FSharp.Compiler.AbstractIL.IL+ILAttribElem+Int16
 FSharp.Compiler.AbstractIL.IL+ILAttribElem: FSharp.Compiler.AbstractIL.IL+ILAttribElem+Int32
 FSharp.Compiler.AbstractIL.IL+ILAttribElem: FSharp.Compiler.AbstractIL.IL+ILAttribElem+Int64
@@ -243,6 +251,7 @@ FSharp.Compiler.AbstractIL.IL+ILAttribElem: ILAttribElem NewBool(Boolean)
 FSharp.Compiler.AbstractIL.IL+ILAttribElem: ILAttribElem NewByte(Byte)
 FSharp.Compiler.AbstractIL.IL+ILAttribElem: ILAttribElem NewChar(Char)
 FSharp.Compiler.AbstractIL.IL+ILAttribElem: ILAttribElem NewDouble(Double)
+FSharp.Compiler.AbstractIL.IL+ILAttribElem: ILAttribElem NewEnum(ILType, ILAttribElem)
 FSharp.Compiler.AbstractIL.IL+ILAttribElem: ILAttribElem NewInt16(Int16)
 FSharp.Compiler.AbstractIL.IL+ILAttribElem: ILAttribElem NewInt32(Int32)
 FSharp.Compiler.AbstractIL.IL+ILAttribElem: ILAttribElem NewInt64(Int64)

tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Quotations/FSharpQuotations.fs

@@ -18,6 +18,10 @@ type E = Microsoft.FSharp.Quotations.Expr;;
 type StaticIndexedPropertyTest() =
     static member IdxProp with get (n : int) = n + 1
 
+type QuotationEnum =
+    | A = 1
+    | B = 2
+
 module Check =
     let argumentException f =
         let mutable ex = false
@@ -103,6 +107,16 @@ type FSharpQuotationsTests() =
         | NewTuple [ Value(:? int as i, _) ; Value(:? string as s, _) ] when i = 1 && s = "" -> ()
         | _ -> Assert.Fail()
 
+    [<Fact>]
+    member x.``Quotation of an enum value preserves the enum type`` () =
+        // Related to https://github.com/dotnet/fsharp/issues/995: an enum literal is quoted as a
+        // Value node carrying the enum type, not the bare underlying integer.
+        match <@ QuotationEnum.B @> with
+        | Value(v, t) ->
+            Assert.Equal(typeof<QuotationEnum>, t)
+            Assert.Equal(box QuotationEnum.B, v)
+        | _ -> Assert.Fail()
+
     [<Fact>]
     member x.``NewTuple literal should not be recognized by NewStructTuple active pattern`` () =
         match <@ (1, "") @> with