Fix issue 11600: to!BigInt(string) should validate input.

[quickfur]

It's pretty hilarious to print the result of to!BigInt("abracadabra"), which is currently accepted by the BigInt code.

About the fix: I know the ddocs of fromDecimalString claims to return false upon invalid input, but this is not implemented, and I don't really like to change biguintFromDecimal's function signature just so I can implement what the ddocs claim. So I'm opting to just throw when an invalid digit is found rather than go back to the bad ole C way of returning an error status that has to be checked by every function up the call stack. We have an exception system for a reason, after all.

[monarchdodra]

LGTM

[monarchdodra]

Quick question: This pull introduces a dependency from the rather "low level" std.internal.math.biguintcore to std.conv, just for a typed exception. Is this acceptable (and worth it)?

[quickfur]

Well, I'm open to suggestions. Should biguintcore define its own exception type? Last I heard, Andrei was against the idea of module-specific exceptions. We should instead of a proper semantic-based exception hierarchy in Phobos that's consistently used across all modules.

[monarchdodra]

Another solution would be for said exceptions to be defined in a more light-weight and centralized area (std.exception). It also means you can catch an exception that is thrown from a submodule you don't actually depend on directly, without needing to import the module. There's more than a few places that could throw a conv.exception

Well, I don't think it's a big deal (especially not blocker here), so I'll just merge this.

[monarchdodra]

Auto-merge toggled on

[JakobOvrum]

We have an exception system for a reason, after all.

Exceptions should be thrown in exceptional circumstances. It's useful to have parsing functions that don't throw because the arguments can be user input, in which case an error would not be exceptional.

[quickfur]

I agree, but parsing functions that don't validate input are obviously wrong.

In this case, this code is only used by std.conv.to, which doesn't support non-throwing conversions, so I wasn't really looking to expand the scope of error-handling here.

[JakobOvrum]

Oh I see, didn't notice that biguintFromDecimal was part of the public interface but didn't have the same error handling policy as fromDecimalString, which calls it.