From 5c32341eeef9b25919d63051a9f9fd92fc55ce6d Mon Sep 17 00:00:00 2001 From: Ben Adams Date: Mon, 5 Sep 2016 04:34:28 +0100 Subject: [PATCH] Improve ThrowHelper (#7048) --- src/mscorlib/src/System/Array.cs | 4 +- .../src/System/Collections/Generic/List.cs | 6 +- .../System/Collections/ObjectModel/Collection.cs | 4 +- src/mscorlib/src/System/ThrowHelper.cs | 322 +-------------------- 4 files changed, 20 insertions(+), 316 deletions(-) diff --git a/src/mscorlib/src/System/Array.cs b/src/mscorlib/src/System/Array.cs index 3f175d9..a97ce9d 100644 --- 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(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(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 6e954c0..ae3356d 100644 --- 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 b9ed1a1..54aa7bb 100644 --- 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 e1cdd30..f327176 100644 --- 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, 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(object value, ExceptionArgument argName) { // Note that default(T) is not equal to null for value types except when T is Nullable. - 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(); } } -- 2.7.4