From cb811332afdc4991396181ab2d8d142b64bc9a14 Mon Sep 17 00:00:00 2001
From: Jon Hanna <jon@hackcraft.net>
Date: Fri, 6 Jan 2017 02:47:29 +0000
Subject: [PATCH] Handle making argument to SameElements safe to enumerate
 twice within it.

Commit migrated from https://github.com/dotnet/corefx/commit/1a5a00d59488b26e67ae1b6fa06437fa5c482ada
---
 .../System/Dynamic/Utils/ExpressionUtils.cs   | 35 +++++++++++++++++--
 .../Linq/Expressions/IndexExpression.cs       |  5 +--
 .../Linq/Expressions/InvocationExpression.cs  |  5 +--
 .../Linq/Expressions/ListInitExpression.cs    |  3 +-
 .../Linq/Expressions/MemberInitExpression.cs  |  3 +-
 .../Linq/Expressions/MemberListBinding.cs     |  3 +-
 .../Linq/Expressions/MemberMemberBinding.cs   |  3 +-
 .../Linq/Expressions/NewArrayExpression.cs    |  5 +--
 .../System/Linq/Expressions/NewExpression.cs  |  5 +--
 .../Expressions/RuntimeVariablesExpression.cs |  3 +-
 .../src/System/Linq/Expressions/SwitchCase.cs |  3 +-
 .../Linq/Expressions/SwitchExpression.cs      |  3 +-
 .../System/Linq/Expressions/TryExpression.cs  |  3 +-
 13 files changed, 44 insertions(+), 35 deletions(-)

diff --git a/src/libraries/Common/src/System/Dynamic/Utils/ExpressionUtils.cs b/src/libraries/Common/src/System/Dynamic/Utils/ExpressionUtils.cs
index 4f5c40d3a69..a24daa153cf 100644
--- a/src/libraries/Common/src/System/Dynamic/Utils/ExpressionUtils.cs
+++ b/src/libraries/Common/src/System/Dynamic/Utils/ExpressionUtils.cs
@@ -227,10 +227,9 @@ internal static ParameterInfo[] GetParametersForValidation(MethodBase method, Ex
             return pis;
         }
 
-        internal static bool SameElements<T>(IEnumerable<T> replacement, IReadOnlyList<T> current) where T : class
+        internal static bool SameElements<T>(ICollection<T> replacement, IReadOnlyList<T> current) where T : class
         {
             Debug.Assert(current != null);
-            Debug.Assert(replacement == null | replacement is ICollection<T>);
             if (replacement == current) // Relatively common case, so particularly useful to take the short-circuit.
             {
                 return true;
@@ -241,8 +240,38 @@ internal static ParameterInfo[] GetParametersForValidation(MethodBase method, Ex
                 return current.Count == 0;
             }
 
+            return SameElementsInCollection(replacement, current);
+        }
+
+        internal static bool SameElements<T>(ref IEnumerable<T> replacement, IReadOnlyList<T> current) where T : class
+        {
+            Debug.Assert(current != null);
+            if (replacement == current) // Relatively common case, so particularly useful to take the short-circuit.
+            {
+                return true;
+            }
+
+            if (replacement == null) // Treat null as empty.
+            {
+                return current.Count == 0;
+            }
+
+            // Ensure arguments is safe to enumerate twice.
+            // If we have to build a collection, build a TrueReadOnlyCollection<T>
+            // so it won't be built a second time if used.
+            ICollection<T> replacementCol = replacement as ICollection<T>;
+            if (replacementCol == null)
+            {
+                replacement = replacementCol = replacement.ToReadOnly();
+            }
+
+            return SameElementsInCollection(replacementCol, current);
+        }
+
+        private static bool SameElementsInCollection<T>(ICollection<T> replacement, IReadOnlyList<T> current) where T : class
+        { 
             int count = current.Count;
-            if (((ICollection<T>)replacement).Count != count)
+            if (replacement.Count != count)
             {
                 return false;
             }
diff --git a/src/libraries/System.Linq.Expressions/src/System/Linq/Expressions/IndexExpression.cs b/src/libraries/System.Linq.Expressions/src/System/Linq/Expressions/IndexExpression.cs
index 4629779ed5c..2964fe6a83e 100644
--- a/src/libraries/System.Linq.Expressions/src/System/Linq/Expressions/IndexExpression.cs
+++ b/src/libraries/System.Linq.Expressions/src/System/Linq/Expressions/IndexExpression.cs
@@ -88,10 +88,7 @@ public IndexExpression Update(Expression @object, IEnumerable<Expression> argume
         {
             if (@object == Object & arguments != null)
             {
-                // Ensure arguments is safe to enumerate twice.
-                // (If this means a second call to ToReadOnly it will return quickly).
-                arguments = arguments as ICollection<Expression> ?? arguments.ToReadOnly();
-                if (ExpressionUtils.SameElements(arguments, Arguments))
+                if (ExpressionUtils.SameElements(ref arguments, Arguments))
                 {
                     return this;
                 }
diff --git a/src/libraries/System.Linq.Expressions/src/System/Linq/Expressions/InvocationExpression.cs b/src/libraries/System.Linq.Expressions/src/System/Linq/Expressions/InvocationExpression.cs
index 43c234adeb8..1ee944326ee 100644
--- a/src/libraries/System.Linq.Expressions/src/System/Linq/Expressions/InvocationExpression.cs
+++ b/src/libraries/System.Linq.Expressions/src/System/Linq/Expressions/InvocationExpression.cs
@@ -58,10 +58,7 @@ public InvocationExpression Update(Expression expression, IEnumerable<Expression
         {
             if (expression == Expression & arguments != null)
             {
-                // Ensure arguments is safe to enumerate twice.
-                // (If this means a second call to ToReadOnly it will return quickly).
-                arguments = arguments as ICollection<Expression> ?? arguments.ToReadOnly();
-                if (ExpressionUtils.SameElements(arguments, Arguments))
+                if (ExpressionUtils.SameElements(ref arguments, Arguments))
                 {
                     return this;
                 }
diff --git a/src/libraries/System.Linq.Expressions/src/System/Linq/Expressions/ListInitExpression.cs b/src/libraries/System.Linq.Expressions/src/System/Linq/Expressions/ListInitExpression.cs
index 30d72e7ee3e..837cbe0d681 100644
--- a/src/libraries/System.Linq.Expressions/src/System/Linq/Expressions/ListInitExpression.cs
+++ b/src/libraries/System.Linq.Expressions/src/System/Linq/Expressions/ListInitExpression.cs
@@ -86,8 +86,7 @@ public ListInitExpression Update(NewExpression newExpression, IEnumerable<Elemen
         {
             if (newExpression == NewExpression & initializers != null)
             {
-                initializers = initializers as ICollection<ElementInit> ?? initializers.ToReadOnly();
-                if (ExpressionUtils.SameElements(initializers, Initializers))
+                if (ExpressionUtils.SameElements(ref initializers, Initializers))
                 {
                     return this;
                 }
diff --git a/src/libraries/System.Linq.Expressions/src/System/Linq/Expressions/MemberInitExpression.cs b/src/libraries/System.Linq.Expressions/src/System/Linq/Expressions/MemberInitExpression.cs
index 955b35a29d3..1b292a79cdb 100644
--- a/src/libraries/System.Linq.Expressions/src/System/Linq/Expressions/MemberInitExpression.cs
+++ b/src/libraries/System.Linq.Expressions/src/System/Linq/Expressions/MemberInitExpression.cs
@@ -124,8 +124,7 @@ public MemberInitExpression Update(NewExpression newExpression, IEnumerable<Memb
         {
             if (newExpression == NewExpression & bindings != null)
             {
-                bindings = bindings as ICollection<MemberBinding> ?? bindings.ToReadOnly();
-                if (ExpressionUtils.SameElements(bindings, Bindings))
+                if (ExpressionUtils.SameElements(ref bindings, Bindings))
                 {
                     return this;
                 }
diff --git a/src/libraries/System.Linq.Expressions/src/System/Linq/Expressions/MemberListBinding.cs b/src/libraries/System.Linq.Expressions/src/System/Linq/Expressions/MemberListBinding.cs
index dbf2e0c96dd..cd86979cacd 100644
--- a/src/libraries/System.Linq.Expressions/src/System/Linq/Expressions/MemberListBinding.cs
+++ b/src/libraries/System.Linq.Expressions/src/System/Linq/Expressions/MemberListBinding.cs
@@ -39,8 +39,7 @@ public MemberListBinding Update(IEnumerable<ElementInit> initializers)
         {
             if (initializers != null)
             {
-                initializers = initializers as ICollection<ElementInit> ?? initializers.ToReadOnly();
-                if (ExpressionUtils.SameElements(initializers, Initializers))
+                if (ExpressionUtils.SameElements(ref initializers, Initializers))
                 {
                     return this;
                 }
diff --git a/src/libraries/System.Linq.Expressions/src/System/Linq/Expressions/MemberMemberBinding.cs b/src/libraries/System.Linq.Expressions/src/System/Linq/Expressions/MemberMemberBinding.cs
index fdef1d903b8..f8472ab7f9d 100644
--- a/src/libraries/System.Linq.Expressions/src/System/Linq/Expressions/MemberMemberBinding.cs
+++ b/src/libraries/System.Linq.Expressions/src/System/Linq/Expressions/MemberMemberBinding.cs
@@ -42,8 +42,7 @@ public MemberMemberBinding Update(IEnumerable<MemberBinding> bindings)
         {
             if (bindings != null)
             {
-                bindings = bindings as ICollection<MemberBinding> ?? bindings.ToReadOnly();
-                if (ExpressionUtils.SameElements(bindings, Bindings))
+                if (ExpressionUtils.SameElements(ref bindings, Bindings))
                 {
                     return this;
                 }
diff --git a/src/libraries/System.Linq.Expressions/src/System/Linq/Expressions/NewArrayExpression.cs b/src/libraries/System.Linq.Expressions/src/System/Linq/Expressions/NewArrayExpression.cs
index 6fdb85f6c09..74b0a8874d3 100644
--- a/src/libraries/System.Linq.Expressions/src/System/Linq/Expressions/NewArrayExpression.cs
+++ b/src/libraries/System.Linq.Expressions/src/System/Linq/Expressions/NewArrayExpression.cs
@@ -66,10 +66,7 @@ public NewArrayExpression Update(IEnumerable<Expression> expressions)
             // Explicit null check here as otherwise wrong parameter name will be used.
             ContractUtils.RequiresNotNull(expressions, nameof(expressions));
 
-            // Ensure expressions is safe to enumerate twice.
-            // (If this means a second call to ToReadOnly it will return quickly).
-            expressions = expressions as ICollection<Expression> ?? expressions.ToReadOnly();
-            if (ExpressionUtils.SameElements(expressions, Expressions))
+            if (ExpressionUtils.SameElements(ref expressions, Expressions))
             {
                 return this;
             }
diff --git a/src/libraries/System.Linq.Expressions/src/System/Linq/Expressions/NewExpression.cs b/src/libraries/System.Linq.Expressions/src/System/Linq/Expressions/NewExpression.cs
index ec8bf3224a0..ac648d8cba9 100644
--- a/src/libraries/System.Linq.Expressions/src/System/Linq/Expressions/NewExpression.cs
+++ b/src/libraries/System.Linq.Expressions/src/System/Linq/Expressions/NewExpression.cs
@@ -82,10 +82,7 @@ protected internal override Expression Accept(ExpressionVisitor visitor)
         /// <returns>This expression if no children changed, or an expression with the updated children.</returns>
         public NewExpression Update(IEnumerable<Expression> arguments)
         {
-            // Ensure arguments is safe to enumerate twice.
-            // (If this means a second call to ToReadOnly it will return quickly).
-            arguments = arguments as ICollection<Expression> ?? arguments.ToReadOnly();
-            if (ExpressionUtils.SameElements(arguments, Arguments))
+            if (ExpressionUtils.SameElements(ref arguments, Arguments))
             {
                 return this;
             }
diff --git a/src/libraries/System.Linq.Expressions/src/System/Linq/Expressions/RuntimeVariablesExpression.cs b/src/libraries/System.Linq.Expressions/src/System/Linq/Expressions/RuntimeVariablesExpression.cs
index 81b3d411e8c..eab4f29bc48 100644
--- a/src/libraries/System.Linq.Expressions/src/System/Linq/Expressions/RuntimeVariablesExpression.cs
+++ b/src/libraries/System.Linq.Expressions/src/System/Linq/Expressions/RuntimeVariablesExpression.cs
@@ -60,8 +60,7 @@ public RuntimeVariablesExpression Update(IEnumerable<ParameterExpression> variab
         {
             if (variables != null)
             {
-                variables = variables as ICollection<ParameterExpression> ?? variables.ToReadOnly();
-                if (ExpressionUtils.SameElements(variables, Variables))
+                if (ExpressionUtils.SameElements(ref variables, Variables))
                 {
                     return this;
                 }
diff --git a/src/libraries/System.Linq.Expressions/src/System/Linq/Expressions/SwitchCase.cs b/src/libraries/System.Linq.Expressions/src/System/Linq/Expressions/SwitchCase.cs
index c29f492ead3..85ab9a0056d 100644
--- a/src/libraries/System.Linq.Expressions/src/System/Linq/Expressions/SwitchCase.cs
+++ b/src/libraries/System.Linq.Expressions/src/System/Linq/Expressions/SwitchCase.cs
@@ -52,8 +52,7 @@ public SwitchCase Update(IEnumerable<Expression> testValues, Expression body)
         {
             if (body == Body & testValues != null)
             {
-                testValues = testValues as ICollection<Expression> ?? testValues.ToReadOnly();
-                if (ExpressionUtils.SameElements(testValues, TestValues))
+                if (ExpressionUtils.SameElements(ref testValues, TestValues))
                 {
                     return this;
                 }
diff --git a/src/libraries/System.Linq.Expressions/src/System/Linq/Expressions/SwitchExpression.cs b/src/libraries/System.Linq.Expressions/src/System/Linq/Expressions/SwitchExpression.cs
index 6b6ae214967..8cef5ba4d66 100644
--- a/src/libraries/System.Linq.Expressions/src/System/Linq/Expressions/SwitchExpression.cs
+++ b/src/libraries/System.Linq.Expressions/src/System/Linq/Expressions/SwitchExpression.cs
@@ -92,8 +92,7 @@ public SwitchExpression Update(Expression switchValue, IEnumerable<SwitchCase> c
         {
             if (switchValue == SwitchValue & defaultBody == DefaultBody & cases != null)
             {
-                cases = cases as ICollection<SwitchCase> ?? cases.ToReadOnly();
-                if (ExpressionUtils.SameElements(cases, Cases))
+                if (ExpressionUtils.SameElements(ref cases, Cases))
                 {
                     return this;
                 }
diff --git a/src/libraries/System.Linq.Expressions/src/System/Linq/Expressions/TryExpression.cs b/src/libraries/System.Linq.Expressions/src/System/Linq/Expressions/TryExpression.cs
index 7c246da2430..a4bbf28ba22 100644
--- a/src/libraries/System.Linq.Expressions/src/System/Linq/Expressions/TryExpression.cs
+++ b/src/libraries/System.Linq.Expressions/src/System/Linq/Expressions/TryExpression.cs
@@ -85,8 +85,7 @@ public TryExpression Update(Expression body, IEnumerable<CatchBlock> handlers, E
         {
             if (body == Body & @finally == Finally & fault == Fault)
             {
-                handlers = handlers as ICollection<CatchBlock> ?? handlers.ToReadOnly();
-                if (ExpressionUtils.SameElements(handlers, Handlers))
+                if (ExpressionUtils.SameElements(ref handlers, Handlers))
                 {
                     return this;
                 }
-- 
GitLab