Added toBytes property to std.bigint.BigInt

[JonathanWilbur]

Per issue 13804, I created a toBytes property for std.bigint.BigInt. This generates an arbitrarily-long array of unsigned bytes that represents the signed, native-endian binary representation of the BigInt.

https://issues.dlang.org/show_bug.cgi?id=13804

[dlang-bot]

Thanks for your pull request and interest in making D better, @JonathanWilbur! We are looking forward to reviewing it, and you should be hearing from a maintainer soon. Please verify that your PR follows this checklist:

• My PR is fully covered with tests (you can see the annotated coverage diff directly on GitHub with CodeCov's browser extension
• My PR is as minimal as possible (smaller, focused PRs are easier to review than big ones)
• I have provided a detailed rationale explaining my changes
• New or modified functions have Ddoc comments (with Params: and Returns:)

Please see CONTRIBUTING.md for more information.

---

If you have addressed all reviews or aren't sure how to proceed, don't hesitate to ping us with a simple comment.

Bugzilla references

Your PR doesn't reference any Bugzilla issue.

If your PR contains non-trivial changes, please reference a Bugzilla issue or create a manual changelog.

Testing this PR locally

If you don't have a local development environment setup, you can use Digger to test this PR:

dub fetch digger
dub run digger -- build "master + phobos#6437"

[JonathanWilbur]

That punycode commit you see there was from a previous pull request. I removed it in a subsequent commit.

[JonathanWilbur]

Oh, also, the main rationale--the one I suspect everybody has for wanting this property--is the ability to convert large integers to arrays of bytes for ASN.1 encoding in RSA data structures. That's why I made it, FWIW.

[Biotronic]

Have you looked at BigUInt (in std/internal/math/biguintcore.d)? Almost every function in BigInt is just a call to a function of BigUint, and I suggest this should be as well.

BigUint would then have a toBytes function that doesn't deal with signs, and BigInt's toBytes implementation would take that value and do 2's complement if necessary. BigUint.data is simply a uint[] and a simple cast should do most of what this PR does.

[JonathanWilbur]

@wilzbach , I do not have a machine for testing on BigEndian. Actually, I assumed that one of the hosts in the D Auto-Tester fleet would be big endian...

[JonathanWilbur]

Also, I don't think that Circle CI failure is my fault: it says that Dscanner failed. I didn't change anything about Dscanner.

[jmdavis]

Actually, I assumed that one of the hosts in the D Auto-Tester fleet would be big endian...

dmd only supports x86 and x86_64, so it doesn't support Big Endian at all, much as the language does. To do anything with Big Endian and D, you need either ldc or gdc and a Big Endian machine that they support. And since they're maintained separately from dmd, we can't test PRs against them. So, unfortunately, anything we've done with Big Endian in Phobos is done with the hope that it works and that when the changes finally get merged into gdc or ldc, if they're broken for Big Endian, someone will report it. Fortunately, not much code in Phobos cares about endianness.

[wilzbach]

Also, I don't think that Circle CI failure is my fault: it says that Dscanner failed. I didn't change anything about Dscanner.

Yep, not your fault. The GC is segfaulting spuriously, but so far no one knows why (or has cared to investigate). The issue is: https://issues.dlang.org/show_bug.cgi?id=18720

[JonathanWilbur]

So what is the next step? Does somebody else need to review this? Do I need to change something?

[Biotronic]

I still hold that the correct location for most of this code is in std/internal/math/biguintcore.d. It should handle the unsigned stuff, and std/bigint.d should only do the 2's complement when that's necessary.

[JonathanWilbur]

@Biotronic , I guess you missed it, but I already moved most of it to biguintcore.d. Please review it again.

[JonathanWilbur]

Just following up on this, @wilzbach (or anybody else), is there a reason this has not been merged yet? Do I need to change anything?

I should also add that the auto-tester appears to run on and off, so it looks like this is still in testing even though it has passed before. There was one strange failure a few days ago, which appears to come from:

make[1]: *** [generated/linux/debug/64/unittest/std/algorithm/searching.o] Killed
make[1]: Leaving directory `/media/ephemeral0/sandbox/at-client/pull-3115292-Linux_64_64/phobos'
make: *** [unittest-debug] Error 2
make: *** Waiting for unfinished jobs....

which I don't think has anything to do with my commit.

[wilzbach]

Thanks for the ping and sorry for not clarifying this. New symbols need the following:

• a changelog entry
• approval from @andralex
• full ddoc documentation headers

Some CI have unfortunately spurious failures. While we are trying to weed them out, it's not always easy, but you can safely ignore such a failure.

[wilzbach]

BTW at the end we need to squash all commits into one, so I recommend to use rebases instead of merge commits as then the final squashing is easier (though it's not a blocking issue if you have troubles with this. We can help you out with this if you need help and force-push over your branch at the end though I'm this case you used the master branch, so you might want to make sure that your punycode commit is safely stored in another branch)