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

'unit' should implement IEquatable<unit> and IComparable<unit> #1345

Open
6 tasks done
charlesroddie opened this issue Jan 7, 2024 · 9 comments
Open
6 tasks done

Comments

@charlesroddie
Copy link

charlesroddie commented Jan 7, 2024

I propose that unit should implement IEquatable<unit> and IComparable<unit>.

Existing:

// https://github.com/dotnet/fsharp/blob/main/src/FSharp.Core/prim-types.fs
type Unit() =
    override _.GetHashCode() = 0

    override _.Equals(obj:obj) = 
        match obj with null -> true | :? Unit -> true | _ -> false

    interface System.IComparable with 
        member _.CompareTo(_obj:obj) = 0

Implementation:

type Unit() =
    override _.GetHashCode() = 0

    override _.Equals(obj:obj) = 
        match obj with null -> true | :? Unit -> true | _ -> false

    interface System.IComparable with 
        member _.CompareTo(_obj:obj) = 0

    interface IEquatable<Unit> with member _.Equals(_) = true
    interface IComparable<Unit> with member _.CompareTo(_) = 0

Pros and Cons

The advantages of making this adjustment to F# are

  • Consistency with other user-defined constructs (records, DUs) which usually implement generic IComparable and IEquatable in addition to non-generic ones where applicable.
  • Implementing the legacy non-generic methods without the proper generic ones is bad practice.
  • Ability to be passed into methods which assume IEquatable<'a>.
    • Prior to an implementation of [Breaking] Avoid boxing on equality and comparison #1280, F# developers can create more performant methods by using a constraint where 'a :> IEquatable<'a> instead of where 'a : equality. I am doing a spike to investigating using IEquatable<'T> almost everywhere and the absence of this feature made this awkward.
    • After the implementation, the compiled code may end up simpler on calling equality with just a call to IEquatable<unit> rather than a more complex equality function.

The disadvantages of making this adjustment to F# are ...

  • The size of F# Core will go up by a few bytes.

Extra information

Estimated cost (XS, S, M, L, XL, XXL):

XS

Related suggestions: (put links to related suggestions here)

Affidavit (please submit!)

Please tick these items by placing a cross in the box:

  • This is not a question (e.g. like one you might ask on StackOverflow) and I have searched StackOverflow for discussions of this issue
  • This is a language change and not purely a tooling change (e.g. compiler bug, editor support, warning/error messages, new warning, non-breaking optimisation) belonging to the compiler and tooling repository
  • This is not something which has obviously "already been decided" in previous versions of F#. If you're questioning a fundamental design decision that has obviously already been taken (e.g. "Make F# untyped") then please don't submit it
  • I have searched both open and closed suggestions on this site and believe this is not a duplicate

Please tick all that apply:

  • This is not a breaking change to the F# language design
  • I or my company would be willing to help implement and/or test this

For Readers

If you would like to see this issue implemented, please click the 👍 emoji on this issue. These counts are used to generally order the suggestions by engagement.

@dsyme
Copy link
Collaborator

dsyme commented Mar 12, 2024

I believe "null" is always emitted as a unit value. To my knowledge there is not even a singleton unit value in FSharp.Core. In a way Unit should be labelled with UseNullAsTrueValue

This means the interfaces are not supported in any real sense. Can your IEquatable code work in this situation? Or does it have to be written to tolerate UseNullAsTrueValue types. Thanks

@abelbraaksma
Copy link
Member

abelbraaksma commented May 8, 2024

@dsyme is right, Unit is always compiled as null. While a type exists, this type cannot explicitly be instantiated.

For instance, calling any of the methods you dumped above will lead to an NRE:

> let x = ();;
val x: unit = ()

> let y = ();;
val y: unit = ()

> x.Equals y;;
System.NullReferenceException: Object reference not set to an instance of an object.
   at <StartupCode$FSI_0005>.$FSI_0005.main@() in C:\Users\abelo\AppData\Local\Temp\stdin:line 4
   at System.RuntimeMethodHandle.InvokeMethod(Object target, Void** arguments, Signature sig, Boolean isConstructor)
   at System.Reflection.MethodBaseInvoker.InvokeWithNoArgs(Object obj, BindingFlags invokeAttr)
Stopped due to error

And if we look at how this compiles, we see that it is always null.

Ability to be passed into methods which assume IEquatable<'a>.

Yes. But if you would ever pass unit to such a method, it will break with an NRE.

After the implementation, the compiled code may end up simpler on calling equality with just a call to IEquatable rather than a more complex equality function.

It will always boil down to: null :> IEquatable<unit>.


That said, I don't see a real downside of doing this. While these methods will never be callable in practice, if there are actual use-cases, I'm not against doing so. It's a benign change. But if that's enough motivation?

@charlesroddie
Copy link
Author

Implementation of this would presumably be (since it would be breaking to make unit a struct) include stopping using null and using a singleton instead. I.e. create the singleton, expose it as a static method, and use the singleton in FSharp.Core. Are there any downsides to this?

@auduchinok
Copy link

Are there any downsides to this?

That would likely break compatibility with all the existing compiled F# libraries and existing compiler versions that people use.

@charlesroddie
Copy link
Author

In what way would it break compatibility? Is there any code that relies on the implementation of Unit being null? For that to happen, code would have to go out of its way to throw an exception if encountering a genuine Unit instead of a null surely?

@Tarmil
Copy link

Tarmil commented May 8, 2024

Libraries compiled before such a change will still use null though. So you could take a unit returned from an existing library, try to call Equals on it, and get a NRE.

@charlesroddie
Copy link
Author

That would happen at the moment so no break is caused. I wouldn't expect returning Unit() instead of null to break anything.

@Tarmil
Copy link

Tarmil commented May 9, 2024

I meant Equals from IEquatable, which unit doesn't implement at the moment. My point is, if unit implements interfaces, then you'll expect to be able to use them, but unit returned from older libraries will fail.

@abelbraaksma
Copy link
Member

abelbraaksma commented May 11, 2024

In what way would it break compatibility?

@charlesroddie, at a minimum, every single public function, method, interface that exposes Unit will be binary incompatible.

Here's an example where you can see that using () is compiled with Unit-type arguments. Check the generated IL.

Is there any code that relies on the implementation of Unit being null?

Yes. I have such code in my helper libraries used in unit testing. And I wouldn't be surprised if such code exists in F# (i.e. in sprintf maybe) and other analytics scenarios.

A trivial example is the following, and it is easy to see how this could help with log messages or debug output in unit tests:

/// Print var info based on the type
let DebugInfo (a: 'T) = 
    match box a with 
    | null when typeof<'T> = typeof<Unit> -> "Unit type"
    | null when typeof<'T> = typeof<option<int>> -> "None value of type int"
    | null when typeof<'T> = typeof<option<string>> -> "None value of type string" // of course, this would be generalized 
    | _ -> "Unknown"

Alternatively, a null-safe GetType() function would follow a similar logic.

For that to happen, code would have to go out of its way to throw an exception if encountering a genuine Unit instead of a null surely?

Not really. I've written code along the lines of above quite often and while it can take a moment before you realize how certain types are implemented and that it requires some creativity in coding dynamic type detection, it is not all too uncommon.


Another consideration, and a strong reason why back in the pre-F# 1.0 days it was decided that None and Unit would be optimized to null is that it is typically much faster to work with than any other value. Furthermore, the JIT is getting increasingly more capable of null-eliding, and processors with their branch-prediction typically work most efficiently if the hot-path is the same almost always (which in the case of null, it obviously is).

Btw, fwiw, Void is also an existing type that is never instantiated...


That said, I still don't see any harm in adding these interfaces to Unit. I'm just curious to what use-cases it helps, though.

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

5 participants