Finalize implementation of the multiplication routines

[AaronKutch]

Before I submit a pull request, should there be another branch or do I just use the master branch?

[Robbepop]

Ideally you implement new features on another branch than master since you then can use master to stay in sync with the origin/upstream master. It is just convention but if everybody sticks to conventions everything is easier.

[AaronKutch]

I attempted to do a pull request but there is a mixture of spaces and tabs being used which messes up the indentation. Do you want to use tabs because that's what it looks like you are using?

[Robbepop]

I personally use what is the default in the ecosystem.

I viewed your pull request but you have closed it already. You can git push --force your fork to overwrite bad states. This will automatically update the current pull request on GitHub. So you do not need to close it. Just re-open your PR, fix the stuff locally, push --force to your branch on GitHub and everything will be fine again. :)

Just one little question: Why have you used macros and macro invokations instead of simple plain functions?

[Robbepop]

I have recently looked into what is required by the implementation of karatsuba algorithm. It seems to be easily implementable but let's see. It is good to have your initial base implementation for medium size ApInt instances first so that I can check both implementations against each other later on.

[AaronKutch]

VScode, git, bugs, my internet connection, and more have been conspiring against me recently and preventing me from uploading stuff. During that time I have almost completed the division algorithm, and I did some code clean up.

I removed the macros since it turns out that the division algorithm needs a specialized type and they should have been functions anyway.

I saw a bunch of stuff in your code like

  let lval = lhs.repr();
  let rval = rhs.repr();
  let result = lval.wrapping_sub(rval);
  *lhs = Digit(result);

and I am replacing it with *lhs = lhs.wrapping_sub(rhs); since I have added those helper methods for my mul and div routines.

I am also fixing negate by adding a wrapping_inc function (that just increments by 1).

That also reminds me that there needs to be a renaming done. I think I will comment more about that after everything is done with this pull request

[Robbepop]

Thank you really really much for all the work you have put in here! I am looking forward to merging your changes.

In general I really like the refactoring of old code and it is okay to me to include it into this PR but in the future it would be awesome if we could package Pull Requests into single pieces of new features instead of bundling them into a huge pile of feature additions and refactorings. This at least tends to keep things simpler and more transparent.

The wrapping_inc method is a nice addition but I was not implementing it since I wasn't sure if we want such an API or if something like wrapping_inc_by(int) is more appropriate. But since you have already implemented it, we will simply go with it now. ;)

Could we please do the renaming decoupled from this Pull-Request? It seems that this Pull Request is already going to be a lot larger than a single closed-up feature addition or bug fix.

[AaronKutch]

Ok I was planning on doing the renaming another pull request

[AaronKutch]

Before this issue is closed, I want to get back to my project and try out some calculations that use the long multiplication.

[Robbepop]

That's a good idea! :) I might do the same. I will release the new version once we close this issue. :)

[AaronKutch]

Because of out-of-date information here, I am continuing this in a new issue