Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Attribute targets on records #17207

Merged
merged 24 commits into from
Jul 8, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
4720fac
Initial test
edgarfgp May 21, 2024
e14eb37
Update record check for targets
edgarfgp May 21, 2024
f0cd5d5
More tests
edgarfgp May 21, 2024
f42a88c
Merge branch 'main' into attribute-targets-on-records
edgarfgp May 22, 2024
ffb497b
Check Record fields
edgarfgp May 22, 2024
669ee19
release notes
edgarfgp May 22, 2024
670972a
Merge branch 'main' into attribute-targets-on-records
edgarfgp May 22, 2024
d43956f
Extend DefaultValueAttribute to use AttributeTargets.Property
edgarfgp May 22, 2024
cd582b6
revert DefaultValueAttribute changes
edgarfgp May 23, 2024
74e1342
TcFieldDecl
edgarfgp May 23, 2024
071611a
update tests
edgarfgp May 23, 2024
476f236
Merge branch 'main' into attribute-targets-on-records
edgarfgp Jun 10, 2024
3ea80b3
update tests
edgarfgp Jun 10, 2024
6e555a5
update test
edgarfgp Jun 11, 2024
5a0b1c1
Update StructAttribute to also AttributeTargets.Class
edgarfgp Jun 11, 2024
389439c
Merge branch 'main' into attribute-targets-on-records
edgarfgp Jun 12, 2024
0062acb
Update 8.0.400.md
psfinaki Jun 12, 2024
a0f3acf
Add clarifying comment
edgarfgp Jun 13, 2024
a63a322
Merge branch 'main' into attribute-targets-on-records
edgarfgp Jun 13, 2024
cc7cc4a
Merge branch 'main' into attribute-targets-on-records
edgarfgp Jun 13, 2024
207ac3b
Merge branch 'main' into attribute-targets-on-records
edgarfgp Jun 14, 2024
993958e
Merge branch 'main' into attribute-targets-on-records
edgarfgp Jun 17, 2024
be11382
Merge branch 'main' into attribute-targets-on-records
edgarfgp Jul 3, 2024
5c6c7c8
We do not check for RQA attribute
edgarfgp Jul 3, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions docs/release-notes/.FSharp.Compiler.Service/8.0.400.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
### Fixed

* Enforce `AttributeTargets` on records. ([PR #17207](https://github.com/dotnet/fsharp/pull/17207))
* Fix a false positive of the `[<TailCall>]` analysis in combination with async. ([Issue #17237](https://github.com/dotnet/fsharp/issues/17237), [PR #17241](https://github.com/dotnet/fsharp/pull/17241))
* Extended #help directive in fsi to show documentation in the REPL. ([PR #17140](https://github.com/dotnet/fsharp/pull/17140))
* Fix internal error when dotting into delegates with multiple type parameters. ([PR #17227](https://github.com/dotnet/fsharp/pull/17227))
Expand Down
3 changes: 2 additions & 1 deletion docs/release-notes/.FSharp.Core/8.0.400.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,8 @@

* Cache delegate in query extensions. ([PR #17130](https://github.com/dotnet/fsharp/pull/17130))
* Update `AllowNullLiteralAttribute` to also use `AttributeTargets.Interface` ([PR #17173](https://github.com/dotnet/fsharp/pull/17173))
* Update `StructAttribute ` to also use `AttributeTargets.Class` ([PR #17207](https://github.com/dotnet/fsharp/pull/17207))

### Breaking Changes

* Fixed argument exception throwing inconsistency - accessing an out-of-bounds collection index will now throw `ArgumentOutOfRangeException` instead of `ArgumentException` ([#17328](https://github.com/dotnet/fsharp/pull/17328))
* Fixed argument exception throwing inconsistency - accessing an out-of-bounds collection index will now throw `ArgumentOutOfRangeException` instead of `ArgumentException` ([#17328](https://github.com/dotnet/fsharp/pull/17328))
8 changes: 8 additions & 0 deletions src/Compiler/Checking/CheckDeclarations.fs
Original file line number Diff line number Diff line change
Expand Up @@ -428,6 +428,7 @@ module TcRecdUnionAndEnumDeclarations =
let g = cenv.g
let m = id.idRange
let attrs, _ = TcAttributesWithPossibleTargets false cenv env AttributeTargets.FieldDecl synAttrs

let attrsForProperty, attrsForField = attrs |> List.partition (fun (attrTargets, _) -> (attrTargets &&& AttributeTargets.Property) <> enum 0)
let attrsForProperty = (List.map snd attrsForProperty)
let attrsForField = (List.map snd attrsForField)
Expand Down Expand Up @@ -2840,6 +2841,7 @@ module EstablishTypeDefinitionCores =
// Allow failure of constructor resolution because Vals for members in the same recursive group are not yet available
let attrs, getFinalAttrs = TcAttributesCanFail cenv envinner AttributeTargets.TyconDecl synAttrs
let hasMeasureAttr = HasFSharpAttribute g g.attrib_MeasureAttribute attrs
let hasStructAttr = HasFSharpAttribute g g.attrib_StructAttribute attrs

let isStructRecordOrUnionType =
match synTyconRepr with
Expand Down Expand Up @@ -2896,6 +2898,12 @@ module EstablishTypeDefinitionCores =

// Run InferTyconKind to raise errors on inconsistent attribute sets
InferTyconKind g (SynTypeDefnKind.Record, attrs, [], [], inSig, true, m) |> ignore

if g.langVersion.SupportsFeature(LanguageFeature.EnforceAttributeTargets) then
if hasStructAttr then
edgarfgp marked this conversation as resolved.
Show resolved Hide resolved
TcAttributesWithPossibleTargets false cenv envinner AttributeTargets.Struct synAttrs |> ignore
else
TcAttributesWithPossibleTargets false cenv envinner AttributeTargets.Class synAttrs |> ignore

// Note: the table of record fields is initially empty
TFSharpTyconRepr (Construct.NewEmptyFSharpTyconData TFSharpRecord)
Expand Down
2 changes: 1 addition & 1 deletion src/FSharp.Core/prim-types.fs
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,7 @@ namespace Microsoft.FSharp.Core
inherit Attribute()
member _.CompiledName = compiledName

[<AttributeUsage (AttributeTargets.Struct ||| AttributeTargets.ReturnValue, AllowMultiple=false)>]
edgarfgp marked this conversation as resolved.
Show resolved Hide resolved
[<AttributeUsage (AttributeTargets.Class ||| AttributeTargets.Struct ||| AttributeTargets.ReturnValue, AllowMultiple=false)>]
[<Sealed>]
type StructAttribute() =
inherit Attribute()
Expand Down
2 changes: 1 addition & 1 deletion src/FSharp.Core/prim-types.fsi
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,7 @@ namespace Microsoft.FSharp.Core
/// <summary>Adding this attribute to a type causes it to be represented using a CLI struct.</summary>
///
/// <category>Attributes</category>
[<AttributeUsage (AttributeTargets.Struct ||| AttributeTargets.ReturnValue ,AllowMultiple=false)>]
[<AttributeUsage (AttributeTargets.Class ||| AttributeTargets.Struct ||| AttributeTargets.ReturnValue ,AllowMultiple=false)>]
[<Sealed>]
type StructAttribute =
inherit Attribute
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,3 +24,11 @@ type SemanticClassificationItem =
type ILTableName(idx: int) =
member __.Index = idx
static member FromIndex n = ILTableName n

[<RequireQualifiedAccess>]
[<CustomClass>]
type Record = { Prop: string }

[<RequireQualifiedAccess>]
[<Struct>]
type StructRecord = { Prop: string }
Original file line number Diff line number Diff line change
Expand Up @@ -7,4 +7,4 @@ type PropertyLevelAttribute() =

type U =
| [<PropertyLevel>] A
| [<PropertyLevel>] B
| [<PropertyLevel>] B
Original file line number Diff line number Diff line change
Expand Up @@ -638,4 +638,25 @@ type InterruptibleLazy<'T> private (valueFactory: unit -> 'T) =
(Error 842, Line 14, Col 3, Line 14, Col 15, "This attribute is not valid for use on this language element")
(Error 842, Line 18, Col 3, Line 18, Col 14, "This attribute is not valid for use on this language element")
(Error 842, Line 19, Col 3, Line 19, Col 15, "This attribute is not valid for use on this language element")
]

// SOURCE= E_AttributeTargetIsClass02.fs # E_AttributeTargetIsClass02.fs
[<Theory; Directory(__SOURCE_DIRECTORY__, Includes=[|"E_AttributeTargetIsClass02.fs"|])>]
let ``E_AttributeTargetIsClass02_fs`` compilation =
compilation
|> verifyCompile
|> shouldSucceed

// SOURCE=E_AttributeTargetIsClass02.fs # E_AttributeTargetIsClass02.fs
[<Theory; Directory(__SOURCE_DIRECTORY__, Includes=[|"E_AttributeTargetIsClass02.fs"|])>]
let ``E_AttributeTargetIsClass02_fs preview`` compilation =
compilation
|> withLangVersionPreview
|> verifyCompile
|> shouldFail
|> withDiagnostics [
(Error 842, Line 15, Col 3, Line 15, Col 18, "This attribute is not valid for use on this language element")
(Error 842, Line 16, Col 3, Line 16, Col 15, "This attribute is not valid for use on this language element")
(Error 842, Line 20, Col 3, Line 20, Col 14, "This attribute is not valid for use on this language element")
(Error 842, Line 21, Col 3, Line 21, Col 18, "This attribute is not valid for use on this language element")
]
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
open System

[<AttributeUsage(AttributeTargets.Class)>]
type ClassTargetAttribute() =
inherit Attribute()

[<AttributeUsage(AttributeTargets.Interface)>]
type InterfaceTargetAttribute() =
inherit Attribute()

[<AttributeUsage(AttributeTargets.Struct)>]
type StructTargetAttribute() =
inherit Attribute()

[<InterfaceTarget>]
[<StructTarget>]
[<ClassTarget>]
type Record = { Prop: string }

[<ClassTarget>]
[<InterfaceTarget>]
[<StructTarget>]
[<Struct>]
psfinaki marked this conversation as resolved.
Show resolved Hide resolved
type StructRecord = { Prop: string }
Original file line number Diff line number Diff line change
Expand Up @@ -12,4 +12,5 @@ type PropertyOrFieldLevelAttribute() =

type SomeUnion =
| [<PropertyLevel>] Case1 of int // Should fail
| [<PropertyOrFieldLevel>] Case2 of int // Should fail
| [<PropertyOrFieldLevel>] Case2 of int // Should fail

Loading