Skip to content

Commit 872352f

Browse files
edgarfgpT-GroCopilot
authored
Fix AttributeUsage.AllowMultiple not inherited for C#-defined attributes (#19315)
* Fix AttributeUsage.AllowMultiple not inherited for C#-defined attributes (#17107) The F# compiler was not walking the inheritance chain for IL-imported (C#) attribute types when checking AllowMultiple. The supersOfTyconRef function only used tcaug_super, which is not populated for IL types. This fix simplifies TryFindAttributeUsageAttribute to only check the current type, and adds a recursive allowsMultiple function in PostInferenceChecks that walks the inheritance chain using GetSuperTypeOfType, which correctly handles both F# and IL-imported types. Based on work by edgarfgp in PR #19315. Fixes #17107 Co-authored-by: edgarfgp <31915729+edgarfgp@users.noreply.github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Use O(1) WellKnownILAttributes fast-path for AttributeUsage lookup Switch TryFindAttributeUsageAttribute to use tryBindTyconRefAttributeCore with ValueSome WellKnownILAttributes.AttributeUsageAttribute, enabling O(1) early exit on IL types that lack [AttributeUsage]. This avoids a full linear attribute scan at each level of the inheritance chain walk. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Tomas Grosup <Tomas.Grosup@gmail.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 763cf17 commit 872352f

7 files changed

Lines changed: 148 additions & 37 deletions

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
@@ -25,6 +25,7 @@
2525
* Fix nativeptr in interfaces leads to TypeLoadException. (Issue [#14508](https://github.com/dotnet/fsharp/issues/14508), [PR #19338](https://github.com/dotnet/fsharp/pull/19338))
2626
* Fix box instruction for literal upcasts. (Issue [#18319](https://github.com/dotnet/fsharp/issues/18319), [PR #19338](https://github.com/dotnet/fsharp/pull/19338))
2727
* Fix Decimal Literal causes InvalidProgramException in Debug builds. (Issue [#18956](https://github.com/dotnet/fsharp/issues/18956), [PR #19338](https://github.com/dotnet/fsharp/pull/19338))
28+
* Fix `AttributeUsage.AllowMultiple` not being inherited for attributes subclassed in C#. ([Issue #17107](https://github.com/dotnet/fsharp/issues/17107), [PR #19315](https://github.com/dotnet/fsharp/pull/19315))
2829

2930
### Added
3031

‎src/Compiler/Checking/PostInferenceChecks.fs‎

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2036,8 +2036,19 @@ and CheckAttribs cenv env (attribs: Attribs) =
20362036
|> Seq.filter (fun (_, count) -> count > 1)
20372037
|> Seq.map fst
20382038
|> Seq.toList
2039-
// Filter for allowMultiple = false
2040-
|> List.filter (fun (tcref, _, m) -> TryFindAttributeUsageAttribute cenv.g m tcref <> Some true)
2039+
// Filter for allowMultiple = false, walking the inheritance chain to find AttributeUsage
2040+
|> List.filter (fun (tcref, _, m) ->
2041+
let rec allowsMultiple (tcref: TyconRef) =
2042+
match TryFindAttributeUsageAttribute cenv.g m tcref with
2043+
| Some res -> res
2044+
| None ->
2045+
generalizedTyconRef cenv.g tcref
2046+
|> GetSuperTypeOfType cenv.g cenv.amap m
2047+
|> Option.bind (tryTcrefOfAppTy cenv.g >> ValueOption.toOption)
2048+
|> Option.map allowsMultiple
2049+
|> Option.defaultValue false
2050+
2051+
not (allowsMultiple tcref))
20412052

20422053
if cenv.reportErrors then
20432054
for tcref, _, m in duplicates do

‎src/Compiler/TypedTree/TypedTreeOps.Attributes.fs‎

Lines changed: 22 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -812,30 +812,29 @@ module internal AttributeHelpers =
812812
| [ Some(:? bool as v: obj) ], _ -> Some v
813813
| _ -> None)
814814

815-
/// Try to find the resolved attributeusage for an type by walking its inheritance tree and picking the correct attribute usage value
815+
/// Try to find the AllowMultiple value of the AttributeUsage attribute on a type definition.
816816
let TryFindAttributeUsageAttribute g m tcref =
817-
[| yield tcref; yield! supersOfTyconRef tcref |]
818-
|> Array.tryPick (fun tcref ->
819-
TryBindTyconRefAttribute
820-
g
821-
m
822-
g.attrib_AttributeUsageAttribute
823-
tcref
824-
(fun (_, named) ->
825-
named
826-
|> List.tryPick (function
827-
| "AllowMultiple", _, _, ILAttribElem.Bool res -> Some res
828-
| _ -> None))
829-
(fun (Attrib(_, _, _, named, _, _, _)) ->
830-
named
831-
|> List.tryPick (function
832-
| AttribNamedArg("AllowMultiple", _, _, AttribBoolArg res) -> Some res
833-
| _ -> None))
834-
(fun (_, named) ->
835-
named
836-
|> List.tryPick (function
837-
| "AllowMultiple", Some(:? bool as res: obj) -> Some res
838-
| _ -> None)))
817+
tryBindTyconRefAttributeCore
818+
g
819+
m
820+
(ValueSome WellKnownILAttributes.AttributeUsageAttribute)
821+
g.attrib_AttributeUsageAttribute
822+
tcref
823+
(fun (_, named) ->
824+
named
825+
|> List.tryPick (function
826+
| "AllowMultiple", _, _, ILAttribElem.Bool res -> Some res
827+
| _ -> None))
828+
(fun (Attrib(_, _, _, named, _, _, _)) ->
829+
named
830+
|> List.tryPick (function
831+
| AttribNamedArg("AllowMultiple", _, _, AttribBoolArg res) -> Some res
832+
| _ -> None))
833+
(fun (_, named) ->
834+
named
835+
|> List.tryPick (function
836+
| "AllowMultiple", Some(:? bool as res: obj) -> Some res
837+
| _ -> None))
839838

840839
/// Try to find a specific attribute on a type definition, where the attribute accepts a string argument.
841840
///

‎src/Compiler/TypedTree/TypedTreeOps.Attributes.fsi‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -194,7 +194,7 @@ module internal AttributeHelpers =
194194
/// Check if a TyconRef has AllowNullLiteralAttribute, returning Some true/Some false/None.
195195
val TyconRefAllowsNull: g: TcGlobals -> tcref: TyconRef -> bool option
196196

197-
/// Try to find the AttributeUsage attribute, looking for the value of the AllowMultiple named parameter
197+
/// Try to find the AllowMultiple value of the AttributeUsage attribute on a type definition.
198198
val TryFindAttributeUsageAttribute: TcGlobals -> range -> TyconRef -> bool option
199199

200200
val (|AttribBitwiseOrExpr|_|): TcGlobals -> Expr -> (Expr * Expr) voption

‎src/Compiler/TypedTree/TypedTreeOps.FreeVars.fs‎

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1542,11 +1542,3 @@ module internal MemberRepresentation =
15421542
match tycon.TypeContents.tcaug_super with
15431543
| None -> g.obj_ty_noNulls
15441544
| Some ty -> ty
1545-
1546-
/// walk a TyconRef's inheritance tree, yielding any parent types as an array
1547-
let supersOfTyconRef (tcref: TyconRef) =
1548-
tcref
1549-
|> Array.unfold (fun tcref ->
1550-
match tcref.TypeContents.tcaug_super with
1551-
| Some(TType_app(sup, _, _)) -> Some(sup, sup)
1552-
| _ -> None)

‎src/Compiler/TypedTree/TypedTreeOps.FreeVars.fsi‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -357,9 +357,6 @@ module internal MemberRepresentation =
357357

358358
val superOfTycon: TcGlobals -> Tycon -> TType
359359

360-
/// walk a TyconRef's inheritance tree, yielding any parent types as an array
361-
val supersOfTyconRef: TyconRef -> TyconRef array
362-
363360
val GetTraitConstraintInfosOfTypars: TcGlobals -> Typars -> TraitConstraintInfo list
364361

365362
val GetTraitWitnessInfosOfTypars: TcGlobals -> numParentTypars: int -> typars: Typars -> TraitWitnessInfos

‎tests/FSharp.Compiler.ComponentTests/Language/AttributeCheckingTests.fs‎

Lines changed: 111 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,3 +77,114 @@ type C() =
7777
|> withReferences [csharpBaseClass]
7878
|> compile
7979
|> shouldSucceed
80+
81+
[<FSharp.Test.FactForNETCOREAPP>]
82+
let ``C# attribute subclass inherits AllowMultiple true from base`` () =
83+
let csharpLib =
84+
CSharp """
85+
using System;
86+
87+
[AttributeUsage(AttributeTargets.All, AllowMultiple = true)]
88+
public class BaseAttribute : Attribute { }
89+
public class ChildAttribute : BaseAttribute { }
90+
""" |> withName "csAttrLib"
91+
92+
FSharp """
93+
module Test
94+
95+
[<Child; Child>]
96+
type C() = class end
97+
"""
98+
|> withReferences [csharpLib]
99+
|> compile
100+
|> shouldSucceed
101+
102+
[<FSharp.Test.FactForNETCOREAPP>]
103+
let ``C# attribute subclass inherits AllowMultiple false from base`` () =
104+
let csharpLib =
105+
CSharp """
106+
using System;
107+
108+
[AttributeUsage(AttributeTargets.All, AllowMultiple = false)]
109+
public class BaseAttribute : Attribute { }
110+
public class ChildAttribute : BaseAttribute { }
111+
""" |> withName "csAttrLib"
112+
113+
FSharp """
114+
module Test
115+
116+
[<Child; Child>]
117+
type C() = class end
118+
"""
119+
|> withReferences [csharpLib]
120+
|> compile
121+
|> shouldFail
122+
|> withSingleDiagnostic (Error 429, Line 4, Col 10, Line 4, Col 15, "The attribute type 'ChildAttribute' has 'AllowMultiple=false'. Multiple instances of this attribute cannot be attached to a single language element.")
123+
124+
[<FSharp.Test.FactForNETCOREAPP>]
125+
let ``C# attribute multi-level inheritance inherits AllowMultiple true`` () =
126+
let csharpLib =
127+
CSharp """
128+
using System;
129+
130+
[AttributeUsage(AttributeTargets.All, AllowMultiple = true)]
131+
public class BaseAttribute : Attribute { }
132+
public class MiddleAttribute : BaseAttribute { }
133+
public class LeafAttribute : MiddleAttribute { }
134+
""" |> withName "csAttrLib"
135+
136+
FSharp """
137+
module Test
138+
139+
[<Leaf; Leaf>]
140+
type C() = class end
141+
"""
142+
|> withReferences [csharpLib]
143+
|> compile
144+
|> shouldSucceed
145+
146+
[<FSharp.Test.FactForNETCOREAPP>]
147+
let ``C# attribute subclass with own AttributeUsage overrides base AllowMultiple`` () =
148+
let csharpLib =
149+
CSharp """
150+
using System;
151+
152+
[AttributeUsage(AttributeTargets.All, AllowMultiple = true)]
153+
public class BaseAttribute : Attribute { }
154+
155+
[AttributeUsage(AttributeTargets.All, AllowMultiple = false)]
156+
public class ChildAttribute : BaseAttribute { }
157+
""" |> withName "csAttrLib"
158+
159+
FSharp """
160+
module Test
161+
162+
[<Child; Child>]
163+
type C() = class end
164+
"""
165+
|> withReferences [csharpLib]
166+
|> compile
167+
|> shouldFail
168+
|> withSingleDiagnostic (Error 429, Line 4, Col 10, Line 4, Col 15, "The attribute type 'ChildAttribute' has 'AllowMultiple=false'. Multiple instances of this attribute cannot be attached to a single language element.")
169+
170+
[<FSharp.Test.FactForNETCOREAPP>]
171+
let ``F# attribute subclass of C# base inherits AllowMultiple true`` () =
172+
let csharpLib =
173+
CSharp """
174+
using System;
175+
176+
[AttributeUsage(AttributeTargets.All, AllowMultiple = true)]
177+
public class BaseAttribute : Attribute { }
178+
""" |> withName "csAttrLib"
179+
180+
FSharp """
181+
module Test
182+
183+
type ChildAttribute() = inherit BaseAttribute()
184+
185+
[<Child; Child>]
186+
type C() = class end
187+
"""
188+
|> withReferences [csharpLib]
189+
|> compile
190+
|> shouldSucceed

0 commit comments

Comments
 (0)