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

2021-07-12 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 mistakenly thought that the implementation was in sync with the spec. 
> This is why this change is also including a unit test of the API for 
> `java.lang.constant.DynamicCallSiteDesc` modulo method `resolveCallSiteDesc` 
> which is covered in test `IndyDescTest` in the same test suite. Also this 
> change needs a CSR as while fixing the implementation of method `::withArgs` 
> I realized that the API of the varargs overloaded version of method `::of` 
> needed some rewording too as it is invoking the same private constructor 
> `::withArgs` is invoking. 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 revision:

  review comments

-

Changes:
  - all: https://git.openjdk.java.net/jdk17/pull/242/files
  - new: https://git.openjdk.java.net/jdk17/pull/242/files/f7fddc99..6702a42f

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk17&pr=242&range=02
 - incr: https://webrevs.openjdk.java.net/?repo=jdk17&pr=242&range=01-02

  Stats: 13 lines in 1 file changed: 0 ins; 3 del; 10 mod
  Patch: https://git.openjdk.java.net/jdk17/pull/242.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk17 pull/242/head:pull/242

PR: https://git.openjdk.java.net/jdk17/pull/242


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

2021-07-12 Thread Mandy Chung
On Mon, 12 Jul 2021 18:02:54 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-8224985](https://bugs.openjdk.java.net/browse/JDK-8224985) and fixed 
>> the API but mistakenly thought that the implementation was in sync with the 
>> spec. This is why this change is also including a unit test of the API for 
>> `java.lang.constant.DynamicCallSiteDesc` modulo method `resolveCallSiteDesc` 
>> which is covered in test `IndyDescTest` in the same test suite. Also this 
>> change needs a CSR as while fixing the implementation of method `::withArgs` 
>> I realized that the API of the varargs overloaded version of method `::of` 
>> needed some rewording too as it is invoking the same private constructor 
>> `::withArgs` is invoking. 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 revision:
> 
>   review comments

Marked as reviewed by mchung (Reviewer).

-

PR: https://git.openjdk.java.net/jdk17/pull/242


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 CSR 
>> [JDK-8224985](https://bugs.openjdk.java.net/browse/JDK-8224985) and fixed 
>> the API but mistakenly thought that the implementation was in sync with the 
>> spec. This is why this change is also including a unit test of the API for 
>> `java.lang.constant.DynamicCallSiteDesc` modulo method `resolveCallSiteDesc` 
>> which is covered in test `IndyDescTest` in the same test suite. Also this 
>> change needs a CSR as while fixing the implementation of method `::withArgs` 
>> I realized that the API of the varargs overloaded version of method `::of` 
>> needed some rewording too as it is invoking the same private constructor 
>> `::withArgs` is invoking. 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 revision:
> 
>   review comments

thanks for the reviews!

-

PR: https://git.openjdk.java.net/jdk17/pull/242