RFR: 8375285: Port fdlibm asinh to Java [v7]

Anton Artemov aartemov at openjdk.org
Wed Feb 4 11:59:43 UTC 2026


On Wed, 4 Feb 2026 10:50:00 GMT, Raffaello Giulietti <rgiulietti at openjdk.org> wrote:

>> Anton Artemov has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   8375285: Addressed reviewer's comments.
>
> src/java.base/share/classes/java/lang/FdLibm.java line 3514:
> 
>> 3512:     /**
>> 3513:      * Return the Inverse Hyperbolic Sine of x
>> 3514:      *
> 
> This is inside a javadoc comment.
> If you want structuring into paragraphs, you need to add `<p>` at the beginning of new paragraphs, except the 1st.
> 
> Alternatively, you could convert to a block comment. In a block comment you can use `<` rather than `<`, but then all the `&...;` HTML named entities make no much sense.

I do not see any other method in FdLibm.java having this type paragraph formatting in place.  I will keep it as it is.

> src/java.base/share/classes/java/lang/FdLibm.java line 3520:
> 
>> 3518:      *      asinh(x) is defined so that asinh(sinh(alpha)) = alpha, -∞ < alpha < ∞
>> 3519:      *      and sinh(asinh(x)) = x, -∞ < x < ∞
>> 3520:      *      It can be written as asinh(x) = ln(|x| + sqrt(x^2 + 1)), -∞ < x < ∞
> 
> Suggestion:
> 
>      *      It can be written as asinh(x) = sign(x) * ln(|x| + sqrt(x^2 + 1)), -∞ < x < ∞

Thanks for spotting.

> src/java.base/share/classes/java/lang/FdLibm.java line 3547:
> 
>> 3545:                 return x + x;                       // x is inf or NaN
>> 3546:             }
>> 3547:             if (ix < 0x3e300000) {                   // |x| < 2**-28
> 
> Suggestion:
> 
>             if (ix < 0x3e30_0000) {                   // |x| < 2**-28

Yes, missed that one.

> src/java.base/share/classes/java/lang/Math.java line 2763:
> 
>> 2761:     /**
>> 2762:      * Returns the inverse hyperbolic sine of a {@code double} value.
>> 2763:      * The inverse hyperbolic sine of <i>x</i> is defined to be a function such that
> 
> Suggestion:
> 
>      * The inverse hyperbolic sine of <i>x</i> is defined to be the function such that
> 
> Using "the" emphasises that the function is unique.

Addressed.

> src/java.base/share/classes/java/lang/Math.java line 2765:
> 
>> 2763:      * The inverse hyperbolic sine of <i>x</i> is defined to be a function such that
>> 2764:      *  asinh({@linkplain Math#sinh sinh(<i>x</i>)}) = <i>x</i> for any <i>x</i>.
>> 2765:      *  Note that both range and domain of the exact asinh are unrestricted.
> 
> Suggestion:
> 
>      *  Note that both domain and range of the exact asinh are unrestricted.
> 
> I think that putting the domain first is more important for usage.

Yes, makes sense, addressed.

> src/java.base/share/classes/java/lang/Math.java line 2773:
> 
>> 2771:      *
>> 2772:      * <li>If the argument is infinity, then the result is
>> 2773:      * negative infinity with the same sign as the argument.
> 
> Suggestion:
> 
>      * infinity with the same sign as the argument.

That is a typo, thanks for spotting.

> src/java.base/share/classes/java/lang/Math.java line 2777:
> 
>> 2775:      * <li>If the argument is NaN, then the result is NaN.
>> 2776:      *
>> 2777:      * <p>The computed result must be within 2.5 ulps of the exact result.
> 
> This paragraph should come _after_ the bulleted list.

Moved out as suggested.

> src/java.base/share/classes/java/lang/StrictMath.java line 2194:
> 
>> 2192:      * @return  The inverse hyperbolic sine of {@code x}.
>> 2193:      * @since 27
>> 2194:      */
> 
> Same comments as for Math.

Addressed.

> test/jdk/java/lang/Math/HyperbolicTests.java line 1413:
> 
>> 1411:      * Therefore,
>> 1412:      *
>> 1413:      * 1. For large values of x asinh(x) ~= signum(x)
> 
> I don't get this.
> signum() is either 1, -1, or 0.

There is nothing wrong with the approx equality itself, but the whole statement is wrong, as the range is unrestricted. I removed this line.

> test/jdk/java/lang/StrictMath/FdlibmTranslit.java line 2761:
> 
>> 2759: 
>> 2760:     /**
>> 2761:     * Return the Inverse Hyperbolic Sine of x
> 
> I suggest turning this into a block comment, and adjust the indentation by one space in the lines below.

I noticed that some methods in this file have doc-style comments, some have block comments. Addressed.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/29273#discussion_r2763627144
PR Review Comment: https://git.openjdk.org/jdk/pull/29273#discussion_r2763627564
PR Review Comment: https://git.openjdk.org/jdk/pull/29273#discussion_r2763627901
PR Review Comment: https://git.openjdk.org/jdk/pull/29273#discussion_r2763628176
PR Review Comment: https://git.openjdk.org/jdk/pull/29273#discussion_r2763628507
PR Review Comment: https://git.openjdk.org/jdk/pull/29273#discussion_r2763628976
PR Review Comment: https://git.openjdk.org/jdk/pull/29273#discussion_r2763629790
PR Review Comment: https://git.openjdk.org/jdk/pull/29273#discussion_r2763630112
PR Review Comment: https://git.openjdk.org/jdk/pull/29273#discussion_r2763630703
PR Review Comment: https://git.openjdk.org/jdk/pull/29273#discussion_r2763631265


More information about the core-libs-dev mailing list