fslang-suggestions icon indicating copy to clipboard operation
fslang-suggestions copied to clipboard

C# record interop (including init only properties of .net 5 and required of .net 7)

Open jbtule opened this issue 4 years ago • 15 comments

I'm not sure what the current interop plan for F# is with init only properties in general is? I assume given a C# class:

   public class CSharpClass{
       public string? Prop1 {get; init;}
       public string? Prop2 {get; init;}
   }

it's using the existing F# init properties syntax

let test = CSharpClass(Prop1="Test", Prop2="Test2")

However, I tested in .net 5.0 preview 7 and the F# compiler has no errors, but builds an "Invalid Program" according to the CLR. Also with the simpler case of initializing inside a constructor.

type TestFsharp() as this= 
    inherit CSharpClass ()
    do 
        this.Prop1 <- "Yo"
        this.Prop2 <- "What"

Also

Unhandled exception. System.InvalidProgramException: Common Language Runtime detected an invalid program.
   at Program.TestFsharp..ctor()
   at Program.main(String[] argv) 

If you try to mutate an init only property outside an initialization block, it also compiles without error, but throws a runtime error. So at the very least that should be a compiler error, and thus am sure .net 5.0 preview 7 F# does not do anything yet for init only properties.

I thought I would take the time to make suggestions for the interop story with 3 cases of init only properties from C#.

So the General Case is a random C# class with an init only setter.

public class CSharpClass{
    public string? Prop1 {get; init;}
    public string? Prop2 {get; init;}
}

Probably handed by the previously mentioned CSharpClass(Prop1="Test", Prop2="Test2") and should work from constructor too.

Then there are 2 C# Record cases. Luckily not allowing inheritance of these records from F# seems like a legit path and would simplify things.

The most attractive C# Record types for developers are the positional records with a primary constructor.

public record CSharpRecord(string Prop1, string Prop2);

With these you may not even need to to use the init only properties in F# because you could use F# Record syntax {Prop1="Test; Prop2="Test2"} and it would be very similar to how you initialize F# records, and Copy them. {source with Prop1="Test1"}

However C# Records don't need to have a primary constructor.

// with empty constructor
public record CSharpRecord {
    public string? Prop1 {get; init;}
    public string? Prop2 {get; init;}
}
//rando constructor
public record CSharpRecord {
    public CSharpRecord(bool rando){
        Prop1 = rando.ToString();
    }
    public string? Prop1 {get; init;}
    public string? Prop2 {get; init;}
}

And then I suppose you'd be left with the general case CSharpRecord(Prop1="Test", Prop2="Test2"), or if there was an empty constructor you could might be able to use the Record syntax safely if you can identify it as a C# Record. And with any record, if you can identify it, it should still possible to syntactically supporting copy {source with Prop1="Test1"} but you'd have to use the <Clone>$ method and init only properties the way C# does. It may be preferred to do this for Positional Records with Primary Constructors, I'm not sure.

Pros and Cons

The advantages of making this adjustment to F# are compiling functioning programs when consuming a C# type with init only properties!

The disadvantages of making this adjustment to F# is its needed for .net 5.0 which is coming up fast! At the very least needs compiler errors.

Extra information

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

Links:

C# 9.0 init only Proposal (mentions F# would be changed): https://github.com/dotnet/csharplang/blob/master/proposals/csharp-9.0/init.md

C# 9.0 Records Proposal: https://github.com/dotnet/csharplang/blob/master/proposals/csharp-9.0/records.md

Related Suggestions:

#903

Affidavit (please submit!)

Please tick this by placing a cross in the box:

  • [x] This is not a question (e.g. like one you might ask on stackoverflow) and I have searched stackoverflow for discussions of this issue
  • [x] I have searched both open and closed suggestions on this site and believe this is not a duplicate
  • [x] 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.

Please tick all that apply:

  • [x] 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 :+1: emoji on this issue. These counts are used to generally order the suggestions by engagement.

jbtule avatar Aug 25 '20 02:08 jbtule

We discussed this leading up to F# 5 but decided to cut it since the main C# feature that uses this - records - are also not completed (for example: metadata emission likely to change, struct records to be added, primary constructor encoding may change, etc.)

It's a very reasonable addition to F# types and will likely happen in the next F# language update

cartermp avatar Aug 25 '20 15:08 cartermp

Potentially relevant comment: https://github.com/dotnet/fsharp/issues/10261#issuecomment-710117542

abelbraaksma avatar Oct 19 '20 11:10 abelbraaksma

It's a very reasonable addition to F# types and will likely happen in the next F# language update

agreed

dsyme avatar Jan 12 '21 14:01 dsyme

Is there any work planned/started about this? This issue is 9 months old, C# is adopting init and records all over the place, but using that from F# not only is impossible, but blows up in runtime with an error not even explaining what's happening.

This hurts especially when you want to setup a shell of the application in C# (like MVC), and put some logic in F# - you cannot really share records both ways

qrzychu avatar May 02 '21 19:05 qrzychu

@qrzychu can't you still consume them via the constructor syntax, but just not use the init property syntax?

image

From what I see, it is possible to consume C# records in F# projects.

Also, for perspective, C# compiler doesn't allow to use records in dotnet framework or even netstandard2.0 projects, without manually adding (hacking) some special code to the projects, but it is not supported; so it is not "all over the place" yet, only libraries targeting net5.0 or above can really use them.

Also for perspective, there are many things that "hurt F#":

  • would the C# team have delayed shipping their record for waiting for the F# team to implement the natural syntax and support for init only (which incurs extra checks on the target framework, etc.) or even allocated resources for this to be possible?
  • would the C# team have implemented support of record syntax in C# for the already "all over the place" F# records in existing codebases in the first place?

It is uphill for F# team to have to stick to schedule dictated by C#, seemingly with them not making a lot of effort to enhance the cross language experience the other way around (showing they are ready to cut corners as well, deeming it doesn't impact much/enough to spend the effort).

The F# team made a cut to not put efforts on this feature yet, to work on features that impact most F# codebases more, they probably made that cut with the knowledge that C# records aren't all over the place and you can already consume them as shown above, so it is reasonable.

Would you be willing to contribute such work to get the feature rolling sooner?

Have you open an issue on C# compiler for it to consume F# records naturally?

smoothdeveloper avatar May 08 '21 07:05 smoothdeveloper

We should do this work very soon.

TIHan avatar May 11 '21 23:05 TIHan

The issue described is the init properties; that is what is causing the issue. I encountered this recently and it's impossible to set an init property in F#.

TIHan avatar May 11 '21 23:05 TIHan

Just to clarify the issue:

  • Constructing C# records in F# is possible, regardless of syntax chosen (positional or property initializers [including out of order])
  • These constructed records run, they do not throw an InvalidProgramException
  • You can set a value, which should not be possible, which does throw

So I don't think there are that many problems when using C# APIs that use these record types (or any init-only member) today. Yes, you can set something that you shouldn't be able to, so we will get that resolved in time. But I don't think there's any issue in working with modern APIs that use records beyond that.

cartermp avatar May 11 '21 23:05 cartermp

The runtime error that occurs is because we had a bug in the compiler; we emitted an extra pop instruction when we shouldn't have. The fix is here: https://github.com/dotnet/fsharp/pull/11552

This should allow us to at least set the properties even though there are no compile-time checks to ensure you can only set them in the initializer. At some point, we will add support for the compile-time checks, just like we did with Span and friends.

TIHan avatar May 12 '21 15:05 TIHan

The runtime error that occurs is because we had a bug in the compiler; we emitted an extra pop instruction when we shouldn't have. The fix is here: dotnet/fsharp#11552

Thanks! I was trying fsharp early in .net 5 cycle with C# records, got bitten by this and figured that was by design.

razzmatazz avatar May 12 '21 15:05 razzmatazz

At some point, we will add support for the compile-time checks, just like we did with Span and friends.

If I understand correctly, the runtime doesn't forbid setting of init members, just the compilers obey this soft restriction when they are fully principled.

If there are no pitfalls and no code generation work because fixed already; just detecting and reporting the new error message in MethodCall.fs, then I can help with this.

edit: @TIHan, if you can point to the original PR bringing the "reading and carrying of init to the TAst", this will give me the right entry of my search.

smoothdeveloper avatar May 12 '21 19:05 smoothdeveloper

There is one pitfall I see, around the "setter in method call" feature of F#:

public class NiceInit {
  public NiceInit() {}
  public int Val {get; init;}
}
type Factory =
  static member mkNiceInit() = NiceInit()
  static member mkNiceInit2() =
    let nice = NiceInit()
    printfn $"{nice}"
    nice
let niceInit = Factory.mkNiceInit(Val = 1)

this theoritically could work for mkNiceInit, but it doesn't seem tractable to account for this case (not sure of implication of inline there, but probably better to prevent it in any circumstance).

  • Am I correct that it isn't going to be supported?
  • Do we want a special error message when the case is detected?

smoothdeveloper avatar May 12 '21 20:05 smoothdeveloper

I am not sure what has changed, but I did some tests:

  • using .NET 5 and F# 5, I get the exception as described in the original topic
  • using .NET 6 (RC) and F# 6 I can use init only properties, but I can always set them

I used the following code to test:

public record CSharpClass
{
    public string Prop1 { get; init; }
}
let test = CSharpClass(Prop1="Bob") // This is what I expect to work
test.Prop1 <- "Alice" // This might be debatable

let create =
    CSharpClass(Prop1="Eve")

let test2 = create()
test2.Prop1 <- "Alice" // This should not work

dnperfors avatar Oct 22 '21 17:10 dnperfors

@dnperfors iirc this is by design as init is respected by c# compiler mostly, on clr they are regular props implemented with “set” . F# compiler does not seem to take additional “init” decoration into account however and this might or might not be an issue..

this is most probably done to not break existing code as code that is based on reflection and sets props continues to work, for example Newtonsoft.json continues to work on c# records exactly because of this compatibility approach

razzmatazz avatar Oct 22 '21 17:10 razzmatazz

Note that subclassing C# records will also run into https://github.com/dotnet/fsharp/issues/13201 because they use covariant return type overrides internally.

chkn avatar Jun 30 '22 23:06 chkn

RFC for the init-only/required parts of this work is here: https://github.com/fsharp/fslang-design/blob/main/FSharp-7.0/FS-1127-init-only-and-required-properties.md

dsyme avatar Oct 27 '22 13:10 dsyme