Re: RFR: 8283667: [vectorapi] Vectorization for masked load with IOOBE with predicate feature [v3]
On Fri, 6 May 2022 14:59:26 GMT, Paul Sandoz wrote: >> Make sense! Thanks for the explanation! > > Doh! of course. This is not the first and will not be the last time i get > caught out by the 2-slot requirement. > It may be useful to do this: > > Node* mask_arg = is_store ? argument(8) : argument(7); Yes, the mask argument is got like: Node* mask = unbox_vector(is_store ? argument(8) : argument(7), mbox_type, elem_bt, num_elem); - PR: https://git.openjdk.java.net/jdk/pull/8035
Re: RFR: JDK-8286347: incorrect use of `{@link}`
On Fri, 6 May 2022 23:30:44 GMT, Jonathan Gibbons wrote:
> Please review a small doc fix to update incorrect use of `{@link}` on arrays
> of primitive types.
Looks fine in and of itself, but not sure how it will interact with the
(presumed) integration of JEP 424: "Foreign Function & Memory API (Preview)"
which will at least move the file, if not otherwise modify it.
-
PR: https://git.openjdk.java.net/jdk/pull/8584
Re: RFR: JDK-8286347: incorrect use of `{@link}`
On Fri, 6 May 2022 23:30:44 GMT, Jonathan Gibbons wrote:
> Please review a small doc fix to update incorrect use of `{@link}` on arrays
> of primitive types.
Marked as reviewed by iris (Reviewer).
-
PR: https://git.openjdk.java.net/jdk/pull/8584
Re: RFR: JDK-6725221 Standardize obtaining boolean properties with defaults
On Thu, 5 May 2022 16:49:07 GMT, Mark Powers wrote: > JDK-6725221 Standardize obtaining boolean properties with defaults Sorry for the confusion. The original intent of this bug 14 years ago was to standardize the seclibs code where simple getProperty calls were needed in the context of an AccessController, without having to provide the doProvided calls. i.e. GetBooleanAction.privilegedGetProperty(). This was not intended not other parts of the JDK. For example: ChannelImpl.java provides a getBooleanProperty() method, which is very similar to GetBooleanAction. I noticed other places in the security where this was being done. Some of the classes like Debug have been rewritten (SSLLogger), so the issue does not appear to exist there any logger. The certpath code is working with Security.getProperty(), so that was a red herring. - PR: https://git.openjdk.java.net/jdk/pull/8559
Re: RFR: JDK-8286347: incorrect use of `{@link}`
On Fri, 6 May 2022 23:30:44 GMT, Jonathan Gibbons wrote:
> Please review a small doc fix to update incorrect use of `{@link}` on arrays
> of primitive types.
Marked as reviewed by mchung (Reviewer).
-
PR: https://git.openjdk.java.net/jdk/pull/8584
RFR: JDK-8286347: incorrect use of `{@link}`
Please review a small doc fix to update incorrect use of `{@link}` on arrays of
primitive types.
-
Commit messages:
- JDK-8286347: incorrect use of `{@link}`
Changes: https://git.openjdk.java.net/jdk/pull/8584/files
Webrev: https://webrevs.openjdk.java.net/?repo=jdk=8584=00
Issue: https://bugs.openjdk.java.net/browse/JDK-8286347
Stats: 7 lines in 1 file changed: 0 ins; 0 del; 7 mod
Patch: https://git.openjdk.java.net/jdk/pull/8584.diff
Fetch: git fetch https://git.openjdk.java.net/jdk pull/8584/head:pull/8584
PR: https://git.openjdk.java.net/jdk/pull/8584
Re: RFR: 8286294 : ForkJoinPool.commonPool().close() spins [v2]
On Fri, 6 May 2022 21:46:58 GMT, Martin Buchholz wrote:
>> --- a/test/jdk/java/util/concurrent/tck/ForkJoinPool19Test.java
>> +++ b/test/jdk/java/util/concurrent/tck/ForkJoinPool19Test.java
>> @@ -55,7 +55,7 @@ public class ForkJoinPool19Test extends JSR166TestCase {
>> }
>>
>> public static Test suite() {
>> -return new TestSuite(ForkJoinPool8Test.class);
>> +return new TestSuite(ForkJoinPool19Test.class);
>> }
>>
>> /**
>
> testLazySubmit will cause jtreg to start hanging.
> If you wait patiently for 1000 seconds, you'll get a stack dump.
Thanks; now fixed (and enabled). (The test didn't do what doc said about joined
"by a worker")
-
PR: https://git.openjdk.java.net/jdk/pull/8577
Re: RFR: 8178355: IdentityHashMap uses identity-based comparison for values everywhere except remove(K,V) and replace(K,V,V)
On Thu, 21 Apr 2022 00:48:00 GMT, Stuart Marks wrote: >> Explicitly implement `remove` and `replace` in `IdentityHashMap` to compare >> values by identity. Updated API documentation of these two methods >> ([Preview](https://cr.openjdk.java.net/~liach/8178355/IdentityHashMap.html#remove(java.lang.Object,java.lang.Object))) >> to mention such behavior. > > Thanks for working on this. Yes I can review and sponsor this change. > > Sorry I got a bit distracted. I started looking at the test here, and this > lead me to inquire about what other tests we have for `IdentityHashMap`, and > the answer is: not enough. See > [JDK-8285295](https://bugs.openjdk.java.net/browse/JDK-8285295). (But that > should be handled separately.) @stuart-marks I have updated the tests to be based off that from [JDK-8285295](https://bugs.openjdk.java.net/browse/JDK-8285295). Anything else that needs an update? - PR: https://git.openjdk.java.net/jdk/pull/8259
Re: RFR: 8286163: micro-optimize Instant.plusSeconds
On Wed, 4 May 2022 20:27:04 GMT, lennartfricke wrote: > Provide micro-benchmark for comparison Seems reasonable to me. plus(long, long) already has this optimisation. - Marked as reviewed by scolebourne (Author). PR: https://git.openjdk.java.net/jdk/pull/8542
Re: RFR: 8178355: IdentityHashMap uses identity-based comparison for values everywhere except remove(K,V) and replace(K,V,V) [v4]
> Explicitly implement `remove` and `replace` in `IdentityHashMap` to compare > values by identity. Updated API documentation of these two methods > ([Preview](https://cr.openjdk.java.net/~liach/8178355/IdentityHashMap.html#remove(java.lang.Object,java.lang.Object))) > to mention such behavior. liach 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: - Move tests - Merge branch 'master' into fix/identityhashmap-default - Fix assertions - Revamp test and changes. Let ci run the tests - Fix indent - 8178355: IdentityHashMap uses identity-based comparison for values everywhere except remove(K,V) and replace(K,V,V) - Changes: - all: https://git.openjdk.java.net/jdk/pull/8259/files - new: https://git.openjdk.java.net/jdk/pull/8259/files/fe91721d..c8468ce2 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=8259=03 - incr: https://webrevs.openjdk.java.net/?repo=jdk=8259=02-03 Stats: 53894 lines in 1878 files changed: 35482 ins; 8470 del; 9942 mod Patch: https://git.openjdk.java.net/jdk/pull/8259.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8259/head:pull/8259 PR: https://git.openjdk.java.net/jdk/pull/8259
Re: RFR: 8286294 : ForkJoinPool.commonPool().close() spins [v2]
On Fri, 6 May 2022 21:28:45 GMT, Martin Buchholz wrote:
>> Tests in this file are not being executed. I think you need:
>>
>> --- a/test/jdk/java/util/concurrent/tck/ForkJoinPool19Test.java
>> +++ b/test/jdk/java/util/concurrent/tck/ForkJoinPool19Test.java
>> @@ -55,7 +55,7 @@ public class ForkJoinPool19Test extends JSR166TestCase {
>> }
>>
>> public static Test suite() {
>> -return new TestSuite(ForkJoinPool8Test.class);
>> +return new TestSuite(ForkJoinPool19Test.class);
>> }
>>
>> /**
>
> --- a/test/jdk/java/util/concurrent/tck/ForkJoinPool19Test.java
> +++ b/test/jdk/java/util/concurrent/tck/ForkJoinPool19Test.java
> @@ -55,7 +55,7 @@ public class ForkJoinPool19Test extends JSR166TestCase {
> }
>
> public static Test suite() {
> -return new TestSuite(ForkJoinPool8Test.class);
> +return new TestSuite(ForkJoinPool19Test.class);
> }
>
> /**
testLazySubmit will cause jtreg to start hanging.
If you wait patiently for 1000 seconds, you'll get a stack dump.
-
PR: https://git.openjdk.java.net/jdk/pull/8577
Re: RFR: 8286294 : ForkJoinPool.commonPool().close() spins [v2]
On Fri, 6 May 2022 20:25:10 GMT, Martin Buchholz wrote:
>> Doug Lea has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Fix doc types
>
> test/jdk/java/util/concurrent/tck/ForkJoinPool19Test.java line 496:
>
>> 494:
>> 495: /**
>> 496: * Implictly closing a new pool using try-with-resources terminates
>> it
>
> Typo "Implictly" - twice
Tests in this file are not being executed. I think you need:
--- a/test/jdk/java/util/concurrent/tck/ForkJoinPool19Test.java
+++ b/test/jdk/java/util/concurrent/tck/ForkJoinPool19Test.java
@@ -55,7 +55,7 @@ public class ForkJoinPool19Test extends JSR166TestCase {
}
public static Test suite() {
-return new TestSuite(ForkJoinPool8Test.class);
+return new TestSuite(ForkJoinPool19Test.class);
}
/**
-
PR: https://git.openjdk.java.net/jdk/pull/8577
Re: RFR: 8286294 : ForkJoinPool.commonPool().close() spins [v2]
On Fri, 6 May 2022 21:27:53 GMT, Martin Buchholz wrote:
>> test/jdk/java/util/concurrent/tck/ForkJoinPool19Test.java line 496:
>>
>>> 494:
>>> 495: /**
>>> 496: * Implictly closing a new pool using try-with-resources
>>> terminates it
>>
>> Typo "Implictly" - twice
>
> Tests in this file are not being executed. I think you need:
>
> --- a/test/jdk/java/util/concurrent/tck/ForkJoinPool19Test.java
> +++ b/test/jdk/java/util/concurrent/tck/ForkJoinPool19Test.java
> @@ -55,7 +55,7 @@ public class ForkJoinPool19Test extends JSR166TestCase {
> }
>
> public static Test suite() {
> -return new TestSuite(ForkJoinPool8Test.class);
> +return new TestSuite(ForkJoinPool19Test.class);
> }
>
> /**
--- a/test/jdk/java/util/concurrent/tck/ForkJoinPool19Test.java
+++ b/test/jdk/java/util/concurrent/tck/ForkJoinPool19Test.java
@@ -55,7 +55,7 @@ public class ForkJoinPool19Test extends JSR166TestCase {
}
public static Test suite() {
-return new TestSuite(ForkJoinPool8Test.class);
+return new TestSuite(ForkJoinPool19Test.class);
}
/**
-
PR: https://git.openjdk.java.net/jdk/pull/8577
Re: RFR: 8286294 : ForkJoinPool.commonPool().close() spins [v2]
> Changes ForkJoinPool.close spec and code to trap close as a no-op if called > on common pool Doug Lea has updated the pull request incrementally with one additional commit since the last revision: Fix doc types - Changes: - all: https://git.openjdk.java.net/jdk/pull/8577/files - new: https://git.openjdk.java.net/jdk/pull/8577/files/57a7c386..5ceb0794 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=8577=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk=8577=00-01 Stats: 2 lines in 1 file changed: 0 ins; 0 del; 2 mod Patch: https://git.openjdk.java.net/jdk/pull/8577.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8577/head:pull/8577 PR: https://git.openjdk.java.net/jdk/pull/8577
Re: RFR: 8285517: System.getenv() returns unexpected value if environment variable has non ASCII character [v4]
On Fri, 6 May 2022 14:29:06 GMT, Ichiroh Takiguchi
wrote:
>> test/jdk/java/lang/System/i18nEnvArg.java line 110:
>>
>>> 108: String s = System.getenv(EUC_JP_TEXT);
>>> 109: ByteArrayOutputStream baos = new ByteArrayOutputStream();
>>> 110: PrintStream ps = new PrintStream(baos);
>>
>> Can utilize try-with-resources pattern.
>
> Use `shouldNotContain()` to find the error message.
I was suggesting `try (ByteArrayOutputStream baos = ...) {` so that no need to
clean them up, but I see you removed them. But I prefer not to use
`shouldNotContain("ERROR: ")` but to check the return value as before.
-
PR: https://git.openjdk.java.net/jdk/pull/8378
Re: RFR: 8285517: System.getenv() returns unexpected value if environment variable has non ASCII character [v5]
On Fri, 6 May 2022 20:03:35 GMT, Naoto Sato wrote: >> Ichiroh Takiguchi has updated the pull request incrementally with one >> additional commit since the last revision: >> >> 8285517: System.getenv() returns unexpected value if environment variable >> has non ASCII character > > src/java.base/share/classes/jdk/internal/util/StaticProperty.java line 77: > >> 75: SUN_JNU_ENCODING = getProperty(props, "sun.jnu.encoding"); >> 76: jnuCharset = Charset.forName(SUN_JNU_ENCODING, >> Charset.defaultCharset()); >> 77: } > > I am not sure it is OK to initialize `Charset` here, as `sun_jnu_encoding` is > initialized in `System.initPhase1()` and pulling `Charset` there may cause > some init order change. I'd only cache the encoding string here. Note the initialization of `sun.jnu.encoding` in System.java:2142'ish. This happens before StaticProperty is initialized; at line 2147. If the `sun.jnu.encoding` is not supported (this is before Providers are enabled) then it is forced to `UTF-8`. So it is the case that the encoding is supported by that point. Note that `Charset.isSupported(...)` does the full lookup in its implementation. If it is not supported, the system still comes up using UTF-8 and a warning is printed at line 2282. Comparing the class initialization order may be a useful thing to cross check. - PR: https://git.openjdk.java.net/jdk/pull/8378
Re: RFR: 8286294 : ForkJoinPool.commonPool().close() spins
On Fri, 6 May 2022 15:05:57 GMT, Doug Lea wrote: > Changes ForkJoinPool.close spec and code to trap close as a no-op if called > on common pool test/jdk/java/util/concurrent/tck/ForkJoinPool19Test.java line 496: > 494: > 495: /** > 496: * Implictly closing a new pool using try-with-resources terminates > it Typo "Implictly" - twice - PR: https://git.openjdk.java.net/jdk/pull/8577
Re: RFR: 8285097: Duplicate XML keys in XPATHErrorResources.java and XSLTErrorResources.java [v3]
On Fri, 6 May 2022 14:33:50 GMT, Shruthi wrote: >> Removing the Duplicate keys present in XSLTErrorResources.java and >> XPATHErrorResources.java >> >> The bug report for the same: https://bugs.openjdk.java.net/browse/JDK-8285097 > > Shruthi has updated the pull request incrementally with one additional commit > since the last revision: > > Replace the ER_RTF_NOT_SUPPORTED_XRTREEFRAGSELECTWRAPPER key in > XPATHErrorResources language files Changing resource bundles is not required as the L10n resource files update would cover that. As you've modified the files, you'll need to update the license header, using XPATHErrorResources_ja.java as an example and update the year and LastModified tag. - PR: https://git.openjdk.java.net/jdk/pull/8318
Re: RFR: 8285517: System.getenv() returns unexpected value if environment variable has non ASCII character [v5]
On Fri, 6 May 2022 14:23:00 GMT, Ichiroh Takiguchi wrote: >> On JDK19 with Linux ja_JP.eucjp locale, >> System.getenv() returns unexpected value if environment variable has >> Japanese EUC characters. >> It seems this issue happens because of JEP 400. >> Arguments for ProcessBuilder have same kind of issue. > > Ichiroh Takiguchi has updated the pull request incrementally with one > additional commit since the last revision: > > 8285517: System.getenv() returns unexpected value if environment variable > has non ASCII character src/java.base/share/classes/jdk/internal/util/StaticProperty.java line 77: > 75: SUN_JNU_ENCODING = getProperty(props, "sun.jnu.encoding"); > 76: jnuCharset = Charset.forName(SUN_JNU_ENCODING, > Charset.defaultCharset()); > 77: } I am not sure it is OK to initialize `Charset` here, as `sun_jnu_encoding` is initialized in `System.initPhase1()` and pulling `Charset` there may cause some init order change. I'd only cache the encoding string here. - PR: https://git.openjdk.java.net/jdk/pull/8378
Integrated: 8285295: Need better testing for IdentityHashMap
On Fri, 22 Apr 2022 03:37:27 GMT, Stuart Marks wrote: > Basic but fairly comprehensive set of tests for `IdentityHashMap`. The patch > in the bug report that breaks `IdentityHashMap` now causes several cases in > this new test to fail. There's more that could be done, but the new tests > cover most of the core functions of `IdentityHashMap`. Unfortunately it seems > difficult to merge this with the existing, comprehensive Collections tests > (e.g., MOAT.java) because those tests implicity rely on `equals()`-based > contract instead of the special-purpose `==`-based contract used by > `IdentityHashMap`. This pull request has now been integrated. Changeset: 5a1d8f7e Author:Stuart Marks URL: https://git.openjdk.java.net/jdk/commit/5a1d8f7e5358d823e9bdeab8056b1de2b050f939 Stats: 678 lines in 1 file changed: 678 ins; 0 del; 0 mod 8285295: Need better testing for IdentityHashMap Reviewed-by: jpai, lancea - PR: https://git.openjdk.java.net/jdk/pull/8354
Re: RFR: 8285295: Need better testing for IdentityHashMap [v5]
> Basic but fairly comprehensive set of tests for `IdentityHashMap`. The patch > in the bug report that breaks `IdentityHashMap` now causes several cases in > this new test to fail. There's more that could be done, but the new tests > cover most of the core functions of `IdentityHashMap`. Unfortunately it seems > difficult to merge this with the existing, comprehensive Collections tests > (e.g., MOAT.java) because those tests implicity rely on `equals()`-based > contract instead of the special-purpose `==`-based contract used by > `IdentityHashMap`. Stuart Marks has updated the pull request incrementally with one additional commit since the last revision: Add comments on tests that were missing them. - Changes: - all: https://git.openjdk.java.net/jdk/pull/8354/files - new: https://git.openjdk.java.net/jdk/pull/8354/files/4bb25edf..fb877d93 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=8354=04 - incr: https://webrevs.openjdk.java.net/?repo=jdk=8354=03-04 Stats: 19 lines in 1 file changed: 19 ins; 0 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/8354.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8354/head:pull/8354 PR: https://git.openjdk.java.net/jdk/pull/8354
Re: RFR: 8285295: Need better testing for IdentityHashMap [v4]
On Fri, 6 May 2022 16:59:16 GMT, Lance Andersen wrote: >> Stuart Marks has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Add some assertions for entrySet.equals and keySet.equals > > I think you have done a nice job on the coverage. > > It would be nice for future maintainers if you consider adding comments for > all of the tests vs. a subset prior to pushing Thanks @LanceAndersen I've added some comments. - PR: https://git.openjdk.java.net/jdk/pull/8354
Re: RFR: JDK-6725221 Standardize obtaining boolean properties with defaults
On Thu, 5 May 2022 16:49:07 GMT, Mark Powers wrote: > JDK-6725221 Standardize obtaining boolean properties with defaults The xtoolkit change is fine. I've not looked at anything else You'll clearly need multiple reviewers for this one. - Marked as reviewed by prr (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/8559
Re: RFR: 8286294 : ForkJoinPool.commonPool().close() spins
On Fri, 6 May 2022 15:05:57 GMT, Doug Lea wrote: > Changes ForkJoinPool.close spec and code to trap close as a no-op if called > on common pool Changes look good, it will need a CSR. - PR: https://git.openjdk.java.net/jdk/pull/8577
Re: RFR: JDK-6725221 Standardize obtaining boolean properties with defaults
On Thu, 5 May 2022 16:49:07 GMT, Mark Powers wrote:
> JDK-6725221 Standardize obtaining boolean properties with defaults
src/java.base/share/classes/java/lang/reflect/AccessibleObject.java line 777:
> 775: if (!printStackPropertiesSet && VM.initLevel() >= 1) {
> 776: printStackWhenAccessFails = GetBooleanAction.
> 777:
> privilegedGetProperty("sun.reflect.debugModuleAccessChecks");
Running with `-Dsun.reflect.debugModuleAccessChecks` should set
printStackWhenAccessFails to true, not false.
-
PR: https://git.openjdk.java.net/jdk/pull/8559
RFR: JDK-6725221 Standardize obtaining boolean properties with defaults
JDK-6725221 Standardize obtaining boolean properties with defaults - Commit messages: - Merge - first iteration Changes: https://git.openjdk.java.net/jdk/pull/8559/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=8559=00 Issue: https://bugs.openjdk.java.net/browse/JDK-6725221 Stats: 27 lines in 10 files changed: 1 ins; 4 del; 22 mod Patch: https://git.openjdk.java.net/jdk/pull/8559.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8559/head:pull/8559 PR: https://git.openjdk.java.net/jdk/pull/8559
Re: RFR: 8262889: Compiler implementation for Record Patterns [v2]
On Fri, 6 May 2022 14:30:10 GMT, Maurizio Cimadamore
wrote:
>> Jan Lahoda has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Reflecting review feedback.
>
> src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Flow.java line 752:
>
>> 750:Iterable> JCCaseLabel> labels) {
>> 751: Set coveredSymbols = new HashSet<>();
>> 752: Map>
>> deconstructionPatternsBySymbol = new HashMap<>();
>
> since you seem to have settled on "recordPattern" for implementation names -
> you can probably revisit some of these names to say "record" instead of
> "deconstruction".
Right. Will do.
> src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Flow.java line 801:
>
>> 799: //i.e. represent all possible combinations.
>> 800: //This is done by categorizing the patterns based on the
>> type covered by the given
>> 801: //starting component.
>
> Example needed here. For instance (I discussed this with @biboudis):
>
>
> record Outer(R r) { };
> sealed interface I { };
> class A implements I { };
> class B implements I { };
> sealed interface R { };
> record Foo(I i) implements R { }
> record Bar(I i) implements R { }
>
> switch (o) {
>case Outer(Foo(A), Foo(A)):
>case Outer(Foo(B), Foo(B)):
>case Outer(Foo(A), Foo(B)):
>case Outer(Foo(B), Foo(A)):
>case Outer(Bar(A), Bar(A)):
>case Outer(Bar(B), Bar(B)):
>case Outer(Bar(A), Bar(B)):
>case Outer(Bar(B), Bar(A)):
> }
>
>
> Which generates two sets:
>
>
>case Outer(Foo(A), Foo(A)):
>case Outer(Foo(B), Foo(B)):
>case Outer(Foo(A), Foo(B)):
>case Outer(Foo(B), Foo(A)):
>
>
> And
>
>
>case Outer(Bar(A), Bar(A)):
>case Outer(Bar(B), Bar(B)):
>case Outer(Bar(A), Bar(B)):
>case Outer(Bar(B), Bar(A)):
>
>
> Sorry for being pedantic - this code is tricky and I'm worried we'll all
> forget exactly how it works in 2 months :-)
Sure. Will try to improve.
-
PR: https://git.openjdk.java.net/jdk/pull/8516
Re: RFR: 8262889: Compiler implementation for Record Patterns [v2]
On Thu, 5 May 2022 18:11:54 GMT, Vicente Romero wrote: >> Jan Lahoda has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Reflecting review feedback. > > src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Attr.java line 4217: > >> 4215: } >> 4216: ListBuffer outBindings = new ListBuffer<>(); >> 4217: List recordTypes = expectedRecordTypes; > > nit: probably a matter of style but why not reusing the already declared > `expectedRecordTypes` declaring a new variable seems unnecessary Please note the full `expectedRecordTypes` are used for error reporting, but the reference to `List` in `recordTypes` is overwritten in the loop (at the time of an error report, it may not longer point to the full expected types, and hence cannot be used for error reporting). - PR: https://git.openjdk.java.net/jdk/pull/8516
Re: RFR: 8262889: Compiler implementation for Record Patterns [v2]
On Fri, 6 May 2022 15:44:22 GMT, Gavin Bierman wrote:
> > From the JLS specdiff
> > > If the type R names a generic record class then it is a compile-time
> > > error if R is not a parameterized type.
> >
> >
> > The following snippet raises a `MatchException`. Shouldn't it be a
> > compile-time error?
> > ```
> > Box r = new Box<>(null);
> >
> > switch (r) {
> > case Box(String s):
> > System.out.println("match");
> > }
> > ```
> >
> >
> >
> >
> >
> >
> >
> >
> >
> >
> >
> > If this is Ok and my understanding is wrong, then why that raises an
> > exception at all? I can make it work (as an unconditional) if I define the
> > Box as `record Box` and now I am confused...
> > ping @GavinBierman @lahodaj
>
> A couple of issues here. (1) This should be a compile-time error. (2) upon
> investigation I think there is a bug with the pattern matching code, because
> the compiler is currently saying that the pattern match here: `Box bs
> = new Box<>(null); if (bs instanceof Box(String s)) {
> System.out.println("match!"); }` does not succeed. (It should do). The
> `MatchException` you are seeing is that the exhaustive pattern switch has no
> matching label (if you put in a default, you don't get the exception).
Right. Will fix. Sorry for that.
-
PR: https://git.openjdk.java.net/jdk/pull/8516
Re: RFR: 8262889: Compiler implementation for Record Patterns [v2]
On Fri, 6 May 2022 14:09:24 GMT, Jan Lahoda wrote: >> 8262889: Compiler implementation for Record Patterns >> >> A first version of a patch that introduces record patterns into javac as a >> preview feature. For the specification, please see: >> http://cr.openjdk.java.net/~gbierman/jep427+405/jep427+405-20220426/specs/patterns-switch-record-patterns-jls.html >> >> There are two notable tricky parts: >> -in the parser, it was necessary to improve the `analyzePattern` method to >> handle nested/record patterns, while still keeping error recovery reasonable >> -in the `TransPatterns`, the desugaring of the record patterns is very >> straightforward - effectivelly the record patterns are desugared into >> guards/conditions. This will likely be improved in some future >> version/preview >> >> `MatchException` has been extended to cover additional cases related to >> record patterns. > > Jan Lahoda has updated the pull request incrementally with one additional > commit since the last revision: > > Reflecting review feedback. src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Attr.java line 4217: > 4215: } > 4216: ListBuffer outBindings = new ListBuffer<>(); > 4217: List recordTypes = expectedRecordTypes; nit: probably a matter of style but why not reusing the already declared `expectedRecordTypes` declaring a new variable seems unnecessary - PR: https://git.openjdk.java.net/jdk/pull/8516
Re: RFR: 8285295: Need better testing for IdentityHashMap [v4]
On Wed, 4 May 2022 19:16:14 GMT, Stuart Marks wrote: >> Basic but fairly comprehensive set of tests for `IdentityHashMap`. The patch >> in the bug report that breaks `IdentityHashMap` now causes several cases in >> this new test to fail. There's more that could be done, but the new tests >> cover most of the core functions of `IdentityHashMap`. Unfortunately it >> seems difficult to merge this with the existing, comprehensive Collections >> tests (e.g., MOAT.java) because those tests implicity rely on >> `equals()`-based contract instead of the special-purpose `==`-based contract >> used by `IdentityHashMap`. > > Stuart Marks has updated the pull request incrementally with one additional > commit since the last revision: > > Add some assertions for entrySet.equals and keySet.equals I think you have done a nice job on the coverage. It would be nice for future maintainers if you consider adding comments for all of the tests vs. a subset prior to pushing - Marked as reviewed by lancea (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/8354
Re: RFR: 8282191: Implementation of Foreign Function & Memory API (Preview) [v41]
On Fri, 6 May 2022 16:48:11 GMT, Maurizio Cimadamore wrote: >> This PR contains the API and implementation changes for JEP-424 [1]. A more >> detailed description of such changes, to avoid repetitions during the review >> process, is included as a separate comment. >> >> [1] - https://openjdk.java.net/jeps/424 > > Maurizio Cimadamore has updated the pull request incrementally with one > additional commit since the last revision: > > Update src/java.base/share/classes/jdk/internal/reflect/Reflection.java > > Co-authored-by: ExE Boss <3889017+exe-b...@users.noreply.github.com> Marked as reviewed by mchung (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/7888
Re: RFR: 8282191: Implementation of Foreign Function & Memory API (Preview) [v41]
> This PR contains the API and implementation changes for JEP-424 [1]. A more > detailed description of such changes, to avoid repetitions during the review > process, is included as a separate comment. > > [1] - https://openjdk.java.net/jeps/424 Maurizio Cimadamore has updated the pull request incrementally with one additional commit since the last revision: Update src/java.base/share/classes/jdk/internal/reflect/Reflection.java Co-authored-by: ExE Boss <3889017+exe-b...@users.noreply.github.com> - Changes: - all: https://git.openjdk.java.net/jdk/pull/7888/files - new: https://git.openjdk.java.net/jdk/pull/7888/files/b71c4e93..f823bf84 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=7888=40 - incr: https://webrevs.openjdk.java.net/?repo=jdk=7888=39-40 Stats: 2 lines in 1 file changed: 1 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/7888.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/7888/head:pull/7888 PR: https://git.openjdk.java.net/jdk/pull/7888
Re: RFR: 8286191: misc tests fail due to JDK-8285987 [v3]
On Fri, 6 May 2022 06:41:31 GMT, Matthias Baesken wrote: >> The isMusl method had to be handled in >> test/lib-test/jdk/test/lib/TestMutuallyExclusivePlatformPredicates.java . >> Additionally, the vm.musl predicate seem not to be available in the >> langtools tests. > > Matthias Baesken has updated the pull request incrementally with one > additional commit since the last revision: > > restore year in ExternalEditorTest, remove test exclusion Looks good. Thanks for resolving both ProblemLists. Hopefully, the real problem and solution on Musl can be found separately. - Marked as reviewed by rriggs (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/8556
Re: RFR: 8285517: System.getenv() returns unexpected value if environment variable has non ASCII character [v2]
On Wed, 4 May 2022 03:01:19 GMT, Ichiroh Takiguchi wrote: >> Do we need to verify the intermediate byte encoding? Could we simply compare >> the text given to environ.put() in Start process and System.getenv() in >> Verify process match? > > Hello @naotoj . > I think if 2nd process' encoder (like UTF-8) and 3rd process' decoder are > same encoding, System.getenv() returns expected data. > Actually the testcase checks 3 parts on Verify process: > 1. Expected environment variable is defined or not (it uses System.getenv()) > 2. Expected argument is received or not > 3. Expected environment variable is encoded by proper encoding > > When I ran this testcase with jdk18.0.1, I got following result: > > Unexpected argument was received: \u6F22\u5B57<->\u7FB2\u221A\uFFFD > Unexpected environment variables: > \xE6\xBC\xA2\xE5\xAD\x97\x3D\xE6\xBC\xA2\xE5\xAD\x97 > > It means 1st test was passed, 2nd and 3rd test were failed. > I don't think environment variable issue can be seen without 3rd test. > Please let me know if you find out another way. This part of the test is very brittle; I'm pretty sure it will fail on AIX that adds its own environment variables. It should not fail if it finds the two entries it expects. It should ignore other entries. I don't see what value it has over checking the entries from System.getEnv(), please elaborate. - PR: https://git.openjdk.java.net/jdk/pull/8378
Re: RFR: 8285517: System.getenv() returns unexpected value if environment variable has non ASCII character [v5]
On Fri, 6 May 2022 14:23:00 GMT, Ichiroh Takiguchi
wrote:
>> On JDK19 with Linux ja_JP.eucjp locale,
>> System.getenv() returns unexpected value if environment variable has
>> Japanese EUC characters.
>> It seems this issue happens because of JEP 400.
>> Arguments for ProcessBuilder have same kind of issue.
>
> Ichiroh Takiguchi has updated the pull request incrementally with one
> additional commit since the last revision:
>
> 8285517: System.getenv() returns unexpected value if environment variable
> has non ASCII character
test/jdk/java/lang/ProcessBuilder/Basic.java line 606:
> 604: ? Charset.forName(jnuEncoding, Charset.defaultCharset())
> 605: : Charset.defaultCharset();
> 606: if (new String(tested.getBytes(cs), cs).equals(tested)) {
Isn't it always true that the round trip encoding to bytes and back (using the
same Charset) to a string is equal()?
And if it is always true, then the if(...) can be removed.
test/jdk/java/lang/System/i18nEnvArg.java line 104:
> 102: String s = System.getenv(EUC_JP_TEXT);
> 103: if (!EUC_JP_TEXT.equals(s)) {
> 104: System.err.println("ERROR: getenv() returns unexpected
> data");
Please add the unexpected data `s` to the output string.
test/jdk/java/lang/System/i18nEnvArg.java line 108:
> 106: if (!EUC_JP_TEXT.equals(args[0])) {
> 107: System.err.print("ERROR: Unexpected argument was
> received: ");
> 108: for(char ch : EUC_JP_TEXT.toCharArray()) {
This is the expected value, the previous "Unexpected" labeling might be
mis-understood.
test/jdk/java/lang/System/i18nEnvArg.java line 111:
> 109:System.err.printf("\\u%04X", (int)ch);
> 110: }
> 111: System.err.print("<->");
This might be clearer if labeled as the actual/incorrect value and on a
separate line.
-
PR: https://git.openjdk.java.net/jdk/pull/8378
Integrated: 8286154: Fix 3rd party notices in test files
On Thu, 5 May 2022 16:13:59 GMT, Naoto Sato wrote: > Trivial fix to 3rd party copyright notices. This pull request has now been integrated. Changeset: 1277f5d8 Author:Naoto Sato URL: https://git.openjdk.java.net/jdk/commit/1277f5d84e9c2863595396a471a61d83f8a0298c Stats: 100 lines in 21 files changed: 64 ins; 0 del; 36 mod 8286154: Fix 3rd party notices in test files Reviewed-by: darcy, joehw, iris - PR: https://git.openjdk.java.net/jdk/pull/8558
Re: RFR: 8286163: micro-optimize Instant.plusSeconds
On Wed, 4 May 2022 20:27:04 GMT, lennartfricke wrote: > Provide micro-benchmark for comparison Hi, thanks for the contribution! How big a speed-up are you observing? Keeping the optimization in `plusSeconds` rather than moving it to `plus(long, long)` means expressions like `instant.plusMillis(1000)` won't be helped, but such expressions might be rarely helped anyway so what you have might be better overall since it doesn't add a branch to the common code. - PR: https://git.openjdk.java.net/jdk/pull/8542
Re: RFR: 8285295: Need better testing for IdentityHashMap [v4]
On Fri, 6 May 2022 06:39:59 GMT, XenoAmess wrote: > > It would be nice if such a test could be written, but as it stands I think > > that `Wrappers.java` test is too simplistic. > > Would adding `Wrappers.java` a method-name white-list mechanism suitable in > this situation? It should really be a method name and type whitelist. - PR: https://git.openjdk.java.net/jdk/pull/8354
Re: RFR: 8282191: Implementation of Foreign Function & Memory API (Preview) [v40]
On Fri, 6 May 2022 11:51:46 GMT, Maurizio Cimadamore wrote: >> This PR contains the API and implementation changes for JEP-424 [1]. A more >> detailed description of such changes, to avoid repetitions during the review >> process, is included as a separate comment. >> >> [1] - https://openjdk.java.net/jeps/424 > > Maurizio Cimadamore has updated the pull request incrementally with one > additional commit since the last revision: > > Add tests for loaderLookup/restricted method corner cases src/java.base/share/classes/jdk/internal/reflect/Reflection.java line 116: > 114: // if there is no caller class, act as if the call came from > unnamed module of system class loader > 115: Module module = currentClass != null ? > 116: currentClass.getModule() : > ClassLoader.getSystemClassLoader().getUnnamedModule(); **Nit:** maybe add a line break Suggestion: currentClass.getModule() : ClassLoader.getSystemClassLoader().getUnnamedModule(); - PR: https://git.openjdk.java.net/jdk/pull/7888
Re: RFR: 8286163: micro-optimize Instant.plusSeconds
On Wed, 4 May 2022 20:27:04 GMT, lennartfricke wrote: > Provide micro-benchmark for comparison Hi, please send an e-Mail to dalibor.to...@oracle.com so that I can mark your account as verified. - PR: https://git.openjdk.java.net/jdk/pull/8542
RFR: 8286163: micro-optimize Instant.plusSeconds
Provide micro-benchmark for comparison - Commit messages: - 8286163: micro-optimize Instant.plusSeconds Changes: https://git.openjdk.java.net/jdk/pull/8542/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=8542=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8286163 Stats: 94 lines in 2 files changed: 93 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/8542.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8542/head:pull/8542 PR: https://git.openjdk.java.net/jdk/pull/8542
Re: RFR: 8262889: Compiler implementation for Record Patterns [v2]
On Fri, 6 May 2022 14:09:24 GMT, Jan Lahoda wrote:
>> 8262889: Compiler implementation for Record Patterns
>>
>> A first version of a patch that introduces record patterns into javac as a
>> preview feature. For the specification, please see:
>> http://cr.openjdk.java.net/~gbierman/jep427+405/jep427+405-20220426/specs/patterns-switch-record-patterns-jls.html
>>
>> There are two notable tricky parts:
>> -in the parser, it was necessary to improve the `analyzePattern` method to
>> handle nested/record patterns, while still keeping error recovery reasonable
>> -in the `TransPatterns`, the desugaring of the record patterns is very
>> straightforward - effectivelly the record patterns are desugared into
>> guards/conditions. This will likely be improved in some future
>> version/preview
>>
>> `MatchException` has been extended to cover additional cases related to
>> record patterns.
>
> Jan Lahoda has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Reflecting review feedback.
> From the JLS specdiff
>
> > If the type R names a generic record class then it is a compile-time error
> > if R is not a parameterized type.
>
> The following snippet raises a `MatchException`. Shouldn't it be a
> compile-time error?
>
> ```
> Box r = new Box<>(null);
>
> switch (r) {
> case Box(String s):
> System.out.println("match");
> }
> ```
>
> If this is Ok and my understanding is wrong, then why that raises an
> exception at all? I can make it work (as an unconditional) if I define the
> Box as `record Box` and now I am confused...
>
> ping @GavinBierman @lahodaj
A couple of issues here. (1) This should be a compile-time error. (2) upon
investigation I think there is a bug with the pattern matching code, because
the compiler is currently saying that the pattern match here: `Box bs =
new Box<>(null); if (bs instanceof Box(String s)) {
System.out.println("match!"); }` does not succeed. (It should do). The
`MatchException` you are seeing is that the exhaustive pattern switch has no
matching label (if you put in a default, you don't get the exception).
-
PR: https://git.openjdk.java.net/jdk/pull/8516
Re: RFR: 4511638: Double.toString(double) sometimes produces incorrect results [v9]
On Fri, 6 May 2022 08:40:40 GMT, Raffaello Giulietti wrote: >> Hello, >> >> here's a PR for a patch submitted on March 2020 >> [1](https://cr.openjdk.java.net/~bpb/4511638/webrev.04/) when Mercurial was >> a thing. >> >> The patch has been edited to adhere to OpenJDK code conventions about >> multi-line (block) comments. Nothing in the code proper has changed, except >> for the addition of redundant but clarifying parentheses in some expressions. >> >> >> Greetings >> Raffaello > > Raffaello Giulietti has updated the pull request incrementally with one > additional commit since the last revision: > > 4511638: Double.toString(double) sometimes produces incorrect results Match the CSR Updated Schubfach writing URL - PR: https://git.openjdk.java.net/jdk/pull/3402
Re: RFR: 8286298: Remove unused methods in sun.invoke.util.VerifyType
On Fri, 6 May 2022 11:32:25 GMT, Claes Redestad wrote: > A few untested and unused methods in `VerifyType` which can be removed. > (Possibly used by native JSR 292 implementations in JDK 7). Looks fine. - Marked as reviewed by bpb (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/8570
RFR: 8286294 : ForkJoinPool.commonPool().close() spins
Changes ForkJoinPool.close spec and code to trap close as a no-op if called on common pool - Commit messages: - Override close as explicit no-op for common pool Changes: https://git.openjdk.java.net/jdk/pull/8577/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=8577=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8286294 Stats: 75 lines in 2 files changed: 75 ins; 0 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/8577.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8577/head:pull/8577 PR: https://git.openjdk.java.net/jdk/pull/8577
Re: RFR: 8283667: [vectorapi] Vectorization for masked load with IOOBE with predicate feature [v3]
On Fri, 6 May 2022 04:49:39 GMT, Xiaohong Gong wrote: >> offset is long so uses two argument slots (5 and 6). >> mask is argument (7). >> offsetInRange is argument(8). > > Make sense! Thanks for the explanation! Doh! of course. This is not the first and will not be the last time i get caught out by the 2-slot requirement. It may be useful to do this: Node* mask_arg = is_store ? argument(8) : argument(7); - PR: https://git.openjdk.java.net/jdk/pull/8035
Re: RFR: 8285097: Duplicate XML keys in XPATHErrorResources.java and XSLTErrorResources.java [v3]
On Wed, 20 Apr 2022 15:48:54 GMT, Tyler Steele wrote:
>> Shruthi has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Replace the ER_RTF_NOT_SUPPORTED_XRTREEFRAGSELECTWRAPPER key in
>> XPATHErrorResources language files
>
> src/java.xml/share/classes/com/sun/org/apache/xpath/internal/res/XPATHErrorResources.java
> line 599:
>
>> 597:
>> 598: { ER_ASNODEITERATOR_NOT_SUPPORTED_XRTREEFRAGSELECTWRAPPER,
>> 599:"asNodeIterator() not supported by XRTreeFragSelectWrapper"},
>
> For this key, please review places where the old key was used to find places
> where the new key was intended. I believe [this
> line](https://github.com/openjdk/jdk/blob/master/src/java.xml/share/classes/com/sun/org/apache/xpath/internal/objects/XRTreeFragSelectWrapper.java#L155)
> is an example.
[Here](https://github.com/openjdk/jdk/blob/64225e19995e81d2e836ce84befea1a01bb6c860/src/java.xml/share/classes/com/sun/org/apache/xpath/internal/res/XPATHErrorResources_de.java#L595)
is another usage where the other key is intended. I expect you will find
similar references in at least some of the other translation files.
-
PR: https://git.openjdk.java.net/jdk/pull/8318
Re: RFR: 8285097: Duplicate XML keys in XPATHErrorResources.java and XSLTErrorResources.java [v2]
On Mon, 2 May 2022 07:39:39 GMT, Shruthi wrote: >> Shruthi has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Updating last modified tag and XRTreeFragSelectWrapper.java > > `/integrate` LGTM. Nicely done @shruacha1234 - PR: https://git.openjdk.java.net/jdk/pull/8318
Re: RFR: 8285097: Duplicate XML keys in XPATHErrorResources.java and XSLTErrorResources.java [v3]
On Fri, 6 May 2022 14:33:50 GMT, Shruthi wrote:
>> Removing the Duplicate keys present in XSLTErrorResources.java and
>> XPATHErrorResources.java
>>
>> The bug report for the same: https://bugs.openjdk.java.net/browse/JDK-8285097
>
> Shruthi has updated the pull request incrementally with one additional commit
> since the last revision:
>
> Replace the ER_RTF_NOT_SUPPORTED_XRTREEFRAGSELECTWRAPPER key in
> XPATHErrorResources language files
Marked as reviewed by backwater...@github.com (no known OpenJDK username).
src/java.xml/share/classes/com/sun/org/apache/xpath/internal/res/XPATHErrorResources.java
line 599:
> 597:
> 598: { ER_ASNODEITERATOR_NOT_SUPPORTED_XRTREEFRAGSELECTWRAPPER,
> 599:"asNodeIterator() not supported by XRTreeFragSelectWrapper"},
For this key, please review places where the old key was used to find places
where the new key was intended. I believe [this
line](https://github.com/openjdk/jdk/blob/master/src/java.xml/share/classes/com/sun/org/apache/xpath/internal/objects/XRTreeFragSelectWrapper.java#L155)
is an example.
-
PR: https://git.openjdk.java.net/jdk/pull/8318
Re: RFR: 8262889: Compiler implementation for Record Patterns [v2]
On Fri, 6 May 2022 14:09:24 GMT, Jan Lahoda wrote:
>> 8262889: Compiler implementation for Record Patterns
>>
>> A first version of a patch that introduces record patterns into javac as a
>> preview feature. For the specification, please see:
>> http://cr.openjdk.java.net/~gbierman/jep427+405/jep427+405-20220426/specs/patterns-switch-record-patterns-jls.html
>>
>> There are two notable tricky parts:
>> -in the parser, it was necessary to improve the `analyzePattern` method to
>> handle nested/record patterns, while still keeping error recovery reasonable
>> -in the `TransPatterns`, the desugaring of the record patterns is very
>> straightforward - effectivelly the record patterns are desugared into
>> guards/conditions. This will likely be improved in some future
>> version/preview
>>
>> `MatchException` has been extended to cover additional cases related to
>> record patterns.
>
> Jan Lahoda has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Reflecting review feedback.
Looks good
src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Flow.java line 752:
> 750:Iterable JCCaseLabel> labels) {
> 751: Set coveredSymbols = new HashSet<>();
> 752: Map>
> deconstructionPatternsBySymbol = new HashMap<>();
since you seem to have settled on "recordPattern" for implementation names -
you can probably revisit some of these names to say "record" instead of
"deconstruction".
src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Flow.java line 801:
> 799: //i.e. represent all possible combinations.
> 800: //This is done by categorizing the patterns based on the
> type covered by the given
> 801: //starting component.
Example needed here. For instance (I discussed this with @biboudis):
record Outer(R r) { };
sealed interface I { };
class A implements I { };
class B implements I { };
sealed interface R { };
record Foo(I i) implements R { }
record Bar(I i) implements R { }
switch (o) {
case Outer(Foo(A), Foo(A)):
case Outer(Foo(B), Foo(B)):
case Outer(Foo(A), Foo(B)):
case Outer(Foo(B), Foo(A)):
case Outer(Bar(A), Bar(A)):
case Outer(Bar(B), Bar(B)):
case Outer(Bar(A), Bar(B)):
case Outer(Bar(B), Bar(A)):
}
Which generates two sets:
case Outer(Foo(A), Foo(A)):
case Outer(Foo(B), Foo(B)):
case Outer(Foo(A), Foo(B)):
case Outer(Foo(B), Foo(A)):
And
case Outer(Bar(A), Bar(A)):
case Outer(Bar(B), Bar(B)):
case Outer(Bar(A), Bar(B)):
case Outer(Bar(B), Bar(A)):
Sorry for being pedantic - this code is tricky and I'm worried we'll all forget
exactly how it works in 2 months :-)
src/jdk.compiler/share/classes/com/sun/tools/javac/parser/JavacParser.java line
1014:
> 1012: pattern = parsePattern(patternPos, mods, type,
> false);
> 1013: } else if (token.kind == LPAREN) {
> 1014: pattern = parsePattern(patternPos, mods, type,
> false);
Nice!
-
Marked as reviewed by mcimadamore (Reviewer).
PR: https://git.openjdk.java.net/jdk/pull/8516
Re: RFR: JDK-8285730: unify _WIN32_WINNT settings [v4]
On Wed, 4 May 2022 08:00:08 GMT, Matthias Baesken wrote: >> Currently we set _WIN32_WINNT at various places in the codebase; this is >> used to target a minimum Windows version we want to support. See also for >> more detailled information : >> https://docs.microsoft.com/en-us/windows/win32/winprog/using-the-windows-headers?redirectedfrom=MSDN#setting-winver-or-_win32_winnt >> Macros for Conditional Declarations >> "Certain functions that depend on a particular version of Windows are >> declared using conditional code. This enables you to use the compiler to >> detect whether your application uses functions that are not supported on its >> target version(s) of Windows." >> >> However currently we have quite a lot of differing settings of _WIN32_WINNT >> in the codebase ; setting _WIN32_WINNT to 0x0601 (Windows 7) where possible >> would make sense because we have this setting already at java_props_md.c >> (so targeting older Windows versions at other places is most likely not >> useful). > > Matthias Baesken has updated the pull request incrementally with one > additional commit since the last revision: > > adjust API level to Windows 8 for security.cpp and do some cleanup Looks good, thanks for doing this cleanup. - Marked as reviewed by ihse (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/8428
Re: RFR: 8285097: Duplicate XML keys in XPATHErrorResources.java and XSLTErrorResources.java [v3]
> Removing the Duplicate keys present in XSLTErrorResources.java and > XPATHErrorResources.java > > The bug report for the same: https://bugs.openjdk.java.net/browse/JDK-8285097 Shruthi has updated the pull request incrementally with one additional commit since the last revision: Replace the ER_RTF_NOT_SUPPORTED_XRTREEFRAGSELECTWRAPPER key in XPATHErrorResources language files - Changes: - all: https://git.openjdk.java.net/jdk/pull/8318/files - new: https://git.openjdk.java.net/jdk/pull/8318/files/d53ca37e..c294a150 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=8318=02 - incr: https://webrevs.openjdk.java.net/?repo=jdk=8318=01-02 Stats: 10 lines in 10 files changed: 0 ins; 0 del; 10 mod Patch: https://git.openjdk.java.net/jdk/pull/8318.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8318/head:pull/8318 PR: https://git.openjdk.java.net/jdk/pull/8318
Re: RFR: 8285517: System.getenv() returns unexpected value if environment variable has non ASCII character [v4]
On Thu, 5 May 2022 01:34:48 GMT, Naoto Sato wrote:
>> Ichiroh Takiguchi has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> 8285517: System.getenv() returns unexpected value if environment variable
>> has non ASCII character
>
> test/jdk/java/lang/System/i18nEnvArg.java line 70:
>
>> 68: Map environ = pb.environment();
>> 69: environ.clear();
>> 70: environ.put("LANG", "ja_JP.eucjp");
>
> There are many duplicate pieces of code here and in the `else` block below.
> Can you simplify this `if` statement more?
Modified.
But I'm not sure, it's expected one.
> test/jdk/java/lang/System/i18nEnvArg.java line 110:
>
>> 108: String s = System.getenv(EUC_JP_TEXT);
>> 109: ByteArrayOutputStream baos = new ByteArrayOutputStream();
>> 110: PrintStream ps = new PrintStream(baos);
>
> Can utilize try-with-resources pattern.
Use `shouldNotContain()` to find the error message.
-
PR: https://git.openjdk.java.net/jdk/pull/8378
Re: RFR: 8285517: System.getenv() returns unexpected value if environment variable has non ASCII character [v2]
On Mon, 2 May 2022 15:00:07 GMT, Roger Riggs wrote: >> Ichiroh Takiguchi has updated the pull request incrementally with one >> additional commit since the last revision: >> >> 8285517: System.getenv() returns unexpected value if environment variable >> has non ASCII character > > Can you clarify the use of different charset properties for the > ProcessEnvironment vs the CommandLine strings? > > They are used to decode or encode strings in the APIs to native functions > respectively. > If I understand `native.encoding` was introduced to reflect the default > encoding of file contents, while `sun.jnu.encoding` is used to encode > filenames. > > I think I would have expected to see `sun.jnu.encoding` to be used in all > contexts when encoding/decoding strings to the native representations. > (Assuming its not required for backward compatibility). Hello @RogerRiggs and @naotoj . I put sun.jnu.encoding related code into s`StaticProperty.java`. Could you review the files again including javadoc comment ? - PR: https://git.openjdk.java.net/jdk/pull/8378
Re: RFR: 8262889: Compiler implementation for Record Patterns [v2]
On Fri, 6 May 2022 14:09:24 GMT, Jan Lahoda wrote:
>> 8262889: Compiler implementation for Record Patterns
>>
>> A first version of a patch that introduces record patterns into javac as a
>> preview feature. For the specification, please see:
>> http://cr.openjdk.java.net/~gbierman/jep427+405/jep427+405-20220426/specs/patterns-switch-record-patterns-jls.html
>>
>> There are two notable tricky parts:
>> -in the parser, it was necessary to improve the `analyzePattern` method to
>> handle nested/record patterns, while still keeping error recovery reasonable
>> -in the `TransPatterns`, the desugaring of the record patterns is very
>> straightforward - effectivelly the record patterns are desugared into
>> guards/conditions. This will likely be improved in some future
>> version/preview
>>
>> `MatchException` has been extended to cover additional cases related to
>> record patterns.
>
> Jan Lahoda has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Reflecting review feedback.
>From the JLS specdiff
> If the type R names a generic record class then it is a compile-time error if
> R is not a parameterized type.
The following snippet raises a `MatchException`. Shouldn't it be a compile-time
error?
Box r = new Box<>(null);
switch (r) {
case Box(String s):
System.out.println("match");
}
-
PR: https://git.openjdk.java.net/jdk/pull/8516
Re: RFR: 8285517: System.getenv() returns unexpected value if environment variable has non ASCII character [v5]
> On JDK19 with Linux ja_JP.eucjp locale, > System.getenv() returns unexpected value if environment variable has Japanese > EUC characters. > It seems this issue happens because of JEP 400. > Arguments for ProcessBuilder have same kind of issue. Ichiroh Takiguchi has updated the pull request incrementally with one additional commit since the last revision: 8285517: System.getenv() returns unexpected value if environment variable has non ASCII character - Changes: - all: https://git.openjdk.java.net/jdk/pull/8378/files - new: https://git.openjdk.java.net/jdk/pull/8378/files/84e6d639..5bd8492f Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=8378=04 - incr: https://webrevs.openjdk.java.net/?repo=jdk=8378=03-04 Stats: 103 lines in 4 files changed: 39 ins; 39 del; 25 mod Patch: https://git.openjdk.java.net/jdk/pull/8378.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8378/head:pull/8378 PR: https://git.openjdk.java.net/jdk/pull/8378
Re: RFR: 8262889: Compiler implementation for Record Patterns [v2]
> 8262889: Compiler implementation for Record Patterns > > A first version of a patch that introduces record patterns into javac as a > preview feature. For the specification, please see: > http://cr.openjdk.java.net/~gbierman/jep427+405/jep427+405-20220426/specs/patterns-switch-record-patterns-jls.html > > There are two notable tricky parts: > -in the parser, it was necessary to improve the `analyzePattern` method to > handle nested/record patterns, while still keeping error recovery reasonable > -in the `TransPatterns`, the desugaring of the record patterns is very > straightforward - effectivelly the record patterns are desugared into > guards/conditions. This will likely be improved in some future version/preview > > `MatchException` has been extended to cover additional cases related to > record patterns. Jan Lahoda has updated the pull request incrementally with one additional commit since the last revision: Reflecting review feedback. - Changes: - all: https://git.openjdk.java.net/jdk/pull/8516/files - new: https://git.openjdk.java.net/jdk/pull/8516/files/56020b0b..90b37c3a Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=8516=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk=8516=00-01 Stats: 193 lines in 18 files changed: 67 ins; 19 del; 107 mod Patch: https://git.openjdk.java.net/jdk/pull/8516.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8516/head:pull/8516 PR: https://git.openjdk.java.net/jdk/pull/8516
Re: RFR: JDK-8266431: Dual-Pivot Quicksort improvements (Radix sort) [v12]
On Mon, 14 Mar 2022 19:12:29 GMT, iaroslavski wrote: >> Sorting: >> >> - adopt radix sort for sequential and parallel sorts on >> int/long/float/double arrays (almost random and length > 6K) >> - fix tryMergeRuns() to better handle case when the last run is a single >> element >> - minor javadoc and comment changes >> >> Testing: >> - add new data inputs in tests for sorting >> - add min/max/infinity values to float/double testing >> - add tests for radix sort > > iaroslavski has updated the pull request incrementally with one additional > commit since the last revision: > > JDK-8266431: Dual-Pivot Quicksort improvements (Radix sort) > > * Improved mixed insertion sort > * Optimized insertion sort > * Improved merging sort > * Optimized soring tests I am improving test code (jmh + robust estimators) to have more robust results on small arrays (20 - 1000). I do hope having new results within 1 or 2 weeks. - PR: https://git.openjdk.java.net/jdk/pull/3938
Re: RFR: JDK-8285730: unify _WIN32_WINNT settings [v4]
On Wed, 4 May 2022 08:00:08 GMT, Matthias Baesken wrote: >> Currently we set _WIN32_WINNT at various places in the codebase; this is >> used to target a minimum Windows version we want to support. See also for >> more detailled information : >> https://docs.microsoft.com/en-us/windows/win32/winprog/using-the-windows-headers?redirectedfrom=MSDN#setting-winver-or-_win32_winnt >> Macros for Conditional Declarations >> "Certain functions that depend on a particular version of Windows are >> declared using conditional code. This enables you to use the compiler to >> detect whether your application uses functions that are not supported on its >> target version(s) of Windows." >> >> However currently we have quite a lot of differing settings of _WIN32_WINNT >> in the codebase ; setting _WIN32_WINNT to 0x0601 (Windows 7) where possible >> would make sense because we have this setting already at java_props_md.c >> (so targeting older Windows versions at other places is most likely not >> useful). > > Matthias Baesken has updated the pull request incrementally with one > additional commit since the last revision: > > adjust API level to Windows 8 for security.cpp and do some cleanup Marked as reviewed by erikj (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/8428
Re: RFR: 8262889: Compiler implementation for Record Patterns
On Wed, 4 May 2022 10:03:13 GMT, Maurizio Cimadamore
wrote:
>> src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Attr.java line 4180:
>>
>>> 4178: type = attribTree(tree.var.vartype, env, varInfo);
>>> 4179: } else {
>>> 4180: type = resultInfo.pt;
>>
>> Looks good - infers the binging var type from the record component being
>> processed. If not in a record, then I suspect resultInfo.pt is just the
>> target expression type (e.g. var in toplevel environment).
>
> That said, I'm not sure how this connects with `instanceof`. This patch
> doesn't seem to alter `visitTypeTest`. In the current code I can see this:
>
> if (tree.pattern.getTag() == BINDINGPATTERN ||
>tree.pattern.getTag() == PARENTHESIZEDPATTERN) {
>attribTree(tree.pattern, env, unknownExprInfo);
>...
>
>
> This seems wrong for two reasons:
>
> * it doesn't take into account the new pattern tag
> * it uses an "unknown" result info when attributing, meaning that any
> toplevel `var` pattern will not be attributed correctly
>
> But we seem to have tests covering record patterns and instanceof. so I'm
> wondering if I'm missing some code update?
Some of the handling inside this ifs is only needed for type test and
parenthesized patterns (as record patterns are never unconditional (total)). I
have an upcoming patch that should improve the tests here, however.
For `var` - the specification does not allow `var` at the top level (14.30.1,
"The LocalVariableType in a top-level type pattern denotes a reference type
(and furthermore is not var)."). In my upcoming patch, I am adding a test to
ensure meaningful behavior for top-level type test patterns with `var`.
-
PR: https://git.openjdk.java.net/jdk/pull/8516
Re: RFR: 8282191: Implementation of Foreign Function & Memory API (Preview) [v39]
On Fri, 6 May 2022 08:42:12 GMT, Maurizio Cimadamore wrote: >> src/java.base/share/classes/jdk/internal/reflect/Reflection.java line 120: >> >>> 118: // if there is no caller class, act as if the call came from >>> an unnamed module >>> 119: Module module = currentClass != null ? >>> 120: currentClass.getModule() : Holder.FALLBACK_MODULE; >> >> This can be simplified to: >> >> Module module = currentClass != null ? >>currentClass.getModule() : >> ClassLoader::getSystemClassLoader().getUnnamedModule(); >> >> >> No need to have the Holder class. > > gah! I missed that :-) I've addressed these comments (thanks!) and also added some tests for these corner cases. - PR: https://git.openjdk.java.net/jdk/pull/7888
Re: RFR: 8282191: Implementation of Foreign Function & Memory API (Preview) [v40]
> This PR contains the API and implementation changes for JEP-424 [1]. A more > detailed description of such changes, to avoid repetitions during the review > process, is included as a separate comment. > > [1] - https://openjdk.java.net/jeps/424 Maurizio Cimadamore has updated the pull request incrementally with one additional commit since the last revision: Add tests for loaderLookup/restricted method corner cases - Changes: - all: https://git.openjdk.java.net/jdk/pull/7888/files - new: https://git.openjdk.java.net/jdk/pull/7888/files/4d24ffa9..b71c4e93 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=7888=39 - incr: https://webrevs.openjdk.java.net/?repo=jdk=7888=38-39 Stats: 248 lines in 10 files changed: 239 ins; 4 del; 5 mod Patch: https://git.openjdk.java.net/jdk/pull/7888.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/7888/head:pull/7888 PR: https://git.openjdk.java.net/jdk/pull/7888
Re: RFR: 8262889: Compiler implementation for Record Patterns
On Fri, 6 May 2022 10:51:33 GMT, Jan Lahoda wrote:
>> I now believe that the check is needed to properly classify patterns based
>> on the type of the i-th component. That said, not sure this should be a
>> subtyping check, or a type equality
>
> A good question. Consider code like:
>
> private void test(R r) {
> switch (r) {
> case R(A a, A v) -> {}
> case R(B b, A v) -> {}
> case R(I i, B v) -> {}
> }
> }
> final class A implements I {}
> sealed interface I permits A, B {}
> final class B implements I {}
> record R(I i1, I i2) {}
>
>
> The switch is exhaustive - all the possible combinations are covered. When
> checking the first component, the code will categorize the patterns like:
>
> A -> R(A a, A v), R(I i, B v)
> B -> R(B b, A v), R(I i, B v)
> I -> R(I i, B v)
>
> this categorization is done using the subtype check, so that `R(I i, B v)`
> will appear in the list for `A`. When checking the second component, the
> possibility for `I` is not exhaustive (does not cover `A` in the second
> component), but the possibilities for `A` and `B` are exhaustive, and they
> together cover `I`.
Ah - makes sense of course - I "just" needed a more convoluted example :-)
-
PR: https://git.openjdk.java.net/jdk/pull/8516
RFR: 8286298: Remove unused methods in sun.invoke.util.VerifyType
A few untested and unused methods in `VerifyType` which can be removed. (Possibly used by native JSR 292 implementations in JDK 7). - Commit messages: - Remove unused methods in sun.invoke.util.VerifyType Changes: https://git.openjdk.java.net/jdk/pull/8570/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=8570=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8286298 Stats: 72 lines in 1 file changed: 0 ins; 71 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/8570.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8570/head:pull/8570 PR: https://git.openjdk.java.net/jdk/pull/8570
Re: RFR: 8262889: Compiler implementation for Record Patterns
On Thu, 5 May 2022 15:17:11 GMT, Maurizio Cimadamore
wrote:
>> You are right. It is the ii) which iteratively checks the component pattern
>> list L.
>
> I now believe that the check is needed to properly classify patterns based on
> the type of the i-th component. That said, not sure this should be a
> subtyping check, or a type equality
A good question. Consider code like:
private void test(R r) {
switch (r) {
case R(A a, A v) -> {}
case R(B b, A v) -> {}
case R(I i, B v) -> {}
}
}
final class A implements I {}
sealed interface I permits A, B {}
final class B implements I {}
record R(I i1, I i2) {}
The switch is exhaustive - all the possible combinations are covered. When
checking the first component, the code will categorize the patterns like:
A -> R(A a, A v), R(I i, B v)
B -> R(B b, A v), R(I i, B v)
I -> R(I i, B v)
this categorization is done using the subtype check, so that `R(I i, B v)` will
appear in the list for `A`. When checking the second component, the possibility
for `I` is not exhaustive (does not cover `A` in the second component), but the
possibilities for `A` and `B` are exhaustive, and they together cover `I`.
-
PR: https://git.openjdk.java.net/jdk/pull/8516
Re: RFR: 8284960: Integration of JEP 426: Vector API (Fourth Incubator) [v2]
On Thu, 5 May 2022 05:47:47 GMT, Jatin Bhateja wrote: >> Hi All, >> >> Patch adds the planned support for new vector operations and APIs targeted >> for [JEP 426: Vector API (Fourth >> Incubator).](https://bugs.openjdk.java.net/browse/JDK-8280173) >> >> Following is the brief summary of changes:- >> >> 1) Extends the scope of existing lanewise API for following new vector >> operations. >>- VectorOperations.BIT_COUNT: counts the number of one-bits >>- VectorOperations.LEADING_ZEROS_COUNT: counts the number of leading zero >> bits >>- VectorOperations.TRAILING_ZEROS_COUNT: counts the number of trailing >> zero bits >>- VectorOperations.REVERSE: reversing the order of bits >>- VectorOperations.REVERSE_BYTES: reversing the order of bytes >>- compress and expand bits: Semantics are based on Hacker's Delight >> section 7-4 Compress, or Generalized Extract. >> >> 2) Adds following new APIs to perform cross lane vector compress and >> expansion operations under the influence of a mask. >>- Vector.compress >>- Vector.expand >>- VectorMask.compress >> >> 3) Adds predicated and non-predicated versions of following new APIs to load >> and store the contents of vector from foreign MemorySegments. >> - Vector.fromMemorySegment >> - Vector.intoMemorySegment >> >> 4) C2 Compiler IR enhancements and optimized X86 and AARCH64 backend support >> for each newly added operation. >> >> >> Patch has been regressed over AARCH64 and X86 targets different AVX levels. >> >> Kindly review and share your feedback. >> >> Best Regards, >> Jatin > > Jatin Bhateja has updated the pull request with a new target base due to a > merge or a rebase. The pull request now contains 10 commits: > > - 8284960: Correcting a typo. > - 8284960: Integrating changes from panama-vector (Add @since 19 tags). > - Merge branch 'master' of http://github.com/openjdk/jdk into JDK-8284960 > - Merge branch 'master' of http://github.com/openjdk/jdk into JDK-8284960 > - Merge branch 'master' of http://github.com/openjdk/jdk into JDK-8284960 > - 8284960: AARCH64 backend changes. > - Merge branch 'master' of http://github.com/openjdk/jdk into JDK-8284960 > - Merge branch 'master' of http://github.com/openjdk/jdk into JDK-8284960 > - Merge branch 'master' of http://github.com/openjdk/jdk into JDK-8284960 > - 8284960: Integration of JEP 426: Vector API (Fourth Incubator) `cpu/aarch64` changes look good. - Marked as reviewed by ngasson (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/8425
Re: RFR: JDK-8285730: unify _WIN32_WINNT settings [v4]
On Wed, 4 May 2022 08:00:08 GMT, Matthias Baesken wrote: >> Currently we set _WIN32_WINNT at various places in the codebase; this is >> used to target a minimum Windows version we want to support. See also for >> more detailled information : >> https://docs.microsoft.com/en-us/windows/win32/winprog/using-the-windows-headers?redirectedfrom=MSDN#setting-winver-or-_win32_winnt >> Macros for Conditional Declarations >> "Certain functions that depend on a particular version of Windows are >> declared using conditional code. This enables you to use the compiler to >> detect whether your application uses functions that are not supported on its >> target version(s) of Windows." >> >> However currently we have quite a lot of differing settings of _WIN32_WINNT >> in the codebase ; setting _WIN32_WINNT to 0x0601 (Windows 7) where possible >> would make sense because we have this setting already at java_props_md.c >> (so targeting older Windows versions at other places is most likely not >> useful). > > Matthias Baesken has updated the pull request incrementally with one > additional commit since the last revision: > > adjust API level to Windows 8 for security.cpp and do some cleanup Hi David, thanks for the review ! - PR: https://git.openjdk.java.net/jdk/pull/8428
Re: RFR: JDK-8285730: unify _WIN32_WINNT settings [v4]
On Wed, 4 May 2022 08:00:08 GMT, Matthias Baesken wrote: >> Currently we set _WIN32_WINNT at various places in the codebase; this is >> used to target a minimum Windows version we want to support. See also for >> more detailled information : >> https://docs.microsoft.com/en-us/windows/win32/winprog/using-the-windows-headers?redirectedfrom=MSDN#setting-winver-or-_win32_winnt >> Macros for Conditional Declarations >> "Certain functions that depend on a particular version of Windows are >> declared using conditional code. This enables you to use the compiler to >> detect whether your application uses functions that are not supported on its >> target version(s) of Windows." >> >> However currently we have quite a lot of differing settings of _WIN32_WINNT >> in the codebase ; setting _WIN32_WINNT to 0x0601 (Windows 7) where possible >> would make sense because we have this setting already at java_props_md.c >> (so targeting older Windows versions at other places is most likely not >> useful). > > Matthias Baesken has updated the pull request incrementally with one > additional commit since the last revision: > > adjust API level to Windows 8 for security.cpp and do some cleanup This seems okay to me in this form. I agree that explicitly setting this is better than unknowingly using API's that might not be available on supported platforms. Thanks. - Marked as reviewed by dholmes (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/8428
Re: RFR: 8284161: Implementation of Virtual Threads (Preview) [v13]
On Fri, 6 May 2022 06:48:46 GMT, Alan Bateman wrote: >> This is the implementation of JEP 425: Virtual Threads (Preview). >> >> We will refresh this PR periodically to pick up changes and fixes from the >> loom repo. >> >> Most of the new mechanisms in the HotSpot VM are disabled by default and >> require running with `--enable-preview` to enable. >> >> The patch has support for x64 and aarch64 on the usual operating systems >> (Linux, macOS, and Windows). There are stubs (calling _Unimplemented_) for >> zero and some of the other ports. Additional ports can be contributed via >> PRs against the fibers branch in the loom repo. >> >> There are changes in many areas. To reduce notifications/mails, the labels >> have been trimmed down for now to hotspot, serviceability and core-libs. We >> can add additional labels, if required, as the PR progresses. >> >> The changes include a refresh of java.util.concurrent and ForkJoinPool from >> Doug Lea's CVS. These changes will probably be proposed and integrated in >> advance of this PR. >> >> The changes include some non-exposed and low-level infrastructure to support >> the (in draft) JEPs for Structured Concurrency and Extent Locals. This is to >> make life a bit easier and avoid having to separate VM changes and juggle >> branches at this time. > > Alan Bateman has updated the pull request with a new target base due to a > merge or a rebase. The pull request now contains 21 commits: > > - Refresh 6ace49bf42e5504c69fa11c564e8e865c0a95fb3 > - Merge with 9425ab2b43b649bd591706bfc820e9c8795a6fdf > - Refresh 12aa09ce7efd48425cc080d0b8761aca0f3e215f > - Merge with 1bb4de2e2868a539846ec48dd43fd623c2ba69a5 > - Refresh d77f7a49af75bcc5b20686805ff82a93a20dde05 > - Merge with 4b2c82200fdc01de868cf414e40a4d891e753f89 > - Refresh 6091080db743ece5f1b2111fcc35a5f2179a403a > - Merge with cfcba1fccc8e3e6a68e1cb1826b70e076d5d83c4 > - Refresh ee9fa8ed05ec22de7a13383052d68aa8aa7832ec > - Merge with jdk-19+20 > - ... and 11 more: > https://git.openjdk.java.net/jdk/compare/9425ab2b...6d968671 > It is understandable to ship the preview feature not implemented on all > platforms. It is even understandable to ship the final product feature that > breaks some platforms in an clearly understandable way, prompting platform > maintainers to implement the agreed-upon, final Java feature. What I am > seeing, though, is that just running `java -version` (!) after integrating a > _preview feature_ is broken! Preview features need to be implemented by a port before it can be released. For JDK 19 this means that the maintainers of ports will have work to do for both JEP 424 and JEP 425 (ifdef is not an option). I think the issue that are concerned about is the interim period between the JEP 425 integration and the port/implementation of this feature to other architectures. I think the answer is that it will vary. It may be that some port maintainers decide to do something very short term so they can run without --enable-preview and buy time before they do the port/implementation for JDK 19. Good progress has been reported on at least ppc64le port and maybe that port be ready before others. So yes, some ports may crash until they receive attention, others (like zero) should be able to run tests that don't use --enable-preview. I've no doubt there will be a flurry of changes post integration. The motivation for Continuations::enabled() was to reduce risk and disable a lot of the new code by default. The motivation wasn't porting but it may be helpful during the interim period. That is probably a topic for loom-dev rather here. - PR: https://git.openjdk.java.net/jdk/pull/8166
Re: RFR: 8262889: Compiler implementation for Record Patterns
On Thu, 5 May 2022 11:57:34 GMT, Aggelos Biboudis
wrote:
>> 8262889: Compiler implementation for Record Patterns
>>
>> A first version of a patch that introduces record patterns into javac as a
>> preview feature. For the specification, please see:
>> http://cr.openjdk.java.net/~gbierman/jep427+405/jep427+405-20220426/specs/patterns-switch-record-patterns-jls.html
>>
>> There are two notable tricky parts:
>> -in the parser, it was necessary to improve the `analyzePattern` method to
>> handle nested/record patterns, while still keeping error recovery reasonable
>> -in the `TransPatterns`, the desugaring of the record patterns is very
>> straightforward - effectivelly the record patterns are desugared into
>> guards/conditions. This will likely be improved in some future
>> version/preview
>>
>> `MatchException` has been extended to cover additional cases related to
>> record patterns.
>
> src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Flow.java line 783:
>
>> 781: }
>> 782: for (Entry> e :
>> categorizedDeconstructionPatterns.entrySet()) {
>> 783: if (coversDeconstructionStartingFromComponent(pos,
>> targetType, e.getValue(), 0)) {
>
> Here, the result of `e.getValue` is a reversed list of the observed patterns.
>
> For the switch below,
>
>
> switch (r) {
> case R(A a, A b) -> 1;
> case R(A a, B b) -> 2;
> case R(B a, A b) -> 3;
> case R(B a, B b) -> 4;
> }
>
>
> The 0th element of that list is the `R(B a, B b)` pattern, the 1st the `R(B
> a, A b)`. I am 99% sure that this is not a problem but I am pointing it out
> regardless, in case there is any underlying danger to that.
Order doesn't matter. I triple checked.
-
PR: https://git.openjdk.java.net/jdk/pull/8516
Re: RFR: 8282191: Implementation of Foreign Function & Memory API (Preview) [v39]
On Thu, 5 May 2022 21:28:32 GMT, Mandy Chung wrote: >> Maurizio Cimadamore has updated the pull request incrementally with one >> additional commit since the last revision: >> >> * Clarify what happens when SymbolLookup::loaderLookup is invoked from JNI >> * Tweak restricted method check to work when there's no caller class > > src/java.base/share/classes/java/lang/foreign/SymbolLookup.java line 161: > >> 159: ClassLoader.getSystemClassLoader(); >> 160: MemorySession loaderSession = (loader == null) ? >> 161: MemorySession.global() : // boot loader never goes away > > The other built-in class loaders (`ClassLoaders::appClassLoader` and > `ClassLoaders::platformClassLoader` are both instance of > `BuiltinClassLoader`) will never be unloaded. Should they use the globel > memory session? good idea > src/java.base/share/classes/jdk/internal/reflect/Reflection.java line 120: > >> 118: // if there is no caller class, act as if the call came from an >> unnamed module >> 119: Module module = currentClass != null ? >> 120: currentClass.getModule() : Holder.FALLBACK_MODULE; > > This can be simplified to: > > Module module = currentClass != null ? >currentClass.getModule() : > ClassLoader::getSystemClassLoader().getUnnamedModule(); > > > No need to have the Holder class. gah! I missed that :-) - PR: https://git.openjdk.java.net/jdk/pull/7888
Re: RFR: 4511638: Double.toString(double) sometimes produces incorrect results [v9]
> Hello, > > here's a PR for a patch submitted on March 2020 > [1](https://cr.openjdk.java.net/~bpb/4511638/webrev.04/) when Mercurial was a > thing. > > The patch has been edited to adhere to OpenJDK code conventions about > multi-line (block) comments. Nothing in the code proper has changed, except > for the addition of redundant but clarifying parentheses in some expressions. > > > Greetings > Raffaello Raffaello Giulietti has updated the pull request incrementally with one additional commit since the last revision: 4511638: Double.toString(double) sometimes produces incorrect results - Changes: - all: https://git.openjdk.java.net/jdk/pull/3402/files - new: https://git.openjdk.java.net/jdk/pull/3402/files/904ba115..e7c4bd25 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=3402=08 - incr: https://webrevs.openjdk.java.net/?repo=jdk=3402=07-08 Stats: 4 lines in 3 files changed: 0 ins; 0 del; 4 mod Patch: https://git.openjdk.java.net/jdk/pull/3402.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/3402/head:pull/3402 PR: https://git.openjdk.java.net/jdk/pull/3402
Re: RFR: 8284161: Implementation of Virtual Threads (Preview) [v12]
On Thu, 5 May 2022 18:54:49 GMT, Alan Bateman wrote: > We mailed porters-dev in Sep 2021 to give a heads up that this feature would > require porting work so it shouldn't be a surprise. We have been open to > including other ports with the initial integration but it was never a goal to > have all the ports done in advance of that. It is understandable to ship the preview feature not implemented on all platforms. It is even understandable to ship the final product feature that breaks some platforms in an clearly understandable way, prompting platform maintainers to implement the agreed-upon, final Java feature. What I am seeing, though, is that just running `java -version` (!) after integrating a *preview feature* is broken! > Most of the new code in the VM is only used when running with > --enable-preview. You'll see several places that test > Continuations::enabled(). It should be possible to get these port running > without -enable-preview without much effort. The feature has to be > implemented to pass all the tests of course. It is not as clear-cut, unfortunately. I see there are substantial changes in deopt machinery with post-call NOPs -- and there maybe more stuff lurking after that one is implemented -- so it is not as simple as changing `Unimplemented()` to guarding with `Continuations::enabled()`. So this looks to me that the core deopt machinery is affected, whether Loom is enabled or not. Is the impact of that deopt machinery change on the VM stability and VM ports discussed, understood, documented somewhere? I would have honestly expected those core changes to be heavily conditionalized with either `ifdef`-s, or runtime checks, or both, so that both unimplemented platforms had the old behavior *and* the implemented platforms had a fallback to old behavior if preview feature was broken. The current code is fine for experimental Loom repo, but I firmly believe mainline should have much stronger safety/reliability requirements. My fear is that an integration like this would wreck JDK 19 release. So my question stands: Are we breaking those platforms? Are we sure the unconditional VM changes are problem-free, implementable everywhere, etc? The answer might be "Yes, we are integrating, let the chips fall where they may", but I also believe that should be the call made by responsible platform/VM architects, who should explicitly weigh in and accept the risk of wide JDK 19 breakage. - PR: https://git.openjdk.java.net/jdk/pull/8166
Re: RFR: 4511638: Double.toString(double) sometimes produces incorrect results [v8]
> Hello, > > here's a PR for a patch submitted on March 2020 > [1](https://cr.openjdk.java.net/~bpb/4511638/webrev.04/) when Mercurial was a > thing. > > The patch has been edited to adhere to OpenJDK code conventions about > multi-line (block) comments. Nothing in the code proper has changed, except > for the addition of redundant but clarifying parentheses in some expressions. > > > Greetings > Raffaello Raffaello Giulietti has updated the pull request incrementally with one additional commit since the last revision: 4511638: Double.toString(double) sometimes produces incorrect results - Changes: - all: https://git.openjdk.java.net/jdk/pull/3402/files - new: https://git.openjdk.java.net/jdk/pull/3402/files/a36ff8ac..904ba115 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=3402=07 - incr: https://webrevs.openjdk.java.net/?repo=jdk=3402=06-07 Stats: 389 lines in 5 files changed: 40 ins; 216 del; 133 mod Patch: https://git.openjdk.java.net/jdk/pull/3402.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/3402/head:pull/3402 PR: https://git.openjdk.java.net/jdk/pull/3402
Re: RFR: JDK-8285730: unify _WIN32_WINNT settings [v4]
On Wed, 4 May 2022 09:08:28 GMT, Matthias Baesken wrote:
> Does this mean that not setting _WIN32_WINNT means :any API is allowed" ?
Hi David , I did one more try with my current setup (VS2017 on a Win10
notebook). I did not set _WIN32_WINNT.
My little test program
#include
#include
int main() {
#ifdef _WIN32_WINNT
printf("_WIN32_WINNT is defined .\n");
#if (_WIN32_WINNT == 0x0600)
printf("Vista API setting\n");
#endif
#if (_WIN32_WINNT == 0x0601)
printf("Win 7 API setting\n");
#endif
#if (_WIN32_WINNT == 0x0602)
printf("Win 8 API setting\n");
#endif
#if (_WIN32_WINNT == 0x0603)
printf("Win 8.1 API setting\n");
#endif
#if (_WIN32_WINNT == 0x0A00)
printf("Win 10 API setting\n");
#endif
#endif
return 0;
}
shows me
_WIN32_WINNT is defined .
Win 10 API setting
So I think with our current compilers in use like VS2017 / VS2019 we allow
Win10 APIs in most of our code except a few places where we set _WIN32_WINNT
and go back to some mixture of older APIs.
Not sure if this is a good thing, we could break for example Win 8.1/Win2012
compatibility easily this way.
-
PR: https://git.openjdk.java.net/jdk/pull/8428
Re: RFR: 4511638: Double.toString(double) sometimes produces incorrect results [v7]
> Hello, > > here's a PR for a patch submitted on March 2020 > [1](https://cr.openjdk.java.net/~bpb/4511638/webrev.04/) when Mercurial was a > thing. > > The patch has been edited to adhere to OpenJDK code conventions about > multi-line (block) comments. Nothing in the code proper has changed, except > for the addition of redundant but clarifying parentheses in some expressions. > > > Greetings > Raffaello Raffaello Giulietti has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 13 commits: - 4511638: Double.toString(double) sometimes produces incorrect results Merge branch 'master' into JDK-4511638 - 4511638: Double.toString(double) sometimes produces incorrect results Adapted hashes in ElementStructureTest.java - 4511638: Double.toString(double) sometimes produces incorrect results Merge branch 'master' into JDK-4511638 - 4511638: Double.toString(double) sometimes produces incorrect results Enhanced intervals in MathUtils. Updated references to Schubfach v4. - 4511638: Double.toString(double) sometimes produces incorrect results Merge branch 'master' into JDK-4511638 - 4511638: Double.toString(double) sometimes produces incorrect results Slight adjustments to Javadoc as suggested in the JDK-8202555 (CSR) comments. - 4511638: Double.toString(double) sometimes produces incorrect results Adjusted hashes in test/langtools/tools/javac/sym/ElementStructureTest.java - 4511638: Double.toString(double) sometimes produces incorrect results Merge branch 'master' into JDK-4511638 - 4511638: Double.toString(double) sometimes produces incorrect results - 4511638: Double.toString(double) sometimes produces incorrect results Refactored test classes to better match OpenJDK conventions. Added tests recommended by Guy Steele and Paul Zimmermann. - ... and 3 more: https://git.openjdk.java.net/jdk/compare/dd06cc63...a36ff8ac - Changes: https://git.openjdk.java.net/jdk/pull/3402/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=3402=06 Stats: 23938 lines in 16 files changed: 23809 ins; 54 del; 75 mod Patch: https://git.openjdk.java.net/jdk/pull/3402.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/3402/head:pull/3402 PR: https://git.openjdk.java.net/jdk/pull/3402
Re: RFR: 8284161: Implementation of Virtual Threads (Preview) [v13]
> This is the implementation of JEP 425: Virtual Threads (Preview). > > We will refresh this PR periodically to pick up changes and fixes from the > loom repo. > > Most of the new mechanisms in the HotSpot VM are disabled by default and > require running with `--enable-preview` to enable. > > The patch has support for x64 and aarch64 on the usual operating systems > (Linux, macOS, and Windows). There are stubs (calling _Unimplemented_) for > zero and some of the other ports. Additional ports can be contributed via PRs > against the fibers branch in the loom repo. > > There are changes in many areas. To reduce notifications/mails, the labels > have been trimmed down for now to hotspot, serviceability and core-libs. We > can add additional labels, if required, as the PR progresses. > > The changes include a refresh of java.util.concurrent and ForkJoinPool from > Doug Lea's CVS. These changes will probably be proposed and integrated in > advance of this PR. > > The changes include some non-exposed and low-level infrastructure to support > the (in draft) JEPs for Structured Concurrency and Extent Locals. This is to > make life a bit easier and avoid having to separate VM changes and juggle > branches at this time. Alan Bateman has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 21 commits: - Refresh 6ace49bf42e5504c69fa11c564e8e865c0a95fb3 - Merge with 9425ab2b43b649bd591706bfc820e9c8795a6fdf - Refresh 12aa09ce7efd48425cc080d0b8761aca0f3e215f - Merge with 1bb4de2e2868a539846ec48dd43fd623c2ba69a5 - Refresh d77f7a49af75bcc5b20686805ff82a93a20dde05 - Merge with 4b2c82200fdc01de868cf414e40a4d891e753f89 - Refresh 6091080db743ece5f1b2111fcc35a5f2179a403a - Merge with cfcba1fccc8e3e6a68e1cb1826b70e076d5d83c4 - Refresh ee9fa8ed05ec22de7a13383052d68aa8aa7832ec - Merge with jdk-19+20 - ... and 11 more: https://git.openjdk.java.net/jdk/compare/9425ab2b...6d968671 - Changes: https://git.openjdk.java.net/jdk/pull/8166/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=8166=12 Stats: 99456 lines in 1130 files changed: 91188 ins; 3598 del; 4670 mod Patch: https://git.openjdk.java.net/jdk/pull/8166.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8166/head:pull/8166 PR: https://git.openjdk.java.net/jdk/pull/8166
Re: RFR: 8286191: misc tests fail due to JDK-8285987 [v3]
On Thu, 5 May 2022 16:14:32 GMT, Daniel D. Daugherty wrote: >> Matthias Baesken has updated the pull request incrementally with one >> additional commit since the last revision: >> >> restore year in ExternalEditorTest, remove test exclusion > > test/langtools/jdk/jshell/ExternalEditorTest.java line 29: > >> 27: * @bug 8143955 8080843 8163816 8143006 8169828 8171130 8162989 8210808 >> 28: * @comment musl/Alpine has problems executing some shell scripts, see >> 8285987 >> 29: * @requires !vm.musl > > So this change backs out an "@requires" that was added by: > > JDK-8285987 executing shell scripts without #! fails on Alpine linux > https://bugs.openjdk.java.net/browse/JDK-8285987 > > Presumably this "@requires" was added for some reason so what's > going to happen if this test is run on Alpine Linux? Also, the fix in > JDK-8285987 updated the copyright year. Do you plan on restoring > it to the original "2017" value? Hi Daniel, I restored the original year 2017. (additionally I removed the langtools exclusion) > Presumably this "@requires" was added for some reason so what's going to > happen if this test is run on Alpine Linux I can only speak about our Alpine setup (3.15 in a container) and there the test fails with error=8, Exec format error. Looks like something similar has been observed as well by these people https://www.openwall.com/lists/musl/2018/03/09/2 https://github.com/scala-steward-org/scala-steward/issues/1374 - PR: https://git.openjdk.java.net/jdk/pull/8556
Re: RFR: 8286191: misc tests fail due to JDK-8285987
On Thu, 5 May 2022 16:33:53 GMT, Daniel D. Daugherty wrote: > ... update the PR's synopsis. Done . - PR: https://git.openjdk.java.net/jdk/pull/8556
Re: RFR: 8285295: Need better testing for IdentityHashMap [v4]
On Thu, 5 May 2022 23:46:24 GMT, Stuart Marks wrote: > It would be nice if such a test could be written, but as it stands I think > that `Wrappers.java` test is too simplistic. Would adding `Wrappers.java` a method-name white-list mechanism suitable in this situation? - PR: https://git.openjdk.java.net/jdk/pull/8354
Re: RFR: 8286191: misc tests fail due to JDK-8285987 [v3]
> The isMusl method had to be handled in > test/lib-test/jdk/test/lib/TestMutuallyExclusivePlatformPredicates.java . > Additionally, the vm.musl predicate seem not to be available in the langtools > tests. Matthias Baesken has updated the pull request incrementally with one additional commit since the last revision: restore year in ExternalEditorTest, remove test exclusion - Changes: - all: https://git.openjdk.java.net/jdk/pull/8556/files - new: https://git.openjdk.java.net/jdk/pull/8556/files/96f508df..2337301c Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=8556=02 - incr: https://webrevs.openjdk.java.net/?repo=jdk=8556=01-02 Stats: 2 lines in 2 files changed: 0 ins; 1 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/8556.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8556/head:pull/8556 PR: https://git.openjdk.java.net/jdk/pull/8556
Re: RFR: 8286191: misc tests fail due to JDK-8285987 [v2]
> The isMusl method had to be handled in > test/lib-test/jdk/test/lib/TestMutuallyExclusivePlatformPredicates.java . > Additionally, the vm.musl predicate seem not to be available in the langtools > tests. Matthias Baesken 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-8286191 - remove from ProblemList - Merge remote-tracking branch 'origin/master' into JDK-8286191 - JDK-8286191 - Changes: - all: https://git.openjdk.java.net/jdk/pull/8556/files - new: https://git.openjdk.java.net/jdk/pull/8556/files/afdc9797..96f508df Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=8556=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk=8556=00-01 Stats: 515 lines in 30 files changed: 194 ins; 107 del; 214 mod Patch: https://git.openjdk.java.net/jdk/pull/8556.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8556/head:pull/8556 PR: https://git.openjdk.java.net/jdk/pull/8556
