Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

ArgOutOfRangeException throw helpers #78222

Merged
merged 17 commits into from
Dec 1, 2022
Merged

ArgOutOfRangeException throw helpers #78222

Show file tree
Hide file tree
Changes from 13 commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
661eef8
add implementation
hrrrrustic Nov 11, 2022
1900b3a
add docs
hrrrrustic Nov 11, 2022
39e6e81
Add some tests
hrrrrustic Nov 11, 2022
600b947
Some replacements in CoreLib root namespace
hrrrrustic Nov 11, 2022
91e279e
Apply suggestions from code review
hrrrrustic Nov 11, 2022
541389f
Inline negativeorzero
hrrrrustic Nov 13, 2022
ad4aff8
fix messages
hrrrrustic Nov 13, 2022
722f5b7
fix throwing
hrrrrustic Nov 13, 2022
3c5fe6d
fix paramnames
hrrrrustic Nov 13, 2022
65d5d23
fix nullability
hrrrrustic Nov 13, 2022
9f81c22
one more paramname fix
hrrrrustic Nov 14, 2022
0709d8c
make private throwers generic
hrrrrustic Nov 23, 2022
0067cbd
Apply suggestions from code review
hrrrrustic Nov 24, 2022
b760a77
apply review comments
hrrrrustic Nov 24, 2022
6893695
few more cleanups
hrrrrustic Nov 24, 2022
b65e7d5
Merge branch 'main' into issue-69590
stephentoub Dec 1, 2022
dd0ad2f
Address my own PR feedback and use helpers in more places
stephentoub Dec 1, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 21 additions & 0 deletions src/libraries/System.Private.CoreLib/src/Resources/Strings.resx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1862,6 +1862,27 @@
<data name="ArgumentOutOfRange_Year" xml:space="preserve">
<value>Year must be between 1 and 9999.</value>
</data>
<data name="ArgumentOutOfRange_Generic_MustBeNonZero" xml:space="preserve">
<value>'{0}' must be a non-zero value.</value>
</data>
<data name="ArgumentOutOfRange_Generic_MustBeNonNegative" xml:space="preserve">
<value>'{0}' must be a non-negative value.</value>
</data>
<data name="ArgumentOutOfRange_Generic_MustBeNonNegativeNonZero" xml:space="preserve">
<value>'{0}' must be a non-negative and non-zero value.</value>
</data>
<data name="ArgumentOutOfRange_Generic_MustBeLowerOrEqual" xml:space="preserve">
<value>'{0}' must be lower than or equal to '{1}'.</value>
stephentoub marked this conversation as resolved.
Show resolved Hide resolved
</data>
<data name="ArgumentOutOfRange_Generic_MustBeLower" xml:space="preserve">
<value>'{0}' must be lower than '{1}'.</value>
</data>
<data name="ArgumentOutOfRange_Generic_MustBeGreaterOrEqual" xml:space="preserve">
<value>'{0}' must be greater than or equal to '{1}'.</value>
</data>
<data name="ArgumentOutOfRange_Generic_MustBeGreater" xml:space="preserve">
<value>'{0}' must be greater than '{1}'.</value>
</data>
<data name="Arithmetic_NaN" xml:space="preserve">
<value>Function does not accept floating point Not-a-Number values.</value>
</data>
Expand Down
119 changes: 119 additions & 0 deletions src/libraries/System.Private.CoreLib/src/System/ArgumentOutOfRangeException.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,9 @@
=============================================================================*/

using System.Runtime.Serialization;
using System.Runtime.CompilerServices;
using System.Diagnostics.CodeAnalysis;
using System.Numerics;

namespace System
{
Expand Down Expand Up @@ -85,5 +88,121 @@ public override string Message

// Gets the value of the argument that caused the exception.
public virtual object? ActualValue => _actualValue;

[DoesNotReturn]
private static void ThrowZero(string? paramName)
{
throw new ArgumentOutOfRangeException(paramName, SR.Format(SR.ArgumentOutOfRange_Generic_MustBeNonZero, paramName));
}

[DoesNotReturn]
private static void ThrowNegative(string? paramName)
{
throw new ArgumentOutOfRangeException(paramName, SR.Format(SR.ArgumentOutOfRange_Generic_MustBeNonNegative, paramName));
}

[DoesNotReturn]
private static void ThrowNegativeOrZero(string? paramName)
{
throw new ArgumentOutOfRangeException(paramName, SR.Format(SR.ArgumentOutOfRange_Generic_MustBeNonNegativeNonZero, paramName));
}

[DoesNotReturn]
private static void ThrowGreater<T>(string? paramName, T other)
{
throw new ArgumentOutOfRangeException(paramName, SR.Format(SR.ArgumentOutOfRange_Generic_MustBeLowerOrEqual, paramName, other));
}

[DoesNotReturn]
private static void ThrowGreaterEqual<T>(string? paramName, T other)
{
throw new ArgumentOutOfRangeException(paramName, SR.Format(SR.ArgumentOutOfRange_Generic_MustBeLower, paramName, other));
}

[DoesNotReturn]
private static void ThrowLess<T>(string? paramName, T other)
{
throw new ArgumentOutOfRangeException(paramName, SR.Format(SR.ArgumentOutOfRange_Generic_MustBeGreaterOrEqual, paramName, other));
}

[DoesNotReturn]
private static void ThrowLessEqual<T>(string? paramName, T other)
{
throw new ArgumentOutOfRangeException(paramName, SR.Format(SR.ArgumentOutOfRange_Generic_MustBeGreater, paramName, other));
}

/// <summary>Throws an <see cref="ArgumentOutOfRangeException"/> if <paramref name="value"/> is zero.</summary>
/// <param name="value">The argument to validate as non-zero.</param>
/// <param name="paramName">The name of the parameter with which <paramref name="value"/> corresponds.</param>
public static void ThrowIfZero<T>(T value, [CallerArgumentExpression(nameof(value))] string? paramName = null)
where T : INumberBase<T>
{
if (T.IsZero(value))
ThrowZero(paramName);
}

/// <summary>Throws an <see cref="ArgumentOutOfRangeException"/> if <paramref name="value"/> is negative.</summary>
/// <param name="value">The argument to validate as non-negative.</param>
/// <param name="paramName">The name of the parameter with which <paramref name="value"/> corresponds.</param>
public static void ThrowIfNegative<T>(T value, [CallerArgumentExpression(nameof(value))] string? paramName = null)
where T : INumberBase<T>
{
if (T.IsNegative(value))
ThrowNegative(paramName);
}

/// <summary>Throws an <see cref="ArgumentOutOfRangeException"/> if <paramref name="value"/> is negative or zero.</summary>
/// <param name="value">The argument to validate as non-zero or non-negative.</param>
/// <param name="paramName">The name of the parameter with which <paramref name="value"/> corresponds.</param>
public static void ThrowIfNegativeOrZero<T>(T value, [CallerArgumentExpression(nameof(value))] string? paramName = null)
where T : INumberBase<T>
{
if (T.IsNegative(value) || T.IsZero(value))
stephentoub marked this conversation as resolved.
Show resolved Hide resolved
ThrowNegativeOrZero(paramName);
}

/// <summary>Throws an <see cref="ArgumentOutOfRangeException"/> if <paramref name="value"/> is greater than <paramref name="other"/>.</summary>
/// <param name="value">The argument to validate as less or equal than <paramref name="other"/>.</param>
/// <param name="other">The value to compare with <paramref name="value"/>.</param>
/// <param name="paramName">The name of the parameter with which <paramref name="value"/> corresponds.</param>
public static void ThrowIfGreaterThan<T>(T value, T other, [CallerArgumentExpression(nameof(value))] string? paramName = null)
where T : IComparable<T>
{
if (value.CompareTo(other) > 0)
stephentoub marked this conversation as resolved.
Show resolved Hide resolved
ThrowGreater(paramName, other);
}

/// <summary>Throws an <see cref="ArgumentOutOfRangeException"/> if <paramref name="value"/> is greater than or equal <paramref name="other"/>.</summary>
/// <param name="value">The argument to validate as less than <paramref name="other"/>.</param>
/// <param name="other">The value to compare with <paramref name="value"/>.</param>
/// <param name="paramName">The name of the parameter with which <paramref name="value"/> corresponds.</param>
public static void ThrowIfGreaterThanOrEqual<T>(T value, T other, [CallerArgumentExpression(nameof(value))] string? paramName = null)
where T : IComparable<T>
{
if (value.CompareTo(other) >= 0)
ThrowGreaterEqual(paramName, other);
}

/// <summary>Throws an <see cref="ArgumentOutOfRangeException"/> if <paramref name="value"/> is less than <paramref name="other"/>.</summary>
/// <param name="value">The argument to validate as greatar than or equal than <paramref name="other"/>.</param>
/// <param name="other">The value to compare with <paramref name="value"/>.</param>
/// <param name="paramName">The name of the parameter with which <paramref name="value"/> corresponds.</param>
public static void ThrowIfLessThan<T>(T value, T other, [CallerArgumentExpression(nameof(value))] string? paramName = null)
where T : IComparable<T>
{
if (value.CompareTo(other) < 0)
ThrowLess(paramName, other);
}

/// <summary>Throws an <see cref="ArgumentOutOfRangeException"/> if <paramref name="value"/> is less than or equal <paramref name="other"/>.</summary>
/// <param name="value">The argument to validate as greatar than than <paramref name="other"/>.</param>
/// <param name="other">The value to compare with <paramref name="value"/>.</param>
/// <param name="paramName">The name of the parameter with which <paramref name="value"/> corresponds.</param>
public static void ThrowIfLessThanOrEqual<T>(T value, T other, [CallerArgumentExpression(nameof(value))] string? paramName = null)
where T : IComparable<T>
{
if (value.CompareTo(other) <= 0)
ThrowLessEqual(paramName, other);
}
}
}
10 changes: 3 additions & 7 deletions src/libraries/System.Private.CoreLib/src/System/BitConverter.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -657,8 +657,7 @@ public static string ToString(byte[] value, int startIndex, int length)
ThrowHelper.ThrowArgumentNullException(ExceptionArgument.value);
if (startIndex < 0 || startIndex >= value.Length && startIndex > 0)
ThrowHelper.ThrowArgumentOutOfRangeException(ExceptionArgument.startIndex, ExceptionResource.ArgumentOutOfRange_IndexMustBeLess);
if (length < 0)
throw new ArgumentOutOfRangeException(nameof(length), SR.ArgumentOutOfRange_GenericPositive);
ArgumentOutOfRangeException.ThrowIfNegative(length);
if (startIndex > value.Length - length)
ThrowHelper.ThrowArgumentException(ExceptionResource.Arg_ArrayPlusOffTooSmall, ExceptionArgument.value);

Expand All @@ -667,11 +666,8 @@ public static string ToString(byte[] value, int startIndex, int length)
return string.Empty;
}

if (length > (int.MaxValue / 3))
{
// (int.MaxValue / 3) == 715,827,882 Bytes == 699 MB
throw new ArgumentOutOfRangeException(nameof(length), SR.Format(SR.ArgumentOutOfRange_LengthTooLarge, int.MaxValue / 3));
}
// (int.MaxValue / 3) == 715,827,882 Bytes == 699 MB
ArgumentOutOfRangeException.ThrowIfGreaterThan(length, (int.MaxValue / 3));
stephentoub marked this conversation as resolved.
Show resolved Hide resolved

return string.Create(length * 3 - 1, (value, startIndex, length), static (dst, state) =>
{
Expand Down
9 changes: 3 additions & 6 deletions src/libraries/System.Private.CoreLib/src/System/Buffer.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,12 +42,9 @@ public static unsafe void BlockCopy(Array src, int srcOffset, Array dst, int dst
}
}

if (srcOffset < 0)
throw new ArgumentOutOfRangeException(nameof(srcOffset), SR.ArgumentOutOfRange_MustBeNonNegInt32);
if (dstOffset < 0)
throw new ArgumentOutOfRangeException(nameof(dstOffset), SR.ArgumentOutOfRange_MustBeNonNegInt32);
if (count < 0)
throw new ArgumentOutOfRangeException(nameof(count), SR.ArgumentOutOfRange_MustBeNonNegInt32);
ArgumentOutOfRangeException.ThrowIfNegative(srcOffset);
ArgumentOutOfRangeException.ThrowIfNegative(dstOffset);
ArgumentOutOfRangeException.ThrowIfNegative(count);

nuint uCount = (nuint)count;
nuint uSrcOffset = (nuint)srcOffset;
Expand Down
45 changes: 15 additions & 30 deletions src/libraries/System.Private.CoreLib/src/System/Convert.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2314,12 +2314,9 @@ public static string ToBase64String(byte[] inArray, int offset, int length, Base
{
ArgumentNullException.ThrowIfNull(inArray);

if (length < 0)
throw new ArgumentOutOfRangeException(nameof(length), SR.ArgumentOutOfRange_IndexMustBeLessOrEqual);
if (offset < 0)
throw new ArgumentOutOfRangeException(nameof(offset), SR.ArgumentOutOfRange_GenericPositive);
if (offset > (inArray.Length - length))
throw new ArgumentOutOfRangeException(nameof(offset), SR.ArgumentOutOfRange_OffsetLength);
ArgumentOutOfRangeException.ThrowIfNegative(length);
ArgumentOutOfRangeException.ThrowIfNegative(offset);
ArgumentOutOfRangeException.ThrowIfGreaterThan(offset, (inArray.Length - length));
stephentoub marked this conversation as resolved.
Show resolved Hide resolved

return ToBase64String(new ReadOnlySpan<byte>(inArray, offset, length), options);
}
Expand Down Expand Up @@ -2371,19 +2368,15 @@ public static unsafe int ToBase64CharArray(byte[] inArray, int offsetIn, int len
ArgumentNullException.ThrowIfNull(inArray);
ArgumentNullException.ThrowIfNull(outArray);

if (length < 0)
throw new ArgumentOutOfRangeException(nameof(length), SR.ArgumentOutOfRange_IndexMustBeLessOrEqual);
if (offsetIn < 0)
throw new ArgumentOutOfRangeException(nameof(offsetIn), SR.ArgumentOutOfRange_GenericPositive);
if (offsetOut < 0)
throw new ArgumentOutOfRangeException(nameof(offsetOut), SR.ArgumentOutOfRange_GenericPositive);
ArgumentOutOfRangeException.ThrowIfNegative(length);
ArgumentOutOfRangeException.ThrowIfNegative(offsetIn);
ArgumentOutOfRangeException.ThrowIfNegative(offsetOut);
if (options < Base64FormattingOptions.None || options > Base64FormattingOptions.InsertLineBreaks)
throw new ArgumentException(SR.Format(SR.Arg_EnumIllegalVal, (int)options), nameof(options));

int inArrayLength = inArray.Length;

if (offsetIn > (inArrayLength - length))
throw new ArgumentOutOfRangeException(nameof(offsetIn), SR.ArgumentOutOfRange_OffsetLength);
ArgumentOutOfRangeException.ThrowIfGreaterThan(offsetIn, (inArrayLength - length));
stephentoub marked this conversation as resolved.
Show resolved Hide resolved

if (inArrayLength == 0)
return 0;
Expand All @@ -2395,8 +2388,7 @@ public static unsafe int ToBase64CharArray(byte[] inArray, int offsetIn, int len
bool insertLineBreaks = options == Base64FormattingOptions.InsertLineBreaks;
int charLengthRequired = ToBase64_CalculateAndValidateOutputLength(length, insertLineBreaks);

if (offsetOut > outArrayLength - charLengthRequired)
throw new ArgumentOutOfRangeException(nameof(offsetOut), SR.ArgumentOutOfRange_OffsetOut);
ArgumentOutOfRangeException.ThrowIfGreaterThan(offsetOut, outArrayLength - charLengthRequired);

if (Vector128.IsHardwareAccelerated && !insertLineBreaks && length >= Base64VectorizationLengthThreshold)
{
Expand Down Expand Up @@ -2784,14 +2776,11 @@ public static byte[] FromBase64CharArray(char[] inArray, int offset, int length)
{
ArgumentNullException.ThrowIfNull(inArray);

if (length < 0)
throw new ArgumentOutOfRangeException(nameof(length), SR.ArgumentOutOfRange_IndexMustBeLessOrEqual);
ArgumentOutOfRangeException.ThrowIfNegative(length);

if (offset < 0)
throw new ArgumentOutOfRangeException(nameof(offset), SR.ArgumentOutOfRange_GenericPositive);
ArgumentOutOfRangeException.ThrowIfNegative(offset);

if (offset > inArray.Length - length)
throw new ArgumentOutOfRangeException(nameof(offset), SR.ArgumentOutOfRange_OffsetLength);
ArgumentOutOfRangeException.ThrowIfGreaterThan(offset, inArray.Length - length);

if (inArray.Length == 0)
{
Expand Down Expand Up @@ -2980,12 +2969,9 @@ public static string ToHexString(byte[] inArray, int offset, int length)
{
ArgumentNullException.ThrowIfNull(inArray);

if (length < 0)
throw new ArgumentOutOfRangeException(nameof(length), SR.ArgumentOutOfRange_IndexMustBeLessOrEqual);
if (offset < 0)
throw new ArgumentOutOfRangeException(nameof(offset), SR.ArgumentOutOfRange_GenericPositive);
if (offset > (inArray.Length - length))
throw new ArgumentOutOfRangeException(nameof(offset), SR.ArgumentOutOfRange_OffsetLength);
ArgumentOutOfRangeException.ThrowIfNegative(length);
ArgumentOutOfRangeException.ThrowIfNegative(offset);
ArgumentOutOfRangeException.ThrowIfGreaterThan(offset, (inArray.Length - length));
stephentoub marked this conversation as resolved.
Show resolved Hide resolved

return ToHexString(new ReadOnlySpan<byte>(inArray, offset, length));
}
Expand All @@ -3000,8 +2986,7 @@ public static string ToHexString(ReadOnlySpan<byte> bytes)
{
if (bytes.Length == 0)
return string.Empty;
if (bytes.Length > int.MaxValue / 2)
throw new ArgumentOutOfRangeException(nameof(bytes), SR.ArgumentOutOfRange_InputTooLarge);
ArgumentOutOfRangeException.ThrowIfGreaterThan(bytes.Length, int.MaxValue / 2, nameof(bytes));

return HexConverter.ToString(bytes, HexConverter.Casing.Upper);
}
Expand Down
5 changes: 1 addition & 4 deletions src/libraries/System.Private.CoreLib/src/System/DateTime.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1700,10 +1700,7 @@ public long ToFileTimeUtc()
}

ticks -= FileTimeOffset;
if (ticks < 0)
{
throw new ArgumentOutOfRangeException(null, SR.ArgumentOutOfRange_FileTimeInvalid);
}
ArgumentOutOfRangeException.ThrowIfNegative(ticks, null);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We're losing context in the message here. We need to decide how much we care.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You marked this as resolved, so replacement is fine?
I just thinking that I can revert that one to make this initial PR like more straightforward without any edge cases. We can return here in another separate PR with replacements

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just thinking that I can revert that one to make this initial PR like more straightforward without any edge cases. We can return here in another separate PR with replacements

Ok, thanks.


return ticks;
}
Expand Down
3 changes: 1 addition & 2 deletions src/libraries/System.Private.CoreLib/src/System/Decimal.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -307,8 +307,7 @@ public Decimal(ReadOnlySpan<int> bits)
//
public Decimal(int lo, int mid, int hi, bool isNegative, byte scale)
{
if (scale > 28)
throw new ArgumentOutOfRangeException(nameof(scale), SR.ArgumentOutOfRange_DecimalScale);
ArgumentOutOfRangeException.ThrowIfGreaterThan(scale, 28);
_lo64 = (uint)lo + ((ulong)(uint)mid << 32);
_hi32 = (uint)hi;
_flags = ((int)scale) << 16;
Expand Down
10 changes: 2 additions & 8 deletions src/libraries/System.Private.CoreLib/src/System/String.Comparison.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -380,10 +380,7 @@ public static int Compare(string? strA, int indexA, string? strB, int indexB, in
return strA == null ? -1 : 1;
}

if (length < 0)
{
throw new ArgumentOutOfRangeException(nameof(length), SR.ArgumentOutOfRange_NegativeLength);
}
ArgumentOutOfRangeException.ThrowIfNegative(length);

if (indexA < 0 || indexB < 0)
{
Expand Down Expand Up @@ -475,10 +472,7 @@ public static int CompareOrdinal(string? strA, int indexA, string? strB, int ind
// COMPAT: Checking for nulls should become before the arguments are validated,
// but other optimizations which allow us to return early should come after.

if (length < 0)
{
throw new ArgumentOutOfRangeException(nameof(length), SR.ArgumentOutOfRange_NegativeCount);
}
ArgumentOutOfRangeException.ThrowIfNegative(length);

if (indexA < 0 || indexB < 0)
{
Expand Down
Loading