On Fri, 16 May 2025 09:45:43 GMT, Shaojin Wen <[email protected]> wrote:
>> Similar to PR #24982
>> Document preconditions on certain DecimalDigits methods that use operations
>> either unsafe and/or without range checks.
>
> Shaojin Wen has updated the pull request incrementally with two additional
> commits since the last revision:
>
> - assert
> - document add warnings
@wenshao, thanks for the kind effort – dropped suggestions to match the
messages with the ones in JLA.
I prefer *all*<sup>1</sup> `public`, `unchecked`-prefixed `DecimalDigits`
methods to start with a sufficient `assert` preamble, to fail as early as
possible. Though I don't want to create redundant extra work, hence double
checking: @jaikiran, @minborg, @liach, WDYT?
<sup>1</sup> Currently, only two `private` methods are introduced `assert`s,
which are fine.
src/java.base/share/classes/jdk/internal/util/DecimalDigits.java line 147:
> 145: * integer.
> 146: * <p>
> 147: * <b>WARNING: Used by trusted callers. Assumes all necessary
> bounds checks have been done by the caller. </b>
Suggestion:
* <b>WARNING: This method does not perform any bound checks. </b>
src/java.base/share/classes/jdk/internal/util/DecimalDigits.java line 198:
> 196: * long.
> 197: * <p>
> 198: * <b>WARNING: Used by trusted callers. Assumes all necessary
> bounds checks have been done by the caller. </b>
Suggestion:
* <b>WARNING: This method does not perform any bound checks. </b>
src/java.base/share/classes/jdk/internal/util/DecimalDigits.java line 251:
> 249: * UTF-16 coder.
> 250: * <p>
> 251: * <b>WARNING: Used by trusted callers. Assumes all necessary
> bounds checks have been done by the caller. </b>
Suggestion:
* <b>WARNING: This method does not perform any bound checks.</b>
src/java.base/share/classes/jdk/internal/util/DecimalDigits.java line 294:
> 292: * UTF-16 coder.
> 293: * <p>
> 294: * <b>WARNING: Used by trusted callers. Assumes all necessary
> bounds checks have been done by the caller. </b>
Suggestion:
* <b>WARNING: This method does not perform any bound checks.</b>
src/java.base/share/classes/jdk/internal/util/DecimalDigits.java line 409:
> 407: * only least significant 16 bits of {@code v} are used.
> 408: * <p>
> 409: * <b>WARNING: Used by trusted callers. Assumes all necessary
> bounds checks have been done by the caller. </b>
Suggestion:
* <b>WARNING: This method does not perform any bound checks.</b>
src/java.base/share/classes/jdk/internal/util/DecimalDigits.java line 440:
> 438: * <p>
> 439: * <b>WARNING: Used by trusted callers. Assumes all necessary
> bounds checks have been done by the caller. </b>
> 440: */
No need for this level of verbosity for private methods.
Suggestion:
src/java.base/share/classes/jdk/internal/util/DecimalDigits.java line 449:
> 447: * <p>
> 448: * <b>WARNING: Used by trusted callers. Assumes all necessary
> bounds checks have been done by the caller. </b>
> 449: */
No need for this level of verbosity for private methods.
Suggestion:
-------------
PR Review: https://git.openjdk.org/jdk/pull/25246#pullrequestreview-2846561005
PR Review Comment: https://git.openjdk.org/jdk/pull/25246#discussion_r2093002611
PR Review Comment: https://git.openjdk.org/jdk/pull/25246#discussion_r2093005955
PR Review Comment: https://git.openjdk.org/jdk/pull/25246#discussion_r2093006521
PR Review Comment: https://git.openjdk.org/jdk/pull/25246#discussion_r2093007264
PR Review Comment: https://git.openjdk.org/jdk/pull/25246#discussion_r2093009890
PR Review Comment: https://git.openjdk.org/jdk/pull/25246#discussion_r2093014576
PR Review Comment: https://git.openjdk.org/jdk/pull/25246#discussion_r2093015088
