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.

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 ImmutableArray readonly #44640

Merged
merged 6 commits into from
Nov 17, 2020
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -93,11 +93,11 @@ public static partial class ImmutableArray
public static System.Collections.Immutable.ImmutableArray<TSource> ToImmutableArray<TSource>(this System.Collections.Generic.IEnumerable<TSource> items) { throw null; }
public static System.Collections.Immutable.ImmutableArray<TSource> ToImmutableArray<TSource>(this System.Collections.Immutable.ImmutableArray<TSource>.Builder builder) { throw null; }
}
public partial struct ImmutableArray<T> : System.Collections.Generic.ICollection<T>, System.Collections.Generic.IEnumerable<T>, System.Collections.Generic.IList<T>, System.Collections.Generic.IReadOnlyCollection<T>, System.Collections.Generic.IReadOnlyList<T>, System.Collections.ICollection, System.Collections.IEnumerable, System.Collections.IList, System.Collections.Immutable.IImmutableList<T>, System.Collections.IStructuralComparable, System.Collections.IStructuralEquatable, System.IEquatable<System.Collections.Immutable.ImmutableArray<T>>
public readonly partial struct ImmutableArray<T> : System.Collections.Generic.ICollection<T>, System.Collections.Generic.IEnumerable<T>, System.Collections.Generic.IList<T>, System.Collections.Generic.IReadOnlyCollection<T>, System.Collections.Generic.IReadOnlyList<T>, System.Collections.ICollection, System.Collections.IEnumerable, System.Collections.IList, System.Collections.Immutable.IImmutableList<T>, System.Collections.IStructuralComparable, System.Collections.IStructuralEquatable, System.IEquatable<System.Collections.Immutable.ImmutableArray<T>>
{
private T[] array;
private object _dummy;
private int _dummyPrimitive;
private readonly T[] array;
private readonly object _dummy;
private readonly int _dummyPrimitive;
public static readonly System.Collections.Immutable.ImmutableArray<T> Empty;
public bool IsDefault { get { throw null; } }
public bool IsDefaultOrEmpty { get { throw null; } }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,7 @@
<Reference Include="System.Linq" />
<Reference Include="System.Resources.ResourceManager" />
<Reference Include="System.Runtime" />
<Reference Include="System.Runtime.CompilerServices.Unsafe" />
<Reference Include="System.Runtime.Extensions" />
<Reference Include="System.Runtime.InteropServices" />
<Reference Include="System.Threading" />
Expand All @@ -114,6 +115,8 @@
'$(TargetFramework)' == 'netstandard2.0' or
$(TargetFramework.StartsWith('net4'))">
<PackageReference Include="System.Memory" Version="$(SystemMemoryVersion)" />
</ItemGroup>
<ItemGroup Condition="'$(TargetFramework)' != '$(NetCoreAppCurrent)'">
<ProjectReference Include="$(LibrariesProjectRoot)System.Runtime.CompilerServices.Unsafe\src\System.Runtime.CompilerServices.Unsafe.ilproj" />
</ItemGroup>
</Project>
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ public partial struct ImmutableArray<T> : IEnumerable<T>, IEquatable<ImmutableAr
/// This would be private, but we make it internal so that our own extension methods can access it.
/// </remarks>
[DebuggerBrowsable(DebuggerBrowsableState.RootHidden)]
internal T[]? array;
internal readonly T[]? array;

/// <summary>
/// Initializes a new instance of the <see cref="ImmutableArray{T}"/> struct
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@

namespace System.Collections.Immutable
{
public partial struct ImmutableArray<T> : IReadOnlyList<T>, IList<T>, IEquatable<ImmutableArray<T>>, IList, IImmutableArray, IStructuralComparable, IStructuralEquatable, IImmutableList<T>
public readonly partial struct ImmutableArray<T> : IReadOnlyList<T>, IList<T>, IEquatable<ImmutableArray<T>>, IList, IImmutableArray, IStructuralComparable, IStructuralEquatable, IImmutableList<T>
{
/// <summary>
/// Gets or sets the element at the specified index in the read-only list.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

using System.Collections.Generic;
using System.Diagnostics.CodeAnalysis;
using System.Runtime.CompilerServices;
using System.Threading;

namespace System.Collections.Immutable
Expand Down Expand Up @@ -121,7 +122,7 @@ public static bool Update<T>(ref ImmutableArray<T> location, Func<ImmutableArray
Requires.NotNull(transformer, nameof(transformer));

bool successful;
T[]? oldArray = Volatile.Read(ref location.array);
T[]? oldArray = Volatile.Read(ref Unsafe.As<ImmutableArray<T>, T[]?>(ref location));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can this use just Unsafe.AsRef to just cast-away the readonly-ness? Unsafe.As is too big hammer.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

makes sense

do
{
ImmutableArray<T> newImmutableArray = transformer(new ImmutableArray<T>(oldArray));
Expand All @@ -131,7 +132,7 @@ public static bool Update<T>(ref ImmutableArray<T> location, Func<ImmutableArray
return false;
}

T[]? interlockedResult = Interlocked.CompareExchange(ref location.array, newImmutableArray.array, oldArray);
T[]? interlockedResult = Interlocked.CompareExchange(ref Unsafe.As<ImmutableArray<T>, T[]?>(ref location), newImmutableArray.array, oldArray);
successful = ReferenceEquals(oldArray, interlockedResult);
oldArray = interlockedResult; // we already have a volatile read that we can reuse for the next loop
}
Expand Down Expand Up @@ -165,7 +166,7 @@ public static bool Update<T, TArg>(ref ImmutableArray<T> location, Func<Immutabl
Requires.NotNull(transformer, nameof(transformer));

bool successful;
T[]? oldArray = Volatile.Read(ref location.array);
T[]? oldArray = Volatile.Read(ref Unsafe.As<ImmutableArray<T>, T[]?>(ref location));
do
{
ImmutableArray<T> newImmutableArray = transformer(new ImmutableArray<T>(oldArray), transformerArgument);
Expand All @@ -175,7 +176,7 @@ public static bool Update<T, TArg>(ref ImmutableArray<T> location, Func<Immutabl
return false;
}

T[]? interlockedResult = Interlocked.CompareExchange(ref location.array, newImmutableArray.array, oldArray);
T[]? interlockedResult = Interlocked.CompareExchange(ref Unsafe.As<ImmutableArray<T>, T[]?>(ref location), newImmutableArray.array, oldArray);
successful = ReferenceEquals(oldArray, interlockedResult);
oldArray = interlockedResult; // we already have a volatile read that we can reuse for the next loop
}
Expand All @@ -195,7 +196,7 @@ public static bool Update<T, TArg>(ref ImmutableArray<T> location, Func<Immutabl
/// <returns>The prior value at the specified <paramref name="location"/>.</returns>
public static ImmutableArray<T> InterlockedExchange<T>(ref ImmutableArray<T> location, ImmutableArray<T> value)
{
return new ImmutableArray<T>(Interlocked.Exchange(ref location.array, value.array));
return new ImmutableArray<T>(Interlocked.Exchange(ref Unsafe.As<ImmutableArray<T>, T[]?>(ref location), value.array));
}

/// <summary>
Expand All @@ -209,7 +210,7 @@ public static ImmutableArray<T> InterlockedExchange<T>(ref ImmutableArray<T> loc
/// <returns>The prior value at the specified <paramref name="location"/>.</returns>
public static ImmutableArray<T> InterlockedCompareExchange<T>(ref ImmutableArray<T> location, ImmutableArray<T> value, ImmutableArray<T> comparand)
{
return new ImmutableArray<T>(Interlocked.CompareExchange(ref location.array, value.array, comparand.array));
return new ImmutableArray<T>(Interlocked.CompareExchange(ref Unsafe.As<ImmutableArray<T>, T[]?>(ref location), value.array, comparand.array));
}

/// <summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ namespace System.Collections.Immutable.Tests
{
public class ImmutableArrayTest : SimpleElementImmutablesTestBase
{
private static readonly ImmutableArray<int> s_emptyDefault;
private static readonly ImmutableArray<int> s_emptyDefault = default; // init explicitly to avoid CS0649
private static readonly ImmutableArray<int> s_empty = ImmutableArray.Create<int>();
private static readonly ImmutableArray<int> s_oneElement = ImmutableArray.Create(1);
private static readonly ImmutableArray<int> s_manyElements = ImmutableArray.Create(1, 2, 3);
Expand Down