CA2201 should not flag `OutOfMemoryException`

[alexrp]

Analyzer

Diagnostic ID: CA2201

Describe the improvement

This rule should not flag throws of OutOfMemoryException. This is a completely reasonable exception to throw when allocating memory by other means than the new keyword. There is even precedent for this in the BCL itself - see NativeMemory.Alloc().

[Evangelink]

I am not sure about it. This seems pretty edge case where I would recommend you to use pragma to ignore the error.

As stated by the doc:

The following exception types are reserved and should be thrown only by the common language runtime:

And the example you are quoting matches the doc.

[alexrp]

Then why isn't e.g. OverflowException on the list? That's exactly the same sort of situation. It's thrown by some intrinsic features of the language/runtime, but is also thrown in library code where doing so makes sense, such as for BigInteger.

[Evangelink]

Stack overflow is part of the list.

[alexrp]

StackOverflowException =/= OverflowException

[Evangelink]

@stephentoub @mavasani What's your thoughts on this?

[stephentoub]

I'm largely ambivalent. The rule is silent by default, and multiple exceptions on the list are in the same category as OutOfMemoryException (e.g. NullReferenceException, IndexOutOfRangeException, etc.): general code should not be throwing them, but there are niche cases where it may be desirable. I don't know why we'd single-out OutOfMemoryException itself.

[alexrp]

NullReferenceException and IndexOutOfRangeException are a bit different in my mind, because we already have ArgumentNullException and ArgumentOutOfRangeException to handle similar cases in custom types. There isn't really a good reason for user code to ever throw the former exceptions; it'll just cause confusion because it's well-understood by most users at this point that the former exceptions are thrown by the runtime and the latter by user code.

Similarly, I can't think of a single reason why it would ever be appropriate for a user to throw StackOverflowException. That exception is supposed to indicate a (usually) fatal situation. We already have InsufficientExecutionStackException for cases where code actively probes for stack space.

As I mentioned, OverflowException is notably absent from the list, despite being a 'special' exception. It makes sense that users might want to throw this exception in custom numeric types since there's already plenty of precedent for this in the BCL itself. There is no equivalent exception that's meant for user code.

Similarly, there isn't anything more appropriate to throw than OutOfMemoryException if you're writing custom allocation code and you hit an OOM condition. Marshal.AllocHGlobal(), NativeMemory.Alloc(), etc have set the precedent that this is the exception to throw in such situations, rather than (e.g.) returning null.

(e.g. NullReferenceException, IndexOutOfRangeException, etc.): general code should not be throwing them, but there are niche cases where it may be desirable.

FWIW, I looked for throw new <type> for these two in the runtime repo and from a quick glance those cases seem to be either a) very low-level runtime implementation code, b) internal implementation details that aren't visible to users, or c) very old code that should have used the Argument* exceptions instead, but cannot be changed now due to compatibility. This seems to be very different from OutOfMemoryException where there are good reasons for users to throw it, even in new code.