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

Introduce SwiftSelf<T> and SwiftIndirectResult to represent Swift structs and enums in C# #100543

Closed
Tracked by #93631
kotlarmilos opened this issue Apr 2, 2024 · 20 comments · Fixed by #102717
Closed
Tracked by #93631
Assignees
Labels
api-approved API was approved in API review, it can be implemented area-System.Runtime.InteropServices
Milestone

Comments

@kotlarmilos
Copy link
Member

kotlarmilos commented Apr 2, 2024

Background and Motivation

The Swift programming language handles the lifetime of structs and enums differently than .NET. When mapping Swift structs and enums to C# in library evolution mode, frozen Swift structs are directly mapped to C# structs with identical field layouts, while non-frozen Swift structs are mapped to C# classes. To support the projection tooling (#95638) for handling these cases, we need to introduce additional runtime support related to passing and returning structs.

The proposal is to update the built-in support for the Swift ABI and introduce a new Swift-specific type result buffer access mechanism.

Proposed API

According to the Swift calling convention, structs are lowered into a sequence of primitives. If the number of primitives is 4 or less, the struct is passed by value via registers. Otherwise, it is passed by reference in the call context register. The proposal would be to change SwiftSelf to be generic so that a user can pass SwiftSelf<T> for a frozen struct type T which then is either enregistered into multiple registers or passed by reference in the call context register.

Additionally, for Swift functions returning non-frozen structs that are projected into C# classes, it is necessary to load return buffer with a memory address. The proposal would be to add a new Swift type SwiftIndirectResult that would provide access to the return buffer register.

Based on the this, we want to update SwiftSelf to handle frozen structs and add a new Swift type to handle returning non-frozen structs.

namespace System.Runtime.InteropServices.Swift
{
-     public readonly unsafe struct SwiftSelf
+    public readonly struct SwiftSelf<T> where T: unmanaged
     {
-         public SwiftSelf(void* value)
+         public SwiftSelf<T>(T value)
         {
             Value = value;
         }

-         public void* Value { get; }
+         public T Value { get; }
     }
+
+     public readonly unsafe struct SwiftIndirectResult
+     {
+         public SwiftIndirectResult(void* value)
+         {
+             Value = value;
+         }
+
+         public void* Value { get; }
+     }
+ }

Usage Examples

This API would be used by the projection tooling. The SwiftSelf<T> is needed when calling non-mutating instance methods on frozen structs to ensure the correct passing of the value type.

using System;
using System.Runtime.InteropServices;
using System.Runtime.InteropServices.Swift;

public struct Point
{
    public double X { get; }
    public double Y { get; }

    public Point(double x, double y)
    {
        X = x;
        Y = y;
    }

    public double DistanceToOrigin()
    {
        var swiftSelf = new SwiftSelf<Point>(this);
        return distanceFromOrigin(swiftSelf);
    }

    [UnmanagedCallConv(CallConvs = new Type[] { typeof(CallConvSwift) })]
    [DllImport("libSwiftPoint.dylib")]
    private static extern double distanceFromOrigin(SwiftSelf<Point> point);
}

The SwiftIndirectResult is required to enable the tooling to invoke functions that return non-frozen structs. The runtime will load the return buffer prior to the call based on the SwiftIndirectResult value. For simplicity, the class does not implement the IDisposable interface.

using System;
using System.Runtime.InteropServices;
using System.Runtime.InteropServices.Swift;

public class Point
{
    private void* _payload;
    private static readonly int _payloadSize = /* Metadata information from value witness table */;

    public Point(double x, double y)
    {
        _payload = Marshal.AllocHGlobal(_payloadSize).ToPointer();
        try
        {
            PIfunc_init(x, y, new SwiftIndirectResult(_payload));
        }
        catch
        {
            Marshal.FreeHGlobal(new IntPtr(payload));
            throw;
        }
    }

    ~Point()
    {
        if (_payload != null)
        {
            Marshal.FreeHGlobal(_payload);
            _payload = IntPtr.Zero;
        }
    }

    private Point(void* payload)
    {
        _payload = payload;
    }

    public static Point CreateWithMagnitudeAndAngle(double magnitude, double angle)
    {
        void* payload = Marshal.AllocHGlobal(_payloadSize).ToPointer();
        try
        {
            PIfunc_createFromMagnitudeAndAngle(magnitude, angle, new SwiftIndirectResult(payload));
            return new Point(payload);
        }
        catch
        {
            Marshal.FreeHGlobal(new IntPtr(payload));
            throw;
        }
    }

    [UnmanagedCallConv(CallConvs = new Type[] { typeof(CallConvSwift) })]
    [DllImport("libSwiftPoint.dylib", EntryPoint = "PIfunc_init")]
    private static extern void PIfunc_init(double x, double y, SwiftIndirectResult payload);

    [UnmanagedCallConv(CallConvs = new Type[] { typeof(CallConvSwift) })]
    [DllImport("libSwiftPoint.dylib")]
    private static extern void PIfunc_createFromMagnitudeAndAngle(double magnitude, double angle, SwiftIndirectResult payload);
}

Please review the proposal and suggest any updates or changes.

/cc: @dotnet/jit-contrib @dotnet/interop-contrib

Copy link
Contributor

Tagging subscribers to this area: @dotnet/interop-contrib
See info in area-owners.md if you want to be subscribed.

@lambdageek
Copy link
Member

lambdageek commented Apr 2, 2024

For simplicity, the class does not implement the IDisposable interface.

This confused me. What you mean is the example with the Point class doesn't use IDisposable ? Not that you considered proposing that SwiftIndirectResult should implement IDisposable and decided against it, right?


void* payload = Marshal.AllocHGlobal(_payloadSize).ToPointer();
       try
       {
           PIfunc_createFromMagnitudeAndAngle(magnitude, angle, new SwiftIndirectResult(payload));
           return new Point(payload);
       }
       catch
       {
           Marshal.FreeHGlobal(new IntPtr(payload));
          throw;
       }

there's a typo here, right? you mean try/finally, not try/catch ? I misunderstood the example. Sorry.

@DaZombieKiller
Copy link
Contributor

there's a typo here, right? you mean try/finally, not try/catch ?

That would free the memory held by the Point instance that gets returned. I believe the code is using try/catch to prevent a memory leak in the event of PIfunc_createFromMagnitudeAndAngle or the Point constructor throwing, because payload would be left dangling otherwise.

@kotlarmilos
Copy link
Member Author

This confused me. What you mean is the example with the Point class doesn't use IDisposable ? Not that you considered proposing that SwiftIndirectResult should implement IDisposable and decided against it, right?

This proposal does not address any memory management concerns. The Point class can be implemented as IDisposable, but it is not the primary focus of this proposal. For memory management details see dotnet/designs#312.

there's a typo here, right? you mean try/finally, not try/catch ?

The catch block should release memory if the initializer on the Swift side throws an exception.

@stephen-hawley
Copy link

swift trivia - it is impossible for a swift init to throw unless it explicitly declares that it does so and even then exceptions in swift are quite different from "normal" exceptions.

Just so I understand - SwiftIndirectResult represents a return value, correct? Does it get handled "out of band" in that it isn't an actual argument, so its position in the pinvoke signature doesn't actually matter?

It might be useful to have an actual void * return value on that constructor so that the difference between a normal constructor and a failable constructor is minor.

example:

public struct WashingMachine {
    // a substantial payload
    public init?(String variety) {
        if (!GlobalInventory.HasVariety (variety)
            return nil
        // initialize the payload
    }
}

In this case you would want to be able to write something like this:

    _payload = Marshal.AllocHGlobal(_payloadSize).ToPointer();
    var result = PIfunc_init((SwiftString)"Bosch", new SwiftIndirectResult(payload));
    if (_payload != result) {
        Marshal.FreeHGlobal(new IntPtr(_payload));
        _payload = null;
        // handle however we feel best.
    }

@kotlarmilos
Copy link
Member Author

kotlarmilos commented Apr 4, 2024

Just so I understand - SwiftIndirectResult represents a return value, correct? Does it get handled "out of band" in that it isn't an actual argument, so its position in the pinvoke signature doesn't actually matter?

It represents a value that is stored in the return buffer and it will be handled in a similar way to SwiftSelf. The position in the signature doesn't matter, but for simplicity, we may decide to impose certain positional limitations.

It might be useful to have an actual void * return value on that constructor so that the difference between a normal constructor and a failable constructor is minor.

Can we utilize SwiftError in this case?

    _payload = Marshal.AllocHGlobal(_payloadSize).ToPointer();
    var swifterror = new SwiftError();
    var result = PIfunc_init((SwiftString)"Bosch", new SwiftIndirectResult(payload), ref swifterror);
    if (swifterror.Value != null) {
        Marshal.FreeHGlobal(new IntPtr(_payload));
        _payload = null;
        // handle however we feel best.
    }

Edit: The above can work only for throwing constructor, not failable constructor.

@kotlarmilos kotlarmilos added the api-suggestion Early API idea and discussion, it is NOT ready for implementation label Apr 4, 2024
@jakobbotsch
Copy link
Member

jakobbotsch commented Apr 4, 2024

From what I see from some examples, it looks like failable initializers are modelled as functions that return optional values in Swift. The projection tooling will need to model it similarly.

https://godbolt.org/z/3jhn7vqsT

I think we should make it illegal to have anything but a void return in signatures that include SwiftIndirectResult. These functions don't return in the normal way.

@kotlarmilos
Copy link
Member Author

From what I see from some examples, it looks like failable initializers are modelled as functions that return optional values in Swift. The projection tooling will need to model it similarly.

Ok. Does this imply that functions returning optional value types always pass results by reference, even though they fit into the lowering sequence?

@jakobbotsch
Copy link
Member

From what I see from some examples, it looks like failable initializers are modelled as functions that return optional values in Swift. The projection tooling will need to model it similarly.

Ok. Does this imply that functions returning optional value types always pass results by reference, even though they fit into the lowering sequence?

No -- take a look at the Failable3 example in that godbolt link -- its init? returns in registers.

@kotlarmilos
Copy link
Member Author

kotlarmilos commented Apr 4, 2024

Sorry, I am not following. Do we have a pattern for determining if a struct is lowered or not in such cases? The tooling should know when to utilize SwiftIndirectResult. Do failable initializers contain an "implicit" primitive?

@jakobbotsch
Copy link
Member

I'm not sure how optionals are represented internally in Swift and what the ABI implications of optionals are. It does not seem to be documented in any of the Swift ABI related documents that I can find.

It appears from the Godbolt link that a Swift optional essentially is a frozen type similar to

@frozen
public struct Optional<T>
{
  public let value : T;
  public let hasValue : Int8;
}

and also that a frozen struct that has member fields that aren't frozen is itself not considered frozen (makes sense). So when we see T? the projection tooling can determine whether SwiftIndirectResult is necessary by whether or not T is frozen. If T is frozen it needs to use some SwiftOptional<T> type that reflects the above layout. If T is not frozen it needs to use SwiftIndirectResult with the added hasValue bool at the end of the memory allocated.

@jakobbotsch
Copy link
Member

I don't think it will be generally possible to represent this with a generic SwiftOptional<T> type on the C# side because of the mismatches between Swift type layout and .NET type layout, so the projection tooling is going to have to create OptionalT variant of every T type it sees.

I'm not sure what the general plans around Swift generics are, but as it stands I do not see any reasonable way to represent them within the runtimes due to this layout algorithm difference. Any ideas on this issue @jkoritzinsky?

@stephen-hawley
Copy link

This is probably not the place to bring up the details of generics, but suffice it to say that I was quite successful in projecting swift generic types into C#. If you keep the payload opaque and use accessors for properties, it's very straightforward.

Optional<T> in swift is an enum, not a struct. Here's how it's implemented in BTfS.

@jakobbotsch
Copy link
Member

I think the primary complications arise when you have various combinations of frozen structs where we are designing things to avoid the overhead of dynamic allocations. For example, how do we represent the type

@frozen
public struct S
{
  public let a : Int?;
  public let b : Int8;
};

in C#? The straightforward translation is

public struct S
{
  public SwiftOptional<nint> a;
  public sbyte b;
}

but I would not expect this to result in the same type layout. I would expect the Swift frozen struct to have b start at offset 9, while I would expect the C# version to have b start at offset 16.

@stephen-hawley
Copy link

Again, I think this is beyond the scope of this discussion. The frozen struct example you give, because it contains a non-trivial enum, is not directly representable in C#, but it will still have accessor functions (swift writes these for every var/let).
Swift trivia: Int? is syntactic sugar onto to SwiftCore.Optional<Int> and since there are no "unused inhabitants" (swift term for bits within a types binary representation that is not used) in Int then it probably puts the discriminator after the size of the datatype. I say probably because swift does not document how it chooses to lay out discriminated unions and this is subject to change.

@jakobbotsch
Copy link
Member

jakobbotsch commented Apr 4, 2024

I don't think this is beyond the scope of this discussion. It has very direct impact on whether we need to reevaluate the entire design around support for frozen types. We discussed in dotnet/designs#313 how enums will be represented and whether they can be represented as C# structs.

Passing S above to a Swift function can be done in registers because S is a frozen type. It can also be returned in registers for the same reason. If we want to support frozen types, then we need runtime support for these mechanisms.

Swift trivia: Int? is syntactic sugar onto to SwiftCore.Optional and since there are no "unused inhabitants" (swift term for bits within a types binary representation that is not used) in Int then it probably puts the discriminator after the size of the datatype. I say probably because swift does not document how it chooses to lay out discriminated unions and this is subject to change.

My understanding is that the ABI was frozen in Swift 5 and that we can rely on these details not to change. Otherwise why are we trying to support frozen types at all?

@jkoritzinsky
Copy link
Member

The ABI was frozen in Swift 5. We're only supporting frozen types because we must (there's no way to avoid the layout requirements).

Even if the layout algorithm is not documented today, it's locked-in and stable enough for us to depend on, at least for frozen types. We may end up being the ones to add the documentation to Swift though if Apple hasn't already done so (not my preference).

It may be worth introducing a mechanism in .NET to say "don't add padding up to alignment at the end of the type" that we could use for Swift types. With non-generics it's easy enough, but it looks like with generics it may be too difficult to do with the tools we have today.

@AaronRobinsonMSFT
Copy link
Member

Seems like we should be able to add the unmanaged constraint?

public readonly unsafe struct SwiftSelf<T> where T: unmanaged

@kotlarmilos kotlarmilos added api-ready-for-review API is ready for review, it is NOT ready for implementation and removed api-suggestion Early API idea and discussion, it is NOT ready for implementation labels Apr 9, 2024
@kotlarmilos
Copy link
Member Author

I have marked this proposal as ready for review since it may take some time to get an API review slot. Meanwhile, we still need to resolve the issue of lowering generic frozen structs. We will reevaluate the design regarding support for frozen structs and update the existing support/this proposal if needed.

@kotlarmilos kotlarmilos added the blocking Marks issues that we want to fast track in order to unblock other important work label Apr 29, 2024
@bartonjs
Copy link
Member

bartonjs commented May 14, 2024

Video

  • Looks good as proposed
  • The original (non-generic) SwiftSelf might no longer be used and may be deleted, but if it's still used then there will just be both SwiftSelf-on-void* and SwiftSelf<T>, which is fine.
namespace System.Runtime.InteropServices.Swift
{
    public readonly struct SwiftSelf<T> where T: unmanaged
    {
        public SwiftSelf<T>(T value)
        {
            Value = value;
        }

        public T Value { get; }
    }
 
    public readonly unsafe struct SwiftIndirectResult
    {
        public SwiftIndirectResult(void* value)
        {
            Value = value;
        }

        public void* Value { get; }
    }
}

@bartonjs bartonjs added api-approved API was approved in API review, it can be implemented and removed api-ready-for-review API is ready for review, it is NOT ready for implementation blocking Marks issues that we want to fast track in order to unblock other important work labels May 14, 2024
@github-actions github-actions bot locked and limited conversation to collaborators Jul 7, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
api-approved API was approved in API review, it can be implemented area-System.Runtime.InteropServices
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

8 participants