Value enumerators do not account for being constructed via `default`, unexpectedly throwing `NullReferenceException`.

[GameMicrowave]

Description

Many value-type enumerators in the standard library collections do not take into account the scenario where they are constructed via default or new(). As a result, calling their MoveNext() method unexpectedly throws a NullReferenceException. The API documentation also does not mention that MoveNext() may throw NullReferenceException, which is clearly an oversight in the design.

These value enumerators internally store some reference-type fields, but their methods access members of those fields without checking for null. Since default and similar means can directly construct such enumerators with all reference fields set to null, this issue arises.

Reproduction Steps

HashSet<int>.Enumerator enumerator = default;
if (enumerator.MoveNext()) {
  // do something
}

Expected behavior

Nothing has happened.

Actual behavior

Throw NullReferenceException

Regression?

No response

Known Workarounds

Add a null check at the very beginning of the method to determine whether it is in the default state. If so, exit immediately.

Configuration

No response

Other information

No response

[dotnet-policy-service]

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

[dotnet-policy-service]

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

[huoyaoyuan]

Many value-type enumerators in the standard library collections do not take into account the scenario where they are constructed via default or new().

This is not a supported usage. Many structs don't have default as a valid state.

Add a null check at the very beginning of the method to determine whether it is in the default state. If so, exit immediately.

This is mostly not acceptable as it will decrease performance for non-promoted usges.

[GameMicrowave]

Many value-type enumerators in the standard library collections do not take into account the scenario where they are constructed via default or new().

This is not a supported usage. Many structs don't have default as a valid state.

Add a null check at the very beginning of the method to determine whether it is in the default state. If so, exit immediately.

This is mostly not acceptable as it will decrease performance for non-promoted usges.

@huoyaoyuan Actually, the null check is always present inherently. When accessing members of a reference type, .NET always performs an implicit null check. With an early explicit null assertion, the JIT can eliminate the subsequent implicit null checks, so the performance is not actually degraded in hot scenarios:

Method	Size	Mean	Error	StdDev
NoCheck	100	45.15 ns	0.242 ns	0.144 ns
WithCheck	100	44.67 ns	0.288 ns	0.171 ns
NoCheck	1000	390.48 ns	1.275 ns	0.843 ns
WithCheck	1000	389.99 ns	0.942 ns	0.560 ns

public bool MoveNext() {
// difference start
    if (_array == null)
        return false;
// difference end
    if ((uint)_index < (uint)_array.Length) {
        _current = _array[_index++];
        return true;
    }
    return false;
}

I believe that correctness should take precedence over marginal performance considerations. In cases where JIT optimization (such as inlining or promotion) does not occur, the added null check incurs a performance overhead of less than 4%. Since this is on the cold path, the actual impact on overall program performance is negligible, while the benefit is maintaining correctness.

[huoyaoyuan]

Actually, the null check is always present inherently. When accessing members of a reference type, .NET always performs an implicit null check.

This is incorrect. There's no null check for accessing field, and NullReferenceException is thrown by catching segment fault. Inherent null check is only inserted for callvirt calls.

I believe that correctness should take precedence over marginal performance considerations.

Invalid usage is not considered for correctness, except for catastrophic cases.

[huoyaoyuan]

With an early explicit null assertion, the JIT can eliminate the subsequent implicit null checks, so the performance is not actually degraded in hot scenarios:

What's the benchmark code? It's probably that JIT inlines the methods, sees the instantiation of non-null field and elemimates the null check at all. However, any non-inlined usages like IEnumerator<T> interface will not benefit from this.