On Tue, 15 Mar 2022 13:49:34 GMT, ExE Boss <d...@openjdk.java.net> wrote:
>> Joe Darcy has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Respond to review feedback.
>
> src/java.base/share/classes/java/lang/constant/ClassDesc.java line 185:
>
>> 183: netRank = Math.addExact(currentDepth, rank);
>> 184: if (netRank > ConstantUtils.MAX_ARRAY_TYPE_DESC_DIMENSIONS)
>> {
>> 185: throw new IllegalArgumentException("rank: " + netRank +
>
> This doesn’t need to use <code>[Math.addExact](<var>currentDepth</var>,
> <var>rank</var>)</code>, as <code><var>currentDepth</var></code> will always
> be in [0, 255], and <code><var>rank</var></code> here is confined to [1,
> 2<sup>31</sup>-1], so it’s possible to check for <code><var>netRank</var>
> < 0</code> and use
> <code>[Integer.toUnsignedString](<var>netRank</var>)</code> (or
> <code>[Integer.toUnsignedLong](<var>netRank</var>)</code> and let
> <code>[StringConcatFactory]</code> deal with it):
>
> Suggestion:
>
> netRank = currentDepth + rank;
> if (netRank > ConstantUtils.MAX_ARRAY_TYPE_DESC_DIMENSIONS ||
> netRank < 0) {
> throw new IllegalArgumentException("rank: " +
> Integer.toUnsignedLong(netRank) +
>
>
> [Integer.toUnsignedLong]:
> https://docs.oracle.com/en/java/javase/17/docs/api/java.base/java/lang/Integer.html#toUnsignedLong(int)
> [Integer.toUnsignedString]:
> https://docs.oracle.com/en/java/javase/17/docs/api/java.base/java/lang/Integer.html#toUnsignedString(int)
> [Math.addExact]:
> https://docs.oracle.com/en/java/javase/17/docs/api/java.base/java/lang/Math.html#addExact(int,int)
> [StringConcatFactory]:
> https://docs.oracle.com/en/java/javase/17/docs/api/java.base/java/lang/invoke/StringConcatFactory.html
Hmm. In this context, I think using addExact is clearer regarding the intention
of the check. Thanks.
-------------
PR: https://git.openjdk.java.net/jdk/pull/7812
