Fix -1 for BigInt factorial - Update gmp.jl

[PallHaraldsson]

Fixes #51488

[PallHaraldsson]

I noticed also this line can be eliminated:

julia/base/combinatorics.jl

Line 20 in 66fe51f

n == 0 && return one(n)

i.e. 2 checks (branches) or even 1 (not 3), by adding 1 (or zero-based indexing; that will in effect happen, +1 optimized out), if thought to be worth it for one more entry in the lookup table.

[jmkuhn]

Add test for factorial(BigInt(-1)) at

julia/test/mpfr.jl

Lines 695 to 704 in 66fe51f

	@testset "factorial" begin
	setprecision(256) do
	x = BigFloat(42)
	@test factorial(x) == factorial(BigInt(42))
	x = BigFloat(10)
	@test factorial(x) == factorial(10)
	@test_throws DomainError factorial(BigFloat(-1))
	@test_throws DomainError factorial(BigFloat(331.3))
	end
	end

[stevengj]

LGTM, other than the need for a @test_throws DomainError factorial(BigInt(-1)).

[PallHaraldsson]

@jmkuhn those tests are maybe in the wrong place. I added at test/gmp.jl, since I kind of want MPFR gone. Actually both.

JeffreySarnoff/ArbNumerics.jl#63 (comment)

[KshitijThareja]

Is this issue still open? I also had the same approach, changing the factorial function in base/gmp.jl and the writing tests for it in test/gmp.jl. Its passed all checks when I tested it locally. Should I make a PR for this issue?

[rfourquet]

Should I make a PR for this issue?

Thanks for offering your help @KshitijThareja , but in this case this PR is already open for solving the issue, and we should rather move forward with it.

[longemen3000]

bump? test fails seem unrelated