Re: RFR: 8267630: Start of release updates for JDK 18 [v9]
> 8267630: Start of release updates for JDK 18 Joe Darcy has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 22 commits: - Merge branch 'master' into 8267630 - Merge branch 'master' into 8267630 - Merge branch 'master' into 8267630 - Merge branch 'master' into 8267630 - Merge branch 'master' into 8267630 - Merge branch 'master' into 8267630 - Merge branch 'master' into 8267630 - Update symbols for JDK 17 b25. - Merge branch 'master' into 8267630 - Merge branch 'master' into 8267630 - ... and 12 more: https://git.openjdk.java.net/jdk/compare/2623b0bf...794f3173 - Changes: https://git.openjdk.java.net/jdk/pull/4175/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=4175=08 Stats: 5334 lines in 63 files changed: 5292 ins; 0 del; 42 mod Patch: https://git.openjdk.java.net/jdk/pull/4175.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4175/head:pull/4175 PR: https://git.openjdk.java.net/jdk/pull/4175
Re: RFR: 8266791: Annotation property which is compiled as an array property but changed to a single element throws NullPointerException [v3]
On Wed, 9 Jun 2021 13:56:40 GMT, Rafael Winterhalter wrote: >> During annotation parsing, the parser assumes that a declared property is of >> an array type if the parsed annotation property is defined as an array. In >> this case, the component type is `null`, and a `NullPointerException` is >> thrown. This change discovers the mismatch and throws an >> `AnnotationTypeMismatchException`. > > Rafael Winterhalter has refreshed the contents of this pull request, and > previous commits have been removed. The incremental views will show > differences compared to the previous content of the PR. test/jdk/java/lang/annotation/AnnotationTypeMismatchException/ArityTypeMismatchTest.java line 2: > 1: /* > 2: * Copyright (c) 2020, Oracle and/or its affiliates. All rights reserved. Copyright year is wrong. - PR: https://git.openjdk.java.net/jdk/pull/3940
Re: RFR: 8267893: Improve jtreg test failure handler do get native/mixed stack traces for cores and live processes [v6]
On Wed, 9 Jun 2021 19:00:39 GMT, Leonid Mesnik wrote: >> EFH is improved to process cores and get mixed stack traces with jhsdb and >> native stack traces with gdb/lldb. It might be useful because hs_err doesn't >> contain info about all threads, sometimes it is even not generated. > > Leonid Mesnik has updated the pull request incrementally with one additional > commit since the last revision: > > fxies Marked as reviewed by iignatyev (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/4234
Re: RFR: 8267630: Start of release updates for JDK 18 [v8]
> 8267630: Start of release updates for JDK 18 Joe Darcy has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 21 commits: - Merge branch 'master' into 8267630 - Merge branch 'master' into 8267630 - Merge branch 'master' into 8267630 - Merge branch 'master' into 8267630 - Merge branch 'master' into 8267630 - Merge branch 'master' into 8267630 - Update symbols for JDK 17 b25. - Merge branch 'master' into 8267630 - Merge branch 'master' into 8267630 - Merge branch 'master' into 8267630 - ... and 11 more: https://git.openjdk.java.net/jdk/compare/b41f3f8e...ae4d6cce - Changes: https://git.openjdk.java.net/jdk/pull/4175/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=4175=07 Stats: 5334 lines in 63 files changed: 5292 ins; 0 del; 42 mod Patch: https://git.openjdk.java.net/jdk/pull/4175.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4175/head:pull/4175 PR: https://git.openjdk.java.net/jdk/pull/4175
Re: RFR: 8266791: Annotation property which is compiled as an array property but changed to a single element throws NullPointerException [v3]
On Wed, 9 Jun 2021 13:56:40 GMT, Rafael Winterhalter wrote: >> During annotation parsing, the parser assumes that a declared property is of >> an array type if the parsed annotation property is defined as an array. In >> this case, the component type is `null`, and a `NullPointerException` is >> thrown. This change discovers the mismatch and throws an >> `AnnotationTypeMismatchException`. > > Rafael Winterhalter has refreshed the contents of this pull request, and > previous commits have been removed. The incremental views will show > differences compared to the previous content of the PR. Please add an overview to test before integrating; thanks. - Marked as reviewed by darcy (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/3940
Re: RFR: 8266766: Arrays of types that cannot be an annotation member do not yield exceptions [v3]
On Wed, 9 Jun 2021 13:46:28 GMT, Rafael Winterhalter wrote: >> If a type is changed from a type that can be a member of an annotation which >> is used in an array, changing it to a type that cannot be an array member >> will be treated as if the type is an annotation type. As a result, no >> exception proxy is created and the type is returned as if it was correctly >> defined. > > Rafael Winterhalter has updated the pull request with a new target base due > to a merge or a rebase. The pull request now contains one commit: > > 8266766: Arrays of types that cannot be an annotation member do not yield > exceptions To aid future maintainers, please add a short summary comment with an outline of what the test is doing before integrating. - Marked as reviewed by darcy (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/3925
Integrated: 8268192: LambdaMetafactory with invokespecial causes VerificationError
On Mon, 7 Jun 2021 23:58:44 GMT, Dan Smith wrote: > Small bug fix. This pull request has now been integrated. Changeset: 58ba48b7 Author:Dan Smith URL: https://git.openjdk.java.net/jdk/commit/58ba48b7b88eff359683aa3271c48b18f1973282 Stats: 27 lines in 3 files changed: 7 ins; 12 del; 8 mod 8268192: LambdaMetafactory with invokespecial causes VerificationError Reviewed-by: psandoz, mchung - PR: https://git.openjdk.java.net/jdk/pull/4403
Re: RFR: 8268192: LambdaMetafactory with invokespecial causes VerificationError [v2]
On Tue, 8 Jun 2021 15:30:33 GMT, Dan Smith wrote: >> Small bug fix. > > Dan Smith has updated the pull request incrementally with one additional > commit since the last revision: > > Clean up validation of implKind REF_invokeSpecial Looks good to me. - Marked as reviewed by mchung (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/4403
Re: RFR: 8268192: LambdaMetafactory with invokespecial causes VerificationError [v2]
On Tue, 8 Jun 2021 15:30:33 GMT, Dan Smith wrote: >> Small bug fix. > > Dan Smith has updated the pull request incrementally with one additional > commit since the last revision: > > Clean up validation of implKind REF_invokeSpecial Marked as reviewed by psandoz (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/4403
Re: RFR: 8268469: Update java.time to use switch expressions
On Wed, 9 Jun 2021 15:41:59 GMT, Patrick Concannon
wrote:
> Hi,
>
> Could someone please review my code for updating the code in the `java.time`
> packages to make use of the switch expressions?
>
> Kind regards,
> Patrick
My biggest comment is the spaces used to align, which I strongly object to. I
agree with @dfuch comments too.
src/java.base/share/classes/java/time/LocalDate.java line 608:
> 606: if (chronoField.isDateBased()) {
> 607: return switch (chronoField) {
> 608: case DAY_OF_MONTH -> ValueRange.of(1,
> lengthOfMonth());
For the record, I hate code that is aligned like this. It makes refactoring
much more noisy, as the author has to readjust all the associated lines. I also
don't believe it is within the spirit of Java's traditional coding guidelines.
src/java.base/share/classes/java/time/chrono/ThaiBuddhistDate.java line 331:
> 329: yield with(isoDate.withYear(nvalue -
> YEARS_DIFFERENCE));
> 330: }
> 331: case ERA -> with(isoDate.withYear((1 -
> getProlepticYear()) - YEARS_DIFFERENCE));
`checkValidIntValue` performs validation, so removing it has changed the
behavoiur
src/java.base/share/classes/java/time/format/SignStyle.java line 129:
> 127: // valid if negative or (positive and lenient)
> 128: case 0 -> !positive || !strict;// NORMAL
> 129: case 1, 4 -> true; // ALWAYS, EXCEEDS_PAD
Extra space before `4`
-
PR: https://git.openjdk.java.net/jdk/pull/4433
Re: RFR: 8268124: Update java.lang to use switch expressions [v5]
On Wed, 9 Jun 2021 10:25:35 GMT, Patrick Concannon wrote: >> Hi, >> >> Could someone please review my code for updating the code in the `java.lang` >> packages to make use of the switch expressions? >> >> Kind regards, >> Patrick > > Patrick Concannon has updated the pull request incrementally with two > additional commits since the last revision: > > - 8268124: moved cases into separate lines in DirectMethodHandleDescImpl > part II > - 8268124: moved cases onto separate lines in DirectMethodHandleDescImpl Marked as reviewed by mchung (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/4312
Re: RFR: 8268124: Update java.lang to use switch expressions [v5]
On Wed, 9 Jun 2021 10:25:35 GMT, Patrick Concannon wrote: >> Hi, >> >> Could someone please review my code for updating the code in the `java.lang` >> packages to make use of the switch expressions? >> >> Kind regards, >> Patrick > > Patrick Concannon has updated the pull request incrementally with two > additional commits since the last revision: > > - 8268124: moved cases into separate lines in DirectMethodHandleDescImpl > part II > - 8268124: moved cases onto separate lines in DirectMethodHandleDescImpl Marked as reviewed by iris (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/4312
Re: RFR: 8267893: Improve jtreg test failure handler do get native/mixed stack traces for cores and live processes [v6]
> EFH is improved to process cores and get mixed stack traces with jhsdb and > native stack traces with gdb/lldb. It might be useful because hs_err doesn't > contain info about all threads, sometimes it is even not generated. Leonid Mesnik has updated the pull request incrementally with one additional commit since the last revision: fxies - Changes: - all: https://git.openjdk.java.net/jdk/pull/4234/files - new: https://git.openjdk.java.net/jdk/pull/4234/files/e70518bc..67b61d01 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=4234=05 - incr: https://webrevs.openjdk.java.net/?repo=jdk=4234=04-05 Stats: 7 lines in 4 files changed: 2 ins; 5 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/4234.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4234/head:pull/4234 PR: https://git.openjdk.java.net/jdk/pull/4234
Re: RFR: 8267893: Improve jtreg test failure handler do get native/mixed stack traces for cores and live processes [v5]
On Wed, 2 Jun 2021 01:00:53 GMT, Leonid Mesnik wrote: >> EFH is improved to process cores and get mixed stack traces with jhsdb and >> native stack traces with gdb/lldb. It might be useful because hs_err doesn't >> contain info about all threads, sometimes it is even not generated. > > Leonid Mesnik has updated the pull request incrementally with one additional > commit since the last revision: > > spaces updated. updated diff - PR: https://git.openjdk.java.net/jdk/pull/4234
Re: RFR: 8267893: Improve jtreg test failure handler do get native/mixed stack traces for cores and live processes [v5]
On Wed, 9 Jun 2021 08:42:13 GMT, Igor Ignatyev wrote: >> Leonid Mesnik has updated the pull request incrementally with one additional >> commit since the last revision: >> >> spaces updated. > > test/failure_handler/src/share/classes/jdk/test/failurehandler/GathererFactory.java > line 32: > >> 30: import java.io.FileWriter; >> 31: import java.io.PrintWriter; >> 32: import java.nio.file.Files; > > I don't see why we need these 3 new imports. fixed > test/failure_handler/src/share/classes/jdk/test/failurehandler/ToolKit.java > line 28: > >> 26: import jdk.test.failurehandler.action.ActionSet; >> 27: import jdk.test.failurehandler.action.ActionHelper; >> 28: import jdk.test.failurehandler.action.PatternAction; > > redundant import fixed > test/failure_handler/src/share/conf/mac.properties line 71: > >> 69: native.lldb.app=lldb >> 70: native.lldb.delimiter=\0 >> 71: native.lldb.args=--core\0%p\0%java\0-o\0thread backtrace all\0-o\0quit > > could you please add a comment similar to the one in common.properties file? fixed > test/failure_handler/src/share/conf/mac.properties line 72: > >> 70: native.lldb.delimiter=\0 >> 71: native.lldb.args=--core\0%p\0%java\0-o\0thread backtrace all\0-o\0quit >> 72: native.lldb.params.timeout=360 > > why does `lldb` require an increases timeout, but `gdb` and `jhsdb` do not? Not sure I remember if there is any reason. I remove it. Let increase it later if it actually needed. - PR: https://git.openjdk.java.net/jdk/pull/4234
Integrated: 8266835: Add a --validate option to the jar tool
On Tue, 11 May 2021 10:51:18 GMT, Jorn Vernee wrote: > This patch adds a `--validate` option to the jar tool which can be used to > validate a jar file that might be malformed. For instance, if a jar is a > multi-release jar, it is malformed if different versions expose different > APIs. > > The implementation is straight forward since there already exists validation > logic that is run when creating or updating a jar. This patch just exposes > that logic directly under a new command line flag. > > I've enhanced the existing ApiValidatorTest to also create malformed jars > using the zip file APIs (the jar tool does not output malformed jars) and run > them through `jar --validate`. > > Note that while the jdk's jar tool does not output malformed jars, > third-party archiving tools might, or the jar could have been manually edited. > > Some prior discussion here: > https://mail.openjdk.java.net/pipermail/core-libs-dev/2021-May/077420.html > > Testing: running jdk/tools/jar test suite locally, tier 1-3 (in progress), > manual testing. This pull request has now been integrated. Changeset: 79010f22 Author:Jorn Vernee URL: https://git.openjdk.java.net/jdk/commit/79010f2254aee8459523800d6049f396b055f123 Stats: 150 lines in 4 files changed: 117 ins; 4 del; 29 mod 8266835: Add a --validate option to the jar tool Reviewed-by: lancea - PR: https://git.openjdk.java.net/jdk/pull/3971
Re: RFR: 8267421: j.l.constant.DirectMethodHandleDesc.Kind.valueOf(int) implementation doesn't conform to the spec regarding REF_invokeInterface handling
On Tue, 8 Jun 2021 16:46:36 GMT, Vicente Romero wrote:
> Please review this PR which is just syncing the implementation of
> DirectMethodHandleDesc.Kind.valueOf(int) with its spec. My reading of the
> method's spec is that if the value of the `refKind` parameter is:
> MethodHandleInfo.REF_invokeInterface, then
> DirectMethodHandleDesc.Kind.valueOf(int, boolean) should be called with a
> value of `true` for its second argument, currently it is invoked with `false`
> regardless of the value of the `refKind` parameter
>
> TIA
test/jdk/java/lang/constant/MethodHandleDescTest.java line 362:
> 360: public void testKind() {
> 361: for (Kind k : Kind.values()) {
> 362: assertEquals(Kind.valueOf(k.refKind),
> Kind.valueOf(k.refKind, k.refKind == MethodHandleInfo.REF_invokeInterface));
Looks like the test does not verify the cases specified by `valueOf(int
refKind, boolean isInterface)`.
i.e. For most values of refKind, there is an exact match regardless of the
value of isInterface except `REF_invokeStatic` and `REF_invokeSpecial`.
Do you mind adding those cases?
-
PR: https://git.openjdk.java.net/jdk/pull/4416
Re: RFR: 8268469: Update java.time to use switch expressions
On Wed, 9 Jun 2021 15:41:59 GMT, Patrick Concannon
wrote:
> Hi,
>
> Could someone please review my code for updating the code in the `java.time`
> packages to make use of the switch expressions?
>
> Kind regards,
> Patrick
src/java.base/share/classes/java/time/Month.java line 491:
> 489: case OCTOBER -> 274 + leap;
> 490: case NOVEMBER -> 305 + leap;
> 491: default -> 335 + leap;
It would be better to keep `DECEMBER` here for clarity - even if only in a
comment - e.g:
case NOVEMBER -> 305 + leap;
// otherwise (DECEMBER)
default-> 335 + leap;
src/java.base/share/classes/java/time/chrono/ThaiBuddhistDate.java line 334:
> 332:
> 333: default -> with(isoDate.with(field, newValue));
> 334: };
My preference would be to keep the imbricated switch structure: it is not
obvious whether this change is correct. Have you verified that
`getChronology().range(chronoField).checkValidIntValue(newValue, chronoField);`
is a no op when chronoField == ERA?
src/java.base/share/classes/java/time/format/DateTimeFormatterBuilder.java line
4992:
> 4990: }
> 4991: default -> throw new
> IllegalStateException("unreachable");
> 4992: };
Not sure I like the new version more than the old, as it seems to lead to more
duplication.
Maybe the 'Y' case should be tested before entering the switch - then the
switch could be used to assign
`var field = switch (chr) {...};`
-
PR: https://git.openjdk.java.net/jdk/pull/4433
Re: RFR: 8268469: Update java.time to use switch expressions
On Wed, 9 Jun 2021 15:41:59 GMT, Patrick Concannon
wrote:
> Hi,
>
> Could someone please review my code for updating the code in the `java.time`
> packages to make use of the switch expressions?
>
> Kind regards,
> Patrick
Looks good overall. Some misplaced comments. Also, copyright years should be
updated.
src/java.base/share/classes/java/time/LocalDateTime.java line 1188:
> 1186: case HOURS -> plusHours(amountToAdd);
> 1187: case HALF_DAYS -> plusDays(amountToAdd /
> 256).plusHours((amountToAdd % 256) * 12);
> 1188: default -> with(date.plus(amountToAdd, unit), time); //
> no overflow (256 is multiple of 2)
The comment is misplaced.
src/java.base/share/classes/java/time/chrono/ChronoLocalDateTimeImpl.java line
310:
> 308: case HOURS -> plusHours(amountToAdd);
> 309: case HALF_DAYS -> plusDays(amountToAdd /
> 256).plusHours((amountToAdd % 256) * 12);
> 310: default -> with(date.plus(amountToAdd, unit), time); //
> no overflow (256 is multiple of 2)
Misplaced comment here too.
src/java.base/share/classes/java/time/format/SignStyle.java line 127:
> 125: boolean parse(boolean positive, boolean strict, boolean fixedWidth) {
> 126: return switch (ordinal()) {
> 127: // valid if negative or (positive and lenient)
The comment should apply only to the `case 0`.
-
PR: https://git.openjdk.java.net/jdk/pull/4433
Re: RFR: 8268469: Update java.time to use switch expressions
On Wed, 9 Jun 2021 15:41:59 GMT, Patrick Concannon wrote: > Hi, > > Could someone please review my code for updating the code in the `java.time` > packages to make use of the switch expressions? > > Kind regards, > Patrick Updates look good Patrick - Marked as reviewed by lancea (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/4433
RFR: 8268469: Update java.time to use switch expressions
Hi, Could someone please review my code for updating the code in the `java.time` packages to make use of the switch expressions? Kind regards, Patrick - Commit messages: - 8268469: Update java.time to use switch expressions Changes: https://git.openjdk.java.net/jdk/pull/4433/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=4433=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8268469 Stats: 388 lines in 23 files changed: 15 ins; 97 del; 276 mod Patch: https://git.openjdk.java.net/jdk/pull/4433.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4433/head:pull/4433 PR: https://git.openjdk.java.net/jdk/pull/4433
Re: RFR: 8265909 : build.tools.dtdbuilder.DTDBuilder.java failed detecting missing path of dtd_home [v2]
On Wed, 9 Jun 2021 00:42:35 GMT, ScientificWare
wrote:
>> This concerns [dtdbuilder
>> tools](https://github.com/openjdk/jdk/tree/master/make/jdk/src/classes/build/tools/dtdbuilder).
>>
>> In jshell, try `System.getProperty("html32") + ""` you'll get a `String`.
>>
>> So, in `DTDBuilder.java` when none `dtd_home` property is set, the
>> `dtd_home` string value is not `null`, causing an exception with the present
>> test.
>>
>> The expected value is `"Must set property 'dtd_home'"`
>>
>> And in this case, should'nt we have a `System.exit(1)` too rather than a
>> simple `return` ?
>
> ScientificWare has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Changes to keep home_dtd null check.
>
> As suggested in conversation moving `+ File.separator` is more elegant than
> changing the `null` check.
Marked as reviewed by erikj (Reviewer).
-
PR: https://git.openjdk.java.net/jdk/pull/3626
Re: RFR: 8266791: Annotation property which is compiled as an array property but changed to a single element throws NullPointerException [v3]
> During annotation parsing, the parser assumes that a declared property is of > an array type if the parsed annotation property is defined as an array. In > this case, the component type is `null`, and a `NullPointerException` is > thrown. This change discovers the mismatch and throws an > `AnnotationTypeMismatchException`. Rafael Winterhalter has refreshed the contents of this pull request, and previous commits have been removed. The incremental views will show differences compared to the previous content of the PR. The pull request contains two new commits since the last revision: - 8266791: Annotation property which is compiled as an array property but changed to a single element throws NullPointerException - 8266766: Arrays of types that cannot be an annotation member do not yield exceptions - Changes: - all: https://git.openjdk.java.net/jdk/pull/3940/files - new: https://git.openjdk.java.net/jdk/pull/3940/files/f126bc2d..4e14cbb9 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=3940=02 - incr: https://webrevs.openjdk.java.net/?repo=jdk=3940=01-02 Stats: 0 lines in 0 files changed: 0 ins; 0 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/3940.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/3940/head:pull/3940 PR: https://git.openjdk.java.net/jdk/pull/3940
Re: RFR: 8268339: Upstream: 8267989: Exceptions thrown during upcalls should be handled [v5]
On Tue, 8 Jun 2021 17:34:58 GMT, Jorn Vernee wrote: >> Hi, >> >> ~This is part 2 of a two part upstreaming process of the patch mentioned in >> the subject line. The patch was split into 2 in order to document 2 separate >> specification changes that arose from a change in the implementation, with 2 >> separate CSRs. The first patch can be found here: >> https://github.com/openjdk/jdk/pull/4395~ >> >> This patch adds uniform exception handling for exceptions thrown during FLA >> upcalls. Currently, these exceptions are either handled similarly to the VMs >> `CATCH` macro, or ignored after which control returns to unsuspecting native >> code, until control returns to Java, which will then handle the exception. >> The handling depends on the invocation mode. >> >> Both of these are bad. The former because a stack trace is not printed and >> instead the VM exits with a fatal error. The latter is bad because an upcall >> completing abruptly and returning to native code which has no idea an >> exception occurred is unsafe, in the sense that invariants about the state >> of the program that the native code depends on might no longer hold. >> >> This patch adds uniform exception handling that replaces both of these cases >> (see `SharedUtils::handleUncaughtException`), which prints the exception >> stack trace, and then unconditionally exits the VM, which is the only safe >> course of action at that point. >> >> Exceptions thrown by upcalls should instead be handled during the upcall >> itself, for instance by translating the exception into an error code that is >> then returned to the native caller, which can deal with it appropriately. >> >> See also the original review for panama-foreign: >> https://github.com/openjdk/panama-foreign/pull/543 >> >> Thanks, >> Jorn >> >> Testing: `jdk_foreign` test suite. >> >> Thanks, >> Jorn > > Jorn Vernee has updated the pull request incrementally with one additional > commit since the last revision: > > Suggest try/catch Throwable in upcallStub javadoc Looks good! - Marked as reviewed by mcimadamore (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/4396
Re: RFR: 8266766: Arrays of types that cannot be an annotation member do not yield exceptions [v3]
> If a type is changed from a type that can be a member of an annotation which > is used in an array, changing it to a type that cannot be an array member > will be treated as if the type is an annotation type. As a result, no > exception proxy is created and the type is returned as if it was correctly > defined. Rafael Winterhalter has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains one commit: 8266766: Arrays of types that cannot be an annotation member do not yield exceptions - Changes: https://git.openjdk.java.net/jdk/pull/3925/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=3925=02 Stats: 130 lines in 2 files changed: 128 ins; 1 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/3925.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/3925/head:pull/3925 PR: https://git.openjdk.java.net/jdk/pull/3925
Re: RFR: 8266791: Annotation property which is compiled as an array property but changed to a single element throws NullPointerException [v2]
> During annotation parsing, the parser assumes that a declared property is of > an array type if the parsed annotation property is defined as an array. In > this case, the component type is `null`, and a `NullPointerException` is > thrown. This change discovers the mismatch and throws an > `AnnotationTypeMismatchException`. Rafael Winterhalter 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 two additional commits since the last revision: - 8266791: Annotation property which is compiled as an array property but changed to a single element throws NullPointerException - 8266766: Arrays of types that cannot be an annotation member do not yield exceptions - Changes: - all: https://git.openjdk.java.net/jdk/pull/3940/files - new: https://git.openjdk.java.net/jdk/pull/3940/files/aeae89ac..f126bc2d Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=3940=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk=3940=00-01 Stats: 607815 lines in 5193 files changed: 511783 ins; 75812 del; 20220 mod Patch: https://git.openjdk.java.net/jdk/pull/3940.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/3940/head:pull/3940 PR: https://git.openjdk.java.net/jdk/pull/3940
Re: RFR: 8266835: Add a --validate option to the jar tool [v3]
On Wed, 9 Jun 2021 13:37:02 GMT, Jorn Vernee wrote: >> src/jdk.jartool/share/classes/sun/tools/jar/resources/jar.properties line >> 241: >> >>> 239: \ --validate Validate a jar. This process will >>> validate that the API\n\ >>> 240: \ exported by a multi-release jar is >>> consistent across all\n\ >>> 241: \ different release versions. >> >> Thanks for the update. I see that that "jar archive" is used in some of the >> existing options so maybe tweak it to: >> >> Validate the contents of the jar archive. >> This option will validate that the API exported by ... > > Done. > > FWIW, I also thought about using "jar file", but the jar tool can also take > input from stdin, so that didn't seem appropriate. Yeah, there are inconsistencies. In other contexts we say "JAR file". In the usage output it has historically used "jar archive". - PR: https://git.openjdk.java.net/jdk/pull/3971
Re: RFR: 8266835: Add a --validate option to the jar tool [v3]
On Wed, 9 Jun 2021 13:02:50 GMT, Alan Bateman wrote: >> Jorn Vernee has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Improve help message. > > src/jdk.jartool/share/classes/sun/tools/jar/resources/jar.properties line 241: > >> 239: \ --validate Validate a jar. This process will >> validate that the API\n\ >> 240: \ exported by a multi-release jar is >> consistent across all\n\ >> 241: \ different release versions. > > Thanks for the update. I see that that "jar archive" is used in some of the > existing options so maybe tweak it to: > > Validate the contents of the jar archive. > This option will validate that the API exported by ... Done. FWIW, I also thought about using "jar file", but the jar tool can also take input from stdin, so that didn't seem appropriate. - PR: https://git.openjdk.java.net/jdk/pull/3971
Re: RFR: 8266835: Add a --validate option to the jar tool [v4]
> This patch adds a `--validate` option to the jar tool which can be used to > validate a jar file that might be malformed. For instance, if a jar is a > multi-release jar, it is malformed if different versions expose different > APIs. > > The implementation is straight forward since there already exists validation > logic that is run when creating or updating a jar. This patch just exposes > that logic directly under a new command line flag. > > I've enhanced the existing ApiValidatorTest to also create malformed jars > using the zip file APIs (the jar tool does not output malformed jars) and run > them through `jar --validate`. > > Note that while the jdk's jar tool does not output malformed jars, > third-party archiving tools might, or the jar could have been manually edited. > > Some prior discussion here: > https://mail.openjdk.java.net/pipermail/core-libs-dev/2021-May/077420.html > > Testing: running jdk/tools/jar test suite locally, tier 1-3 (in progress), > manual testing. Jorn Vernee has updated the pull request incrementally with one additional commit since the last revision: Update help message - Changes: - all: https://git.openjdk.java.net/jdk/pull/3971/files - new: https://git.openjdk.java.net/jdk/pull/3971/files/c4cfe847..6efd89de Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=3971=03 - incr: https://webrevs.openjdk.java.net/?repo=jdk=3971=02-03 Stats: 4 lines in 1 file changed: 1 ins; 0 del; 3 mod Patch: https://git.openjdk.java.net/jdk/pull/3971.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/3971/head:pull/3971 PR: https://git.openjdk.java.net/jdk/pull/3971
Integrated: 8266598: Exception values for AnnotationTypeMismatchException are not always informative
On Wed, 5 May 2021 21:36:45 GMT, Rafael Winterhalter wrote: > This improves the messages that are provided by > `AnnotationTypeMismatchException`s. The message provided by > AnnotationTypeMismatchExceptions is deliberately undefined such that this > should not break any contract. This pull request has now been integrated. Changeset: 7b1e4024 Author:Rafael Winterhalter Committer: Joel Borggrén-Franck URL: https://git.openjdk.java.net/jdk/commit/7b1e4024c02e6e831502e20cdbf54efb6240d12b Stats: 28 lines in 3 files changed: 15 ins; 3 del; 10 mod 8266598: Exception values for AnnotationTypeMismatchException are not always informative Reviewed-by: jfranck - PR: https://git.openjdk.java.net/jdk/pull/3892
Re: RFR: 8266835: Add a --validate option to the jar tool [v3]
On Tue, 8 Jun 2021 18:32:36 GMT, Jorn Vernee wrote: >> This patch adds a `--validate` option to the jar tool which can be used to >> validate a jar file that might be malformed. For instance, if a jar is a >> multi-release jar, it is malformed if different versions expose different >> APIs. >> >> The implementation is straight forward since there already exists validation >> logic that is run when creating or updating a jar. This patch just exposes >> that logic directly under a new command line flag. >> >> I've enhanced the existing ApiValidatorTest to also create malformed jars >> using the zip file APIs (the jar tool does not output malformed jars) and >> run them through `jar --validate`. >> >> Note that while the jdk's jar tool does not output malformed jars, >> third-party archiving tools might, or the jar could have been manually >> edited. >> >> Some prior discussion here: >> https://mail.openjdk.java.net/pipermail/core-libs-dev/2021-May/077420.html >> >> Testing: running jdk/tools/jar test suite locally, tier 1-3 (in progress), >> manual testing. > > Jorn Vernee has updated the pull request incrementally with one additional > commit since the last revision: > > Improve help message. src/jdk.jartool/share/classes/sun/tools/jar/resources/jar.properties line 241: > 239: \ --validate Validate a jar. This process will validate > that the API\n\ > 240: \ exported by a multi-release jar is > consistent across all\n\ > 241: \ different release versions. Thanks for the update. I see that that "jar archive" is used in some of the existing options so maybe tweak it to: Validate the contents of the jar archive. This option will validate that the API exported by ... - PR: https://git.openjdk.java.net/jdk/pull/3971
Re: RFR: 8266598: Exception values for AnnotationTypeMismatchException are not always informative [v4]
On Wed, 9 Jun 2021 10:02:35 GMT, Rafael Winterhalter wrote: >> This improves the messages that are provided by >> `AnnotationTypeMismatchException`s. The message provided by >> AnnotationTypeMismatchExceptions is deliberately undefined such that this >> should not break any contract. > > Rafael Winterhalter has refreshed the contents of this pull request, and > previous commits have been removed. The incremental views will show > differences compared to the previous content of the PR. The pull request > contains one new commit since the last revision: > > 8266598: Exception values for AnnotationTypeMismatchException are not > always informative LGTM - Marked as reviewed by jfranck (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/3892
Re: RFR: 8266835: Add a --validate option to the jar tool [v3]
On Tue, 8 Jun 2021 18:32:36 GMT, Jorn Vernee wrote: >> This patch adds a `--validate` option to the jar tool which can be used to >> validate a jar file that might be malformed. For instance, if a jar is a >> multi-release jar, it is malformed if different versions expose different >> APIs. >> >> The implementation is straight forward since there already exists validation >> logic that is run when creating or updating a jar. This patch just exposes >> that logic directly under a new command line flag. >> >> I've enhanced the existing ApiValidatorTest to also create malformed jars >> using the zip file APIs (the jar tool does not output malformed jars) and >> run them through `jar --validate`. >> >> Note that while the jdk's jar tool does not output malformed jars, >> third-party archiving tools might, or the jar could have been manually >> edited. >> >> Some prior discussion here: >> https://mail.openjdk.java.net/pipermail/core-libs-dev/2021-May/077420.html >> >> Testing: running jdk/tools/jar test suite locally, tier 1-3 (in progress), >> manual testing. > > Jorn Vernee has updated the pull request incrementally with one additional > commit since the last revision: > > Improve help message. If the changes look good like this, please review the accompanying CSR for the command line option changes: https://bugs.openjdk.java.net/browse/JDK-8267402 Thanks - PR: https://git.openjdk.java.net/jdk/pull/3971
Integrated: 8264859: Implement Context-Specific Deserialization Filters
On Wed, 12 May 2021 13:58:44 GMT, Roger Riggs wrote: > JEP 415: Context-specific Deserialization Filters extends the deserialization > filtering mechanisms with more flexible and customizable protections against > malicious deserialization. See JEP 415: https://openjdk.java.net/jeps/415. > The `java.io.ObjectInputFilter` and `java.io.ObjectInputStream` classes are > extended with additional > configuration mechanisms and filter utilities. > > javadoc for `ObjectInputFilter`, `ObjectInputFilter.Config`, and > `ObjectInputStream`: > > http://cr.openjdk.java.net/~rriggs/filter-factory/java.base/java/io/ObjectInputFilter.html This pull request has now been integrated. Changeset: 13d61804 Author:Roger Riggs URL: https://git.openjdk.java.net/jdk/commit/13d618042112aa761ef256aa35ec0a8b808cd78b Stats: 2594 lines in 9 files changed: 2409 ins; 40 del; 145 mod 8264859: Implement Context-Specific Deserialization Filters Reviewed-by: bchristi, dfuchs, chegar - PR: https://git.openjdk.java.net/jdk/pull/3996
Re: RFR: 8267421: j.l.constant.DirectMethodHandleDesc.Kind.valueOf(int) implementation doesn't conform to the spec regarding REF_invokeInterface handling
On Tue, 8 Jun 2021 16:46:36 GMT, Vicente Romero wrote: > Please review this PR which is just syncing the implementation of > DirectMethodHandleDesc.Kind.valueOf(int) with its spec. My reading of the > method's spec is that if the value of the `refKind` parameter is: > MethodHandleInfo.REF_invokeInterface, then > DirectMethodHandleDesc.Kind.valueOf(int, boolean) should be called with a > value of `true` for its second argument, currently it is invoked with `false` > regardless of the value of the `refKind` parameter > > TIA Looks good to me. - Marked as reviewed by jvernee (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/4416
Re: RFR: 8268428: Test java/foreign/TestResourceScope.java fails: expected [M] but found [N] [v2]
> This tests fails intermittently but very rarely, on Windows it seems. The > subtest in question is `testLockSharedMultiThread`. This test is spawning a > number of threads, each of which: > > * increments a counter atomically > * acquire scope > * waits some time > * release scope > * decrement counter (in finally block) > > The main test thread tries (while the counter value is > 0) to repeatedly > close the scope, and asserts that if the scope closes successfully, then the > counter value should be exactly zero. > > Looking at the test closely, I realized there's an issue: the order in which > counter is incremented and scope is acquired is wrong. As such it is possible > for counter to be increased, but then for subsequent acquire to fail (e.g. if > segment is already closed). This would lead to the main test thread to fail, > as it would appear as if the segment has been closed successfully, but the > counter value is not zero. > > The fix is to only increment the counter after a successful acquire. I have > also tweaked the test so that it keeps trying closing the scope, even if the > counter value reaches zero permanently (e.g. all other threads have > completed). Maurizio Cimadamore has updated the pull request incrementally with one additional commit since the last revision: Update wrong copyright date - Changes: - all: https://git.openjdk.java.net/jdk/pull/4427/files - new: https://git.openjdk.java.net/jdk/pull/4427/files/eb0c5d50..13f87671 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=4427=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk=4427=00-01 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/4427.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4427/head:pull/4427 PR: https://git.openjdk.java.net/jdk/pull/4427
Re: RFR: 8268428: Test java/foreign/TestResourceScope.java fails: expected [M] but found [N] [v2]
On Wed, 9 Jun 2021 11:31:52 GMT, Maurizio Cimadamore wrote: >> This tests fails intermittently but very rarely, on Windows it seems. The >> subtest in question is `testLockSharedMultiThread`. This test is spawning a >> number of threads, each of which: >> >> * increments a counter atomically >> * acquire scope >> * waits some time >> * release scope >> * decrement counter (in finally block) >> >> The main test thread tries (while the counter value is > 0) to repeatedly >> close the scope, and asserts that if the scope closes successfully, then the >> counter value should be exactly zero. >> >> Looking at the test closely, I realized there's an issue: the order in which >> counter is incremented and scope is acquired is wrong. As such it is >> possible for counter to be increased, but then for subsequent acquire to >> fail (e.g. if segment is already closed). This would lead to the main test >> thread to fail, as it would appear as if the segment has been closed >> successfully, but the counter value is not zero. >> >> The fix is to only increment the counter after a successful acquire. I have >> also tweaked the test so that it keeps trying closing the scope, even if the >> counter value reaches zero permanently (e.g. all other threads have >> completed). > > Maurizio Cimadamore has updated the pull request incrementally with one > additional commit since the last revision: > > Update wrong copyright date Marked as reviewed by dfuchs (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/4427
Re: RFR: 8266791: Annotation property which is compiled as an array property but changed to a single element throws NullPointerException
On Sun, 9 May 2021 21:59:29 GMT, Rafael Winterhalter wrote: > During annotation parsing, the parser assumes that a declared property is of > an array type if the parsed annotation property is defined as an array. In > this case, the component type is `null`, and a `NullPointerException` is > thrown. This change discovers the mismatch and throws an > `AnnotationTypeMismatchException`. Link to CSR: https://bugs.openjdk.java.net/browse/JDK-8268452 - PR: https://git.openjdk.java.net/jdk/pull/3940
Re: RFR: 8268428: Test java/foreign/TestResourceScope.java fails: expected [M] but found [N]
On Wed, 9 Jun 2021 10:33:34 GMT, Maurizio Cimadamore wrote: > This tests fails intermittently but very rarely, on Windows it seems. The > subtest in question is `testLockSharedMultiThread`. This test is spawning a > number of threads, each of which: > > * increments a counter atomically > * acquire scope > * waits some time > * release scope > * decrement counter (in finally block) > > The main test thread tries (while the counter value is > 0) to repeatedly > close the scope, and asserts that if the scope closes successfully, then the > counter value should be exactly zero. > > Looking at the test closely, I realized there's an issue: the order in which > counter is incremented and scope is acquired is wrong. As such it is possible > for counter to be increased, but then for subsequent acquire to fail (e.g. if > segment is already closed). This would lead to the main test thread to fail, > as it would appear as if the segment has been closed successfully, but the > counter value is not zero. > > The fix is to only increment the counter after a successful acquire. I have > also tweaked the test so that it keeps trying closing the scope, even if the > counter value reaches zero permanently (e.g. all other threads have > completed). I agree with the analysis and test code changes. Trivially you should also update the copyright years on this file. - Marked as reviewed by dfuchs (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/4427
Re: RFR: 8268124: Update java.lang to use switch expressions [v5]
On Wed, 9 Jun 2021 10:25:35 GMT, Patrick Concannon wrote: >> Hi, >> >> Could someone please review my code for updating the code in the `java.lang` >> packages to make use of the switch expressions? >> >> Kind regards, >> Patrick > > Patrick Concannon has updated the pull request incrementally with two > additional commits since the last revision: > > - 8268124: moved cases into separate lines in DirectMethodHandleDescImpl > part II > - 8268124: moved cases onto separate lines in DirectMethodHandleDescImpl Marked as reviewed by dfuchs (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/4312
Re: RFR: 8266766: Arrays of types that cannot be an annotation member do not yield exceptions
On Fri, 7 May 2021 18:58:02 GMT, Rafael Winterhalter wrote: > If a type is changed from a type that can be a member of an annotation which > is used in an array, changing it to a type that cannot be an array member > will be treated as if the type is an annotation type. As a result, no > exception proxy is created and the type is returned as if it was correctly > defined. Link to CSR: https://bugs.openjdk.java.net/browse/JDK-8268447 - PR: https://git.openjdk.java.net/jdk/pull/3925
Re: RFR: 8266766: Arrays of types that cannot be an annotation member do not yield exceptions [v2]
> If a type is changed from a type that can be a member of an annotation which > is used in an array, changing it to a type that cannot be an array member > will be treated as if the type is an annotation type. As a result, no > exception proxy is created and the type is returned as if it was correctly > defined. Rafael Winterhalter has refreshed the contents of this pull request, and previous commits have been removed. The incremental views will show differences compared to the previous content of the PR. The pull request contains one new commit since the last revision: 8266766: Arrays of types that cannot be an annotation member do not yield exceptions - Changes: - all: https://git.openjdk.java.net/jdk/pull/3925/files - new: https://git.openjdk.java.net/jdk/pull/3925/files/55284e94..3e3eb032 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=3925=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk=3925=00-01 Stats: 0 lines in 0 files changed: 0 ins; 0 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/3925.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/3925/head:pull/3925 PR: https://git.openjdk.java.net/jdk/pull/3925
RFR: 8268428: Test java/foreign/TestResourceScope.java fails: expected [M] but found [N]
This tests fails intermittently but very rarely, on Windows it seems. The subtest in question is `testLockSharedMultiThread`. This test is spawning a number of threads, each of which: * increments a counter atomically * acquire scope * waits some time * release scope * decrement counter (in finally block) The main test thread tries (while the counter value is > 0) to repeatedly close the scope, and asserts that if the scope closes successfully, then the counter value should be exactly zero. Looking at the test closely, I realized there's an issue: the order in which counter is incremented and scope is acquired is wrong. As such it is possible for counter to be increased, but then for subsequent acquire to fail (e.g. if segment is already closed). This would lead to the main test thread to fail, as it would appear as if the segment has been closed successfully, but the counter value is not zero. The fix is to only increment the counter after a successful acquire. I have also tweaked the test so that it keeps trying closing the scope, even if the counter value reaches zero permanently (e.g. all other threads have completed). - Commit messages: - Fix TestResourceScope locking test Changes: https://git.openjdk.java.net/jdk/pull/4427/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=4427=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8268428 Stats: 14 lines in 1 file changed: 6 ins; 3 del; 5 mod Patch: https://git.openjdk.java.net/jdk/pull/4427.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4427/head:pull/4427 PR: https://git.openjdk.java.net/jdk/pull/4427
Re: RFR: 8268113: Re-use Long.hashCode() where possible [v6]
> There is a few JDK classes duplicating the contents of Long.hashCode() for > hash code calculation. They should explicitly delegate to Long.hashCode(). Сергей Цыпанов 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: - Merge branch 'master' into 8268113 - Merge branch 'master' into 8268113 - Merge branch 'master' into 8268113 - 8268113: Inline local vars where reasonable - 8268113: Delegate to Double.hashCode() - 8268113: Re-use Long.hashCode() where possible - Changes: - all: https://git.openjdk.java.net/jdk/pull/4309/files - new: https://git.openjdk.java.net/jdk/pull/4309/files/76af35c6..3a7b5515 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=4309=05 - incr: https://webrevs.openjdk.java.net/?repo=jdk=4309=04-05 Stats: 5577 lines in 149 files changed: 3466 ins; 1405 del; 706 mod Patch: https://git.openjdk.java.net/jdk/pull/4309.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4309/head:pull/4309 PR: https://git.openjdk.java.net/jdk/pull/4309
Re: RFR: 8268124: Update java.lang to use switch expressions [v5]
On Wed, 9 Jun 2021 10:25:35 GMT, Patrick Concannon wrote: >> Hi, >> >> Could someone please review my code for updating the code in the `java.lang` >> packages to make use of the switch expressions? >> >> Kind regards, >> Patrick > > Patrick Concannon has updated the pull request incrementally with two > additional commits since the last revision: > > - 8268124: moved cases into separate lines in DirectMethodHandleDescImpl > part II > - 8268124: moved cases onto separate lines in DirectMethodHandleDescImpl Marked as reviewed by lancea (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/4312
Re: RFR: 8268124: Update java.lang to use switch expressions [v3]
On Thu, 3 Jun 2021 17:04:58 GMT, Mandy Chung wrote:
>> Patrick Concannon has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> 8268124: small refactoring; fixed misplaced comment and added missing
>> lambda operator
>
> src/java.base/share/classes/java/lang/constant/DirectMethodHandleDescImpl.java
> line 138:
>
>> 136: public String lookupDescriptor() {
>> 137: return switch (kind) {
>> 138: case VIRTUAL, SPECIAL,
>
> Nit: I prefer to have each case in a separate line (in this switch and also
> the switch in `resolveConstantDesc`.
Hi Mandy. Thanks for your suggestion. I've now moved each case onto its own
line. Please see ccc9db7 / df5b34e
-
PR: https://git.openjdk.java.net/jdk/pull/4312
Re: RFR: 8268124: Update java.lang to use switch expressions [v5]
> Hi, > > Could someone please review my code for updating the code in the `java.lang` > packages to make use of the switch expressions? > > Kind regards, > Patrick Patrick Concannon has updated the pull request incrementally with two additional commits since the last revision: - 8268124: moved cases into separate lines in DirectMethodHandleDescImpl part II - 8268124: moved cases onto separate lines in DirectMethodHandleDescImpl - Changes: - all: https://git.openjdk.java.net/jdk/pull/4312/files - new: https://git.openjdk.java.net/jdk/pull/4312/files/7907f3eb..df5b34e5 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=4312=04 - incr: https://webrevs.openjdk.java.net/?repo=jdk=4312=03-04 Stats: 12 lines in 1 file changed: 6 ins; 0 del; 6 mod Patch: https://git.openjdk.java.net/jdk/pull/4312.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4312/head:pull/4312 PR: https://git.openjdk.java.net/jdk/pull/4312
Re: RFR: 8268124: Update java.lang to use switch expressions [v4]
> Hi, > > Could someone please review my code for updating the code in the `java.lang` > packages to make use of the switch expressions? > > Kind regards, > Patrick Patrick Concannon 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 four additional commits since the last revision: - Merge remote-tracking branch 'origin/master' into JDK-8268124 - 8268124: small refactoring; fixed misplaced comment and added missing lambda operator - Merge remote-tracking branch 'origin/master' into JDK-8268124 - 8268124: Update java.lang to use switch expressions - Changes: - all: https://git.openjdk.java.net/jdk/pull/4312/files - new: https://git.openjdk.java.net/jdk/pull/4312/files/a8706b02..7907f3eb Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=4312=03 - incr: https://webrevs.openjdk.java.net/?repo=jdk=4312=02-03 Stats: 463928 lines in 821 files changed: 455007 ins; 5063 del; 3858 mod Patch: https://git.openjdk.java.net/jdk/pull/4312.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4312/head:pull/4312 PR: https://git.openjdk.java.net/jdk/pull/4312
Re: RFR: 8268124: Update java.lang to use switch expressions [v4]
On Wed, 2 Jun 2021 16:10:19 GMT, Rémi Forax wrote: >> Patrick Concannon 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 four additional >> commits since the last revision: >> >> - Merge remote-tracking branch 'origin/master' into JDK-8268124 >> - 8268124: small refactoring; fixed misplaced comment and added missing >> lambda operator >> - Merge remote-tracking branch 'origin/master' into JDK-8268124 >> - 8268124: Update java.lang to use switch expressions > > src/java.base/share/classes/java/lang/invoke/MemberName.java line 331: > >> 329: assert(false) : this+" != >> "+MethodHandleNatives.refKindName((byte)originalRefKind); >> 330: yield true; >> 331: } > > this code always yield true, better to check if the assert are enabled, do > the switch in that case and always return true Thanks for your suggestion. I've incorporated it into commit a8706b0 - PR: https://git.openjdk.java.net/jdk/pull/4312
Re: RFR: 8266598: Exception values for AnnotationTypeMismatchException are not always informative [v3]
On Sat, 8 May 2021 09:51:46 GMT, Rafael Winterhalter wrote: >> This improves the messages that are provided by >> `AnnotationTypeMismatchException`s. The message provided by >> AnnotationTypeMismatchExceptions is deliberately undefined such that this >> should not break any contract. > > Rafael Winterhalter has refreshed the contents of this pull request, and > previous commits have been removed. The incremental views will show > differences compared to the previous content of the PR. Moved the bug number to the same line on both tests. - PR: https://git.openjdk.java.net/jdk/pull/3892
Re: RFR: 8266598: Exception values for AnnotationTypeMismatchException are not always informative [v4]
> This improves the messages that are provided by > `AnnotationTypeMismatchException`s. The message provided by > AnnotationTypeMismatchExceptions is deliberately undefined such that this > should not break any contract. Rafael Winterhalter has refreshed the contents of this pull request, and previous commits have been removed. The incremental views will show differences compared to the previous content of the PR. The pull request contains one new commit since the last revision: 8266598: Exception values for AnnotationTypeMismatchException are not always informative - Changes: - all: https://git.openjdk.java.net/jdk/pull/3892/files - new: https://git.openjdk.java.net/jdk/pull/3892/files/9acdb5d0..064334c4 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=3892=03 - incr: https://webrevs.openjdk.java.net/?repo=jdk=3892=02-03 Stats: 4 lines in 2 files changed: 0 ins; 2 del; 2 mod Patch: https://git.openjdk.java.net/jdk/pull/3892.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/3892/head:pull/3892 PR: https://git.openjdk.java.net/jdk/pull/3892
Re: RFR: 8266598: Exception values for AnnotationTypeMismatchException are not always informative [v3]
On Sat, 8 May 2021 09:51:46 GMT, Rafael Winterhalter wrote: >> This improves the messages that are provided by >> `AnnotationTypeMismatchException`s. The message provided by >> AnnotationTypeMismatchExceptions is deliberately undefined such that this >> should not break any contract. > > Rafael Winterhalter has refreshed the contents of this pull request, and > previous commits have been removed. The incremental views will show > differences compared to the previous content of the PR. Changes requested by jfranck (Reviewer). test/jdk/java/lang/annotation/AnnotationTypeMismatchException/AnnotationTypeMismatchTest.java line 27: > 25: * @test > 26: * @bug 8228988 > 27: * @bug 8266598 @bug numbers are usually on the same line: "* @bug 8228988 8266598" test/jdk/java/lang/annotation/AnnotationTypeMismatchException/EnumTypeMismatchTest.java line 27: > 25: * @test > 26: * @bug 8228988 > 27: * @bug 8266598 Same, here, single line - PR: https://git.openjdk.java.net/jdk/pull/3892
Re: RFR: 8265518: C1: Intrinsic support for Preconditions.checkIndex [v13]
> The JDK codebase re-created many variants of checkIndex(`grep -I -r > 'cehckIndex' jdk/`). A notable variant is java.nio.Buffer.checkIndex, which > annotated with @IntrinsicCandidate and it only has a corresponding C1 > intrinsic version. > > In fact, there is an utility method > `jdk.internal.util.Preconditions.checkIndex`(wrapped by > java.lang.Objects.checkIndex) that behaves the same as these variants of > checkIndex, we can replace these re-created variants of checkIndex by > Objects.checkIndex, it would significantly reduce duplicated code and enjoys > performance improvement because Preconditions.checkIndex is > @IntrinsicCandidate and it has a corresponding intrinsic method in HotSpot. > > But, the problem is currently HotSpot only implements the C2 version of > Preconditions.checkIndex. To reuse it global-widely in JDK code, I think we > can firstly implement its C1 counterpart. There are also a few kinds of stuff > we can do later: > > 1. Replace all variants of checkIndex by Objects.checkIndex in the whole JDK > codebase. > 2. Remove Buffer.checkIndex and obsolete/deprecate InlineNIOCheckIndex flag > > Testing: cds, compiler and jdk Yi Yang has updated the pull request incrementally with one additional commit since the last revision: more comment - Changes: - all: https://git.openjdk.java.net/jdk/pull/3615/files - new: https://git.openjdk.java.net/jdk/pull/3615/files/63f1c30d..87d8b399 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=3615=12 - incr: https://webrevs.openjdk.java.net/?repo=jdk=3615=11-12 Stats: 2 lines in 1 file changed: 2 ins; 0 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/3615.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/3615/head:pull/3615 PR: https://git.openjdk.java.net/jdk/pull/3615
Re: RFR: 8266598: Exception values for AnnotationTypeMismatchException are not always informative [v3]
On Sat, 8 May 2021 09:51:46 GMT, Rafael Winterhalter wrote: >> This improves the messages that are provided by >> `AnnotationTypeMismatchException`s. The message provided by >> AnnotationTypeMismatchExceptions is deliberately undefined such that this >> should not break any contract. > > Rafael Winterhalter has refreshed the contents of this pull request, and > previous commits have been removed. The incremental views will show > differences compared to the previous content of the PR. Link to CSR: https://bugs.openjdk.java.net/browse/JDK-8266768 - PR: https://git.openjdk.java.net/jdk/pull/3892
Re: RFR: 8267893: Improve jtreg test failure handler do get native/mixed stack traces for cores and live processes [v5]
On Wed, 2 Jun 2021 01:00:53 GMT, Leonid Mesnik wrote: >> EFH is improved to process cores and get mixed stack traces with jhsdb and >> native stack traces with gdb/lldb. It might be useful because hs_err doesn't >> contain info about all threads, sometimes it is even not generated. > > Leonid Mesnik has updated the pull request incrementally with one additional > commit since the last revision: > > spaces updated. Changes requested by iignatyev (Reviewer). test/failure_handler/src/share/classes/jdk/test/failurehandler/GathererFactory.java line 32: > 30: import java.io.FileWriter; > 31: import java.io.PrintWriter; > 32: import java.nio.file.Files; I don't see why we need these 3 new imports. test/failure_handler/src/share/classes/jdk/test/failurehandler/ToolKit.java line 28: > 26: import jdk.test.failurehandler.action.ActionSet; > 27: import jdk.test.failurehandler.action.ActionHelper; > 28: import jdk.test.failurehandler.action.PatternAction; redundant import test/failure_handler/src/share/conf/linux.properties line 62: > 60: cores=native.gdb > 61: native.gdb.app=gdb > 62: native.gdb.args=%java\0-c\0%p\0-batch\0-ex\0thread apply all backtrace could you please add a comment similar to the one in `common.properties` file? test/failure_handler/src/share/conf/mac.properties line 71: > 69: native.lldb.app=lldb > 70: native.lldb.delimiter=\0 > 71: native.lldb.args=--core\0%p\0%java\0-o\0thread backtrace all\0-o\0quit could you please add a comment similar to the one in common.properties file? test/failure_handler/src/share/conf/mac.properties line 72: > 70: native.lldb.delimiter=\0 > 71: native.lldb.args=--core\0%p\0%java\0-o\0thread backtrace all\0-o\0quit > 72: native.lldb.params.timeout=360 why does `lldb` require an increases timeout, but `gdb` and `jhsdb` do not? - PR: https://git.openjdk.java.net/jdk/pull/4234
Re: RFR: 8265518: C1: Intrinsic support for Preconditions.checkIndex [v12]
> The JDK codebase re-created many variants of checkIndex(`grep -I -r > 'cehckIndex' jdk/`). A notable variant is java.nio.Buffer.checkIndex, which > annotated with @IntrinsicCandidate and it only has a corresponding C1 > intrinsic version. > > In fact, there is an utility method > `jdk.internal.util.Preconditions.checkIndex`(wrapped by > java.lang.Objects.checkIndex) that behaves the same as these variants of > checkIndex, we can replace these re-created variants of checkIndex by > Objects.checkIndex, it would significantly reduce duplicated code and enjoys > performance improvement because Preconditions.checkIndex is > @IntrinsicCandidate and it has a corresponding intrinsic method in HotSpot. > > But, the problem is currently HotSpot only implements the C2 version of > Preconditions.checkIndex. To reuse it global-widely in JDK code, I think we > can firstly implement its C1 counterpart. There are also a few kinds of stuff > we can do later: > > 1. Replace all variants of checkIndex by Objects.checkIndex in the whole JDK > codebase. > 2. Remove Buffer.checkIndex and obsolete/deprecate InlineNIOCheckIndex flag > > Testing: cds, compiler and jdk Yi Yang has updated the pull request incrementally with two additional commits since the last revision: - c1 can not handle 0 constant value when using cmp - fix - Changes: - all: https://git.openjdk.java.net/jdk/pull/3615/files - new: https://git.openjdk.java.net/jdk/pull/3615/files/289d752c..63f1c30d Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=3615=11 - incr: https://webrevs.openjdk.java.net/?repo=jdk=3615=10-11 Stats: 51 lines in 1 file changed: 23 ins; 17 del; 11 mod Patch: https://git.openjdk.java.net/jdk/pull/3615.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/3615/head:pull/3615 PR: https://git.openjdk.java.net/jdk/pull/3615
Integrated: 8240349: jlink should not leave partial image output directory on failure
On Mon, 7 Jun 2021 11:00:00 GMT, Athijegannathan Sundararajan wrote: > jlink should clean up output directory on any failure. should not leave > partially filled output. This pull request has now been integrated. Changeset: 4d1cf51b Author:Athijegannathan Sundararajan URL: https://git.openjdk.java.net/jdk/commit/4d1cf51b1d4a5e812c9f78b0104e40fbc4883a6c Stats: 71 lines in 5 files changed: 66 ins; 0 del; 5 mod 8240349: jlink should not leave partial image output directory on failure Reviewed-by: jlaskey, alanb - PR: https://git.openjdk.java.net/jdk/pull/4386
Re: RFR: 8240349: jlink should not leave partial image output directory on failure [v2]
On Tue, 8 Jun 2021 14:29:38 GMT, Athijegannathan Sundararajan wrote: >> jlink should clean up output directory on any failure. should not leave >> partially filled output. > > Athijegannathan Sundararajan has updated the pull request incrementally with > one additional commit since the last revision: > > Fixed resouce closing issue for lib/moduls file. Pre-existing output > directory should not be deleted when exiting. New version looks good. Good spot by Jorn that ImageFileCreator wasn't closing. - Marked as reviewed by alanb (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/4386
