RFR: JDK-8283075: Bad `IllegalArgumentException` message when calling `ClassDesc.arrayType(int)` which results in a rank > 255
Improving the exception messages for out-of-supported-range array types. I'll update copyrights before pushing. - Commit messages: - Appease jcheck - Update @bug line of test - JDK-8283075: Bad `IllegalArgumentException` message when calling `ClassDesc.arrayType(int)` which results in a rank > 255 Changes: https://git.openjdk.java.net/jdk/pull/7812/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=7812&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8283075 Stats: 29 lines in 2 files changed: 24 ins; 0 del; 5 mod Patch: https://git.openjdk.java.net/jdk/pull/7812.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/7812/head:pull/7812 PR: https://git.openjdk.java.net/jdk/pull/7812
Re: RFR: JDK-8283075: Bad IllegalArgumentException message when calling ClassDesc.arrayType(int) which results in a rank > 255
On Mon, 14 Mar 2022 19:56:17 GMT, Joe Darcy wrote:
> Improving the exception messages for out-of-supported-range array types.
>
> I'll update copyrights before pushing.
src/java.base/share/classes/java/lang/constant/ClassDesc.java line 186:
> 184: if (rank <= 0 || netRank >
> ConstantUtils.MAX_ARRAY_TYPE_DESC_DIMENSIONS)
> 185: throw new IllegalArgumentException("rank: " + netRank +
> 186:" exceeds maximum
> supported dimension of " +
if the `rank < 0` it won't be exceeding the maximum supported dimensions so
this message only applies to the second condition
-
PR: https://git.openjdk.java.net/jdk/pull/7812
Re: RFR: JDK-8283075: Bad IllegalArgumentException message when calling ClassDesc.arrayType(int) which results in a rank > 255 [v2]
> Improving the exception messages for out-of-supported-range array types. > > I'll update copyrights before pushing. Joe Darcy has updated the pull request incrementally with one additional commit since the last revision: Respond to review feedback. - Changes: - all: https://git.openjdk.java.net/jdk/pull/7812/files - new: https://git.openjdk.java.net/jdk/pull/7812/files/51388988..1723d710 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=7812&range=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=7812&range=00-01 Stats: 14 lines in 1 file changed: 9 ins; 5 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/7812.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/7812/head:pull/7812 PR: https://git.openjdk.java.net/jdk/pull/7812
Re: RFR: JDK-8283075: Bad IllegalArgumentException message when calling ClassDesc.arrayType(int) which results in a rank > 255 [v2]
On Mon, 14 Mar 2022 21:27:29 GMT, Joe Darcy wrote: >> Improving the exception messages for out-of-supported-range array types. >> >> I'll update copyrights before pushing. > > Joe Darcy has updated the pull request incrementally with one additional > commit since the last revision: > > Respond to review feedback. lgtm - Marked as reviewed by vromero (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/7812
Re: RFR: JDK-8283075: Bad IllegalArgumentException message when calling ClassDesc.arrayType(int) which results in a rank > 255 [v2]
On Mon, 14 Mar 2022 21:27:29 GMT, Joe Darcy wrote: >> Improving the exception messages for out-of-supported-range array types. >> >> I'll update copyrights before pushing. > > Joe Darcy has updated the pull request incrementally with one additional > commit since the last revision: > > Respond to review feedback. Marked as reviewed by alanb (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/7812
Re: RFR: JDK-8283075: Bad IllegalArgumentException message when calling ClassDesc.arrayType(int) which results in a rank > 255 [v2]
On Mon, 14 Mar 2022 21:27:29 GMT, Joe Darcy wrote:
>> Improving the exception messages for out-of-supported-range array types.
>>
>> I'll update copyrights before pushing.
>
> Joe Darcy has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Respond to review feedback.
Changes requested by plevart (Reviewer).
src/java.base/share/classes/java/lang/constant/ClassDesc.java line 179:
> 177: int netRank;
> 178: if (rank <= 0) {
> 179: throw new IllegalArgumentException("rank " + rank + "is not
> a positive value");
There's a space missing in the IAE message after the rank number. For example,
if rank == 0, the message would be: "rank 0is not positive value" ...
-
PR: https://git.openjdk.java.net/jdk/pull/7812
Re: RFR: JDK-8283075: Bad IllegalArgumentException message when calling ClassDesc.arrayType(int) which results in a rank > 255 [v2]
On Tue, 15 Mar 2022 07:50:29 GMT, Peter Levart 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 179:
>
>> 177: int netRank;
>> 178: if (rank <= 0) {
>> 179: throw new IllegalArgumentException("rank " + rank + "is not
>> a positive value");
>
> There's a space missing in the IAE message after the rank number. For
> example, if rank == 0, the message would be: "rank 0is not positive value" ...
Suggestion:
throw new IllegalArgumentException("rank " + rank + " is not a
positive value");
-
PR: https://git.openjdk.java.net/jdk/pull/7812
Re: RFR: JDK-8283075: Bad IllegalArgumentException message when calling ClassDesc.arrayType(int) which results in a rank > 255 [v2]
On Mon, 14 Mar 2022 21:27:29 GMT, Joe Darcy wrote:
>> Improving the exception messages for out-of-supported-range array types.
>>
>> I'll update copyrights before pushing.
>
> 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 [Math.addExact](currentDepth,
rank), as currentDepth will always be
in [0, 255], and rank here is confined to [1,
231-1], so it’s possible to check for netRank <
0 and use [Integer.toUnsignedString](netRank)
(or [Integer.toUnsignedLong](netRank) and let
[StringConcatFactory] 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
-
PR: https://git.openjdk.java.net/jdk/pull/7812
Re: RFR: JDK-8283075: Bad IllegalArgumentException message when calling ClassDesc.arrayType(int) which results in a rank > 255 [v3]
> Improving the exception messages for out-of-supported-range array types. > > I'll update copyrights before pushing. Joe Darcy has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains six additional commits since the last revision: - Update copyright and fix typo. - Merge branch 'master' into JDK-8283075 - Respond to review feedback. - Appease jcheck - Update @bug line of test - JDK-8283075: Bad `IllegalArgumentException` message when calling `ClassDesc.arrayType(int)` which results in a rank > 255 - Changes: - all: https://git.openjdk.java.net/jdk/pull/7812/files - new: https://git.openjdk.java.net/jdk/pull/7812/files/1723d710..514a57a1 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=7812&range=02 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=7812&range=01-02 Stats: 1594 lines in 67 files changed: 1085 ins; 268 del; 241 mod Patch: https://git.openjdk.java.net/jdk/pull/7812.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/7812/head:pull/7812 PR: https://git.openjdk.java.net/jdk/pull/7812
Re: RFR: JDK-8283075: Bad IllegalArgumentException message when calling ClassDesc.arrayType(int) which results in a rank > 255 [v2]
On Tue, 15 Mar 2022 13:50:03 GMT, ExE Boss wrote:
>> src/java.base/share/classes/java/lang/constant/ClassDesc.java line 179:
>>
>>> 177: int netRank;
>>> 178: if (rank <= 0) {
>>> 179: throw new IllegalArgumentException("rank " + rank + "is
>>> not a positive value");
>>
>> There's a space missing in the IAE message after the rank number. For
>> example, if rank == 0, the message would be: "rank 0is not positive value"
>> ...
>
> Suggestion:
>
> throw new IllegalArgumentException("rank " + rank + " is not a
> positive value");
Good catch; will fix before pushing.
-
PR: https://git.openjdk.java.net/jdk/pull/7812
Re: RFR: JDK-8283075: Bad IllegalArgumentException message when calling ClassDesc.arrayType(int) which results in a rank > 255 [v2]
On Tue, 15 Mar 2022 13:49:34 GMT, ExE Boss 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 [Math.addExact](currentDepth,
> rank), as currentDepth will always
> be in [0, 255], and rank here is confined to [1,
> 231-1], so it’s possible to check for netRank
> < 0 and use
> [Integer.toUnsignedString](netRank) (or
> [Integer.toUnsignedLong](netRank) and let
> [StringConcatFactory] 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
