Improve ThrowHelper (#7048)

diff --git a/src/mscorlib/src/System/Array.cs b/src/mscorlib/src/System/Array.cs
index 3f175d9159c7b4eeaa7b7cd2885044a982ac52bc..a97ce9d53301bb717e923741c9e97ef4af9f6593 100644 (file)
--- a/src/mscorlib/src/System/Array.cs
+++ b/src/mscorlib/src/System/Array.cs
@@ -2714,7 +2714,7 @@ namespace System {
             //! or you may introduce a security hole!
             T[] _this = JitHelpers.UnsafeCast<T[]>(this);
             if ((uint)index >= (uint)_this.Length) {
-                ThrowHelper.ThrowArgumentOutOfRangeException();
+                ThrowHelper.ThrowArgumentOutOfRange_IndexException();
             }
 
             return _this[index];
@@ -2726,7 +2726,7 @@ namespace System {
             //! or you may introduce a security hole!
             T[] _this = JitHelpers.UnsafeCast<T[]>(this);
             if ((uint)index >= (uint)_this.Length) {
-                ThrowHelper.ThrowArgumentOutOfRangeException();
+                ThrowHelper.ThrowArgumentOutOfRange_IndexException();
             }
 
             _this[index] = value;

diff --git a/src/mscorlib/src/System/Collections/Generic/List.cs b/src/mscorlib/src/System/Collections/Generic/List.cs
index 6e954c03ed7be43b6e565d01ab4cb3beb3f10485..ae3356d37273f64cee848bcb05207b03f90027e1 100644 (file)
--- a/src/mscorlib/src/System/Collections/Generic/List.cs
+++ b/src/mscorlib/src/System/Collections/Generic/List.cs
@@ -174,7 +174,7 @@ namespace System.Collections.Generic {
             get {
                 // Following trick can reduce the range check by one
                 if ((uint) index >= (uint)_size) {
-                    ThrowHelper.ThrowArgumentOutOfRangeException();
+                    ThrowHelper.ThrowArgumentOutOfRange_IndexException();
                 }
                 Contract.EndContractBlock();
                 return _items[index]; 
@@ -182,7 +182,7 @@ namespace System.Collections.Generic {
 
             set {
                 if ((uint) index >= (uint)_size) {
-                    ThrowHelper.ThrowArgumentOutOfRangeException();
+                    ThrowHelper.ThrowArgumentOutOfRange_IndexException();
                 }
                 Contract.EndContractBlock();
                 _items[index] = value;
@@ -870,7 +870,7 @@ namespace System.Collections.Generic {
         // 
         public void RemoveAt(int index) {
             if ((uint)index >= (uint)_size) {
-                ThrowHelper.ThrowArgumentOutOfRangeException();
+                ThrowHelper.ThrowArgumentOutOfRange_IndexException();
             }
             Contract.EndContractBlock();
             _size--;

diff --git a/src/mscorlib/src/System/Collections/ObjectModel/Collection.cs b/src/mscorlib/src/System/Collections/ObjectModel/Collection.cs
index b9ed1a15f98a82e051cd8bca8fbd211674810c77..54aa7bb09db090a31020e1fee889046635dd1ecb 100644 (file)
--- a/src/mscorlib/src/System/Collections/ObjectModel/Collection.cs
+++ b/src/mscorlib/src/System/Collections/ObjectModel/Collection.cs
@@ -49,7 +49,7 @@ namespace System.Collections.ObjectModel
                 }
                 
                 if (index < 0 || index >= items.Count) {
-                    ThrowHelper.ThrowArgumentOutOfRangeException();
+                    ThrowHelper.ThrowArgumentOutOfRange_IndexException();
                 }
 
                 SetItem(index, value);
@@ -118,7 +118,7 @@ namespace System.Collections.ObjectModel
             }
 
             if (index < 0 || index >= items.Count) {
-                ThrowHelper.ThrowArgumentOutOfRangeException();
+                ThrowHelper.ThrowArgumentOutOfRange_IndexException();
             }
 
             RemoveItem(index);

diff --git a/src/mscorlib/src/System/ThrowHelper.cs b/src/mscorlib/src/System/ThrowHelper.cs
index e1cdd30081af1cbe1a5a6579d4a94eff7c22b7e4..f3271769cc25f4abab4a9c6347b2588c556ec14b 100644 (file)
--- a/src/mscorlib/src/System/ThrowHelper.cs
+++ b/src/mscorlib/src/System/ThrowHelper.cs
@@ -42,8 +42,9 @@ namespace System {
 
     [Pure]
     internal static class ThrowHelper {    
-        internal static void ThrowArgumentOutOfRangeException() {        
-            ThrowArgumentOutOfRangeException(ExceptionArgument.index, ExceptionResource.ArgumentOutOfRange_Index);            
+        internal static void ThrowArgumentOutOfRange_IndexException() {
+            throw new ArgumentOutOfRangeException(GetArgumentName(ExceptionArgument.index),
+                                                    Environment.GetResourceString(GetResourceName(ExceptionResource.ArgumentOutOfRange_Index)));
         }
 
         internal static void ThrowWrongKeyTypeArgumentException(object key, Type targetType) {
@@ -110,9 +111,12 @@ namespace System {
         }
 
         // Allow nulls for reference types and Nullable<U>, but not for value types.
+        // Aggressively inline so the jit evaluates the if in place and either drops the call altogether
+        // Or just leaves null test and call to the Non-returning ThrowHelper.ThrowArgumentNullException
+        [MethodImpl(MethodImplOptions.AggressiveInlining)]
         internal static void IfNullAndNullsAreIllegalThenThrow<T>(object value, ExceptionArgument argName) {
             // Note that default(T) is not equal to null for value types except when T is Nullable<U>. 
-            if (value == null && !(default(T) == null))
+            if (!(default(T) == null) && value == null)
                 ThrowHelper.ThrowArgumentNullException(argName);
         }
 
@@ -120,320 +124,20 @@ namespace System {
         // This function will convert an ExceptionArgument enum value to the argument name string.
         //
         internal static string GetArgumentName(ExceptionArgument argument) {
-            string argumentName = null;
+            Contract.Assert(Enum.IsDefined(typeof(ExceptionArgument), argument),
+                "The enum value is not defined, please check the ExceptionArgument Enum.");
 
-            switch (argument) {
-                case ExceptionArgument.action:
-                    argumentName = "action";
-                    break;
-
-                case ExceptionArgument.array:
-                    argumentName = "array";
-                    break;
-
-                case ExceptionArgument.arrayIndex:
-                    argumentName = "arrayIndex";
-                    break;
-
-                case ExceptionArgument.capacity:
-                    argumentName = "capacity";
-                    break;
-
-                case ExceptionArgument.collection:
-                    argumentName = "collection";
-                    break;
-
-                case ExceptionArgument.comparison:
-                    argumentName = "comparison";
-                    break;
-
-                case ExceptionArgument.list:
-                    argumentName = "list";
-                    break;
-
-                case ExceptionArgument.converter:
-                    argumentName = "converter";
-                    break;
-
-                case ExceptionArgument.count:
-                    argumentName = "count";
-                    break;
-
-                case ExceptionArgument.dictionary:
-                    argumentName = "dictionary";
-                    break;
-
-                case ExceptionArgument.dictionaryCreationThreshold:
-                    argumentName = "dictionaryCreationThreshold";
-                    break;
-
-                case ExceptionArgument.index:
-                    argumentName = "index";
-                    break;
-
-                case ExceptionArgument.info:
-                    argumentName = "info";
-                    break;
-
-                case ExceptionArgument.key:
-                    argumentName = "key";
-                    break;
-
-                case ExceptionArgument.match:
-                    argumentName = "match";
-                    break;
-
-                case ExceptionArgument.obj:
-                    argumentName = "obj";
-                    break;
-
-                case ExceptionArgument.queue:
-                    argumentName = "queue";
-                    break;
-
-                case ExceptionArgument.stack:
-                    argumentName = "stack";
-                    break;
-
-                case ExceptionArgument.startIndex:
-                    argumentName = "startIndex";
-                    break;
-
-                case ExceptionArgument.value:
-                    argumentName = "value";
-                    break;
-
-                case ExceptionArgument.name:
-                    argumentName = "name";
-                    break;
-
-                case ExceptionArgument.mode:
-                    argumentName = "mode";
-                    break;
-
-                case ExceptionArgument.item:
-                    argumentName = "item";
-                    break;
-
-                case ExceptionArgument.options:
-                    argumentName = "options";
-                    break;
-
-                case ExceptionArgument.view:
-                    argumentName = "view";
-                    break;
-
-               case ExceptionArgument.sourceBytesToCopy:
-                    argumentName = "sourceBytesToCopy";
-                    break;
-
-                default:
-                    Contract.Assert(false, "The enum value is not defined, please checked ExceptionArgumentName Enum.");
-                    return string.Empty;
-            }
-
-            return argumentName;
+            return argument.ToString();
         }
 
         //
         // This function will convert an ExceptionResource enum value to the resource string.
         //
         internal static string GetResourceName(ExceptionResource resource) {
-            string resourceName = null;
-
-            switch (resource) {
-                case ExceptionResource.Argument_ImplementIComparable:
-                    resourceName = "Argument_ImplementIComparable";
-                    break;
-
-                case ExceptionResource.Argument_AddingDuplicate:
-                    resourceName = "Argument_AddingDuplicate";
-                    break;
-
-                case ExceptionResource.ArgumentOutOfRange_BiggerThanCollection:
-                    resourceName = "ArgumentOutOfRange_BiggerThanCollection";
-                    break;
-
-                case ExceptionResource.ArgumentOutOfRange_Count:
-                    resourceName = "ArgumentOutOfRange_Count";
-                    break;
-
-                case ExceptionResource.ArgumentOutOfRange_Index:
-                    resourceName = "ArgumentOutOfRange_Index";
-                    break;
-
-                case ExceptionResource.ArgumentOutOfRange_InvalidThreshold:
-                    resourceName = "ArgumentOutOfRange_InvalidThreshold";
-                    break;
-
-                case ExceptionResource.ArgumentOutOfRange_ListInsert:
-                    resourceName = "ArgumentOutOfRange_ListInsert";
-                    break;
-
-                case ExceptionResource.ArgumentOutOfRange_NeedNonNegNum:
-                    resourceName = "ArgumentOutOfRange_NeedNonNegNum";
-                    break;
-
-                case ExceptionResource.ArgumentOutOfRange_SmallCapacity:
-                    resourceName = "ArgumentOutOfRange_SmallCapacity";
-                    break;
-
-                case ExceptionResource.Arg_ArrayPlusOffTooSmall:
-                    resourceName = "Arg_ArrayPlusOffTooSmall";
-                    break;
-
-                case ExceptionResource.Arg_RankMultiDimNotSupported:
-                    resourceName = "Arg_RankMultiDimNotSupported";
-                    break;
-
-                case ExceptionResource.Arg_NonZeroLowerBound:
-                    resourceName = "Arg_NonZeroLowerBound";
-                    break;
-
-                case ExceptionResource.Argument_InvalidArrayType:
-                    resourceName = "Argument_InvalidArrayType";
-                    break;
-
-                case ExceptionResource.Argument_InvalidOffLen:
-                    resourceName = "Argument_InvalidOffLen";
-                    break;
-
-                case ExceptionResource.Argument_ItemNotExist:
-                    resourceName = "Argument_ItemNotExist";
-                    break;                    
-
-                case ExceptionResource.InvalidOperation_CannotRemoveFromStackOrQueue:
-                    resourceName = "InvalidOperation_CannotRemoveFromStackOrQueue";
-                    break;
-
-                case ExceptionResource.InvalidOperation_EmptyQueue:
-                    resourceName = "InvalidOperation_EmptyQueue";
-                    break;
-
-                case ExceptionResource.InvalidOperation_EnumOpCantHappen:
-                    resourceName = "InvalidOperation_EnumOpCantHappen";
-                    break;
-
-                case ExceptionResource.InvalidOperation_EnumFailedVersion:
-                    resourceName = "InvalidOperation_EnumFailedVersion";
-                    break;
-
-                case ExceptionResource.InvalidOperation_EmptyStack:
-                    resourceName = "InvalidOperation_EmptyStack";
-                    break;
-
-                case ExceptionResource.InvalidOperation_EnumNotStarted:
-                    resourceName = "InvalidOperation_EnumNotStarted";
-                    break;
-
-                case ExceptionResource.InvalidOperation_EnumEnded:
-                    resourceName = "InvalidOperation_EnumEnded";
-                    break;
-
-                case ExceptionResource.NotSupported_KeyCollectionSet:
-                    resourceName = "NotSupported_KeyCollectionSet";
-                    break;
-
-                case ExceptionResource.NotSupported_ReadOnlyCollection:
-                    resourceName = "NotSupported_ReadOnlyCollection";
-                    break;
-
-                case ExceptionResource.NotSupported_ValueCollectionSet:
-                    resourceName = "NotSupported_ValueCollectionSet";
-                    break;
-
-
-                case ExceptionResource.NotSupported_SortedListNestedWrite:
-                    resourceName = "NotSupported_SortedListNestedWrite";
-                    break;
-
-
-                case ExceptionResource.Serialization_InvalidOnDeser:
-                    resourceName = "Serialization_InvalidOnDeser";
-                    break;
-
-                case ExceptionResource.Serialization_MissingKeys:
-                    resourceName = "Serialization_MissingKeys";
-                    break;
-
-                case ExceptionResource.Serialization_NullKey:
-                    resourceName = "Serialization_NullKey";
-                    break;
-
-                case ExceptionResource.Argument_InvalidType:
-                    resourceName = "Argument_InvalidType";
-                    break;
-
-                case ExceptionResource.Argument_InvalidArgumentForComparison:
-                    resourceName = "Argument_InvalidArgumentForComparison";                    
-                    break;
-
-                case ExceptionResource.InvalidOperation_NoValue:
-                    resourceName = "InvalidOperation_NoValue";                    
-                    break;
-
-                case ExceptionResource.InvalidOperation_RegRemoveSubKey:
-                    resourceName = "InvalidOperation_RegRemoveSubKey";                    
-                    break;
-
-                case ExceptionResource.Arg_RegSubKeyAbsent:
-                    resourceName = "Arg_RegSubKeyAbsent";                    
-                    break;
-
-                case ExceptionResource.Arg_RegSubKeyValueAbsent:
-                    resourceName = "Arg_RegSubKeyValueAbsent";                    
-                    break;
-                    
-                case ExceptionResource.Arg_RegKeyDelHive:
-                    resourceName = "Arg_RegKeyDelHive";                    
-                    break;
-
-                case ExceptionResource.Security_RegistryPermission:
-                    resourceName = "Security_RegistryPermission";                    
-                    break;
-
-                case ExceptionResource.Arg_RegSetStrArrNull:
-                    resourceName = "Arg_RegSetStrArrNull";                    
-                    break;
-
-                case ExceptionResource.Arg_RegSetMismatchedKind:
-                    resourceName = "Arg_RegSetMismatchedKind";                    
-                    break;
-
-                case ExceptionResource.UnauthorizedAccess_RegistryNoWrite:
-                    resourceName = "UnauthorizedAccess_RegistryNoWrite";
-                    break;
-
-                case ExceptionResource.ObjectDisposed_RegKeyClosed:
-                    resourceName = "ObjectDisposed_RegKeyClosed";
-                    break;
-
-                case ExceptionResource.Arg_RegKeyStrLenBug:
-                    resourceName = "Arg_RegKeyStrLenBug";
-                    break;
-
-                case ExceptionResource.Argument_InvalidRegistryKeyPermissionCheck:
-                    resourceName = "Argument_InvalidRegistryKeyPermissionCheck";
-                    break;
-
-                case ExceptionResource.NotSupported_InComparableType:
-                    resourceName = "NotSupported_InComparableType";
-                    break;
-
-                case ExceptionResource.Argument_InvalidRegistryOptionsCheck:
-                    resourceName = "Argument_InvalidRegistryOptionsCheck";
-                    break;
-
-                case ExceptionResource.Argument_InvalidRegistryViewCheck:
-                    resourceName = "Argument_InvalidRegistryViewCheck";
-                    break;
-
-                default:
-                    Contract.Assert( false, "The enum value is not defined, please checked ExceptionArgumentName Enum.");
-                    return string.Empty;
-            }
+            Contract.Assert(Enum.IsDefined(typeof(ExceptionResource), resource),
+                "The enum value is not defined, please check the ExceptionResource Enum.");
 
-            return resourceName;
+            return resource.ToString();
         }
 
     }