refactor Base.GMP.MPZ.import! to be consistent with Base.GMP.MPZ.export!

[NegaScout]

rename Base.GMP.MPZ.export! "dynamic" variable from a to x, since it is more in line with comment on L141 in base/gmp.jl

julia/base/gmp.jl

Lines 141 to 146 in cab11bb

	# wrapping of libgmp functions
	# - "output parameters" are labeled x, y, z, and are returned when appropriate
	# - constant input parameters are labeled a, b, c
	# - a method modifying its input has a "!" appended to its name, according to Julia's conventions
	# - some convenient methods are added (in addition to the pure MPZ ones), e.g. `add(a, b) = add!(BigInt(), a, b)`
	# and `add!(x, a) = add!(x, x, a)`.

add tests, that numbers and vectors can be exported and imported correctly and composed import and export is identity

[LilithHafner]

Thanks!

AFAICT, import! is an internal function that is neither used nor tested. Perhaps it should be removed? As of 6d71254, this PR makes a breaking change to import! so if we run pkgeval we should be able to tell if anyone depends on this internal function. export! is used once and untested so adding tests for it is very nice.

This invokes pkgeval: @nanosoldier runtests()

[NegaScout]

Oh, I wanted to run some kind of grep at least, if it is used somewhere, but I forgot. Sorry. However, this functionality is very handy for "interpreting memory as number". I am not sure if there is more use cases. However it would be nice for any serious RSA implementation (and ElGamal at least I think, possibly more cryptographic schemes), which needs to interpret memory as a number to sign it or encrypt/decrypt it.

I would like to see movement in the opposite direction and would like to see more support for this. Otherwise, some iteration based construction for this use case would be needed, unless some Julia native BigInt type is present in Base.

I also think, any big number serialization/deserialization would benefit from this.

However, I can implement any proposed changes or feel free to close this, if it is not desired change

[giordano]

No uses outside of Base according to https://juliahub.com/ui/Search?type=symbols&q=import!

[nanosoldier]

The package evaluation job you requested has completed - possible new issues were detected.
The full report is available.

[LilithHafner]

Generally, I lean toward avoiding new public API unless there is a very compelling case. The fact that nobody has used this internal function all this time makes me doubt that it is valuable commonly enough to warrant including in the public API. For those reasons, I prefer deleting insert! rather than publicizing it.

That said, it would be nice to hear other folks' opinions on whether we should keep/publicize/remove Base.GMP.MPZ.import!.

Let's talk about it at the next triage meeting Thursday at 1:15 ET.

[LilithHafner]

Triage talked about this and considered weather we want to provide a user facing wrapper of MPZ or if we just want to keep these internal and use only what we need to implement BigInts and other Base functions. Making a good user-facing wrapper of MPZ would be fairly large and we don't want to put that in Base. Instead, it makes more sense for a fully featured MPZ wrapper to exist as a package (open invitation to anyone who wants to create that package :) ). Making import! public without also making many more functions public wouldn't really make sense.

For import!, specifically, it makes sense to delete the function from Base because we're not using it and don't want to add it to the public API.

[NegaScout]

Ok, fair. It makes sense since there is still much in MPZ that would be cool to have in Julia but does not have bindings. So, lets rebase it and keep only the test and argument rename?

[LilithHafner]

It would also be good to move the definition of import! from base/ to test/