Re: [jdk17] RFR: 8270025: DynamicCallSiteDesc::withArgs doesn't throw NPE [v3]
> 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]
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]
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