Re: RFR: JDK-8210031: implementation for JVM Constants API

2018-12-06 Thread Vicente Romero
Hi Rogers, Thanks for your comments, I have uploaded another webrev [1], some comments below: [1] http://cr.openjdk.java.net/~vromero/8210031/webrev.14/ On 12/6/18 11:07 AM, Roger Riggs wrote: Hi Vicente, On 12/05/2018 09:13 PM, Vicente Romero wrote: Hi Roger, Thanks for your comments,

Re: RFR: JDK-8210031: implementation for JVM Constants API

2018-12-06 Thread Roger Riggs
Hi Vicente, On 12/05/2018 09:13 PM, Vicente Romero wrote: Hi Roger, Thanks for your comments, see my comments below. I have published another iteration at [1] Thanks, Vicente [1] http://cr.openjdk.java.net/~vromero/8210031/webrev.12/ - bootStrapArgs() - the return is always non-null,

Re: RFR: JDK-8210031: implementation for JVM Constants API

2018-12-05 Thread Vicente Romero
Hi Mandy, On 12/5/18 1:12 PM, Mandy Chung wrote: On 12/3/18 11:12 AM, Vicente Romero wrote: Hi all, Can I have the final nod to the JVM constants API, there have been some changes since the last review iteration. Thanks to the internal and external developers that have taken the time to

Re: RFR: JDK-8210031: implementation for JVM Constants API

2018-12-05 Thread Roger Riggs
Hi, Editorial comments java.lang.contant package javadoc:  - In the Nominal Descriptors  2nd paragraph:  "of of" -> "of" -  DirectMethodHandleDescImpl - Its unusual to see a Class named *Impl in    a developer facing API.  And if its intended, it should be linked. - "These classes provides"

Re: RFR: JDK-8210031: implementation for JVM Constants API

2018-12-05 Thread Mandy Chung
On 12/3/18 11:12 AM, Vicente Romero wrote: Hi all, Can I have the final nod to the JVM constants API, there have been some changes since the last review iteration. Thanks to the internal and external developers that have taken the time to provide feedback so far. The links to the last

Re: RFR: JDK-8210031: implementation for JVM Constants API

2018-12-03 Thread Vicente Romero
Hi all, Can I have the final nod to the JVM constants API, there have been some changes since the last review iteration. Thanks to the internal and external developers that have taken the time to provide feedback so far. The links to the last versions are: webrev:

Re: RFR: JDK-8210031: implementation for JVM Constants API

2018-10-19 Thread Vicente Romero
Hi all, Thanks Mandy for the review. I have uploaded a new iteration [1]. Vicente [1] http://cr.openjdk.java.net/~vromero/8210031/webrev.02/ On 10/18/18 9:55 PM, Mandy Chung wrote: On 10/15/18 11:12 AM, Vicente Romero wrote: [1] http://cr.openjdk.java.net/~vromero/8210031/webrev.01 I

Re: RFR: JDK-8210031: implementation for JVM Constants API

2018-10-18 Thread Mandy Chung
On 10/15/18 11:12 AM, Vicente Romero wrote: [1] http://cr.openjdk.java.net/~vromero/8210031/webrev.01 I reviewed java.lang.invoke change in details.  I have skimmed through the new classes. I will look at the new tests next. @since 12 is missing in the new APIs VarHandle.java 1887

Re: RFR: JDK-8210031: implementation for JVM Constants API

2018-10-18 Thread Vicente Romero
Hi all, Please review also the CSR at [1]. Thanks, Vicente [1] https://bugs.openjdk.java.net/browse/JDK-8202031 On 10/15/18 2:12 PM, Vicente Romero wrote: Hi all, sorry for the repeated number of mails on this issue. I have added a direct link to the patch the right link to the webrev is

Re: RFR: JDK-8210031: implementation for JVM Constants API

2018-10-15 Thread Vicente Romero
Hi all, sorry for the repeated number of mails on this issue. I have added a direct link to the patch the right link to the webrev is [1] there is a previous version at [2] if you want to see the differences with the last version. Basically we have dropped the `implements Constable` for the

Re: RFR: JDK-8210031: implementation for JVM Constants API

2018-10-15 Thread Vicente Romero
adding core-libs in the loop On 10/10/2018 12:30 PM, Vicente Romero wrote: Hi all, I have updated the webrev [1], this version removes the `implements Constable` from the symbolic descriptor classes. Feedback is mostly appreciated, Thanks, Vicente [1]