From a037f0ca7c558e85a0d7534a30667327cc3b5603 Mon Sep 17 00:00:00 2001
From: Stephen Toub <stoub@microsoft.com>
Date: Sat, 13 Aug 2022 16:36:04 -0400
Subject: [PATCH] Streamline String.Substring (#73882)

- Split the one-arg Substring from the two-arg Substring to avoid unnecessary checks in the former
- Employ the same argument validation checks as Span, and then delegate to a helper that does more detailed checking to throw the right exception
- Avoid duplicative checks in the body
- Reorder checks in one-arg overload to do success paths before error paths where possible
---
 .../src/System/String.Manipulation.cs         | 48 ++++++++++++++-----
 1 file changed, 35 insertions(+), 13 deletions(-)

diff --git a/src/libraries/System.Private.CoreLib/src/System/String.Manipulation.cs b/src/libraries/System.Private.CoreLib/src/System/String.Manipulation.cs
index b386f7e5c36b3..e9df30a7d78ec 100644
--- a/src/libraries/System.Private.CoreLib/src/System/String.Manipulation.cs
+++ b/src/libraries/System.Private.CoreLib/src/System/String.Manipulation.cs
@@ -1798,43 +1798,65 @@ private static void CheckStringSplitOptions(StringSplitOptions options)
 
         // Returns a substring of this string.
         //
-        public string Substring(int startIndex) => Substring(startIndex, Length - startIndex);
-
-        public string Substring(int startIndex, int length)
+        public string Substring(int startIndex)
         {
-            if (startIndex < 0)
+            if (startIndex == 0)
             {
-                throw new ArgumentOutOfRangeException(nameof(startIndex), SR.ArgumentOutOfRange_StartIndex);
+                return this;
             }
 
-            if (startIndex > Length)
+            int length = Length - startIndex;
+            if (length == 0)
             {
-                throw new ArgumentOutOfRangeException(nameof(startIndex), SR.ArgumentOutOfRange_StartIndexLargerThanLength);
+                return Empty;
             }
 
-            if (length < 0)
+            if ((uint)startIndex > (uint)Length)
             {
-                throw new ArgumentOutOfRangeException(nameof(length), SR.ArgumentOutOfRange_NegativeLength);
+                ThrowSubstringArgumentOutOfRange(startIndex, length);
             }
 
-            if (startIndex > Length - length)
+            return InternalSubString(startIndex, length);
+        }
+
+        public string Substring(int startIndex, int length)
+        {
+#if TARGET_64BIT
+            // See comment in Span<T>.Slice for how this works.
+            if ((ulong)(uint)startIndex + (ulong)(uint)length > (ulong)(uint)Length)
+#else
+            if ((uint)startIndex > (uint)Length || (uint)length > (uint)(Length - startIndex))
+#endif
             {
-                throw new ArgumentOutOfRangeException(nameof(length), SR.ArgumentOutOfRange_IndexLength);
+                ThrowSubstringArgumentOutOfRange(startIndex, length);
             }
 
             if (length == 0)
             {
-                return string.Empty;
+                return Empty;
             }
 
-            if (startIndex == 0 && length == this.Length)
+            if (length == Length)
             {
+                Debug.Assert(startIndex == 0);
                 return this;
             }
 
             return InternalSubString(startIndex, length);
         }
 
+        [DoesNotReturn]
+        private void ThrowSubstringArgumentOutOfRange(int startIndex, int length)
+        {
+            (string paramName, string message) =
+                startIndex < 0 ? (nameof(startIndex), SR.ArgumentOutOfRange_StartIndex) :
+                startIndex > Length ? (nameof(startIndex), SR.ArgumentOutOfRange_StartIndexLargerThanLength) :
+                length < 0 ? (nameof(length), SR.ArgumentOutOfRange_NegativeLength) :
+                (nameof(length), SR.ArgumentOutOfRange_IndexLength);
+
+            throw new ArgumentOutOfRangeException(paramName, message);
+        }
+
         private string InternalSubString(int startIndex, int length)
         {
             Debug.Assert(startIndex >= 0 && startIndex <= this.Length, "StartIndex is out of range!");