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.

Sign up for GitHub

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

Revert back to use the wrong attribute targets for union case declarations #18346

Open
wants to merge 2 commits into
base: release/dev17.13
Choose a base branch
from
Open

Revert back to use the wrong attribute targets for union case declarations #18346

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
bbc4e24
Revert back to use the wrong attribute targets for union case declara…
Feb 28, 2025
927bb97
release notes
Feb 28, 2025
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
8 changes: 8 additions & 0 deletions docs/release-notes/.FSharp.Compiler.Service/9.0.201.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
### Fixed
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(just putting in a dummy comment to prevent accidental merging - mark as resolved once tactics is approved for this change)

* Revert back to use the wrong attribute targets for union case declarations.([Issue #18298](https://github.com/dotnet/fsharp/issues/18298), [PR #18346](https://github.com/dotnet/fsharp/pull/18346))

### Added

### Changed

### Breaking Changes
25 changes: 12 additions & 13 deletions src/Compiler/Checking/CheckDeclarations.fs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -580,19 +580,18 @@ module TcRecdUnionAndEnumDeclarations =
let attrs =
(*
The attributes of a union case decl get attached to the generated "static factory" method.
Enforce union-cases AttributeTargets:
- AttributeTargets.Method
type SomeUnion =
| Case1 of int // Compiles down to a static method
- AttributeTargets.Property
type SomeUnion =
| Case1 // Compiles down to a static property
*)
if g.langVersion.SupportsFeature(LanguageFeature.EnforceAttributeTargets) then
let target = if rfields.IsEmpty then AttributeTargets.Property else AttributeTargets.Method
TcAttributes cenv env target synAttrs
else
TcAttributes cenv env AttributeTargets.UnionCaseDecl synAttrs
Enforce union-cases AttributeTargets:
- AttributeTargets.Method
type SomeUnion =
| Case1 of int // Compiles down to a static method so the target should be AttributeTargets.Method
- AttributeTargets.Property
type SomeUnion =
| Case1 // Compiles down to a static property so the target should be AttributeTargets.Property
*)
// See https://github.com/dotnet/fsharp/issues/18298
// For backward compatibility, we can't enforce the correct the attribute targets for union case declarations.
// In the future, we should have a dedicated `FsharpAttributeTargets` that will help us enforce F# the attribute targets.
TcAttributes cenv env AttributeTargets.UnionCaseDecl synAttrs

Construct.NewUnionCase id rfields recordTy attrs xmlDoc vis

Expand Down
9 changes: 3 additions & 6 deletions ...tTests/Conformance/BasicGrammarElements/CustomAttributes/AttributeUsage/AttributeUsage.fs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -481,6 +481,7 @@ module CustomAttributes_AttributeUsage =
(Error 842, Line 14, Col 5, Line 14, Col 15, "This attribute is not valid for use on this language element")
]

// https://github.com/dotnet/fsharp/issues/18298
// SOURCE=E_AttributeTargetIsField03.fs # E_AttributeTargetIsField03.fs
[<Theory; Directory(__SOURCE_DIRECTORY__, Includes=[|"E_AttributeTargetIsField03.fs"|])>]
let ``E_AttributeTargetIsField03_fs`` compilation =
Expand All @@ -490,7 +491,6 @@ module CustomAttributes_AttributeUsage =
|> shouldFail
|> withDiagnostics [
(Error 842, Line 14, Col 5, Line 14, Col 15, "This attribute is not valid for use on this language element")
(Error 842, Line 15, Col 5, Line 15, Col 25, "This attribute is not valid for use on this language element")
]

// SOURCE=E_AttributeTargetIsProperty01.fs # E_AttributeTargetIsField03.fs
Expand All @@ -501,17 +501,14 @@ module CustomAttributes_AttributeUsage =
|> verifyCompile
|> shouldSucceed

// https://github.com/dotnet/fsharp/issues/18298
// SOURCE=E_AttributeTargetIsProperty01.fs # E_AttributeTargetIsField03.fs
[<Theory; Directory(__SOURCE_DIRECTORY__, Includes=[|"E_AttributeTargetIsProperty01.fs"|])>]
let ``E_AttributeTargetIsProperty01_fs`` compilation =
compilation
|> withLangVersion90
|> verifyCompile
|> shouldFail
|> withDiagnostics [
(Error 842, Line 14, Col 5, Line 14, Col 18, "This attribute is not valid for use on this language element")
(Error 842, Line 15, Col 5, Line 15, Col 25, "This attribute is not valid for use on this language element")
]
|> shouldSucceed

// SOURCE=E_AttributeTargetIsCtor01.fs # E_AttributeTargetIsCtor01.fs
[<Theory; Directory(__SOURCE_DIRECTORY__, Includes=[|"E_AttributeTargetIsCtor01.fs"|])>]
Expand Down
Loading