fix mod for mixes of Signed and Unsigned

[oscardssmith]

Previously this was just overfowing producing wrong answers (both for the sign convention and just the wrong modulo class)
fixes #57851

[StefanKarpinski]

It should be noted that this changes the return type in these mixed cases, but that does seem like a bug fix. Given that the sign of the result comes from the second argument, it only seems sensible that the signedness also comes from the second argument. In fact, it would be find if the result type was simply the type of the second argument.

[oscardssmith]

This doesn't change the result type.

[StefanKarpinski]

Oh, nvm then.

[oscardssmith]

(it actually did change the result type, but that was a bug)

[KristofferC]

This broke DarkCurves.jl (https://s3.amazonaws.com/julialang-reports/nanosoldier/pkgeval/by_hash/4991eb8_vs_8561cc3/DarkCurves.primary.log)

Error During Test at /home/pkgeval/.julia/packages/DarkCurves/ULKGM/test/curve.test.jl:20
  Test threw exception
  Expression: double(b) == convert(ptp, double(b_ref))
  MethodError: convert(::Type{MLUInt{4, UInt64}}, ::MLInt{4, UInt64}) is ambiguous.
  
  Candidates:
    convert(::Type{MLUInt{N, T}}, x::Integer) where {N, T}
      @ DarkIntegers ~/.julia/packages/DarkIntegers/wu36S/src/multi_limb.jl:96
    convert(::Type{V}, x::MLInt{N, T}) where {V<:Unsigned, N, T}
      @ DarkIntegers ~/.julia/packages/DarkIntegers/wu36S/src/multi_limb_signed.jl:115
  
  Possible fix, define
    convert(::Type{MLUInt{N, T}}, ::MLInt{N, T}) where {T, N, N, T}
  
  Stacktrace:
    [1] rem(x::MLInt{4, UInt64}, T::Type{MLUInt{4, UInt64}})
      @ Base ./int.jl:632
    [2] +(a::MLInt{4, UInt64}, b::MLUInt{4, UInt64})
      @ Base ./int.jl:1017
    [3] mod(x::Int64, y::MLUInt{4, UInt64})
      @ Base ./int.jl:291
    [4] MgModUInt
      @ ~/.julia/packages/DarkIntegers/wu36S/src/modulo_int_montgomery.jl:42 [inlined]
    [5] convert(::Type{MgModUInt{MLUInt{4, UInt64}, {(0xfffffffefffffc2f, 0xffffffffffffffff, 0xffffffffffffffff, 0xffffffffffffffff)}}}, x::Int64)
      @ Base ./number.jl:7
    [6] _promote
      @ ./promotion.jl:375 [inlined]
    [7] promote
      @ ./promotion.jl:400 [inlined]
    [8] ==(x::MgModUInt{MLUInt{4, UInt64}, {(0xfffffffefffffc2f, 0xffffffffffffffff, 0xffffffffffffffff, 0xffffffffffffffff)}}, y::Int64)
      @ Base ./promotion.jl:483
    [9] +(p::DarkCurves.AffinePoint{Curve_secp256k1, MgModUInt{MLUInt{4, UInt64}, {(0xfffffffefffffc2f, 0xffffffffffffffff, 0xffffffffffffffff, 0xffffffffffffffff)}}}, q::DarkCurves.AffinePoint{Curve_secp256k1, MgModUInt{MLUInt{4, UInt64}, {(0xfffffffefffffc2f, 0xffffffffffffffff, 0xffffffffffffffff, 0xffffffffffffffff)}}})
      @ DarkCurves ~/.julia/packages/DarkCurves/ULKGM/src/point_affine.jl:42
   [10] double(x::DarkCurves.AffinePoint{Curve_secp256k1, MgModUInt{MLUInt{4, UInt64}, {(0xfffffffefffffc2f, 0xffffffffffffffff, 0xffffffffffffffff, 0xffffffffffffffff)}}})
      @ Main ~/.julia/packages/DarkCurves/ULKGM/test/TestUtils.jl:10
   [11] macro expansion
      @ /opt/julia/share/julia/stdlib/v1.11/Test/src/Test.jl:676 [inlined]
   [12] macro expansion
      @ ~/.julia/packages/DarkCurves/ULKGM/test/curve.test.jl:20 [inlined]
   [13] (::var"#2#7")(curve_type::Type, point_type::Type)
      @ Main ~/.julia/packages/Jute/Rd9q9/src/macros.jl:112
Error During Test at /home/pkgeval/.julia/packages/DarkCurves/ULKGM/test/curve.test.jl:21

Can be reproduced with

using DarkIntegers
using DarkCurves
point_type = DarkCurves.AffinePoint
curve_type = Curve_secp256k1
stp = curve_scalar_type(curve_type)

y = stp.parameters[2]
mod(123, y)

I'll drop it from being backported.

[StefanKarpinski]

@KristofferC: if it was depending on an incorrect answer, that's acceptable breakage. We are allowed to fix bugs. My understanding of this change is that it is strictly a bug fix. Or is your point just that we shouldn't include it in a patch release?

[oscardssmith]

it's not that this was depending on an incorrect answer, it's that this is a type for which the new implementation is apparently insufficiently generic for some reason that I don't yet understand.

[KristofferC]

It wasn't obvious what the lesser evil / greater good was to me, and there was some pressure to get out a 1.11 quickly, so I just went with keeping packages working. We can reconsider for next 1.11.