runtime: Add new ObjectDisposedException constructor overload

Video

We discussed the method pattern here, and debated for a bit on whether the parameters should be nullable and we concluded no. Since we’re early in the release we have time to learn that was wrong. (And going “more nullable” is not breaking)

namespace System
{
    public partial class ObjectDisposedException
    {
        [System.Diagnostics.CodeAnalysis.DoesNotReturnAttribute]
        [System.Diagnostics.StackTraceHidden]
        public static void Throw(System.Object instance) => throw null;
        [System.Diagnostics.StackTraceHidden]
        [System.Diagnostics.CodeAnalysis.DoesNotReturnAttribute]
        public static void Throw(System.Type type) => throw null;
    }
}

One notable change from the PR state: We added StackTraceHidden to both methods.

Of the hundreds of call sites changed in https://github.com/dotnet/runtime/pull/58684, I think all but maybe 4 were of the form:

if (condition)
{
    ObjectDisposedException.Throw(...);
}

and two of the remaining ones could have been written that way. In addition to or instead of Throw, should we have a ThrowIf?

public static void ThrowIf(bool condition, object instance)
{
    if (condition)
    {
        Throw(instance);
    }
}

such that those hundreds of call sites change from:

if (condition)
{
    ObjectDisposedException.Throw(...);
}

to instead be:

ObjectDisposedException.ThrowIf(condition, ...);

?

Reset to api-ready-for-review since the PR seems to have gone ahead with Steve’s suggestion. So we should discuss Steve’s suggestion.

namespace System
{
    public partial class ObjectDisposedException
    {
        [System.Diagnostics.CodeAnalysis.DoesNotReturnAttribute]
        [System.Diagnostics.StackTraceHidden]
        public static void Throw(System.Object instance) => throw null;
        [System.Diagnostics.CodeAnalysis.DoesNotReturnAttribute]
        public static void Throw(System.Type type) => throw null;
    }
}

Part of the motivation being that new ObjectDisposedException(null) is currently valid (uses the default message), and as the new ctors thus introduce a null-literal compile-time ambiguity/break. The other part being that it doesn’t seem likely that anyone really wants to do anything with an ODE other than throw it.

Also suggestion, public static void Throw<T>() => Throw(typeof(T));. (FWIW, I don’t think it’s useful, since there’s no inference or return type from it)

The reflection caches are collectible. If it is just one time occurence, the GC will take care of them.

I would not worry about cost of computing the type name for the purpose of this API. It is the least of the problem for throwing ObjectDisposedException.

For new ObjectDisposedException(GetType().Name); and new ObjectDisposedException(nameof(NameOfThisType)); would it be better to have the type as a generic parameter so the jit can see the real type and provide a static simple implementation that doesn’t rely on runtime GetType() or reflection?

public sealed class ObjectDisposedException
{
+    public static ObjectDisposedException CreateFor<T>() => return new ObjectDisposedException(nameof(T));
}