Add ObjectDisposedException.Throw for object instance and type

[Bibletoon]

Closes #51700
I've decided to make static methods instead of new ctors because of breaking changes with null argument (like there and there)

Should I also change old calls like that throw new ObjectDisposedException(GetType().FullName); or throw new ObjectDisposedException(GetType().Name); to new method?

[dotnet-issue-labeler]

Note regarding the new-api-needs-documentation label:

This serves as a reminder for when your PR is modifying a ref *.cs file and adding/modifying public APIs, to please make sure the API implementation in the src *.cs file is documented with triple slash comments, so the PR reviewers can sign off that change.

[dotnet-issue-labeler]

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

[dnfadmin]

All CLA requirements met.

[marek-safar]

Should I also change old calls like that throw new ObjectDisposedException(GetType().FullName); or throw new ObjectDisposedException(GetType().Name); to new method?

Yes please but in separate PR.

[stephentoub]

I've decided to make static methods instead of new ctors because of breaking changes with null argument (like there and there)

The APIs were approved as constructors. We can't merge this with different APIs unless that's officially revisited (my comment about using static members instead was just a musing).
cc: @terrajobst, @bartonjs

[stephentoub]

Should I also change old calls like that throw new ObjectDisposedException(GetType().FullName); or throw new ObjectDisposedException(GetType().Name); to new method?

Yes please but in separate PR.

FWIW, my preference is actually to do it in the same PR. That helps to validate the design of the API by seeing whether there are any places we'd like to change that can't be changed for some reason, or otherwise discovering issues with the shape of the API. That's still possible if it's rolled out separately, but only after it's already been merged, at which point we then need to go back and revise the public API, which could have already been adopted elsewhere up-stack by that point, and now we need to deal with a breaking change (within the same release).

[stephentoub]

I've decided to make static methods instead of new ctors because of breaking changes with null argument (like there and there)

I don't think these arguments should be nullable (it's also not what was approved in the API review). Allowing these to be null is promoting poor usage, and the cases you highlighted are good examples of how those exceptions are lacking appropriate details for debugging because null was passed in. We wouldn't consider it breaking to pass in this instead of null for those two examples.

[tmds]

Maybe also add?

public static void Throw<T>() => Throw(typeof(T));

[bartonjs]

I've reset the issue to api-ready-for-review to discuss the implementation-vs-spec mismatch.

[bartonjs]

LGTM, but I defer to @stephentoub in case he has last minute second thoughts 😄

[ghost]

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

Issue Details

---

Closes #51700
I've decided to make static methods instead of new ctors because of breaking changes with null argument (like there and there)

Should I also change old calls like that throw new ObjectDisposedException(GetType().FullName); or throw new ObjectDisposedException(GetType().Name); to new method?

Author:	Bibletoon
Assignees:	-
Labels:	area-System.Runtime, new-api-needs-documentation
Milestone:	-

[jeffhandley]

There are a couple of formatting tweaks needed; otherwise this looks good. I've queued up suggestions for those formatting tweaks; I'll go ahead and commit them.

The test failure is #47968.

[jeffhandley]

The other failure is #60706, which has been fixed in main already.

[jeffhandley]

Thank you for the contribution, @Bibletoon! Nice work and thanks for iterating through the feedback.

[EgorBo]

Improvements on ubuntu x64 dotnet/perf-autofiling-issues#2086

[stephentoub]

Improvements on ubuntu x64

Why/how would this change have impacted that particular benchmark?

[EgorBo]

Improvements on ubuntu x64

Why/how would this change have impacted that particular benchmark?

this commit was the only suspect in improvements of that benchmark

but most likely it's just a noise. I assumed ThrowHelpers helped some IPAddress-related functions to become inlining-friendly