Skip to content

Commit

Permalink
PR feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
@elinor-fung
elinor-fung committed Apr 11, 2022
1 parent cff2b5d commit c5ae13d
Show file tree
Hide file tree
Showing 6 changed files with 95 additions and 86 deletions.
1 change: 0 additions & 1 deletion docs/design/libraries/LibraryImportGenerator/Compatibility.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,6 @@ When marshalling as [ANSI](https://docs.microsoft.com/windows/win32/intl/code-pa
- Best-fit mapping will be disabled and no exception will be thrown for unmappable characters. In the built-in system, this behaviour was configured through [`DllImportAttribute.BestFitMapping`] and [`DllImportAttribute.ThrowOnUnmappableChar`]. The generated marshalling code will have the equivalent behaviour of `BestFitMapping=false` and `ThrowOnUnmappableChar=false`.

The p/invoke source generator does not provide an equivalent to using `CharSet.Auto` in the built-in system. If platform-dependent behaviour is desired, it is left to the user to define different p/invokes with different marshalling configurations.
Similarly, `UnmanagedType.LPStr` will only mean ANSI rather than ANSI on Windows and UTF-8 on non-Windows. `UnmanagedType.LPUTF8Str` or `StringMarshalling.Utf8` can be used for UTF-8 and it is left to the user to define different p/invokes if platform-dependent behaviour is desired.

### `bool` marshalling

Expand Down
40 changes: 24 additions & 16 deletions ...braries/System.Private.CoreLib/src/System/Runtime/InteropServices/AnsiStringMarshaller.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,15 +13,15 @@ namespace System.Runtime.InteropServices
Features = CustomTypeMarshallerFeatures.UnmanagedResources | CustomTypeMarshallerFeatures.TwoStageMarshalling | CustomTypeMarshallerFeatures.CallerAllocatedBuffer)]
public unsafe ref struct AnsiStringMarshaller
{
private byte* allocated;
private Span<byte> span;
private byte* _allocated;
private readonly Span<byte> _span;

/// <summary>
/// Initializes a new instance of the <see cref="AnsiStringMarshaller"/>.
/// </summary>
/// <param name="str">The string to marshal.</param>
public AnsiStringMarshaller(string str)
: this(str, default(Span<byte>))
: this(str, default)
{ }

/// <summary>
Expand All @@ -30,26 +30,30 @@ public AnsiStringMarshaller(string str)
/// <param name="str">The string to marshal.</param>
/// <param name="buffer">Buffer that may be used for marshalling.</param>
/// <remarks>
/// The <paramref name="buffer"/> must not be movable - that is, it should not be
/// on the managed heap or it should be pinned.
/// <seealso cref="CustomTypeMarshallerFeatures.CallerAllocatedBuffer"/>
/// </remarks>
public AnsiStringMarshaller(string str, Span<byte> buffer)
{
allocated = null;
span = default;
_allocated = null;
if (str is null)
{
_span = default;
return;
}

// +1 for null terminator
// + 1 for the null character from the user. + 1 for the null character we put in.
int maxLength = (str.Length + 1) * Marshal.SystemMaxDBCSCharSize + 1;
if (buffer.Length >= maxLength)
// + 1 for null terminator
int maxByteCount = (str.Length + 1) * Marshal.SystemMaxDBCSCharSize + 1;
if (buffer.Length >= maxByteCount)
{
int length = Marshal.StringToAnsiString(str, (byte*)Unsafe.AsPointer(ref MemoryMarshal.GetReference(buffer)), buffer.Length);
span = buffer.Slice(0, length);
Marshal.StringToAnsiString(str, (byte*)Unsafe.AsPointer(ref MemoryMarshal.GetReference(buffer)), buffer.Length);
_span = buffer;
}
else
{
allocated = (byte*)Marshal.StringToCoTaskMemAnsi(str);
_allocated = (byte*)Marshal.StringToCoTaskMemAnsi(str);
_span = default;
}
}

Expand All @@ -59,7 +63,7 @@ public AnsiStringMarshaller(string str, Span<byte> buffer)
/// <remarks>
/// <seealso cref="CustomTypeMarshallerFeatures.TwoStageMarshalling"/>
/// </remarks>
public byte* ToNativeValue() => allocated != null ? allocated : (byte*)Unsafe.AsPointer(ref MemoryMarshal.GetReference(span));
public byte* ToNativeValue() => _allocated != null ? _allocated : (byte*)Unsafe.AsPointer(ref MemoryMarshal.GetReference(_span));

/// <summary>
/// Sets the native value representing the string.
Expand All @@ -68,22 +72,26 @@ public AnsiStringMarshaller(string str, Span<byte> buffer)
/// <remarks>
/// <seealso cref="CustomTypeMarshallerFeatures.TwoStageMarshalling"/>
/// </remarks>
public void FromNativeValue(byte* value) => allocated = value;
public void FromNativeValue(byte* value) => _allocated = value;

/// <summary>
/// Returns the managed string.
/// </summary>
/// <remarks>
/// <seealso cref="CustomTypeMarshallerDirection.Out"/>
/// </remarks>
public string? ToManaged() => allocated == null ? null : new string((sbyte*)allocated);
public string? ToManaged() => _allocated == null ? null : new string((sbyte*)_allocated);

/// <summary>
/// Frees native resources.
/// </summary>
/// <remarks>
/// <seealso cref="CustomTypeMarshallerFeatures.UnmanagedResources"/>
/// </remarks>
public void FreeNative() => Marshal.FreeCoTaskMem((IntPtr)allocated);
public void FreeNative()
{
if (_allocated != null)
Marshal.FreeCoTaskMem((IntPtr)_allocated);
}
}
}
52 changes: 27 additions & 25 deletions ...raries/System.Private.CoreLib/src/System/Runtime/InteropServices/Utf16StringMarshaller.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,16 +13,16 @@ namespace System.Runtime.InteropServices
Features = CustomTypeMarshallerFeatures.UnmanagedResources | CustomTypeMarshallerFeatures.TwoStageMarshalling | CustomTypeMarshallerFeatures.CallerAllocatedBuffer)]
public unsafe ref struct Utf16StringMarshaller
{
private ushort* allocated;
private Span<ushort> span;
private bool isNullString;
private ushort* _allocated;
private readonly Span<ushort> _span;
private bool _isNullString;

/// <summary>
/// Initializes a new instance of the <see cref="Utf16StringMarshaller"/>.
/// </summary>
/// <param name="str">The string to marshal.</param>
public Utf16StringMarshaller(string str)
: this(str, default(Span<ushort>))
: this(str, default)
{
}

Expand All @@ -32,29 +32,31 @@ public Utf16StringMarshaller(string str)
/// <param name="str">The string to marshal.</param>
/// <param name="buffer">Buffer that may be used for marshalling.</param>
/// <remarks>
/// The <paramref name="buffer"/> must not be movable - that is, it should not be
/// on the managed heap or it should be pinned.
/// <seealso cref="CustomTypeMarshallerFeatures.CallerAllocatedBuffer"/>
/// </remarks>
public Utf16StringMarshaller(string str, Span<ushort> buffer)
{
isNullString = false;
span = default;
_isNullString = false;
_allocated = null;
if (str is null)
{
allocated = null;
isNullString = true;
_isNullString = true;
_span = default;
}
else if ((str.Length + 1) < buffer.Length)
{
span = buffer;
str.AsSpan().CopyTo(MemoryMarshal.Cast<ushort, char>(buffer));
_span = buffer;
str.CopyTo(MemoryMarshal.Cast<ushort, char>(buffer));
// Supplied memory is in an undefined state so ensure
// there is a trailing null in the buffer.
span[str.Length] = '\0';
allocated = null;
_span[str.Length] = '\0';
}
else
{
allocated = (ushort*)Marshal.StringToCoTaskMemUni(str);
_allocated = (ushort*)Marshal.StringToCoTaskMemUni(str);
_span = default;
}
}

Expand All @@ -63,10 +65,10 @@ public Utf16StringMarshaller(string str, Span<ushort> buffer)
/// </summary>
public ref ushort GetPinnableReference()
{
if (allocated != null)
return ref Unsafe.AsRef<ushort>(allocated);
if (_allocated != null)
return ref Unsafe.AsRef<ushort>(_allocated);

return ref span.GetPinnableReference();
return ref _span.GetPinnableReference();
}

/// <summary>
Expand All @@ -75,7 +77,7 @@ public ref ushort GetPinnableReference()
/// <remarks>
/// <seealso cref="CustomTypeMarshallerFeatures.TwoStageMarshalling"/>
/// </remarks>
public ushort* ToNativeValue() => allocated != null ? allocated : (ushort*)Unsafe.AsPointer(ref MemoryMarshal.GetReference(span));
public ushort* ToNativeValue() => _allocated != null ? _allocated : (ushort*)Unsafe.AsPointer(ref MemoryMarshal.GetReference(_span));

/// <summary>
/// Sets the native value representing the string.
Expand All @@ -86,8 +88,8 @@ public ref ushort GetPinnableReference()
/// </remarks>
public void FromNativeValue(ushort* value)
{
allocated = value;
isNullString = value == null;
_allocated = value;
_isNullString = value == null;
}

/// <summary>
Expand All @@ -98,13 +100,13 @@ public void FromNativeValue(ushort* value)
/// </remarks>
public string? ToManaged()
{
if (isNullString)
if (_isNullString)
return null;

if (allocated != null)
return new string((char*)allocated);
if (_allocated != null)
return new string((char*)_allocated);

return MemoryMarshal.Cast<ushort, char>(span).ToString();
return MemoryMarshal.Cast<ushort, char>(_span).ToString();
}

/// <summary>
Expand All @@ -115,8 +117,8 @@ public void FromNativeValue(ushort* value)
/// </remarks>
public void FreeNative()
{
if (allocated != null)
Marshal.FreeCoTaskMem((IntPtr)allocated);
if (_allocated != null)
Marshal.FreeCoTaskMem((IntPtr)_allocated);
}
}
}
50 changes: 26 additions & 24 deletions ...braries/System.Private.CoreLib/src/System/Runtime/InteropServices/Utf8StringMarshaller.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,23 +14,15 @@ namespace System.Runtime.InteropServices
Features = CustomTypeMarshallerFeatures.UnmanagedResources | CustomTypeMarshallerFeatures.TwoStageMarshalling | CustomTypeMarshallerFeatures.CallerAllocatedBuffer)]
public unsafe ref struct Utf8StringMarshaller
{
private byte* allocated;
private Span<byte> span;

// Conversion from a 2-byte 'char' in UTF-16 to bytes in UTF-8 has a maximum of 3 bytes per 'char'
// Two bytes ('char') in UTF-16 can be either:
// - Code point in the Basic Multilingual Plane: all 16 bits are that of the code point
// - Part of a pair for a code point in the Supplementary Planes: 10 bits are that of the code point
// In UTF-8, 3 bytes are need to represent the code point in first and 4 bytes in the second. Thus, the
// maximum number of bytes per 'char' is 3.
private const int MaxByteCountPerChar = 3;
private byte* _allocated;
private readonly Span<byte> _span;

/// <summary>
/// Initializes a new instance of the <see cref="Utf8StringMarshaller"/>.
/// </summary>
/// <param name="str">The string to marshal.</param>
public Utf8StringMarshaller(string str)
: this(str, default(Span<byte>))
: this(str, default)
{ }

/// <summary>
Expand All @@ -39,27 +31,37 @@ public Utf8StringMarshaller(string str)
/// <param name="str">The string to marshal.</param>
/// <param name="buffer">Buffer that may be used for marshalling.</param>
/// <remarks>
/// The <paramref name="buffer"/> must not be movable - that is, it should not be
/// on the managed heap or it should be pinned.
/// <seealso cref="CustomTypeMarshallerFeatures.CallerAllocatedBuffer"/>
/// </remarks>
public Utf8StringMarshaller(string str, Span<byte> buffer)
{
allocated = null;
span = default;
_allocated = null;
if (str is null)
{
_span = default;
return;
}

// + 1 for number of characters in case left over high surrogate is ?
// * <MaxByteCountPerChar> (3 for UTF-8)
// +1 for null terminator
if (buffer.Length >= (str.Length + 1) * MaxByteCountPerChar + 1)
// + 1 for null terminator
int maxByteCount = Encoding.UTF8.GetMaxByteCount(str.Length) + 1;
if (buffer.Length >= maxByteCount)
{
int byteCount = Encoding.UTF8.GetBytes(str, buffer);
buffer[byteCount] = 0; // null-terminate
span = buffer;
_span = buffer;
}
else
{
allocated = (byte*)Marshal.StringToCoTaskMemUTF8(str);
_allocated = (byte*)Marshal.AllocCoTaskMem(maxByteCount);
int byteCount;
fixed (char* ptr = str)
{
byteCount = Encoding.UTF8.GetBytes(ptr, str.Length, _allocated, maxByteCount);
}
_allocated[byteCount] = 0; // null-terminate
_span = default;
}
}

Expand All @@ -69,7 +71,7 @@ public Utf8StringMarshaller(string str, Span<byte> buffer)
/// <remarks>
/// <seealso cref="CustomTypeMarshallerFeatures.TwoStageMarshalling"/>
/// </remarks>
public byte* ToNativeValue() => allocated != null ? allocated : (byte*)Unsafe.AsPointer(ref MemoryMarshal.GetReference(span));
public byte* ToNativeValue() => _allocated != null ? _allocated : (byte*)Unsafe.AsPointer(ref MemoryMarshal.GetReference(_span));

/// <summary>
/// Sets the native value representing the string.
Expand All @@ -78,15 +80,15 @@ public Utf8StringMarshaller(string str, Span<byte> buffer)
/// <remarks>
/// <seealso cref="CustomTypeMarshallerFeatures.TwoStageMarshalling"/>
/// </remarks>
public void FromNativeValue(byte* value) => allocated = value;
public void FromNativeValue(byte* value) => _allocated = value;

/// <summary>
/// Returns the managed string.
/// </summary>
/// <remarks>
/// <seealso cref="CustomTypeMarshallerDirection.Out"/>
/// </remarks>
public string? ToManaged() => allocated == null ? null : Marshal.PtrToStringUTF8((IntPtr)allocated);
public string? ToManaged() => _allocated == null ? null : Marshal.PtrToStringUTF8((IntPtr)_allocated);

/// <summary>
/// Frees native resources.
Expand All @@ -96,8 +98,8 @@ public Utf8StringMarshaller(string str, Span<byte> buffer)
/// </remarks>
public void FreeNative()
{
if (allocated != null)
Marshal.FreeCoTaskMem((IntPtr)allocated);
if (_allocated != null)
Marshal.FreeCoTaskMem((IntPtr)_allocated);
}
}
}
Loading

0 comments on commit c5ae13d

Please sign in to comment.