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

Request: make C# 7's discriminated unions compatible with F# #9039

Closed
DavidArno opened this issue Feb 23, 2016 · 19 comments
Closed

Request: make C# 7's discriminated unions compatible with F# #9039

DavidArno opened this issue Feb 23, 2016 · 19 comments

Comments

@DavidArno
Copy link

Assuming C# 7 supports discriminated unions, then, if I define the following in F#:

type Shape =
    | Rectangle of width : float * length : float
    | Circle of radius : float
    | Line of length: float

then C#'s pattern matching features should support that type as a sealed ADT type set and it should handle it just as if I'd defined it in C# as:

abstract sealed class Shape
{
    class Rectangle(float width, float length);
    class Circle(float radius);
    class Line(float length);
}

If possible, the two definitions should generate the same IL.

Further, whilst there will likely be practical problems around this extra idea, the option type in Microsoft.FSharp.Core should be used by the new pattern matching features to, for example, avoid having to use out parameters with the new is operator as discussed in #9005.

@alrz
Copy link
Contributor

alrz commented Feb 23, 2016

the option type in Microsoft.FSharp.Core should be used by the new pattern matching features

You mean they should take a dependency on F# runtime lib? That's beyond unacceptable, if anything, a C# AST for option type might find its way to BCL and then it's F# that should maintain compatibility with it. I agree that this will be really helpful but it adds unnecessary burden to C# design. I think since BCL is written in C#, this should be taken care of on F#'s side.

@DavidArno
Copy link
Author

@alrz,

How would you propose that the option type be moved like this without it becoming a breaking change for F#? Moving Microsoft.FSharp.Core into the BCL might be the only way to achieve this.

@HaloFour
Copy link

While I see the convenience in having compatibility between the two languages F# currently produces some quite specific types to accomplish pattern matching and expects some very specific conventions. To actually achieve this goal of compatibility would require that C# effectively implement pattern matching identically to F#, which I doubt is the goal of the C# team.

@HaloFour
Copy link

I've been toying around with this a little and I've been unsuccessful at defining a C# class that F# will recognize as an ADT as is. I started with a public F# definition in a library and copied the structure as closely as possible yet F# still balks. This includes all of the F# CompilerMappingAttribute attributes to denote the type, the static factory methods and the instance properties. I am probably still missing something, but the F# compiler seems very finicky.

If compatibility between the two languages does become a worthwhile goal (and I do see the utility of it) I think that the two teams should collaborate over the public interface of what makes an ADT. Future versions of the F# compiler could adhere to this convention.

Also, for the concerns with Option<T>, I agree with @alrz that the C# compiler should definitely not take a dependency on the F# core libraries. What is more likely to happen is that if the BCL gets an Option<T> type that a newer F# compiler will switch to using the BCL version rather than the F# version. This is what the F# team when tuples were added to the BCL.

@DavidArno
Copy link
Author

@HaloFour,

I didn't realise the F# team switched their tuple model when they were added to the BCL. That does indeed suggest they could switch option too, without it being a breaking change. Good news all around!

@axel-habermaier
Copy link
Contributor

@DavidArno: As far as I know, F# was still in beta then. It is of course a breaking change as the namespace of the type changes. At least when doing C# interop. Within F#, it probably wouldn't matter.

@HaloFour
Copy link

@axel-habermaier

F# was released as 1.0 in 2005 but the BCL didn't get tuples until 2010. You can still target F# projects to previous versions of the .NET framework and it will fallback to the tuple classes in FSharp.Core.dll. But those tuple classes were at least defined in the System namespace.

Update: FSharp.Core.dll does define the assembly-level attributes to forward System.Tuple to mscorlib.dll, as well as a few other types like Lazy<T> and IObservable<T>.

Unfortunately F#'s Option<T> type is located in the Microsoft.FSharp.Core namespace, and it's also actually called FSharpOption<T>, not Option<T>.

@alrz
Copy link
Contributor

alrz commented Feb 23, 2016

Just to mention, F#'s option is not a regular DU and it has been specialized (e.g. None is null). However it's planned to implement another type for option as a struct which still makes it a special case (that's quite interesting, I wonder if struct DUs could make it to the language as a standard construct).

@DavidArno
Copy link
Author

@alrz,

I like the idea of DU's being structs, but I've got hung up on the syntax. Assuming we are going for the following syntax for class DU's:

abstract sealed class DU
{
    type 1 def;
    type 2 def;
    ...
}

Then what do we put for structs? The following seems silly, as abstract and sealed are both invalid terms for structs:

abstract sealed struct DU
{
    type 1 def;
    type 2 def;
    ...
}

Whilst the compiler could be modified to accept that combination as a way of telling it I'm declaring a DU, it seems clunky. But hang-ups on syntax ugliness seems a very poor reason to not inplement DU's as structs.

@svick
Copy link
Contributor

svick commented Feb 24, 2016

@HaloFour

I've been toying around with this a little and I've been unsuccessful at defining a C# class that F# will recognize as an ADT as is. […] I am probably still missing something, but the F# compiler seems very finicky.

I believe what you're missing is the undocumented FSharpSignatureData resource that the F# compiler emits to assemblies. It contains, among other things, additional metadata about F# types, like what its F# type is.

@HaloFour
Copy link

@svick I had noticed that resource and suspected that it (or other assembly level metadata) might be involved. It's weird that F# uses both that and type/member attributes to specify that metadata, you'd think that there'd be enough information by simply examining the type. Anyhow, I think that demonstrates that it wouldn't be suitable for C# to produce F#-compatible ADTs. But I do think that there would be value in the two teams collaborating together to decide on a compatible contract going forward.

@xoofx
Copy link
Member

xoofx commented Oct 31, 2016

FYI, wrote a blog post about valuetype discriminated unions... While I'm more in favor of valuetype for performance reasons, we could have a syntax that could support both options.

@alrz
Copy link
Contributor

alrz commented Oct 31, 2016

@xoofx Great post! That's something along the lines of #6739. A minor point: It'd be nice if enum struct generate a type-safe union (with FieldOffset, etc), but currently it will be blocked on CLR limitations (e.g. #8456). However, I'd be ok with something similar to Java enums (same thing that is proposed in #6739) but it seems that it's patented. If anything, it would be really helpful if at least we handle completeness for enum struct because that is not applicable to regular enums per #11438 (comment) :

We don't handle completeness for enum types, as there are a huge number of values even if only a few are named.

@xoofx
Copy link
Member

xoofx commented Nov 1, 2016

For the explicit layout problem with T , this can be solved by packing non T-blitable types into a internal struct with explicit layout, and place T types/object types on their own slots. The structured enum would not be declared with an ExplicitLayout, only parts of its internal representation/packing.

So suppose you have a type like this:

public struct enum Variant<T>
{
   V1(int x),
   V2(float y, T add),
   V3(object z)
}

The generated parent struct Variant<T> could look like this:

public struct Variant<T>
{
  protected $PackedStruct $packed;  // used by V1.x and V2.y
  protected T $field1; // used by V2.add
  protected object $field2;  // used by V3.z

  [StructLayout(LayoutKind.ExplicitLayout) struct $PackedStruct{ [FieldOffset(0)] int x; [FieldOffset] float y; }
}

@DavidArno
Copy link
Author

@xoofx,

Re the comment on your blog:

Unlike the issue “Request: make C# 7’s discriminated unions compatible with F#”, I really don’t want to have this compatibility with F# as it would completely obliterate the performance and efficiency that C# deserve.

Are these two things truly mutually exclusive? I see two potential options here:

  1. Just as F# is changing its tuple model from the class-based Tuple<...> set to the struct-based ValueTuple<...> set, could F# not likewise change the underlying implementation of DU's to your struct enum proposal?
  2. Alternatively, is there any reason why C# couldn't offer both abstract sealed class DU's that were then interchangeable with F#'s DU types, as well as non-compatible, but more performant, struct enum based DU's?

@xoofx
Copy link
Member

xoofx commented Nov 1, 2016

@DavidArno indeed, as I responded later to a user comment, I don't mind having both, as long as we have a valuetype DU. It would make actually the compatibility possible (though I would not project it the same way F# is doing it today, by avoiding is usage).

@alrz
Copy link
Contributor

alrz commented Nov 1, 2016

@xoofx In your example $PackedStruct is still a generic type and causes a type load error. It probably need to be declared as a non-nested type.

@xoofx
Copy link
Member

xoofx commented Nov 1, 2016

@alrz ah! maybe, haven't checked actually... 😅 but yeah, putting it outside will solve the issue.

@gafter
Copy link
Member

gafter commented May 8, 2018

Discussion should continue at dotnet/csharplang#485

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

8 participants