Sorry I haven't reviewed this -- I'm mostly pondering what the public API should look like. I think you're probably right that powm should be a trait, and maybe the others too, but I don't want to get too fine-grained about this. I'm also not fond of public fields as in GcdResult.

I'm a bit concerned that the trait bounds are leaking implementation details. If we change the implementation and require different traits, it can easily become a breaking change.

I'm also not fond of public fields as in GcdResult.

Why? It is similar to returning a tuple, except that the fields are named.

You only need the trait bounds for a generic or default impl. The alternative is to only macro-impl the primitives here, then implement bigint separately, and leave others to their own. That's not necessarily the greatest, but how many other Integer types are there anyway?

I fought with this for Roots -- I started with a default impl, but I wasn't happy with the performance I could get generically. (That's a bit of an indictment against generic num, I know...)

My basic objection to public fields is that it makes it hard to make changes to the struct. You're right that it's an improvement over a plain tuple though, which also couldn't change. The more flexible path is an opaque struct with accessors, but then you can't easily unpack values.

The alternative is to only macro-impl the primitives here, then implement bigint separately, and leave others to their own. That's not necessarily the greatest, but how many other Integer types are there anyway?

This would work. The other integer types mostly implement it anyway. What should the traits look like? Putting several functions into one trait makes it hard to extend, having one trait per function is probably inconvenient.

My basic objection to public fields is that it makes it hard to make changes to the struct. You're right that it's an improvement over a plain tuple though, which also couldn't change. The more flexible path is an opaque struct with accessors, but then you can't easily unpack values.

In this case, the only possible change I can think of is returning (a / gcd(a, b), b / gcd(a, b)) in the struct as well. The field names are unlikely to change, because their names don't really matter.

We could add a private field:

pub struct GcdResult<T> {
    /// Greatest common divisor.
    pub gcd: T,
    /// Coefficients such that: gcd(a, b) = c1*a + c2*b
    pub c1: T, pub c2: T,
    seal: ()
}

It would allow adding fields in future.

Is there any hope for this getting merged? Would love to do modular inverses on bigint.

I rebased on master and made GcdResult extendable as suggested by @strake.

@cuviper Should I move the added functions to a new NumberTheory trait?

Extended GCD was already merged in #19, so this should now build on top of that if possible.

Should I move the added functions to a new NumberTheory trait?

Can it just be added to Integer? The new methods can have additional where constraints if needed for the default impl.

Extended GCD was already merged in #19, so this should now build on top of that if possible.

My implementation is not equivalent: It does not use Clone, but relies on NumRef and RefNum instead. It also consumes the input, instead of taking it by reference and forcing a clone.

The implementation currently part of Integer has some possibly redundant allocations, which are unavoidable without changing the signature of the method.

I don't know which of the two signatures is preferable. The one on master with Clone, or the one in this PR with NumRef and RefNum (which would be a breaking change)? This also affects the other functions (normalize, inverse and powm), which in their current implementation (in this PR) use NumRef and RefNum to avoid allocations.

@cuviper Which option do you prefer?

1. Add NumRef and RefNum to the relevant Integer methods (avoiding unnecessary allocations but requiring a breaking change).
2. Rewrite the new methods to be compatible with the existing interface, by introducing avoidable allocations, but without breaking changes.

FWIW, there's a 'num-modular` crate that have full support for modular operations.