Re: RFR: 8262889: Compiler implementation for Record Patterns [v5]

2022-05-10 Thread Vicente Romero
On Tue, 10 May 2022 09:57:48 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/

Re: RFR: 8262889: Compiler implementation for Record Patterns [v4]

2022-05-09 Thread Vicente Romero
On Mon, 9 May 2022 14:37:35 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/j

Re: RFR: 8262889: Compiler implementation for Record Patterns [v4]

2022-05-09 Thread Vicente Romero
On Mon, 9 May 2022 14:37:35 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/j

Re: RFR: 8262889: Compiler implementation for Record Patterns [v4]

2022-05-09 Thread Vicente Romero
On Fri, 6 May 2022 17:40:25 GMT, Jan Lahoda wrote: >> 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

Re: RFR: 8262889: Compiler implementation for Record Patterns [v2]

2022-05-06 Thread Vicente Romero
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/j

Re: RFR: 8262889: Compiler implementation for Record Patterns

2022-05-05 Thread Vicente Romero
On Thu, 5 May 2022 15:21:49 GMT, Maurizio Cimadamore 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/je

Re: RFR: 8282274: Compiler implementation for Pattern Matching for switch (Third Preview) [v5]

2022-04-20 Thread Vicente Romero
On Tue, 19 Apr 2022 18:47:16 GMT, Jan Lahoda wrote: >> This is a (preliminary) patch for javac implementation for the third preview >> of pattern matching for switch (type patterns in switches). >> >> Draft JLS: >> http://cr.openjdk.java.net/~gbierman/PatternSwitchPlusRecordPatterns/PatternSwit

Re: RFR: 8282274: Compiler implementation for Pattern Matching for switch (Third Preview) [v5]

2022-04-20 Thread Vicente Romero
On Tue, 19 Apr 2022 18:47:16 GMT, Jan Lahoda wrote: >> This is a (preliminary) patch for javac implementation for the third preview >> of pattern matching for switch (type patterns in switches). >> >> Draft JLS: >> http://cr.openjdk.java.net/~gbierman/PatternSwitchPlusRecordPatterns/PatternSwit

Re: RFR: 8282274: Compiler implementation for Pattern Matching for switch (Third Preview) [v5]

2022-04-20 Thread Vicente Romero
On Tue, 19 Apr 2022 18:47:16 GMT, Jan Lahoda wrote: >> This is a (preliminary) patch for javac implementation for the third preview >> of pattern matching for switch (type patterns in switches). >> >> Draft JLS: >> http://cr.openjdk.java.net/~gbierman/PatternSwitchPlusRecordPatterns/PatternSwit

Re: RFR: 8282274: Compiler implementation for Pattern Matching for switch (Third Preview) [v3]

2022-04-15 Thread Vicente Romero
On Tue, 12 Apr 2022 13:18:14 GMT, Jan Lahoda wrote: >> This is a (preliminary) patch for javac implementation for the third preview >> of pattern matching for switch (type patterns in switches). >> >> Draft JLS: >> http://cr.openjdk.java.net/~gbierman/PatternSwitchPlusRecordPatterns/PatternSwit

Re: RFR: 8282274: Compiler implementation for Pattern Matching for switch (Third Preview) [v3]

2022-04-15 Thread Vicente Romero
On Tue, 12 Apr 2022 13:18:14 GMT, Jan Lahoda wrote: >> This is a (preliminary) patch for javac implementation for the third preview >> of pattern matching for switch (type patterns in switches). >> >> Draft JLS: >> http://cr.openjdk.java.net/~gbierman/PatternSwitchPlusRecordPatterns/PatternSwit

Re: RFR: 8282274: Compiler implementation for Pattern Matching for switch (Third Preview) [v3]

2022-04-14 Thread Vicente Romero
On Thu, 14 Apr 2022 08:46:50 GMT, Aggelos Biboudis wrote: >> Jan Lahoda has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Cleanup. > > src/jdk.compiler/share/classes/com/sun/tools/javac/tree/TreeInfo.java line > 1377: > >> 1375:

Re: RFR: 8282274: Compiler implementation for Pattern Matching for switch (Third Preview) [v3]

2022-04-13 Thread Vicente Romero
On Tue, 12 Apr 2022 13:18:14 GMT, Jan Lahoda wrote: >> This is a (preliminary) patch for javac implementation for the third preview >> of pattern matching for switch (type patterns in switches). >> >> Draft JLS: >> http://cr.openjdk.java.net/~gbierman/PatternSwitchPlusRecordPatterns/PatternSwit

Integrated: 8284361: Updating ASM to 9.3 for JDK 19

2022-04-08 Thread Vicente Romero
On Thu, 7 Apr 2022 03:33:02 GMT, Vicente Romero wrote: > We recently updated our ASM version to 9.2 and just this week version 9.3 was > announced with support for JDK19 so it makes sense to update to this last > version. > > Thanks in advance for the reviews, > Vicente Thi

Re: RFR: 8284361: Updating ASM to 9.3 for JDK 19

2022-04-08 Thread Vicente Romero
On Fri, 8 Apr 2022 16:44:15 GMT, Mandy Chung wrote: >> We recently updated our ASM version to 9.2 and just this week version 9.3 >> was announced with support for JDK19 so it makes sense to update to this >> last version. >> >> Thanks in advance for the reviews, >> Vicente > > Looks okay. Mo

RFR: 8284361: Updating ASM to 9.3 for JDK 19

2022-04-06 Thread Vicente Romero
We recently updated our ASM version to 9.2 and just this week version 9.3 was announced with support for JDK19 so it makes sense to update to this last version. Thanks in advance for the reviews, Vicente - Commit messages: - updating test ValidateJarWithSealedAndRecord - adaptati

Re: RFR: 8282508: Updating ASM to 9.2 for JDK 19 [v3]

2022-03-30 Thread Vicente Romero
On Wed, 30 Mar 2022 02:56:41 GMT, Vicente Romero wrote: >> Please review this PR which is updating the ASM included in the JDK to ASM >> 9.2. This version should be included in JDK19. There are basically two >> commits here, one that was automatically generated and that mostl

Re: RFR: 8282508: Updating ASM to 9.2 for JDK 19 [v3]

2022-03-29 Thread Vicente Romero
mons.RemappingMethodAdapter > jdk.internal.org.objectweb.asm.commons.RemappingAnnotationAdapter > > This is because package: `jdk.jfr.internal.instrument` needs them. > > TIA Vicente Romero has updated the pull request incrementally with one additional commit since the last revision: fixing files missing new line at the e

Re: RFR: 8282508: Updating ASM to 9.2 for JDK 19

2022-03-28 Thread Vicente Romero
On Mon, 28 Mar 2022 17:08:12 GMT, Lance Andersen wrote: > With this fix, I believe > [JDK-8282446](https://bugs.openjdk.java.net/browse/JDK-8282446) should also > be addressed. Thanks for mentioning this. I have uploaded another commit [41ae618e3a5ce696e3400a8654d98801226d7c1b](https://github

Re: RFR: 8282508: Updating ASM to 9.2 for JDK 19 [v2]

2022-03-28 Thread Vicente Romero
mons.RemappingMethodAdapter > jdk.internal.org.objectweb.asm.commons.RemappingAnnotationAdapter > > This is because package: `jdk.jfr.internal.instrument` needs them. > > TIA Vicente Romero has updated the pull request incrementally with one additional commit since the last revision: addressing review commen

Re: RFR: 8282508: Updating ASM to 9.2 for JDK 19

2022-03-28 Thread Vicente Romero
On Mon, 28 Mar 2022 17:50:25 GMT, Rémi Forax wrote: > I suppose that you are raising commons.RemappingMethodAdapter and > commons.RemappingAnnotationAdapter from the dead because you want to fix the > code in jdk.jfr.internal.instrument later ? correct, I would prefer the team maintaining jdk.

RFR: 8282508: Updating ASM to 9.2 for JDK 19

2022-03-28 Thread Vicente Romero
Please review this PR which is updating the ASM included in the JDK to ASM 9.2. This version should be included in JDK19. TIA - Commit messages: - additional adaptations after the automatic conversion - 8282508: Updating ASM to 9.2 for JDK 19 Changes: https://git.openjdk.java.net

Re: RFR: JDK-8283075: Bad IllegalArgumentException message when calling ClassDesc.arrayType(int) which results in a rank > 255 [v2]

2022-03-14 Thread Vicente Romero
On Mon, 14 Mar 2022 21:27:29 GMT, Joe Darcy wrote: >> Improving the exception messages for out-of-supported-range array types. >> >> I'll update copyrights before pushing. > > Joe Darcy has updated the pull request incrementally with one additional > commit since the last revision: > > Respo

Re: RFR: JDK-8283075: Bad IllegalArgumentException message when calling ClassDesc.arrayType(int) which results in a rank > 255

2022-03-14 Thread Vicente Romero
On Mon, 14 Mar 2022 19:56:17 GMT, Joe Darcy wrote: > Improving the exception messages for out-of-supported-range array types. > > I'll update copyrights before pushing. src/java.base/share/classes/java/lang/constant/ClassDesc.java line 186: > 184: if (rank <= 0 || netRank > > Constant

Integrated: 8213905: reflection not working for type annotations applied to exception types in the inner class constructor

2022-01-25 Thread Vicente Romero
On Thu, 16 Dec 2021 17:44:04 GMT, Vicente Romero wrote: > Hi, > > Please review this change that is fixing a bug in reflection in particular in > `sun.reflect.annotation.TypeAnnotationParser::buildAnnotatedTypes` the > current code is assuming that for inner class constructors,

Re: RFR: 8213905: reflection not working for type annotations applied to exception types in the inner class constructor

2022-01-21 Thread Vicente Romero
On Fri, 21 Jan 2022 12:41:37 GMT, Jan Lahoda wrote: > To me, looks good. thanks - PR: https://git.openjdk.java.net/jdk/pull/6869

Re: RFR: 8213905: reflection not working for type annotations applied to exception types in the inner class constructor

2021-12-16 Thread Vicente Romero
On Thu, 16 Dec 2021 18:01:54 GMT, Joe Darcy wrote: > Hi Vicente. > > Please file a CSR for the behavioral change. sure, thanks - PR: https://git.openjdk.java.net/jdk/pull/6869

RFR: 8213905: reflection not working for type annotations applied to exception types in the inner class constructor

2021-12-16 Thread Vicente Romero
Hi, Please review this change that is fixing a bug in reflection in particular in `sun.reflect.annotation.TypeAnnotationParser::buildAnnotatedTypes` the current code is assuming that for inner class constructors it is always working on type annotations on parameters, but it is also invoked to e

Re: RFR: 8261847: performance of java.lang.Record::toString should be improved [v6]

2021-11-23 Thread Vicente Romero
On Tue, 23 Nov 2021 15:09:53 GMT, Claes Redestad wrote: > Thanks for adding the comment and fixing typos. sure, thanks to you - PR: https://git.openjdk.java.net/jdk/pull/6403

Re: RFR: 8261847: performace of java.lang.Record::toString should be improved [v6]

2021-11-23 Thread Vicente Romero
On Tue, 23 Nov 2021 15:05:47 GMT, Vicente Romero wrote: >> Please review this PR which aims to optimize the implementation of the >> `toString` method we provide for records. A benchmark comparing the >> implementation we are providing for records with lombok found out that

Re: RFR: 8261847: performace of java.lang.Record::toString should be improved [v6]

2021-11-23 Thread Vicente Romero
5.718 ± 4.633 ns/op > MyBenchmark.record_hash_code avgt4 4.628 ± 4.368 ns/op > MyBenchmark.record_to_string avgt4 26.791 ± 1.817 ns/op > <--- After > MyBenchmark.value_class_to_string avgt 4 35.473 ± 2.626 ns/op > MyBenchmark.value_

Re: RFR: 8261847: performace of java.lang.Record::toString should be improved [v5]

2021-11-22 Thread Vicente Romero
5.718 ± 4.633 ns/op > MyBenchmark.record_hash_code avgt4 4.628 ± 4.368 ns/op > MyBenchmark.record_to_string avgt4 26.791 ± 1.817 ns/op > <--- After > MyBenchmark.value_class_to_string avgt 4 35.473 ± 2.626 ns/op > MyBenchmark.value_

Re: RFR: 8261847: performace of java.lang.Record::toString should be improved [v4]

2021-11-22 Thread Vicente Romero
On Mon, 22 Nov 2021 15:53:53 GMT, Claes Redestad wrote: > FWIW I did a few experiments trying to move the chunking to `SCF`. Most made > things worse, but this is getting close: > https://github.com/openjdk/jdk/compare/master...cl4es:scf_split?expand=1 > > The threshold for when the JIT fails

Re: RFR: 8261847: performace of java.lang.Record::toString should be improved [v3]

2021-11-22 Thread Vicente Romero
On Sat, 20 Nov 2021 12:10:40 GMT, Jim Laskey wrote: >> @stuart-marks yes, a general purpose splitting logic moved into the >> `StringConcatFactory` would be able to get rid of the arbitrary 200 slot >> limit (we would hit a harder but less arbitrary limit at around 253 instead). >> >> @JimLask

Re: RFR: 8261847: performace of java.lang.Record::toString should be improved [v4]

2021-11-22 Thread Vicente Romero
On Sun, 21 Nov 2021 00:29:46 GMT, Vicente Romero wrote: >> Please review this PR which aims to optimize the implementation of the >> `toString` method we provide for records. A benchmark comparing the >> implementation we are providing for records with lombok found out that

Re: RFR: 8261847: performace of java.lang.Record::toString should be improved [v4]

2021-11-20 Thread Vicente Romero
5.718 ± 4.633 ns/op > MyBenchmark.record_hash_code avgt4 4.628 ± 4.368 ns/op > MyBenchmark.record_to_string avgt4 26.791 ± 1.817 ns/op > <--- After > MyBenchmark.value_class_to_string avgt 4 35.473 ± 2.626 ns/op > MyBenchmark.value_

Re: RFR: 8261847: performace of java.lang.Record::toString should be improved [v3]

2021-11-20 Thread Vicente Romero
On Fri, 19 Nov 2021 05:07:23 GMT, Vicente Romero wrote: >> Please review this PR which aims to optimize the implementation of the >> `toString` method we provide for records. A benchmark comparing the >> implementation we are providing for records with lombok found out that

Re: RFR: 8261847: performace of java.lang.Record::toString should be improved [v3]

2021-11-18 Thread Vicente Romero
On Fri, 19 Nov 2021 05:28:10 GMT, Guoxiong Li wrote: > FYI: this patch also seems to solve > [JDK-8265747](https://bugs.openjdk.java.net/browse/JDK-8265747). yep, although I prefer to keep [JDK-8265747](https://bugs.openjdk.java.net/browse/JDK-8265747) because it is also referring to the hash

Re: RFR: 8261847: performace of java.lang.Record::toString should be improved [v3]

2021-11-18 Thread Vicente Romero
On Fri, 19 Nov 2021 05:07:23 GMT, Vicente Romero wrote: >> Please review this PR which aims to optimize the implementation of the >> `toString` method we provide for records. A benchmark comparing the >> implementation we are providing for records with lombok found out that

Re: RFR: 8261847: performace of java.lang.Record::toString should be improved [v3]

2021-11-18 Thread Vicente Romero
5.718 ± 4.633 ns/op > MyBenchmark.record_hash_code avgt4 4.628 ± 4.368 ns/op > MyBenchmark.record_to_string avgt4 26.791 ± 1.817 ns/op > <--- After > MyBenchmark.value_class_to_string avgt 4 35.473 ± 2.626 ns/op > MyBenchmark.value_

Re: RFR: 8261847: performace of java.lang.Record::toString should be improved [v2]

2021-11-17 Thread Vicente Romero
On Wed, 17 Nov 2021 22:00:30 GMT, Vicente Romero wrote: >> Please review this PR which aims to optimize the implementation of the >> `toString` method we provide for records. A benchmark comparing the >> implementation we are providing for records with lombok found out that

Re: RFR: 8261847: performace of java.lang.Record::toString should be improved [v2]

2021-11-17 Thread Vicente Romero
5.718 ± 4.633 ns/op > MyBenchmark.record_hash_code avgt4 4.628 ± 4.368 ns/op > MyBenchmark.record_to_string avgt4 26.791 ± 1.817 ns/op > <--- After > MyBenchmark.value_class_to_string avgt 4 35.473 ± 2.626 ns/op > MyBenchmark.value_

Re: RFR: 8261847: performace of java.lang.Record::toString should be improved

2021-11-17 Thread Vicente Romero
On Tue, 16 Nov 2021 05:24:38 GMT, Vicente Romero wrote: > Please review this PR which aims to optimize the implementation of the > `toString` method we provide for records. A benchmark comparing the > implementation we are providing for records with lombok found out that lombok

RFR: 8261847: performace of java.lang.Record::toString should be improved

2021-11-15 Thread Vicente Romero
Please review this PR which aims to optimize the implementation of the `toString` method we provide for records. A benchmark comparing the implementation we are providing for records with lombok found out that lombok is much faster mainly because our implementation uses `String::format`. This f

Integrated: 8272347: ObjectMethods::bootstrap should specify NPE if any argument except lookup is null

2021-08-30 Thread Vicente Romero
On Mon, 23 Aug 2021 18:08:02 GMT, Vicente Romero wrote: > Please review this simple PR along with the associated CSR. The PR is > basically adding a line the the specification of method > `java.lang.runtime.ObjectMethods::bootstrap` stating under what conditions a > NPE wi

Re: RFR: 8272347: ObjectMethods::bootstrap should specify NPE if any argument except lookup is null [v4]

2021-08-30 Thread Vicente Romero
://bugs.openjdk.java.net/browse/JDK-8272852) Vicente Romero 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 five additional commits since the last re

Re: RFR: 8272347: ObjectMethods::bootstrap should specify NPE if any argument except lookup is null [v3]

2021-08-30 Thread Vicente Romero
On Mon, 30 Aug 2021 01:45:49 GMT, Mandy Chung wrote: >> Vicente Romero has updated the pull request incrementally with one >> additional commit since the last revision: >> >> clarifying that the names parameter is ignored in some cases > > src/java.base/s

Re: RFR: 8272347: ObjectMethods::bootstrap should specify NPE if any argument except lookup is null [v3]

2021-08-25 Thread Vicente Romero
://bugs.openjdk.java.net/browse/JDK-8272852) Vicente Romero has updated the pull request incrementally with one additional commit since the last revision: clarifying that the names parameter is ignored in some cases - Changes: - all: https://git.openjdk.java.net/jdk/pull/5

Re: RFR: 8272347: ObjectMethods::bootstrap should specify NPE if any argument except lookup is null [v2]

2021-08-25 Thread Vicente Romero
On 8/25/21 4:45 PM, Mandy Chung wrote: On 8/25/21 12:08 PM, Vicente Romero wrote: On Wed, 25 Aug 2021 02:17:12 GMT, Mandy Chung wrote: Hi Mandy, I have changed the implementation of the method to explicitly require all arguments but lookup to be non-null as suggested by Brian. I have

Re: RFR: 8272347: ObjectMethods::bootstrap should specify NPE if any argument except lookup is null [v2]

2021-08-25 Thread Vicente Romero
On Wed, 25 Aug 2021 02:17:12 GMT, Mandy Chung wrote: >> Hi Mandy, I have changed the implementation of the method to explicitly >> require all arguments but lookup to be non-null as suggested by Brian. I >> have also covered, I think, all the missing test cases in test >> `ObjectMethodsTest`,

Re: RFR: 8272347: ObjectMethods::bootstrap should specify NPE if any argument except lookup is null [v2]

2021-08-23 Thread Vicente Romero
On Mon, 23 Aug 2021 23:13:58 GMT, Mandy Chung wrote: >> Vicente Romero has updated the pull request incrementally with one >> additional commit since the last revision: >> >> addressing review comments > > src/java.base/share/classes/java/lang/runtime/ObjectMe

Re: RFR: 8272347: ObjectMethods::bootstrap should specify NPE if any argument except lookup is null [v2]

2021-08-23 Thread Vicente Romero
://bugs.openjdk.java.net/browse/JDK-8272852) Vicente Romero has updated the pull request incrementally with one additional commit since the last revision: addressing review comments - Changes: - all: https://git.openjdk.java.net/jdk/pull/5226/files - new: https://git.openjdk.java

Re: RFR: 8272347: ObjectMethods::bootstrap should specify NPE if any argument except lookup is null

2021-08-23 Thread Vicente Romero
(type); requireNonNull(recordClass); requireNonNull(names); requireNonNull(getters); will do, thanks, Vicente On 8/23/2021 4:04 PM, Brian Goetz wrote: +1 On 8/23/2021 2:22 PM, Vicente Romero wrote: Please review this simple PR along with the associated CSR. The PR is basically adding a line the

RFR: 8272347: ObjectMethods::bootstrap should specify NPE if any argument except lookup is null

2021-08-23 Thread Vicente Romero
Please review this simple PR along with the associated CSR. The PR is basically adding a line the the specification of method `java.lang.runtime.ObjectMethods::bootstrap` stating under what conditions a NPE will be thrown. TIA link to the [CSR](https://bugs.openjdk.java.net/browse/JDK-8272852)

[jdk17] Integrated: 8270025: DynamicCallSiteDesc::withArgs doesn't throw NPE

2021-07-13 Thread Vicente Romero
On Sat, 10 Jul 2021 19:03:36 GMT, Vicente Romero wrote: > Please review this PR that is fixing a mismatch between the implementation > for method `java.lang.constant.DynamicCallSiteDesc::withArgs` and its > implementation. I made a mistake while working on a recent CSR > [JDK-82

Re: [jdk17] RFR: 8270025: DynamicCallSiteDesc::withArgs doesn't throw NPE [v3]

2021-07-13 Thread Vicente Romero
On Mon, 12 Jul 2021 18:06:30 GMT, Vicente Romero wrote: >> Please review this PR that is fixing a mismatch between the implementation >> for method `java.lang.constant.DynamicCallSiteDesc::withArgs` and its >> implementation. I made a mistake while working on a recent CS

Re: [jdk17] RFR: 8270025: DynamicCallSiteDesc::withArgs doesn't throw NPE [v3]

2021-07-12 Thread Vicente Romero
nvoking. So it didn't make sense for the API of one method > to be more restrictive than the other. Please review also the accompanying > CSR. > > Thanks, > Vicente Vicente Romero has updated the pull request incrementally with one additional commit since the last revis

Re: [jdk17] RFR: 8270025: DynamicCallSiteDesc::withArgs doesn't throw NPE [v2]

2021-07-12 Thread Vicente Romero
nvoking. So it didn't make sense for the API of one method > to be more restrictive than the other. Please review also the accompanying > CSR. > > Thanks, > Vicente Vicente Romero has updated the pull request incrementally with one additional commit since the last revisio

Re: [jdk17] RFR: 8270025: DynamicCallSiteDesc::withArgs doesn't throw NPE

2021-07-12 Thread Vicente Romero
On Mon, 12 Jul 2021 17:11:01 GMT, Mandy Chung wrote: >> Please review this PR that is fixing a mismatch between the implementation >> for method `java.lang.constant.DynamicCallSiteDesc::withArgs` and its >> implementation. I made a mistake while working on a recent CSR >> [JDK-8224985](https:/

[jdk17] RFR: 8270025: DynamicCallSiteDesc::withArgs doesn't throw NPE

2021-07-10 Thread Vicente Romero
Please review this PR that is fixing a mismatch between the implementation for method `java.lang.constant.DynamicCallSiteDesc::withArgs` and its implementation. I made a mistake while working on a recent CSR [JDK-8224985](https://bugs.openjdk.java.net/browse/JDK-8224985) and fixed the API but m

Integrated: 8266407: remove jdk.internal.javac.PreviewFeature.Feature.SEALED_CLASSES

2021-07-06 Thread Vicente Romero
On Wed, 23 Jun 2021 20:57:24 GMT, Vicente Romero wrote: > Enum constant: jdk.internal.javac.PreviewFeature.Feature.SEALED_CLASSES was > not removed when Sealed Classes were made final because the build was failing > without it. Now that the feature is final we should be able to safel

RFR: 8266407: remove jdk.internal.javac.PreviewFeature.Feature.SEALED_CLASSES

2021-06-23 Thread Vicente Romero
Enum constant: jdk.internal.javac.PreviewFeature.Feature.SEALED_CLASSES was not removed when Sealed Classes were made final because the build was failing without it. Now that the feature is final we should be able to safely removed it. This is the intention of this patch. TIA, Vicente

[jdk17] Integrated: 8267421: j.l.constant.DirectMethodHandleDesc.Kind.valueOf(int) implementation doesn't conform to the spec regarding REF_invokeInterface handling

2021-06-21 Thread Vicente Romero
On Fri, 11 Jun 2021 15:01:20 GMT, Vicente Romero wrote: > This PR is a copy of [PR-4416](https://github.com/openjdk/jdk/pull/4416) > which was intended to openjdk/jdk. > > Please review this PR which is syncing the implementation of > `DirectMethodHandleDesc.Kind.va

Re: [jdk17] RFR: 8267421: j.l.constant.DirectMethodHandleDesc.Kind.valueOf(int) implementation doesn't conform to the spec regarding REF_invokeInterface handling [v2]

2021-06-21 Thread Vicente Romero
On Tue, 22 Jun 2021 00:45:36 GMT, Mandy Chung wrote: >> Vicente Romero has updated the pull request incrementally with one >> additional commit since the last revision: >> >> updating after review comments > > test/jdk/java/lang/constant/MethodHandleDes

Re: [jdk17] RFR: 8267421: j.l.constant.DirectMethodHandleDesc.Kind.valueOf(int) implementation doesn't conform to the spec regarding REF_invokeInterface handling [v4]

2021-06-21 Thread Vicente Romero
is invoked with false > regardless of the value of the refKind parameter > > TIA Vicente Romero has updated the pull request incrementally with one additional commit since the last revision: addressing review comments - Changes: - all: https://git.openjdk

Re: [jdk17] RFR: 8267421: j.l.constant.DirectMethodHandleDesc.Kind.valueOf(int) implementation doesn't conform to the spec regarding REF_invokeInterface handling [v3]

2021-06-21 Thread Vicente Romero
is invoked with false > regardless of the value of the refKind parameter > > TIA Vicente Romero 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 conta

Re: [jdk17] RFR: 8267421: j.l.constant.DirectMethodHandleDesc.Kind.valueOf(int) implementation doesn't conform to the spec regarding REF_invokeInterface handling [v2]

2021-06-11 Thread Vicente Romero
On Fri, 11 Jun 2021 18:17:10 GMT, Vicente Romero wrote: >> This PR is a copy of [PR-4416](https://github.com/openjdk/jdk/pull/4416) >> which was intended to openjdk/jdk. >> >> Please review this PR which is syncing the implementation of >> `DirectMethodHan

Re: [jdk17] RFR: 8267421: j.l.constant.DirectMethodHandleDesc.Kind.valueOf(int) implementation doesn't conform to the spec regarding REF_invokeInterface handling [v2]

2021-06-11 Thread Vicente Romero
is invoked with false > regardless of the value of the refKind parameter > > TIA Vicente Romero has updated the pull request incrementally with one additional commit since the last revision: updating after review comments - Changes: - all: https://git.openjdk

Re: [jdk17] RFR: 8267421: j.l.constant.DirectMethodHandleDesc.Kind.valueOf(int) implementation doesn't conform to the spec regarding REF_invokeInterface handling

2021-06-11 Thread Vicente Romero
On Fri, 11 Jun 2021 15:15:12 GMT, Vicente Romero wrote: >> This PR is a copy of [PR-4416](https://github.com/openjdk/jdk/pull/4416) >> which was intended to openjdk/jdk. >> >> Please review this PR which is syncing the implementation of >> `DirectMethodHan

Re: [jdk17] RFR: 8267421: j.l.constant.DirectMethodHandleDesc.Kind.valueOf(int) implementation doesn't conform to the spec regarding REF_invokeInterface handling

2021-06-11 Thread Vicente Romero
On Fri, 11 Jun 2021 15:01:20 GMT, Vicente Romero wrote: > This PR is a copy of [PR-4416](https://github.com/openjdk/jdk/pull/4416) > which was intended to openjdk/jdk. > > Please review this PR which is syncing the implementation of > `DirectMethodHandleDesc.Kind.va

Re: RFR: 8267421: j.l.constant.DirectMethodHandleDesc.Kind.valueOf(int) implementation doesn't conform to the spec regarding REF_invokeInterface handling [v2]

2021-06-11 Thread Vicente Romero
On Thu, 10 Jun 2021 23:26:21 GMT, Vicente Romero wrote: >> Please review this PR which is just syncing the implementation of >> DirectMethodHandleDesc.Kind.valueOf(int) with its spec. My reading of the >> method's spec is that if the value of the

Withdrawn: 8267421: j.l.constant.DirectMethodHandleDesc.Kind.valueOf(int) implementation doesn't conform to the spec regarding REF_invokeInterface handling

2021-06-11 Thread Vicente Romero
On Tue, 8 Jun 2021 16:46:36 GMT, Vicente Romero wrote: > Please review this PR which is just syncing the implementation of > DirectMethodHandleDesc.Kind.valueOf(int) with its spec. My reading of the > method's spec is that if the value of the `refKind

[jdk17] RFR: 8267421: j.l.constant.DirectMethodHandleDesc.Kind.valueOf(int) implementation doesn't conform to the spec regarding REF_invokeInterface handling

2021-06-11 Thread Vicente Romero
This PR is a copy of [PR-4416](https://github.com/openjdk/jdk/pull/4416) which was intended to openjdk/jdk. Please review this PR which is syncing the implementation of `DirectMethodHandleDesc.Kind.valueOf(int)` and `DirectMethodHandleDesc.Kind.valueOf(int, boolean)` with its spec. My reading

Re: RFR: 8267421: j.l.constant.DirectMethodHandleDesc.Kind.valueOf(int) implementation doesn't conform to the spec regarding REF_invokeInterface handling [v2]

2021-06-10 Thread Vicente Romero
On Wed, 9 Jun 2021 17:22:30 GMT, Mandy Chung wrote: >> Vicente Romero has updated the pull request incrementally with one >> additional commit since the last revision: >> >> addressing review changes > > test/jdk/java/lang/constant/MethodHandleDescTest.java l

Re: RFR: 8267421: j.l.constant.DirectMethodHandleDesc.Kind.valueOf(int) implementation doesn't conform to the spec regarding REF_invokeInterface handling [v2]

2021-06-10 Thread Vicente Romero
dleDesc.Kind.valueOf(int, boolean) should be called with a > value of `true` for its second argument, currently it is invoked with `false` > regardless of the value of the `refKind` parameter > > TIA Vicente Romero has updated the pull request incrementally with one additional

Re: RFR: 8267421: j.l.constant.DirectMethodHandleDesc.Kind.valueOf(int) implementation doesn't conform to the spec regarding REF_invokeInterface handling

2021-06-10 Thread Vicente Romero
On Wed, 9 Jun 2021 17:22:30 GMT, Mandy Chung wrote: > Looks like the test does not verify the cases specified by `valueOf(int > refKind, boolean isInterface)`. > i.e. For most values of refKind, there is an exact match regardless of the > value of isInterface except `REF_invokeStatic` and `REF_

RFR: 8267421: j.l.constant.DirectMethodHandleDesc.Kind.valueOf(int) implementation doesn't conform to the spec regarding REF_invokeInterface handling

2021-06-08 Thread Vicente Romero
Please review this PR which is just syncing the implementation of DirectMethodHandleDesc.Kind.valueOf(int) with its spec. My reading of the method's spec is that if the value of the `refKind` parameter is: MethodHandleInfo.REF_invokeInterface, then DirectMethodHandleDesc.Kind.valueOf(int, boole

Re: RFR: 8262891: Compiler implementation for Pattern Matching for switch (Preview) [v12]

2021-06-04 Thread Vicente Romero
On Fri, 4 Jun 2021 09:46:31 GMT, Jan Lahoda wrote: >> This is a preview of a patch implementing JEP 406: Pattern Matching for >> switch (Preview): >> https://bugs.openjdk.java.net/browse/JDK-8213076 >> >> The current draft of the specification is here: >> http://cr.openjdk.java.net/~gbierman/je

Integrated: 8224158: assertion related to NPE at DynamicCallSiteDesc::withArgs should be reworded

2021-05-24 Thread Vicente Romero
On Mon, 17 May 2021 16:53:24 GMT, Vicente Romero wrote: > This is a very small patch that is just rewording the spec for > DynamicCallSiteDesc::withArgs. Basically adding that NPE can also be thrown > if the content of the argument is `null` > > TIA for the review This pull

Re: RFR: 8224158: assertion related to NPE at DynamicCallSiteDesc::withArgs should be reworded

2021-05-21 Thread Vicente Romero
On Mon, 17 May 2021 16:53:24 GMT, Vicente Romero wrote: > This is a very small patch that is just rewording the spec for > DynamicCallSiteDesc::withArgs. Basically adding that NPE can also be thrown > if the content of the argument is `null` > > TIA for the review any rev

Re: RFR: 8265130: Make ConstantDesc class hierarchy sealed [v4]

2021-05-21 Thread Vicente Romero
On Fri, 21 May 2021 08:53:51 GMT, Gavin Bierman wrote: >> Hi all, >> >> Please review this patch to make the ConstantDesc hierarchy `sealed`, as was >> promised in its Javadoc, now that sealed classes are finalising in JDK 17. >> >> Thanks, >> Gavin > > Gavin Bierman has updated the pull requ

Integrated: 8260517: implement Sealed Classes as a standard feature in Java

2021-05-20 Thread Vicente Romero
On Fri, 16 Apr 2021 01:08:57 GMT, Vicente Romero wrote: > Please review this PR that intents to make sealed classes a final feature in > Java. This PR contains compiler and VM changes. In line with similar PRs, > which has made preview features final, this one is mostly removin

Re: RFR: 8260517: implement Sealed Classes as a standard feature in Java [v8]

2021-05-19 Thread Vicente Romero
ases. Please also review the > related [CSR](https://bugs.openjdk.java.net/browse/JDK-8265090) > > Thanks > > note: this PR is related to > [PR-3528](https://github.com/openjdk/jdk/pull/3528) and must be integrated > after it. Vicente Romero has updated the pull reques

Re: RFR: 8224158: assertion related to NPE at DynamicCallSiteDesc::withArgs should be reworded

2021-05-18 Thread Vicente Romero
On Mon, 17 May 2021 16:53:24 GMT, Vicente Romero wrote: > This is a very small patch that is just rewording the spec for > DynamicCallSiteDesc::withArgs. Basically adding that NPE can also be thrown > if the content of the argument is `null` > > TIA for the review ping --

RFR: 8224158: assertion related to NPE at DynamicCallSiteDesc::withArgs should be reworded

2021-05-17 Thread Vicente Romero
This is a very small patch that is just rewording the spec for DynamicCallSiteDesc::withArgs. Basically adding that NPE can also be thrown if the content of the argument is `null` TIA for the review - Commit messages: - 8224158: assertion related to NPE at DynamicCallSiteDesc::wit

Re: RFR: 8264561: javap get NegativeArraySizeException on bad instruction

2021-05-17 Thread Vicente Romero
On Mon, 17 May 2021 13:11:56 GMT, Adam Sotona wrote: > Actual javap implementation reacts on corrupted TABLESWITCH or LOOKUPSWITCH > bytecode instructions resulting to negative array allocation with > NegativeArraySizeException, which is reported to user with stack trace and as > serious inter

Re: RFR: 8260517: implement Sealed Classes as a standard feature in Java [v7]

2021-05-06 Thread Vicente Romero
ases. Please also review the > related [CSR](https://bugs.openjdk.java.net/browse/JDK-8265090) > > Thanks > > note: this PR is related to > [PR-3528](https://github.com/openjdk/jdk/pull/3528) and must be integrated > after it. Vicente Romero has updated the pull reques

Re: RFR: 8260517: implement Sealed Classes as a standard feature in Java [v6]

2021-05-01 Thread Vicente Romero
ases. Please also review the > related [CSR](https://bugs.openjdk.java.net/browse/JDK-8265090) > > Thanks > > note: this PR is related to > [PR-3528](https://github.com/openjdk/jdk/pull/3528) and must be integrated > after it. Vicente Romero has updated the pull request

Re: RFR: 8260517: implement Sealed Classes as a standard feature in Java [v5]

2021-04-30 Thread Vicente Romero
ases. Please also review the > related [CSR](https://bugs.openjdk.java.net/browse/JDK-8265090) > > Thanks > > note: this PR is related to > [PR-3528](https://github.com/openjdk/jdk/pull/3528) and must be integrated > after it. Vicente Romero has updated the pull reques

Re: RFR: 8260517: implement Sealed Classes as a standard feature in Java [v4]

2021-04-22 Thread Vicente Romero
ases. Please also review the > related [CSR](https://bugs.openjdk.java.net/browse/JDK-8265090) > > Thanks > > note: this PR is related to > [PR-3528](https://github.com/openjdk/jdk/pull/3528) and must be integrated > after it. Vicente Romero has updated the pull reques

Re: RFR: 8260517: implement Sealed Classes as a standard feature in Java [v3]

2021-04-21 Thread Vicente Romero
On Wed, 21 Apr 2021 14:42:39 GMT, Maurizio Cimadamore wrote: > Compiler changes look good (I have not checked SymbolGenerator). > > Why were some tests removed? thanks for the review. The removed tests were already covered in langtools regression tests, so I only removed duplicated tests ---

Re: RFR: 8260517: implement Sealed Classes as a standard feature in Java [v3]

2021-04-15 Thread Vicente Romero
ases. Please also review the > related [CSR](https://bugs.openjdk.java.net/browse/JDK-8265090) > > Thanks > > note: this PR is related to > [PR-3528](https://github.com/openjdk/jdk/pull/3528) and must be integrated > after it. Vicente Romero has updated the pull request

Re: RFR: 8260517: implement Sealed Classes as a standard feature in Java [v2]

2021-04-15 Thread Vicente Romero
On Fri, 16 Apr 2021 02:10:05 GMT, David Holmes wrote: > Hi Vicente, > > Hotspot and hotspot tests all look fine. One query: why was this test removed? > > test/hotspot/jtreg/runtime/sealedClasses/AbstractSealedTest.java > > is that functionality tested elsewhere? (The other deleted test seemed

Re: RFR: 8260517: implement Sealed Classes as a standard feature in Java [v2]

2021-04-15 Thread Vicente Romero
ases. Please also review the > related [CSR](https://bugs.openjdk.java.net/browse/JDK-8265090) > > Thanks > > note: this PR is related to > [PR-3528](https://github.com/openjdk/jdk/pull/3528) and must be integrated > after it. Vicente Romero has updated the pull request

RFR: 8260517: implement Sealed Classes as a standard feature

2021-04-15 Thread Vicente Romero
Please review this PR that intents to make sealed classes a final feature in Java. This PR contains compiler and VM changes. In line with similar PRs, which has made preview features final, this one is mostly removing preview related comments from APIs plus options in test cases. Thanks --

Re: RFR: 8260403: javap should be more robust in the face of invalid class files

2021-02-25 Thread Vicente Romero
On Thu, 25 Feb 2021 13:01:30 GMT, Adam Sotona wrote: > Please review javap fix to handle java.lang.IllegalStateException for classes > with invalid Signature attribute. > New test (T8260403) parsing class with invalid Signature attribute (as > described in the bug) is included. > Fix wraps java

RFR: 8260959: remove RECORDS from PreviewFeature.Feature enum

2021-02-02 Thread Vicente Romero
Please review this simple fix that is removing the RECORDS enum constant from the PreviewFeature.Feature enum, now that RECORDS are final in 16 this constant can be safely removed. Thanks, Vicente - Commit messages: - 8260959: remove RECORDS from PreviewFeature.Feature enum Chang

Re: [jdk16] RFR: 8256693: getAnnotatedReceiverType parameterizes types too eagerly [v2]

2020-12-18 Thread Vicente Romero
On Thu, 17 Dec 2020 10:07:13 GMT, Joel Borggrén-Franck wrote: >> The fix for JDK-8256693 too often produces a ParameterizedType as the result >> of getAnnotatedReceiverType().getType() . A ParameterizedType is necessary >> only when this type or any of its transitive owner types has type >> p

Re: [jdk16] RFR: 8256693: getAnnotatedReceiverType parameterizes types too eagerly

2020-12-16 Thread Vicente Romero
On Wed, 16 Dec 2020 19:57:32 GMT, Joel Borggrén-Franck wrote: > Yes, not great, but at least it isn't brittle when running the test, only > when changing it. I'd like to take a separate pass over the tests for 17 if > possible. what about declaring a static final field for that value instead

Re: [jdk16] RFR: 8256693: getAnnotatedReceiverType parameterizes types too eagerly

2020-12-16 Thread Vicente Romero
On Wed, 16 Dec 2020 09:41:47 GMT, Joel Borggrén-Franck wrote: > The fix for JDK-8256693 too often produces a ParameterizedType as the result > of getAnnotatedReceiverType().getType() . A ParameterizedType is necessary > only when this type or any of its transitive owner types has type paramete

  1   2   3   4   >