[Swift language feature] Implement Swift.String wrapper in C#

[kotlarmilos]

Description

This PR projects Swift.String used in products(for:).

In Swift, String is defined as frozen struct String with an internal payload of Swift.Data. The projection is a thin C# wrapper around Swift.Data, with methods to convert between UTF-8 and UTF-16 encodings.

Out of Scope

• In Swift, the string payload requires release of internals, which is not implemented in this PR. The plan is to review and refactor memory handling of projected types as outlined in [Swift ABI compatibility] Implement memory ownership between Swift and C# bindings #2851.
• This projection does not expose the full API surface of Swift.String.

Fixes #2831

[vitek-karas]

I think we should have a discussion if we want to project string (which is what this PR does) or if we want to always marshal string. Personal preference would be to marshal string whenever possible (params, return value), but I guess we need the projection just like we will likely need boxes for other primitive types (mainly due to protocol conformance).
Either way we should have this decision discussed and written down somewhere.

[kotlarmilos]

I think we should have a discussion if we want to project string

Let's collect input on this PR.

Personal preference would be to marshal string whenever possible (params, return value)

Could you provide more details?

I guess we need the projection just like we will likely need boxes for other primitive types (mainly due to protocol conformance).

Yes + it is lowered as (i64, ptr).

[vitek-karas]

More details on the personal preference:
Let's say we have a swift code like this:

Label("Your name please:")  // Assuming Label init takes a string parameter, which it doesn't, but it's a good example

When projected to C# I would want to be able to write:

new Label("Your name please:");

Basically I should not be forced to write things like:

new Label(new SwiftString("Your name please:"));

But we might be able to achieve this with implicit casts.

[kotlarmilos]

When projected to C# I would want to be able to write:

new Label("Your name please:");

Yes, this is a UX decision.

The preferred Swift encoding is UTF-8: https://www.swift.org/blog/utf8-string/. Currently, we access it through ContiguousArray and use a closure to access the array’s contiguous storage.

[jkurdek]

I understand this is here as it requires some swift code. But I dont think the label 'struct tests' describes it very well. Maybe we should create a new test file?

[kotlarmilos]

Yes, sounds good.

[jkurdek]

NIT: This is used outside of string extensively right? I think we encountered pointers using that when projecting crypto kit - maybe it deserves it own file

[kotlarmilos]

Yes, sounds good.

[kotlarmilos]

Moved and created a tracking issue: #2992.

[jkotas]

How much do you care about performance?

This will allocate a marshalled delegate on every call that is quite expensive operation. It would be a lot more efficient to use function pointers and use the context argument to pass the local context.

[kotlarmilos]

Yes, this is a good point. We should implement the correct layout and memory management for the closure context. Also, we should be able to reduce the number of calls using a thin wrapper.

Created a tracking issue: #2990

[jkotas]

Doing this properly using function pointers is like 10-line edit on the new code added in this PR...

[kotlarmilos]

Ok, let's fix that in this PR.

[kotlarmilos]

Follow-up tasks:

• [Swift ABI compatibility] Implement memory ownership between Swift and C# bindings #2851
• [Swift language features] Implement Foundation.Data in C# #2992

[jkotas]

Should the callback have default calling convention or Swift calling convention? (Related to my other comment about function pointers.)

[jkotas]

Also, should the callback signature have the closure context as the argument?

[kotlarmilos]

Yes.

ToString has been refactored to use function pointers. The length is passed through the closure context to the callback via the self register. A utf8 buffer is allocated, and the content of the contiguous array is copied into it. The buffer is then marshalled using PtrToStringUTF8, and the native buffer is deallocated.

[jkotas]

This is a better way to do it without an extra copy

[kotlarmilos]

This is a better way to do it without an extra copy

Changed ToStringCallbackContext to a ref type to allow the callback to take an unmanaged pointer, as it contains managed fields.

[vitek-karas]

Thanks!

[kotlarmilos]

@jkotas Thanks for help! If you have any additional feedback regarding the function pointers, please let me know and we can address them in a follow-up.

[jkotas]

Changed ToStringCallbackContext to a ref type to allow the callback to take an unmanaged pointer, as it contains managed fields.

That's just an annoying C# warning. It is fine to disable it. We have it disabled globally in dotnet/runtime. We use the pattern that I have suggested in many places. One example from many: https://github.com/dotnet/runtime/blob/c6fd0543b67aec813b16fc9458afd2ec2716178b/src/libraries/System.Private.CoreLib/src/System/Globalization/CultureData.Nls.cs#L370-L403

There is nothing wrong with taking an address of a struct on stack that contains object references. You just need to be careful about what you are doing - like you need to be careful everywhere else in unsafe code. GCHandle and the reference type instead of struct are just a bunch of unnecessary overhead.