Hello and thank you for this PR! It looks fine but you should add tests as well, probably in test/numbers.jl.

Since these definitions are identical to the ones for AbstractFloat, I think an alternative solution could consist in replacing AbstractFloat with Real instead in these definitions:

which I believe is also more in the spirit of what the docstring of round specifies:

To extend round to new numeric types, it is typically sufficient to define Base.round(x::NewType, r::RoundingMode).

But I'm not completely sure that's fine either. Tentatively pinging @simonbyrne since I think you're familiar with these, any preference?

Thank you so much for the quick response! I've added some tests now. I wasn't sure about using Real since there are existing definitions for Rational that might be in conflict, so I just stuck to duplicating the code a little bit instead😅

Since Rational <: Real and given the signatures of the function, the rules of dispatch with subtyping would ensure that the Rational functions get called on Rational inputs, and the Real one for subtypes of Real with no specialized methods. But anyway, that can always be for another PR, this one looks fine as it is!

I think we may want to extend the type signatures all the way to Number. Extended reals such as Dual from DualNumbers.jl may be rounded to Integers in this way as well. That said, this PR is already an improvement, so we can generalize to Real or Number later.

Bump on merging this if it's okay?

CI failed look unrelated.

But it is always helpful to re-run the CI. You can do this by clicking on "Update branch" button.

I prefer the solution @Liozou proposed of widening the existing type signatures over the approach of adding more methods because it requires less code, leaves fewer special cases, and applies more seamlessly to user-defined types. I think it should be safe to extend these definitions to ::Real. In the case of reals that cannot be rounded to the given integer type, we will get an error like this

julia> round(UInt, -5.3)
ERROR: InexactError: trunc(UInt64, -5.0)
Stacktrace:
 [1] trunc
   @ ./float.jl:760 [inlined]
 [2] round(#unused#::Type{UInt64}, x::Float64)
   @ Base ./float.jl:359
 [3] top-level scope
   @ REPL[8]:1

Mathematically, it makes sense to ask for the next highest integer after a real, so I think we should provide generic methods to support this type of query.

Thanks for the feedback! I've done what you've suggested and extended the already existing signature to Real. I'm not sure if there's another file where these methods should be moved to, but for now they're in base/float.jl.

There's a test failure that indicates an ambiguous method because there are now two definitions. There's a Rational specific definition here that seems to be in conflict. I think the solution could be to separate out the method definitions like so:

function round(::Type{T}, x::Rational{Tr}, r::RoundingMode=RoundNearest) where {T,Tr}
    if iszero(denominator(x))
        return convert(T, copysign(unsafe_rational(one(Tr), zero(Tr)), numerator(x)))
    end
    convert(T, div(numerator(x), denominator(x), r))
end

function round(::Type{T}, x::Rational{Tr}, r::RoundingMode=RoundNearest) where {T<:Integer, Tr}
    convert(T, div(numerator(x), denominator(x), r))
end

but I'm not an expert so I'd appreciate any help I get 😅

Now I see what you meant by

there are existing definitions for Rational that might be in conflict

You were quite right!

This sounds like a good way to resolve the ambiguity. The second method does not need the Tr type parameter anymore because it is only used in the !(T <: Integer) branch. There should not be any runtime impact from removing it.

Ah, the edge cases continue to crop up 😅 this may take time to fix completely. Most of the test failures seem to indicate that there need to be more specific methods defined (either that or there are now ambiguous methods because Real has a ton of subtypes being affected)

I think #45598 (comment) is still unresolved

I think #45598 (comment) is still unresolved

Whoops yeah, had missed that, resolved now!

Rounding could probably use some refactoring, but this PR doesn't need to wait for that.

From a past triage #50812 (comment)

Triage disscussed this and while there are some annoyances to how this works, round documents that it's error throwing behaviour is similar to convert, and if convert doesn't error then neither should round.

Which, as I interpret it, means triage agreed with the changes made in this PR. (specifically with widening the type signature to ::Real)

So thank you for your contribution, @theabhirath. I think we did the right thing here (or right-ish, nothing is really right or wrong in PL, just lots of opinions)

Cool, happy to help 😄! (and also to have my first contribution to the base Julia repo 🎉 )