Make ImmutableArray readonly (dotnet#44640)

* Add reference to Unsafe. * Mark ImmutableArray as readonly struct. * Update ImmutableInterlocked to use Unsafe.As. * Also include S.R.CS.Unsafe for netstandard1.0. * Add explicit '=default' to avoid CS0649. * Use Unsafe.AsRef instead of Unsafe.As.
zhk0603 · Nov 17, 2020 · 223fd3a · 223fd3a
1 parent 184e5aa
commit 223fd3a
Show file tree

Hide file tree

Showing 6 changed files with 17 additions and 13 deletions.
diff --git a/src/libraries/System.Collections.Immutable/ref/System.Collections.Immutable.cs b/src/libraries/System.Collections.Immutable/ref/System.Collections.Immutable.cs
@@ -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; } }

diff --git a/src/libraries/System.Collections.Immutable/src/System.Collections.Immutable.csproj b/src/libraries/System.Collections.Immutable/src/System.Collections.Immutable.csproj
@@ -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" />
@@ -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>
diff --git a/...System.Collections.Immutable/src/System/Collections/Immutable/ImmutableArray_1.Minimal.cs b/...System.Collections.Immutable/src/System/Collections/Immutable/ImmutableArray_1.Minimal.cs
@@ -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

diff --git a/...braries/System.Collections.Immutable/src/System/Collections/Immutable/ImmutableArray_1.cs b/...braries/System.Collections.Immutable/src/System/Collections/Immutable/ImmutableArray_1.cs
@@ -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.

diff --git a/...ies/System.Collections.Immutable/src/System/Collections/Immutable/ImmutableInterlocked.cs b/...ies/System.Collections.Immutable/src/System/Collections/Immutable/ImmutableInterlocked.cs
@@ -3,6 +3,7 @@
 
 using System.Collections.Generic;
 using System.Diagnostics.CodeAnalysis;
+using System.Runtime.CompilerServices;
 using System.Threading;
 
 namespace System.Collections.Immutable
@@ -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.AsRef(in location.array));
             do
             {
                 ImmutableArray<T> newImmutableArray = transformer(new ImmutableArray<T>(oldArray));
@@ -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.AsRef(in location.array), newImmutableArray.array, oldArray);
                 successful = ReferenceEquals(oldArray, interlockedResult);
                 oldArray = interlockedResult; // we already have a volatile read that we can reuse for the next loop
             }
@@ -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.AsRef(in location.array));
             do
             {
                 ImmutableArray<T> newImmutableArray = transformer(new ImmutableArray<T>(oldArray), transformerArgument);
@@ -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.AsRef(in location.array), newImmutableArray.array, oldArray);
                 successful = ReferenceEquals(oldArray, interlockedResult);
                 oldArray = interlockedResult; // we already have a volatile read that we can reuse for the next loop
             }
@@ -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.AsRef(in location.array), value.array));
         }
 
         /// <summary>
@@ -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.AsRef(in location.array), value.array, comparand.array));
         }
 
         /// <summary>

diff --git a/src/libraries/System.Collections.Immutable/tests/ImmutableArrayTest.cs b/src/libraries/System.Collections.Immutable/tests/ImmutableArrayTest.cs
@@ -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);