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.

Sign up for GitHub

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

Make System.Guid readonly #44629

Merged
merged 5 commits into from
Nov 17, 2020
Merged

Make System.Guid readonly #44629

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
aea1f1e
Make System.Guid readonly
GrabYourPitchforks Nov 13, 2020
b9fa769
Update Guid.Unix.cs
GrabYourPitchforks Nov 13, 2020
2e42ec9
PR feedback: Make GuidResult mutable
GrabYourPitchforks Nov 13, 2020
9440168
Fix remarks in Unsafe.cs
GrabYourPitchforks Nov 14, 2020
5f60cbb
PR feedback & remove Unsafe.Unbox
GrabYourPitchforks Nov 16, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/libraries/System.Private.CoreLib/src/Resources/Strings.resx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -362,7 +362,7 @@
<value>Property Get method was not found.</value>
</data>
<data name="Arg_GuidArrayCtor" xml:space="preserve">
<value>Byte array for GUID must be exactly {0} bytes long.</value>
<value>Byte array for Guid must be exactly {0} bytes long.</value>
</data>
<data name="Arg_HandleNotAsync" xml:space="preserve">
<value>Handle does not support asynchronous operations. The parameters to the FileStream constructor may need to be changed to indicate that the handle was opened synchronously (that is, it was not opened for overlapped I/O).</value>
Expand Down
6 changes: 4 additions & 2 deletions src/libraries/System.Private.CoreLib/src/System/Guid.Unix.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using Internal.Runtime.CompilerServices;

namespace System
{
public partial struct Guid
Expand All @@ -26,9 +28,9 @@ public static unsafe Guid NewGuid()
unchecked
{
// time_hi_and_version
g._c = (short)((g._c & ~VersionMask) | RandomGuidVersion);
Unsafe.AsRef(in g._c) = (short)((g._c & ~VersionMask) | RandomGuidVersion);
// clock_seq_hi_and_reserved
g._d = (byte)((g._d & ~ClockSeqHiAndReservedMask) | ClockSeqHiAndReservedValue);
Unsafe.AsRef(in g._d) = (byte)((g._d & ~ClockSeqHiAndReservedMask) | ClockSeqHiAndReservedValue);
}

return g;
Expand Down
177 changes: 92 additions & 85 deletions src/libraries/System.Private.CoreLib/src/System/Guid.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
using System.Diagnostics;
using System.Diagnostics.CodeAnalysis;
using System.Globalization;
using System.Numerics;
using System.Runtime.CompilerServices;
using System.Runtime.InteropServices;
using System.Runtime.Versioning;
Expand All @@ -17,21 +18,21 @@ namespace System
[Serializable]
[NonVersionable] // This only applies to field layout
[TypeForwardedFrom("mscorlib, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089")]
public partial struct Guid : IFormattable, IComparable, IComparable<Guid>, IEquatable<Guid>, ISpanFormattable
public readonly partial struct Guid : IFormattable, IComparable, IComparable<Guid>, IEquatable<Guid>, ISpanFormattable
{
public static readonly Guid Empty;

private int _a; // Do not rename (binary serialization)
private short _b; // Do not rename (binary serialization)
private short _c; // Do not rename (binary serialization)
private byte _d; // Do not rename (binary serialization)
private byte _e; // Do not rename (binary serialization)
private byte _f; // Do not rename (binary serialization)
private byte _g; // Do not rename (binary serialization)
private byte _h; // Do not rename (binary serialization)
private byte _i; // Do not rename (binary serialization)
private byte _j; // Do not rename (binary serialization)
private byte _k; // Do not rename (binary serialization)
private readonly int _a; // Do not rename (binary serialization)
private readonly short _b; // Do not rename (binary serialization)
private readonly short _c; // Do not rename (binary serialization)
private readonly byte _d; // Do not rename (binary serialization)
private readonly byte _e; // Do not rename (binary serialization)
private readonly byte _f; // Do not rename (binary serialization)
private readonly byte _g; // Do not rename (binary serialization)
private readonly byte _h; // Do not rename (binary serialization)
private readonly byte _i; // Do not rename (binary serialization)
private readonly byte _j; // Do not rename (binary serialization)
private readonly byte _k; // Do not rename (binary serialization)

// Creates a new guid from an array of bytes.
public Guid(byte[] b) :
Expand Down Expand Up @@ -133,17 +134,39 @@ private enum GuidParseThrowStyle : byte
}

// This will store the result of the parsing. And it will eventually be used to construct a Guid instance.
// We'll eventually reinterpret_cast<> a GuidResult as a Guid, so we need to give it a sequential
// layout and ensure that its early fields match the layout of Guid exactly.
[StructLayout(LayoutKind.Explicit)]
private struct GuidResult
{
[FieldOffset(0)]
internal uint _a;
[FieldOffset(4)]
internal uint _bc;
[FieldOffset(4)]
internal ushort _b;
[FieldOffset(6)]
internal ushort _c;
[FieldOffset(8)]
internal uint _defg;
[FieldOffset(8)]
internal ushort _de;
[FieldOffset(8)]
internal byte _d;
[FieldOffset(10)]
internal ushort _fg;
[FieldOffset(12)]
internal uint _hijk;

[FieldOffset(16)]
private readonly GuidParseThrowStyle _throwStyle;
internal Guid _parsedGuid;

internal GuidResult(GuidParseThrowStyle canThrow) : this()
{
_throwStyle = canThrow;
}

internal void SetFailure(bool overflow, string failureMessageID)
internal readonly void SetFailure(bool overflow, string failureMessageID)
{
if (_throwStyle == GuidParseThrowStyle.None)
{
Expand All @@ -162,6 +185,12 @@ internal void SetFailure(bool overflow, string failureMessageID)

throw new FormatException(SR.GetResourceString(failureMessageID));
}

[MethodImpl(MethodImplOptions.AggressiveInlining)]
public readonly Guid ToGuid()
{
return Unsafe.As<GuidResult, Guid>(ref Unsafe.AsRef(in this));
}
}

// Creates a new guid based on the value in the string. The value is made up
Expand All @@ -182,7 +211,7 @@ public Guid(string g)
bool success = TryParseGuid(g, ref result);
Debug.Assert(success, "GuidParseThrowStyle.All means throw on all failures");

this = result._parsedGuid;
this = result.ToGuid();
}

public static Guid Parse(string input) =>
Expand All @@ -194,7 +223,7 @@ public static Guid Parse(ReadOnlySpan<char> input)
bool success = TryParseGuid(input, ref result);
Debug.Assert(success, "GuidParseThrowStyle.AllButOverflow means throw on all failures");

return result._parsedGuid;
return result.ToGuid();
}

public static bool TryParse([NotNullWhen(true)] string? input, out Guid result)
Expand All @@ -213,7 +242,7 @@ public static bool TryParse(ReadOnlySpan<char> input, out Guid result)
var parseResult = new GuidResult(GuidParseThrowStyle.None);
if (TryParseGuid(input, ref parseResult))
{
result = parseResult._parsedGuid;
result = parseResult.ToGuid();
return true;
}
else
Expand Down Expand Up @@ -249,7 +278,7 @@ public static Guid ParseExact(ReadOnlySpan<char> input, ReadOnlySpan<char> forma
_ => throw new FormatException(SR.Format_InvalidGuidFormatSpecification),
};
Debug.Assert(success, "GuidParseThrowStyle.AllButOverflow means throw on all failures");
return result._parsedGuid;
return result.ToGuid();
}

public static bool TryParseExact([NotNullWhen(true)] string? input, [NotNullWhen(true)] string? format, out Guid result)
Expand Down Expand Up @@ -300,7 +329,7 @@ public static bool TryParseExact(ReadOnlySpan<char> input, ReadOnlySpan<char> fo

if (success)
{
result = parseResult._parsedGuid;
result = parseResult.ToGuid();
return true;
}
else
Expand Down Expand Up @@ -373,33 +402,29 @@ private static bool TryParseExactD(ReadOnlySpan<char> guidString, ref GuidResult
return false;
}

ref Guid g = ref result._parsedGuid;

if (TryParseHex(guidString.Slice(0, 8), out Unsafe.As<int, uint>(ref g._a)) && // _a
if (TryParseHex(guidString.Slice(0, 8), out result._a) && // _a
TryParseHex(guidString.Slice(9, 4), out uint uintTmp)) // _b
{
g._b = (short)uintTmp;
result._b = (ushort)uintTmp;

if (TryParseHex(guidString.Slice(14, 4), out uintTmp)) // _c
{
g._c = (short)uintTmp;
result._c = (ushort)uintTmp;

if (TryParseHex(guidString.Slice(19, 4), out uintTmp)) // _d, _e
{
g._d = (byte)(uintTmp >> 8);
g._e = (byte)uintTmp;
// _d, _e must be stored as a big-endian ushort
result._de = BitConverter.IsLittleEndian ? BinaryPrimitives.ReverseEndianness((ushort)uintTmp) : (ushort)uintTmp;

if (TryParseHex(guidString.Slice(24, 4), out uintTmp)) // _f, _g
{
g._f = (byte)(uintTmp >> 8);
g._g = (byte)uintTmp;
// _f, _g must be stored as a big-endian ushort
result._fg = BitConverter.IsLittleEndian ? BinaryPrimitives.ReverseEndianness((ushort)uintTmp) : (ushort)uintTmp;

if (uint.TryParse(guidString.Slice(28, 8), NumberStyles.AllowHexSpecifier, null, out uintTmp)) // _h, _i, _j, _k
{
g._h = (byte)(uintTmp >> 24);
g._i = (byte)(uintTmp >> 16);
g._j = (byte)(uintTmp >> 8);
g._k = (byte)uintTmp;
// _h, _i, _j, _k must be stored as a big-endian uint
result._hijk = BitConverter.IsLittleEndian ? BinaryPrimitives.ReverseEndianness(uintTmp) : uintTmp;

return true;
}
Expand All @@ -422,27 +447,24 @@ private static bool TryParseExactN(ReadOnlySpan<char> guidString, ref GuidResult
return false;
}

ref Guid g = ref result._parsedGuid;

if (uint.TryParse(guidString.Slice(0, 8), NumberStyles.AllowHexSpecifier, null, out Unsafe.As<int, uint>(ref g._a)) && // _a
if (uint.TryParse(guidString.Slice(0, 8), NumberStyles.AllowHexSpecifier, null, out result._a) && // _a
uint.TryParse(guidString.Slice(8, 8), NumberStyles.AllowHexSpecifier, null, out uint uintTmp)) // _b, _c
{
g._b = (short)(uintTmp >> 16);
g._c = (short)uintTmp;
// _b, _c are independently in machine-endian order
if (BitConverter.IsLittleEndian) { uintTmp = BitOperations.RotateRight(uintTmp, 16); }
result._bc = uintTmp;

if (uint.TryParse(guidString.Slice(16, 8), NumberStyles.AllowHexSpecifier, null, out uintTmp)) // _d, _e, _f, _g
{
g._d = (byte)(uintTmp >> 24);
g._e = (byte)(uintTmp >> 16);
g._f = (byte)(uintTmp >> 8);
g._g = (byte)uintTmp;
// _d, _e, _f, _g must be stored as a big-endian uint
if (BitConverter.IsLittleEndian) { uintTmp = BinaryPrimitives.ReverseEndianness(uintTmp); }
result._defg = uintTmp;

if (uint.TryParse(guidString.Slice(24, 8), NumberStyles.AllowHexSpecifier, null, out uintTmp)) // _h, _i, _j, _k
{
g._h = (byte)(uintTmp >> 24);
g._i = (byte)(uintTmp >> 16);
g._j = (byte)(uintTmp >> 8);
g._k = (byte)uintTmp;
// _h, _i, _j, _k must be stored as big-endian uint
if (BitConverter.IsLittleEndian) { uintTmp = BinaryPrimitives.ReverseEndianness(uintTmp); }
result._hijk = uintTmp;

return true;
}
Expand Down Expand Up @@ -507,7 +529,7 @@ private static bool TryParseExactX(ReadOnlySpan<char> guidString, ref GuidResult
}

bool overflow = false;
if (!TryParseHex(guidString.Slice(numStart, numLen), out Unsafe.As<int, uint>(ref result._parsedGuid._a), ref overflow) || overflow)
if (!TryParseHex(guidString.Slice(numStart, numLen), out result._a, ref overflow) || overflow)
{
result.SetFailure(overflow, overflow ? nameof(SR.Overflow_UInt32) : nameof(SR.Format_GuidInvalidChar));
return false;
Expand All @@ -529,7 +551,7 @@ private static bool TryParseExactX(ReadOnlySpan<char> guidString, ref GuidResult
}

// Read in the number
if (!TryParseHex(guidString.Slice(numStart, numLen), out result._parsedGuid._b, ref overflow) || overflow)
if (!TryParseHex(guidString.Slice(numStart, numLen), out result._b, ref overflow) || overflow)
{
result.SetFailure(overflow, overflow ? nameof(SR.Overflow_UInt32) : nameof(SR.Format_GuidInvalidChar));
return false;
Expand All @@ -551,7 +573,7 @@ private static bool TryParseExactX(ReadOnlySpan<char> guidString, ref GuidResult
}

// Read in the number
if (!TryParseHex(guidString.Slice(numStart, numLen), out result._parsedGuid._c, ref overflow) || overflow)
if (!TryParseHex(guidString.Slice(numStart, numLen), out result._c, ref overflow) || overflow)
{
result.SetFailure(overflow, overflow ? nameof(SR.Overflow_UInt32) : nameof(SR.Format_GuidInvalidChar));
return false;
Expand Down Expand Up @@ -611,7 +633,7 @@ private static bool TryParseExactX(ReadOnlySpan<char> guidString, ref GuidResult
nameof(SR.Format_GuidInvalidChar));
return false;
}
Unsafe.Add(ref result._parsedGuid._d, i) = (byte)byteVal;
Unsafe.Add(ref result._d, i) = (byte)byteVal;
}

// Check for last '}'
Expand All @@ -631,10 +653,10 @@ private static bool TryParseExactX(ReadOnlySpan<char> guidString, ref GuidResult
return true;
}

private static bool TryParseHex(ReadOnlySpan<char> guidString, out short result, ref bool overflow)
private static bool TryParseHex(ReadOnlySpan<char> guidString, out ushort result, ref bool overflow)
{
bool success = TryParseHex(guidString, out uint tmp, ref overflow);
result = (short)tmp;
result = (ushort)tmp;
return success;
}

Expand Down Expand Up @@ -728,7 +750,7 @@ public byte[] ToByteArray()
var g = new byte[16];
if (BitConverter.IsLittleEndian)
{
MemoryMarshal.TryWrite<Guid>(g, ref this);
MemoryMarshal.TryWrite<Guid>(g, ref Unsafe.AsRef(in this));
}
else
{
Expand All @@ -742,7 +764,7 @@ public bool TryWriteBytes(Span<byte> destination)
{
if (BitConverter.IsLittleEndian)
{
return MemoryMarshal.TryWrite(destination, ref this);
return MemoryMarshal.TryWrite(destination, ref Unsafe.AsRef(in this));
}

// slower path for BigEndian
Expand All @@ -769,33 +791,27 @@ public bool TryWriteBytes(Span<byte> destination)
public override int GetHashCode()
{
// Simply XOR all the bits of the GUID 32 bits at a time.
return _a ^ Unsafe.Add(ref _a, 1) ^ Unsafe.Add(ref _a, 2) ^ Unsafe.Add(ref _a, 3);
ref int r = ref Unsafe.AsRef(in _a);
return r ^ Unsafe.Add(ref r, 1) ^ Unsafe.Add(ref r, 2) ^ Unsafe.Add(ref r, 3);
}

// Returns true if and only if the guid represented
// by o is the same as this instance.
public override bool Equals(object? o)
{
Guid g;
// Check that o is a Guid first
if (o == null || !(o is Guid))
return false;
else g = (Guid)o;
public override bool Equals(object? o) => o is Guid g && EqualsCore(this, g);

// Now compare each of the elements
return g._a == _a &&
Unsafe.Add(ref g._a, 1) == Unsafe.Add(ref _a, 1) &&
Unsafe.Add(ref g._a, 2) == Unsafe.Add(ref _a, 2) &&
Unsafe.Add(ref g._a, 3) == Unsafe.Add(ref _a, 3);
}
public bool Equals(Guid g) => EqualsCore(this, g);

public bool Equals(Guid g)
private static bool EqualsCore(in Guid left, in Guid right)
{
// Now compare each of the elements
return g._a == _a &&
Unsafe.Add(ref g._a, 1) == Unsafe.Add(ref _a, 1) &&
Unsafe.Add(ref g._a, 2) == Unsafe.Add(ref _a, 2) &&
Unsafe.Add(ref g._a, 3) == Unsafe.Add(ref _a, 3);
ref int rA = ref Unsafe.AsRef(in left._a);
ref int rB = ref Unsafe.AsRef(in right._a);

// Compare each element

return rA == rB
&& Unsafe.Add(ref rA, 1) == Unsafe.Add(ref rB, 1)
&& Unsafe.Add(ref rA, 2) == Unsafe.Add(ref rB, 2)
&& Unsafe.Add(ref rA, 3) == Unsafe.Add(ref rB, 3);
}

private static int GetResult(uint me, uint them) => me < them ? -1 : 1;
Expand Down Expand Up @@ -930,18 +946,9 @@ public int CompareTo(Guid value)
return 0;
}

public static bool operator ==(Guid a, Guid b) =>
a._a == b._a &&
Unsafe.Add(ref a._a, 1) == Unsafe.Add(ref b._a, 1) &&
Unsafe.Add(ref a._a, 2) == Unsafe.Add(ref b._a, 2) &&
Unsafe.Add(ref a._a, 3) == Unsafe.Add(ref b._a, 3);

public static bool operator !=(Guid a, Guid b) =>
// Now compare each of the elements
a._a != b._a ||
Unsafe.Add(ref a._a, 1) != Unsafe.Add(ref b._a, 1) ||
Unsafe.Add(ref a._a, 2) != Unsafe.Add(ref b._a, 2) ||
Unsafe.Add(ref a._a, 3) != Unsafe.Add(ref b._a, 3);
public static bool operator ==(Guid a, Guid b) => EqualsCore(a, b);

public static bool operator !=(Guid a, Guid b) => !EqualsCore(a, b);

public string ToString(string? format)
{
Expand Down
Loading