dotnet / dotnet-api-docs

.NET API reference documentation (.NET 5+, .NET Core, .NET Framework)
https://docs.microsoft.com/dotnet/api/
Other
688 stars 1.52k forks source link

Incomplete doc on boundary conditions in BigInter.Pow #9900

Open brettshearer opened 2 months ago

brettshearer commented 2 months ago

Consider this awesome code:

var g = 0xffffffffffffffff; var bi = new BigInteger(g); bi = BigInteger.Pow(bi, Int32.MaxValue);

Consider this awesome exception:

System.OverflowException: 'Arithmetic operation resulted in an overflow.'

Consider this documentation: https://learn.microsoft.com/en-us/dotnet/api/system.numerics.biginteger.pow?view=net-8.0

Specifically this incomplete part:

Exceptions ArgumentOutOfRangeException exponent is negative.

Is this a central location describing exceptions that may be thrown across all of BigInteger, or should all methods be documented separately?

dotnet-issue-labeler[bot] commented 2 months ago

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.

tannergooding commented 2 months ago

System.OverflowException: 'Arithmetic operation resulted in an overflow.'

Overflow (much like OutOfMemory) is largely an implementation detail here. The general issue is that BigInteger can't actually grow indefinitely, it is bounded by the memory you have on your actual machine and the limits of the GC. If we were to document it, it would need to be documented more broadly across most BigInteger APIs and indicate that any of them could throw OverflowException if the resulting BigInteger didn't fit within the internal limits.

The current internal limit is Array.MaxLength / sizeof(uint) (which is 2_147_483_591 / 4, which is 536_870_897 uint, or rather 2_147_483_588 bytes. However, this isn't quite accurate as a number of APIs break down at this size (may not always be able to return results even if the allocation were to succeed). The functional limit for any BigInteger implementation is actually is no more than int.MaxValue (2_147_483_647) bits, which is 268_435_455 bytes. This then becomes 67_108_853 uint or rather 268_435_452 bytes. If we ever change this to use nuint internally, then the limit becomes 33_554_431 ulong or rather 268_435_448 bytes. Regardless of whether we used uint[] or nuint[] as the backing store, this ends up being functionally just less than a 256MB allocation as the limit that makes sense across all current BigInteger APIs. Such a limit allows you to represent numbers up to around 2^2_147_483_584, which would have approx. 646_456_973.98 decimal digits (10^646_456_973). This is such a large number that it starts getting into the realm where there are only theoretical applications for it and where specialized programs with customized types would be the appropriate thing to use at that point.

I wouldn't be opposed to documenting that there is a limit across all BigInteger producing APIs and therefore any API could result in an OutOfMemory or Overflow exception if this limit were exceeded. I'd leave it up to the doc team (CC. @gewarren) on the best way to surface this. -- My naive expectation is we'd want to have some general remarks section across all such APIs that links to a general place the details around these limits are documented (which would probably be part of the remarks for the documentation page on the type).

colejohnson66 commented 1 month ago

IMO, documenting OutOfMemoryException on every API is the same as documenting StackOverflowException or InvalidProgramException (corrupted DLL). Sure, any call could result in them, but you're not supposed to recover from them. I, personally, only document exceptions that are deliberately thrown. As such (on the other hand), documenting OverflowException is something that I'd personally agree with.

brettshearer commented 1 month ago

Thanks - agree it is pretty picky, but with no mention of overflow (and the definition on the front page of Represents an arbitrarily large signed integer.) there is a gap. Even if the overflow exception mentioned it when occurring.

tannergooding commented 1 month ago

but with no mention of overflow (and the definition on the front page of Represents an arbitrarily large signed integer.) there is a gap.

The general issue is that its an implementation detail. BigInteger is an "arbitrarily large signed integer". The problem is that "arbitrarily large" is fundamentally bounded by per machine physical limitations.

It's something we can probably call out in the remarks section, as I described, but it's not something that can really be more directly called out without causing confusion for the primary use-cases.