[API Proposal]: ArgumentOutOfRangeException.ThrowIfNegativeOrGreaterThanOrEqual

[xtqqczze]

Background and motivation

For the common case of validating an index to a span, array, string, etc.

Can be implemented in one branch as (uint)value >= (uint)other as opposed to two branches with the following:

ArgumentOutOfRangeException.ThrowIfNegative(arrayIndex);
ArgumentOutOfRangeException.ThrowIfGreaterThan(arrayIndex, array.Length);

API Proposal

public class ArgumentOutOfRangeException : ArgumentException
{
  public static void ThrowIfNegativeOrGreaterThan<T>(T value, T other, [CallerArgumentExpression(nameof(value))] string? paramName = null)
    where T : INumberBase<T>
  public static void ThrowIfNegativeOrGreaterThanOrEqual<T>(T value, T other, [CallerArgumentExpression(nameof(value))] string? paramName = null)
    where T : INumberBase<T>
}

API Usage

  private T[] array
  public T this[int arrayIndex]
  {
    get
    {
      ArgumentOutOfRangeException.ThrowIfNegativeOrGreaterThanOrEqual(arrayIndex, array.Length);
      return array[arrayIndex];
    }
  }

Alternatives

• The proposed API, but internal only.
• The JIT could be taught to recognize a range check and convert into a single branch.

[dotnet-issue-labeler]

I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label.

[skyoxZ]

I think we don't need INumberBase<T>, int is enough. And the method name could be simplified to ThrowIfNotWholeNumberLessThan.

[ghost]

Tagging subscribers to this area: @dotnet/area-system-runtime
See info in area-owners.md if you want to be subscribed.

Issue Details

---

Background and motivation

For the common case of validating an index to a span, array, string, etc.

Can be implemented in one branch as (uint)value >= (uint)other as opposed to two branches with the following:

ArgumentOutOfRangeException.ThrowIfNegative(arrayIndex);
ArgumentOutOfRangeException.ThrowIfGreaterThan(arrayIndex, array.Length);

API Proposal

namespace System

public class ArgumentOutOfRangeException : ArgumentException
{
  public static void ThrowIfNegativeOrGreaterThan<T>(T value, T other, [CallerArgumentExpression(nameof(value))] string? paramName = null)
    where T : INumberBase<T>
  public static void ThrowIfNegativeOrGreaterThanOrEqual<T>(T value, T other, [CallerArgumentExpression(nameof(value))] string? paramName = null)
    where T : INumberBase<T>
}

API Usage

  private T[] array
  public T this[int arrayIndex]
  {
    get
    {
      ArgumentOutOfRangeException.ThrowIfNegativeOrGreaterThanOrEqual(arrayIndex, array.Length);
      return array[arrayIndex];
    }
  }

Alternatives

• The proposed API, but internal only.
• The JIT could be taught to recognize a range check and convert into a single branch.

Author:	xtqqczze
Assignees:	-
Labels:	api-suggestion, area-System.Runtime, untriaged
Milestone:	-

[tannergooding]

ThrowIfNegativeOrGreaterThan is a verbose and not very "intuitive" name in my opinion. It's the kind of name that you have to stop and think about what it means.

In my own libraries, I use the name ThrowIfNotInBounds and ThrowIfNotInInsertBounds instead. In my case I take int length directly and assert that length is positive. Doing the same for a public API in the BCL is probably not possible, but taking a T[], Span<T>, and ReadOnlySpan<T> would be. We could then consider taking ICollection<T> to cover other cases, but I imagine the majority case would be covered by the first three.

[ghost]

This issue has been marked needs-author-action and may be missing some important information.

[stephentoub]

Doing the same for a public API in the BCL is probably not possible, but taking a T[], Span, and ReadOnlySpan would be. We could then consider taking ICollection to cover other cases, but I imagine the majority case would be covered by the first three.

Note that #77749 proposes a ThrowIfBufferTooSmall, which I left some comments on in #77749 (comment). I'd be concerned about taking an ICollection<T> that could lead to interface dispatch (though hopefully it could be eliminated by the JIT in some cases) in an API that's intended to be super cheap.

[ghost]

This issue has been automatically marked no-recent-activity because it has not had any activity for 14 days. It will be closed if no further activity occurs within 14 more days. Any new comment (by anyone, not necessarily the author) will remove no-recent-activity.

[ghost]

This issue will now be closed since it had been marked no-recent-activity but received no further activity in the past 14 days. It is still possible to reopen or comment on the issue, but please note that the issue will be locked if it remains inactive for another 30 days.