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

Identify F# single case discriminated unions as SumType #8739

Merged
merged 1 commit into from
Nov 30, 2023
Merged

Identify F# single case discriminated unions as SumType #8739

merged 1 commit into from
Nov 30, 2023

Conversation

gfix
Copy link
Contributor

@gfix gfix commented Nov 22, 2023

Unlike discriminated unions with more than two cases, single case discriminated unions do not inherit from an abstract base class, thus, for single case discriminated unions we need to look for the CompilationMappingAttribute on the type itself.

Single case discriminated unions do have a declared instance member named "Item", so the FSharpUnionCaseTypeDescription correctly identifies this.

Fixes #8715

Microsoft Reviewers: Open in CodeFlow

@gfix
Copy link
Contributor Author

gfix commented Nov 23, 2023

@dotnet-policy-service agree company="NRK"

else if (baseType.GetAttributes(compilationAttributeType, out compilationAttributes) && compilationAttributes.Length > 0)
{
sumTypeCandidate = baseType;
}
Copy link
Member

Choose a reason for hiding this comment

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

Do we ever need to worry about deeper inheritance chains than this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not that I know of. There is however an issue with DUs of 4 cases or more, but not due to inheritance. I will open separate issue for that.

Single case discriminated unions do not inherit from an abstract
base class, thus, for single case discriminated unions we need to
look for the CompilationMappingAttribute on the type itself.

Fix 8715
@ReubenBond ReubenBond merged commit 96c61ce into dotnet:main Nov 30, 2023
ReubenBond pushed a commit to ReubenBond/orleans that referenced this pull request Dec 2, 2023
Single case discriminated unions do not inherit from an abstract
base class, thus, for single case discriminated unions we need to
look for the CompilationMappingAttribute on the type itself.

Fix 8715
ReubenBond added a commit that referenced this pull request Dec 2, 2023
Single case discriminated unions do not inherit from an abstract
base class, thus, for single case discriminated unions we need to
look for the CompilationMappingAttribute on the type itself.

Fix 8715

Co-authored-by: Geir Fiksdal <geirfiks@gmail.com>
ReubenBond added a commit that referenced this pull request Dec 2, 2023
Single case discriminated unions do not inherit from an abstract
base class, thus, for single case discriminated unions we need to
look for the CompilationMappingAttribute on the type itself.

Fix 8715

Co-authored-by: Geir Fiksdal <geirfiks@gmail.com>
@github-actions github-actions bot locked and limited conversation to collaborators Dec 31, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

F# single case discriminated unions not serialized correctly
2 participants