Skip to content

Commit c91ff9c

Browse files
CopilotT-GroCopilot
authored
Handle Const.Zero for primitive value types in custom attribute codegen (#19484)
* Initial plan * Fix internal error when using custom attribute with optional value type argument (fixes #8353) Handle Const.Zero for all primitive value types in GenAttribArg, which is used when [<Optional>] is specified without [<DefaultParameterValue>] on attribute constructor parameters. Previously only System.Object, System.String, and System.Type were handled, causing an internal error for bool, int, and other value types. Co-authored-by: T-Gro <46543583+T-Gro@users.noreply.github.com> Agent-Logs-Url: https://github.com/dotnet/fsharp/sessions/6f81842d-a307-4b7e-9ef2-ffa78948f9c6 * Add comprehensive test coverage for all primitive value types in optional attribute args Co-authored-by: T-Gro <46543583+T-Gro@users.noreply.github.com> Agent-Logs-Url: https://github.com/dotnet/fsharp/sessions/6f81842d-a307-4b7e-9ef2-ffa78948f9c6 * Merge main, add release notes for #8353 fix Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: T-Gro <46543583+T-Gro@users.noreply.github.com> Co-authored-by: Tomas Grosup <Tomas.Grosup@gmail.com> Co-authored-by: Tomas Grosup <tomasgrosup@microsoft.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 8e82779 commit c91ff9c

4 files changed

Lines changed: 176 additions & 1 deletion

File tree

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@
4545
* Fix signature generation: `namespace global` header dropped from generated signature. ([Issue #19593](https://github.com/dotnet/fsharp/issues/19593), [PR #19609](https://github.com/dotnet/fsharp/pull/19609))
4646
* Fix signature generation: SRTP constraints use postfix syntax that fails conformance, now uses explicit type param declarations. ([Issue #19594](https://github.com/dotnet/fsharp/issues/19594), [PR #19609](https://github.com/dotnet/fsharp/pull/19609))
4747
* Fix signature generation: type params with special characters missing backtick escaping. ([Issue #19595](https://github.com/dotnet/fsharp/issues/19595), [PR #19609](https://github.com/dotnet/fsharp/pull/19609))
48+
* Fix internal error when using custom attribute with `[<Optional>]` value type parameter and no `[<DefaultParameterValue>]`. ([Issue #8353](https://github.com/dotnet/fsharp/issues/8353), [PR #19484](https://github.com/dotnet/fsharp/pull/19484))
4849

4950
### Added
5051

‎src/Compiler/CodeGen/IlxGen.fs‎

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10307,7 +10307,7 @@ and GenAttribArg amap g eenv x (ilArgTy: ILType) =
1030710307
| Expr.Const(Const.Zero, _, _), ILType.Array _ -> ILAttribElem.Null
1030810308

1030910309
// Detect standard constants
10310-
| Expr.Const(c, m, _), _ ->
10310+
| Expr.Const(c, m, ty), _ ->
1031110311
let tynm = ilArgTy.TypeSpec.Name
1031210312
let isobj = (tynm = "System.Object")
1031310313

@@ -10334,6 +10334,22 @@ and GenAttribArg amap g eenv x (ilArgTy: ILType) =
1033410334
| Const.Zero when isobj -> ILAttribElem.Null
1033510335
| Const.Zero when tynm = "System.String" -> ILAttribElem.String None
1033610336
| Const.Zero when tynm = "System.Type" -> ILAttribElem.Type None
10337+
| Const.Zero when tynm = "System.Boolean" -> ILAttribElem.Bool false
10338+
| Const.Zero when tynm = "System.SByte" -> ILAttribElem.SByte 0y
10339+
| Const.Zero when tynm = "System.Int16" -> ILAttribElem.Int16 0s
10340+
| Const.Zero when tynm = "System.Int32" -> ILAttribElem.Int32 0
10341+
| Const.Zero when tynm = "System.Int64" -> ILAttribElem.Int64 0L
10342+
| Const.Zero when tynm = "System.Byte" -> ILAttribElem.Byte 0uy
10343+
| Const.Zero when tynm = "System.UInt16" -> ILAttribElem.UInt16 0us
10344+
| Const.Zero when tynm = "System.UInt32" -> ILAttribElem.UInt32 0u
10345+
| Const.Zero when tynm = "System.UInt64" -> ILAttribElem.UInt64 0UL
10346+
| Const.Zero when tynm = "System.Single" -> ILAttribElem.Single 0.0f
10347+
| Const.Zero when tynm = "System.Double" -> ILAttribElem.Double 0.0
10348+
| Const.Zero when tynm = "System.Char" -> ILAttribElem.Char '\000'
10349+
| Const.Zero when isEnumTy g ty ->
10350+
let underlyingTy = underlyingTypeOfEnumTy g ty
10351+
let underlyingIlTy = GenType amap m eenv.tyenv underlyingTy
10352+
GenAttribArg amap g eenv (Expr.Const(Const.Zero, m, underlyingTy)) underlyingIlTy
1033710353
| Const.String i when isobj || tynm = "System.String" -> ILAttribElem.String(Some i)
1033810354
| _ -> error (InternalError("The type '" + tynm + "' may not be used as a custom attribute value", m))
1033910355

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

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -303,6 +303,14 @@ module CustomAttributes_Basic =
303303
|> verifyCompileAndRun
304304
|> shouldSucceed
305305

306+
// SOURCE=OptionalAttributeArgs.fs # OptionalAttributeArgs.fs
307+
// Regression test for https://github.com/dotnet/fsharp/issues/8353
308+
[<Theory; Directory(__SOURCE_DIRECTORY__, Includes=[|"OptionalAttributeArgs.fs"|])>]
309+
let ``OptionalAttributeArgs_fs`` compilation =
310+
compilation
311+
|> verifyCompileAndRun
312+
|> shouldSucceed
313+
306314
// SOURCE=W_ReturnType03b.fs SCFLAGS="--test:ErrorRanges" # W_ReturnType03b.fs
307315
[<Theory; Directory(__SOURCE_DIRECTORY__, Includes=[|"W_ReturnType03b.fs"|])>]
308316
let ``W_ReturnType03b_fs`` compilation =

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

Lines changed: 150 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,150 @@
1+
// #Regression #Conformance #DeclarationElements #Attributes
2+
// Regression test for https://github.com/dotnet/fsharp/issues/8353
3+
// Verify that custom attributes with [<Optional>] parameters (no DefaultParameterValue) compile for all value types
4+
// and that the emitted default values are correct at runtime.
5+
6+
open System
7+
open System.Runtime.InteropServices
8+
9+
type BoolAttribute(name: string, flag: bool) =
10+
inherit Attribute()
11+
member _.Flag = flag
12+
new([<Optional>] flag: bool) = BoolAttribute("", flag)
13+
14+
type IntAttribute(name: string, value: int) =
15+
inherit Attribute()
16+
member _.Value = value
17+
new([<Optional>] value: int) = IntAttribute("", value)
18+
19+
type ByteAttribute(name: string, value: byte) =
20+
inherit Attribute()
21+
member _.Value = value
22+
new([<Optional>] value: byte) = ByteAttribute("", value)
23+
24+
type SByteAttribute(name: string, value: sbyte) =
25+
inherit Attribute()
26+
member _.Value = value
27+
new([<Optional>] value: sbyte) = SByteAttribute("", value)
28+
29+
type Int16Attribute(name: string, value: int16) =
30+
inherit Attribute()
31+
member _.Value = value
32+
new([<Optional>] value: int16) = Int16Attribute("", value)
33+
34+
type Int64Attribute(name: string, value: int64) =
35+
inherit Attribute()
36+
member _.Value = value
37+
new([<Optional>] value: int64) = Int64Attribute("", value)
38+
39+
type UInt16Attribute(name: string, value: uint16) =
40+
inherit Attribute()
41+
member _.Value = value
42+
new([<Optional>] value: uint16) = UInt16Attribute("", value)
43+
44+
type UInt32Attribute(name: string, value: uint32) =
45+
inherit Attribute()
46+
member _.Value = value
47+
new([<Optional>] value: uint32) = UInt32Attribute("", value)
48+
49+
type UInt64Attribute(name: string, value: uint64) =
50+
inherit Attribute()
51+
member _.Value = value
52+
new([<Optional>] value: uint64) = UInt64Attribute("", value)
53+
54+
type FloatAttribute(name: string, value: float) =
55+
inherit Attribute()
56+
member _.Value = value
57+
new([<Optional>] value: float) = FloatAttribute("", value)
58+
59+
type SingleAttribute(name: string, value: float32) =
60+
inherit Attribute()
61+
member _.Value = value
62+
new([<Optional>] value: float32) = SingleAttribute("", value)
63+
64+
type CharAttribute(name: string, value: char) =
65+
inherit Attribute()
66+
member _.Value = value
67+
new([<Optional>] value: char) = CharAttribute("", value)
68+
69+
// Enum types with different underlying types
70+
type MyIntEnum = A = 0 | B = 1
71+
72+
type MyByteEnum = A = 0uy | B = 1uy
73+
74+
type EnumIntAttribute(name: string, value: MyIntEnum) =
75+
inherit Attribute()
76+
member _.Value = value
77+
new([<Optional>] value: MyIntEnum) = EnumIntAttribute("", value)
78+
79+
type EnumByteAttribute(name: string, value: MyByteEnum) =
80+
inherit Attribute()
81+
member _.Value = value
82+
new([<Optional>] value: MyByteEnum) = EnumByteAttribute("", value)
83+
84+
[<Bool>]
85+
type T1() = class end
86+
87+
[<Int>]
88+
type T2() = class end
89+
90+
[<Byte>]
91+
type T3() = class end
92+
93+
[<Float>]
94+
type T4() = class end
95+
96+
[<Single>]
97+
type T5() = class end
98+
99+
[<Char>]
100+
type T6() = class end
101+
102+
[<SByte>]
103+
type T7() = class end
104+
105+
[<Int16>]
106+
type T8() = class end
107+
108+
[<Int64>]
109+
type T9() = class end
110+
111+
[<UInt16>]
112+
type T10() = class end
113+
114+
[<UInt32>]
115+
type T11() = class end
116+
117+
[<UInt64>]
118+
type T12() = class end
119+
120+
[<EnumInt>]
121+
type T13() = class end
122+
123+
[<EnumByte>]
124+
type T14() = class end
125+
126+
// Verify default values at runtime via reflection
127+
let inline getAttr<'a when 'a :> Attribute> (t: Type) = t.GetCustomAttributes(typeof<'a>, false).[0] :?> 'a
128+
129+
let check (name: string) (actual: 'a) (expected: 'a) =
130+
if actual <> expected then
131+
failwithf "%s: expected %A but got %A" name expected actual
132+
133+
[<EntryPoint>]
134+
let main _ =
135+
check "bool" (getAttr<BoolAttribute>(typeof<T1>)).Flag false
136+
check "int" (getAttr<IntAttribute>(typeof<T2>)).Value 0
137+
check "byte" (getAttr<ByteAttribute>(typeof<T3>)).Value 0uy
138+
check "float" (getAttr<FloatAttribute>(typeof<T4>)).Value 0.0
139+
check "single" (getAttr<SingleAttribute>(typeof<T5>)).Value 0.0f
140+
check "char" (getAttr<CharAttribute>(typeof<T6>)).Value '\000'
141+
check "sbyte" (getAttr<SByteAttribute>(typeof<T7>)).Value 0y
142+
check "int16" (getAttr<Int16Attribute>(typeof<T8>)).Value 0s
143+
check "int64" (getAttr<Int64Attribute>(typeof<T9>)).Value 0L
144+
check "uint16" (getAttr<UInt16Attribute>(typeof<T10>)).Value 0us
145+
check "uint32" (getAttr<UInt32Attribute>(typeof<T11>)).Value 0u
146+
check "uint64" (getAttr<UInt64Attribute>(typeof<T12>)).Value 0UL
147+
check "enum_int" (getAttr<EnumIntAttribute>(typeof<T13>)).Value MyIntEnum.A
148+
check "enum_byte" (getAttr<EnumByteAttribute>(typeof<T14>)).Value MyByteEnum.A
149+
0
150+

0 commit comments

Comments
 (0)