[SwiftBindings] Added TypeMetadata, removed SwiftTypeInfo #2804
Conversation
| @@ -292,7 +293,7 @@ private List<BaseDecl> CollectDeclarations(IEnumerable<Node> nodes, BaseDecl par | |||
| { | |||
| decl = CreateClassDecl(node, parentDecl, moduleDecl); | |||
| } | |||
| typeRecord.SwiftTypeInfo = swiftTypeInfo; | |||
| typeRecord.SwiftTypeInfo = swiftTypeInfo; | |||
There was a problem hiding this comment.
This doesn't look intentional? Unindent by one space?
| /// </summary> | ||
| [Flags] | ||
| public enum TypeMetadataFlags { | ||
| /// <summary> |
There was a problem hiding this comment.
Would it make sense to add a None = 0 value, since this enum represents flags?
| MetadataKindIsNonType = 0x400, | ||
| /// <summary> | ||
| /// The metadata doesn't live on the heap | ||
| /// </summary> | ||
| MetadataKindIsNonHeap = 0x200, | ||
| /// <summary> | ||
| /// The type is private to the runtime | ||
| /// </summary> | ||
| MetadataKindIsRuntimePrivate = 0x100, |
There was a problem hiding this comment.
Is MetadataKind a common prefix for all values, or will there be more flags that aren't MetadataKind flags? If the former, the prefix should be removed:
| MetadataKindIsNonType = 0x400, | |
| /// <summary> | |
| /// The metadata doesn't live on the heap | |
| /// </summary> | |
| MetadataKindIsNonHeap = 0x200, | |
| /// <summary> | |
| /// The type is private to the runtime | |
| /// </summary> | |
| MetadataKindIsRuntimePrivate = 0x100, | |
| IsNonType = 0x400, | |
| /// <summary> | |
| /// The metadata doesn't live on the heap | |
| /// </summary> | |
| IsNonHeap = 0x200, | |
| /// <summary> | |
| /// The type is private to the runtime | |
| /// </summary> | |
| IsRuntimePrivate = 0x100, |
| get { | ||
| ThrowOnInvalid (); | ||
| long val = ReadPointerSizedInt (handle); | ||
| if (val == 0) |
There was a problem hiding this comment.
Looks like there's a mix of spaces and tabs here.
Also you can't load dylibs for other than the current platform (so you won't be able to load dylibs for iOS on macOS for instance). |
| /// <summary> | ||
| /// Represents the type metadata for a Swift type | ||
| /// </summary> | ||
| public readonly struct TypeMetadata : IEquatable<TypeMetadata> { |
There was a problem hiding this comment.
How this correlates with SwiftMetadata? Do we plan to use metadata for type semantics on the C# side or only as an unmanaged pointer for callconv?
There was a problem hiding this comment.
SwiftMetadata is based off the same type, but the difference here is that I'm not trying to define it as a ref struct but an struct with an opaque pointer so that it's easy to manipulate, compare, hash, and pass to and from p/invokes (the most common operations by far) and because it is going to escape the stack.
SwiftMetadata as is feels like a binding-time type not a runtime type.
I learned early on that we can't rely on it at binding time because of generics. It appears the only thing it gets used for at binding time is putting in a Size value in the StructLayout attribute.
For this I have several questions: 1 - do we need that? 2 - if we do, can't we get it in some other way (for example, follow the steps in the doc'n here.
In the larger picture, we need a binding-time type database, which is it's own thing, but certainly one of the things we can put into in optional properties for Size, Stride, and Alignment so that we can calculate a new Struct's size. But like I said, that's a larger discussion.
There was a problem hiding this comment.
1 - do we need that?
I think this might be relevant #2796 (comment). tldr: we need to check
There was a problem hiding this comment.
It's not really relevant since we can't know unbound generic type sizes at compile time ever. Having the Size at compile time is only beneficial for frozen structs and frozen enums.
There was a problem hiding this comment.
Thanks for bringing that in! My only concern at the moment is that we have now both TypeMetadata and SwiftMetadata which duplicate some code and as far I can understand duplicate some functionality. I think we should discuss the boundaries of responsibility / merge those two
| /// Represents the type metadata for a Swift type | ||
| /// </summary> | ||
| public readonly struct TypeMetadata : IEquatable<TypeMetadata> { | ||
| readonly IntPtr handle; |
There was a problem hiding this comment.
Will this become SwiftHandle in the future?
There was a problem hiding this comment.
That's a great question - since this is a private member and we should never (1) need to access this outside of the TypeMetadata (and I don't think we ever would) and (2) get this type from any other place that as an implicit argument passed to us from Swift or as a return from an explicit call, it's fine to leave it as an IntPtr.
My inclination is that the binding time functionality of |
This is a very simple implementation of TypeMetadata to get @jkurdek unblocked.
It includes access to the metadata kind and not a lot else for now.
The goal of this is to be used as a runtime type not a compile time type.
For the time being I removed SwiftTypeInfo which uses info pulled directly from the data structures in the dylib. We should discuss this in more detail in terms of how we want to get this information. As background, it's not a great idea to fully load a dylib at compile time since it can execute arbitrary code (see
__attribute__((constructor))) just by being loaded and may have dependencies that we don't know about or may load dependencies that conflict with the binding tool itself.TypeMetadata is designed to be used in any place that a Swift call needs a type metadata object, such as implicit arguments in a generic function or in an existential type. To get one, you should use this as the return type for a Metadata Accessor function or the appropriate Swift runtime call to generate/get them from a cache for tuples, functions etc.
Added issues to address:
#2800
#2801
#2802
#2803