-
Notifications
You must be signed in to change notification settings - Fork 204
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
[Swift Binding] Projecting frozen structs #2797
Comments
Nice findings -- There are two ways to resolve this:
We need to decide which tradeoff to make. Implementing this in the projections would bring part of the ABI into the projections. Handling it at the runtime level would require an annotation for structs to indicate if it is frozen. Let's try to align this with discussion in #2796 (comment). /cc: @jkoritzinsky |
@kotlarmilos I did show incorrect code earlier, I edited the issue - now it should be ok. I think now it is clear that the layout is not know until runtime |
Correct, I confirmed that
I'm okay with these rules. Let's see how generics are resolved, and if they don't require runtime-level handling, I think we can proceed with this change. |
Bringing the PR discussion in: Let's treat representation and runtime lowering separately for a moment. If a frozen struct doesn't require a copy constructor when moved in memory, I would still consider it a value type (struct). Additionally, as we discussed offline, the projection will flip if a generic used becomes frozen. From the user’s perspective, this can occur without their control, which is my primary concern when introducing this rule. I understand that it doesn't match the lowering on the ABI level. However, I see this as implementation detail that belongs to the runtime. @amanasifkhalid I would like to hear your perspective. Originally posted by @kotlarmilos in #2796 (comment) I think the problem is that user-facing C# Originally posted by @matouskozak in #2796 (comment)
So for generics strictly. I think we cannot tell at compile time whether a generic will have or wont have to use a copy constructor as we have no idea what the generic parameter will be. Some instances of the generic, @matouskozak I think you got it right.
Making something non-frozen frozen is an ABI breaking change. I do not think we can do much about it. One way of solving this would be representing every struct as a class on C# side and abstracting the implementation details
Do you mean every generic struct? In this case, the size and alignment of an object of opaque layout, as well as whether it is trivial or bitwise movable, is determined by querying its value witness table. If contains frozen structs, the helper can be used. Originally posted by @kotlarmilos in #2796 (comment) I meant all structs. Its more of stretch idea thing than actual proposal. I feel that projecting some as structs and some as classes creates a weird UX for developers who are not Swift professionals. I would prefer to have uniformity - e.g. all structs to be classes. I do understand however that this would require a lot custom runtime work and would bring its own set of problems. Anyway it is a different discussion. Originally posted by @jkurdek in #2796 (comment)
It seems we need this layer of abstraction for structs with opaque layouts (those not known until runtime). In other cases, this abstraction isn’t necessary. We should prioritize simplicity, ensuring that users reading Swift docs know what to expect. If a struct can be projected directly, we should do so.
Let's do cost estimate for this. /cc: @jkoritzinsky Originally posted by @kotlarmilos in #2796 (comment) |
@jakobbotsch can give you a more informed perspective on CoreCLR's Swift lowering implementation. |
@kotlarmilos. I agree with the simplicity part. But I don't think I agree with the "know what to expect" part. I feel that we are currently flowing ABI details into public API. The developer might look into the Swift docs -- see two different |
This is legitimate concern. Let's look at data then. I run a brief search over StoreKit, SwiftUI, and Foundation. StoreKit: 40 structs / 20 enums / 0 classes @frozen public enum PriceIncreaseStatus
@frozen public enum VerificationResult<SignedType> SwiftUI: 691 structs / 42 enums / 13 classes @frozen internal struct LimitedAvailabilityTableColumnContent<TableRowValue, TableColumnSortComparator> : SwiftUI.TableColumnContent where TableRowValue : Swift.Identifiable, TableColumnSortComparator : Foundation.SortComparator {
@frozen public struct SequenceGesture<First, Second> : SwiftUICore.Gesture where First : SwiftUICore.Gesture, Second : SwiftUICore.Gesture {
@frozen public struct SubscriptionView<PublisherType, Content> : SwiftUICore.View where PublisherType : Combine.Publisher, Content : SwiftUICore.View, PublisherType.Failure == Swift.Never {
@propertyWrapper @frozen public struct GestureState<Value> : SwiftUICore.DynamicProperty {
@frozen public struct GestureStateGesture<Base, State> : SwiftUICore.Gesture where Base : SwiftUICore.Gesture {
@frozen public struct _DiscreteSymbolEffectModifier<T> : SwiftUICore.ViewModifier, SwiftUICore._GraphInputsModifier where T : Swift.Equatable {
@frozen public struct _TaskValueModifier<Value> : SwiftUICore.ViewModifier where Value : Swift.Equatable {
@frozen public struct _GeometryActionModifier<Value> where Value : Swift.Equatable {
@frozen public struct _GeometryActionModifier2<Value> where Value : Swift.Equatable {
@frozen public struct _ContentShapeKindModifier<ContentShape> : SwiftUICore.ViewModifier where ContentShape : SwiftUICore.Shape {
@frozen public struct _PreferenceActionModifier<Key> : SwiftUICore.ViewModifier where Key : SwiftUICore.PreferenceKey, Key.Value : Swift.Equatable {
@frozen @propertyWrapper public struct SceneStorage<Value> : SwiftUICore.DynamicProperty {
@frozen @propertyWrapper public struct AppStorage<Value> : SwiftUICore.DynamicProperty {
@frozen @_Concurrency.MainActor @preconcurrency internal struct AccessibilityTupleRotorContent<T> : SwiftUI.AccessibilityRotorContent {
@frozen @_Concurrency.MainActor @preconcurrency internal struct AccessibilityOptionalRotorContent<Content> where Content : SwiftUI.AccessibilityRotorContent {
@frozen @propertyWrapper public struct FocusState<Value> : SwiftUICore.DynamicProperty where Value : Swift.Hashable {
@frozen @_Concurrency.MainActor @preconcurrency internal struct _TupleTabContent<T, U> where T : Swift.Hashable {
@frozen public struct _AnchorWritingModifier<AnchorValue, Key> : SwiftUICore.ViewModifier where Key : SwiftUICore.PreferenceKey {
@frozen public struct ViewThatFits<Content> : SwiftUICore.View where Content : SwiftUICore.View {
@frozen public struct _CoordinateSpaceModifier<Name> : SwiftUICore.ViewModifier, Swift.Equatable where Name : Swift.Hashable {
@frozen public struct EquatableView<Content> : SwiftUICore.View where Content : Swift.Equatable, Content : SwiftUICore.View {
@frozen public struct _ScrollViewBoundsModifier<Result> : SwiftUICore.ViewModifier where Result : SwiftUICore.ViewModifier {
@frozen public struct _ScrollViewBoundsModifier2<Result> : SwiftUICore.ViewModifier where Result : SwiftUICore.ViewModifier {
@frozen public struct Grid<Content> where Content : SwiftUICore.View {
@frozen public struct GridRow<Content> where Content : SwiftUICore.View {
@frozen public struct _AnchorTransformModifier<AnchorValue, Key> : SwiftUICore.ViewModifier where Key : SwiftUICore.PreferenceKey {
@frozen public struct _ContentShapeModifier<ContentShape> : SwiftUICore.ViewModifier where ContentShape : SwiftUICore.Shape {
@frozen public struct _IdentifiedModifier<Identifier> : SwiftUICore.ViewModifier, Swift.Equatable where Identifier : Swift.Hashable {
@frozen public struct ScrollViewReader<Content> : SwiftUICore.View where Content : SwiftUICore.View {
@frozen public struct TupleTableColumnContent<RowValue, Sort, T> : SwiftUI.TableColumnContent where RowValue : Swift.Identifiable, Sort : Foundation.SortComparator {
@frozen public struct _ShadowView<Content> : SwiftUICore.View where Content : SwiftUICore.Shape {
@frozen public struct _IgnoredByLayoutEffect<Base> : SwiftUICore.GeometryEffect where Base : SwiftUICore.GeometryEffect {
@propertyWrapper @frozen public struct AccessibilityFocusState<Value> : SwiftUICore.DynamicProperty where Value : Swift.Hashable {
@frozen public struct TupleTableRowContent<Value, T> : SwiftUI.TableRowContent where Value : Swift.Identifiable {
@frozen public struct _ContainerValueWritingModifier<Value> {
@frozen @propertyWrapper @preconcurrency @_Concurrency.MainActor public struct FocusedObject<ObjectType> : SwiftUICore.DynamicProperty where ObjectType : Combine.ObservableObject { Foundation: 269 structs / 100 enums / 7 classes @jkurdek Could you try to estimate the number of non-frozen nested types? It appears that frozen generic types are a minority in both StoreKit and SwiftUI. I think we need more data before deciding to proceed with all-struct-to-class projections. |
Note
This was edited to contain correct code. The previous version of the code contained incorrect LLVM-IR (did not account for crossing the module boundary).
So far we have considered frozen structs to have known layout at compile time and thus we projected them into c# as structs. However not every frozen struct's layout will be know at compile time. (this also applies to generic types to #2796)
Lets consider the following Swift code:
FrozenStruct contains a field of type NonFrozenStruct. This makes its layout and size unknown at compile time.
Swift will pass this struct as if it was non-frozen.
Below we can see the llvm-ir for the returnData struct. Frozen struct will be passed usinga a pointer, and the function will utilise
IndirectResult
.As the layout is unknown at compile time, the correct way to handle this on C# side would be to represent
FrozenStruct
as a class.This means that we need to revisit the rules when we can project Swift struct to C# struct.
Following https://github.com/dotnet/designs/blob/main/proposed/swift-interop.md#structsvalue-types the rules to project Swift struct into c# struct will be:
\cc @kotlarmilos @stephen-hawley @amanasifkhalid @matouskozak
The text was updated successfully, but these errors were encountered: