On Tue, 15 Mar 2022 17:09:03 GMT, Joe Darcy <da...@openjdk.org> wrote:
>> 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. Notice the overflow case is already handled by the `ArithmeticException` caught in the larger try block. ------------- PR: https://git.openjdk.java.net/jdk/pull/7812