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

Structs: DUs and active patterns that have no fields or only simple sturcts should also be structs #705

Closed
Thorium opened this issue Mar 15, 2024 · 3 comments

Comments

@Thorium
Copy link
Member

Thorium commented Mar 15, 2024

I'm stealing the great idea from @nojaf and adding it here as possible development as well. :-)

From FSharp.Core 6.0.1 onwards (F# 6):

A DU without any fields should be a struct:

[<Struct>]
type MyType =
| A
| B

Enums are fine, and this is a struct already:

type MyType =
| A = 1
| B = 2

Enums with simple fields like only boolean, int, Guid or struct could be also recommended to be structs, but that should be configurable, because that needs the name for the field, and changes may be a breaking change for existing API:

[<Struct>]
type MyType =
| A of a: int 
| B of b: struct(bool*bool)

Partial active pattern that returns empty (), boolean, guid or int, should also be marked as [<return: Struct>] and use ValueOption instead of Option:

[<return: Struct>]
let (|MyThing|_|) x =
   if x then ValueSome()
   else ValueNone

This comes from conversation in here:
fsprojects/FSharp.Formatting#906

@knocte
Copy link
Collaborator

knocte commented Mar 15, 2024

So this would be a rule whose purpose is optimization? (speed? memory?)

@Thorium
Copy link
Member Author

Thorium commented Mar 15, 2024

Yes, both. Similarly like testing IEnumerable isEmpty is optimization over "length > 0".

@Thorium
Copy link
Member Author

Thorium commented Apr 2, 2024

I'm closing this because this is already done in ionide-analyzers project. Over time that will already improve general F# code-bases on a level appropriate, there is no point of doing double amount of work implementing it here.

@Thorium Thorium closed this as completed Apr 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants