Rune seems to be missing the non-generic IComparable interface. Because of this, comparison operators from F# can not be used with the type. Was there a design decision behind omitting this? If not, could this get implemented?
There's mitigation code down below if you're effected by this in F#. I don't think there's a mitigation for C#/VB.
Interesting. To make a formal API proposal out of this:
namespace System.Text {
- public readonly struct Rune : IComparable<Rune>, IEquatable<Rune> {
+ public readonly struct Rune : IComparable<Rune>, IEquatable<Rune>, IComparable {
+ public int CompareTo(object? obj);
}
}
@GrabYourPitchforks
Not an intentional omission. Thanks for the report. And now I've learned that in API reviews I should be on the lookout for this pattern to improve compatibility across languages. :)
@GrabYourPitchforks I miss this all the time in my own libraries. Some of the generic interfaces include their non generic ones implicitly. Some don't. I always forget which don't, and usually don't even notice until I'm implement F# bindings later.
FWIW I ran a bit of reflection against CoreLib in netcoreapp3.1 and it looks like System.Text.Rune is the only public type that doesn't do this.
I'm running into a problem related to this now, and wanna see if any of you have ideas about how to tackle this cleanly, or at least mitigate the issue in effected targets.
I've been backporting Rune in some libraries I develop/maintain. I've actually got it basically done and passing all but one of the official tests, and have it set up properly to forward to the official one on newer targets. There's no issue here. For this entire project there's official F# bindings, and my tests are in F# to achieve better coverage, which is where I found this issue in the first place.
While I can add IComparable to the backported implementation, I don't know of any way to deal with that for the .NET Core 3.0/3.1 implementation. That creates an odd "hole".
I've seen several ways to implement interfaces over the years. And while I'm not 100% sure, I believe .NET has them as vtables, in which case there's no possible way to add an interface to an existing type.
I could address this on the F# side by conditionally adding in some code to handle it. It involves heavy abuse of the type system, but it would allow the operators to still be called as intended; albeit only for users of my libraries. However this still leaves the C# side with a hole in it. I can't think of any situation where someone would be using Rune in a weakly typed context, but if there's one thing I've definitely learned over the years it's that you can't assume anything about how people will use generics.
I'm probably just stuck documenting the omission, huh?
Could you describe the exact F# scenario that is blocking you at the moment? Is it using comparison operators, or Map/Set types?
~~~~fsharp
member _.Operators_And_CompareTo(left:uint32, right:uint32) =
let l = Rune(left)
let r = Rune(right)
Assert.AreEqual((left = right), (l = r))
Assert.AreEqual((left <> right), (l <> r))
Assert.AreEqual((left < right), (l < r))
Assert.AreEqual((left <= right), (l <= r))
Assert.AreEqual((left > right), (l > r))
Assert.AreEqual((left >= right), (l >= r))
Assert.AreEqual(left.CompareTo(right), l.CompareTo(r))
~~~~
Essentially a direct translation of your official tests for Rune. It would show up in C#/VB in any instance where IComparable is used, however, such as sorting a collection. I know I just said I couldn't think of a situation where this would be done, but as I'm writing this I realized there's an obvious one. Char and Rune have the same sort rules, and could be put into a weakly typed collection and sorted; except for the missing interface.
For the purposes of your test you could perhaps define a wrapper type that implements IComparable based on Rune's ICompatable<T> implementation.
That would certainly work.
I could address this on the F# side by conditionally adding in some code to handle it. It involves heavy abuse of the type system, but it would allow the operators to still be called as intended; albeit only for users of my libraries.
I was hoping to get it working for my downstream as well; to mitigate this issue as much as I can. Like I said, I already know how to address this on the F# side. I can post that code here once I get it worked out, in case anyone else is effected by this and needs mitigation.
Disclaimer: This is straight up abuse of the F# type system and is potentially dangerous. Do not do this because it's neat. This is only meant to address this issue for users of F# on the effected target runtimes.
~~~~fsharp
namespace Stringier
// Because of #2340 (https://github.com/dotnet/runtime/issues/2340), Rune originally did not
// implement IComparable. This was a mistake. These fixes allow comparison operators to still
// work with Rune. Hopefully these do not interfere with the intended behavior of the operator.
// In case they do, this module must be manually opened.
module OperatorFixes =
let inline (<)(left:^a)(right:^b) =
(^a : (member CompareTo : ^b -> int) (left, right)) < 0
let inline (<=)(left:^a)(right:^b) =
(^a : (member CompareTo : ^b -> int) (left, right)) <= 0
let inline (>)(left:^a)(right:^b) =
(^a : (member CompareTo : ^b -> int) (left, right)) > 0
let inline (>=)(left:^a)(right:^b) =
(^a : (member CompareTo : ^b -> int) (left, right)) >= 0
~~~~
You'll want to adjust the conditional compilation as required for your scenario, and obviously change the namespace to whatever you're using.
I've done some testing to ensure correct behavior for comparison still occurs, but I simply can't test enough to be sure. As such, it should be assumed this is going to break something, somewhere.
Do not [<AutoOpen>] this module!
Open in only in the contexts where you are comparing Rune, and let F# do its normal thing everywhere else.
@GrabYourPitchforks what is the status of this issue? I am seeing you have marked for 5.0 release.
Not yet approved. Hoping we can quickly approve it during the next backlog review.
C#
namespace System.Text
{
public partial struct Rune : IComparable<Rune>, IComparable
{
}
}
Per offline discussions with Phil, it looks like doing this via an _explicit_ interface implementation is sufficient to address the scenario. I'd rather do it this way so that it doesn't pollute the normal API surface.
Most helpful comment
FWIW I ran a bit of reflection against CoreLib in netcoreapp3.1 and it looks like System.Text.Rune is the only public type that doesn't do this.