Re: RFR:DK-8148457(Remove jdk.nashorn.tools.FXShell class and associated build.xml cruft)

2016-06-03 Thread Michael Haupt
Hi Srini, lower-case thumbs up, and I'll be happy to sponsor the push. Best, Michael > Am 03.06.2016 um 14:39 schrieb Srinivas Dama : > > Hi, > > Please review: > http://cr.openjdk.java.net/~sdama/8148457/webrev.00/ > Bug:

Re: RFR:DK-8148457(Remove jdk.nashorn.tools.FXShell class and associated build.xml cruft)

2016-06-03 Thread Sundararajan Athijegannathan
+1 On 6/3/2016 6:09 PM, Srinivas Dama wrote: > Hi, > > Please review: > http://cr.openjdk.java.net/~sdama/8148457/webrev.00/ > Bug: https://bugs.openjdk.java.net/browse/JDK-8148457 > > I have verified all debug logs generated out of make targets for jdk9 forest > build and > nashorn ant

Re: RFR:DK-8148457(Remove jdk.nashorn.tools.FXShell class and associated build.xml cruft)

2016-06-03 Thread Jim Laskey (Oracle)
+1 > On Jun 3, 2016, at 9:39 AM, Srinivas Dama wrote: > > Hi, > > Please review: > http://cr.openjdk.java.net/~sdama/8148457/webrev.00/ > Bug: https://bugs.openjdk.java.net/browse/JDK-8148457 > > I have verified all debug logs generated out of make targets for jdk9

Re: RFR 8158467: AccessControlException is thrown on public Java class access if "script app loader" is set to null

2016-06-02 Thread Hannes Wallnoefer
+1 Am 2016-06-02 um 07:10 schrieb Sundararajan Athijegannathan: Please review http://cr.openjdk.java.net/~sundar/8158467/webrev.00/ for https://bugs.openjdk.java.net/browse/JDK-8158467 Thanks, -Sundar

Re: RFR 8158467: AccessControlException is thrown on public Java class access if "script app loader" is set to null

2016-06-02 Thread Michael Haupt
Hi Sundar, lower-case thumbs up! Best, Michael > Am 02.06.2016 um 07:10 schrieb Sundararajan Athijegannathan > : > > Please review http://cr.openjdk.java.net/~sundar/8158467/webrev.00/ for > https://bugs.openjdk.java.net/browse/JDK-8158467 > >

Re: Review request for JDK-8156714: Parsing issue with automatic semicolon insertion

2016-06-01 Thread Silas Baronda
Sorry for the late reply, but is this [1] going to be backported to 8u? This would be extremely helpful for webpack generated files used for Server Side Rendering. [1]: https://bugs.openjdk.java.net/browse/JDK-8156714 —Silas On Fri, May 13, 2016 at 12:12 PM, Sundararajan Athijegannathan <

Re: Review request for JDK-8156714: Parsing issue with automatic semicolon insertion

2016-06-01 Thread Silas Baronda
Sorry for the noise. Look forward to this fix! Thanks :) —Silas On Wed, Jun 1, 2016 at 9:34 AM, Sundararajan Athijegannathan < sundararajan.athijegannat...@oracle.com> wrote: > It has been backported to 8u already! > > https://bugs.openjdk.java.net/browse/JDK-8157926 > >

Re: Review request for JDK-8156714: Parsing issue with automatic semicolon insertion

2016-06-01 Thread Sundararajan Athijegannathan
It has been backported to 8u already! https://bugs.openjdk.java.net/browse/JDK-8157926 http://hg.openjdk.java.net/jdk8u/jdk8u-dev/nashorn/rev/133a3c6c906e -Sundar On 6/1/2016 7:02 PM, Silas Baronda wrote: > Sorry for the late reply, but is this [1] going to be backported to > 8u? This would

Re: [8u] approval request for 8158338: Nashorn's ScriptLoader split delegation has to be adjusted

2016-06-01 Thread Seán Coffey
Approved for jdk8u-dev. Regards, Sean. On 01/06/16 14:02, Marcus Lagergren wrote: +1 On 01 Jun 2016, at 14:13, Sundararajan Athijegannathan wrote: Please approve. Bug: https://bugs.openjdk.java.net/browse/JDK-8158338 jdk9 review thread:

Re: [8u] approval request for 8158338: Nashorn's ScriptLoader split delegation has to be adjusted

2016-06-01 Thread Marcus Lagergren
+1 > On 01 Jun 2016, at 14:13, Sundararajan Athijegannathan > wrote: > > Please approve. > > Bug: https://bugs.openjdk.java.net/browse/JDK-8158338 > > jdk9 review thread: > http://mail.openjdk.java.net/pipermail/nashorn-dev/2016-June/006259.html > >

Re: RFR(S) 8158338: Nashorn's ScriptLoader split delegation has to adjusted

2016-06-01 Thread Jim Laskey (Oracle)
+1 > On Jun 1, 2016, at 6:14 AM, Sundararajan Athijegannathan > wrote: > > I had to update webrev to handle the case of appLoader being null in > ScriptLoader. I had used Class.forName(String, boolean, ClassLoader). > But that methods introduces a

Re: RFR(S) 8158338: Nashorn's ScriptLoader split delegation has to adjusted

2016-06-01 Thread Sundararajan Athijegannathan
Just clean code so that Module Layer parent/child mimics class loader delegation chain. Thanks for the review! -Sundar On 6/1/2016 3:03 PM, Hannes Wallnöfer wrote: > This looks good to me. > > I’m wondering: what would be the consequences of class loader inheritance not > reflecting module

Re: RFR(S) 8158338: Nashorn's ScriptLoader split delegation has to adjusted

2016-06-01 Thread Hannes Wallnöfer
This looks good to me. I’m wondering: what would be the consequences of class loader inheritance not reflecting module layer inheritance? Hannes > Am 01.06.2016 um 11:14 schrieb Sundararajan Athijegannathan > : > > I had to update webrev to handle

Re: RFR(S) 8158338: Nashorn's ScriptLoader split delegation has to adjusted

2016-06-01 Thread Sundararajan Athijegannathan
I had to update webrev to handle the case of appLoader being null in ScriptLoader. I had used Class.forName(String, boolean, ClassLoader). But that methods introduces a security check when caller is not bootstrap (like nashorn) and ClassLoader is null! In any case, bootloader delegation has

Re: RFR(S) 8158338: Nashorn's ScriptLoader split delegation has to adjusted

2016-06-01 Thread Sundararajan Athijegannathan
Hi, Thanks for the review. We have an existing test that depends on "split" delegation implemented in the ScriptLoader. http://hg.openjdk.java.net/jdk9/dev/nashorn/file/7fb2bf00347b/test/script/basic/JDK-8024619.js The current code adjustment is about which loader is "parent" and which one

Re: RFR(S) 8158338: Nashorn's ScriptLoader split delegation has to adjusted

2016-06-01 Thread Marcus Lagergren
This looks good. Is it possible to test it? /M > On 01 Jun 2016, at 08:46, Sundararajan Athijegannathan > wrote: > > Please review http://cr.openjdk.java.net/~sundar/8158338/webrev.00/ for > https://bugs.openjdk.java.net/browse/JDK-8158338 > > Thanks,

Re: RFR 8158250: nashorn ant javadoc targets are broken

2016-06-01 Thread Marcus Lagergren
+1 > On 31 May 2016, at 17:00, Sundararajan Athijegannathan > wrote: > > Please review http://cr.openjdk.java.net/~sundar/8158250/webrev.00/ for > https://bugs.openjdk.java.net/browse/JDK-8158250 > > Thanks, > > -Sundar >

Re: "not an object"

2016-05-31 Thread Sundararajan Athijegannathan
Hi, Java objects are treated "as is" in nashorn. i.e., Nashorn uses wrapperless Java object access. So, Java objects are *not* ECMAScript objects. Dynamic property addition, prototype manipulation etc. won't work on Java objects. But, you can create a helper ECMAScript object whose properties

Re: RFR 8158250: nashorn ant javadoc targets are broken

2016-05-31 Thread Michael Haupt
Hi Sundar, lower-case thumbs up! Best, Michael > Am 31.05.2016 um 17:00 schrieb Sundararajan Athijegannathan > : > > Please review http://cr.openjdk.java.net/~sundar/8158250/webrev.00/ for > https://bugs.openjdk.java.net/browse/JDK-8158250 > >

Re: RFR 8158250: nashorn ant javadoc targets are broken

2016-05-31 Thread Hannes Wallnoefer
Looks good! Am 2016-05-31 um 17:00 schrieb Sundararajan Athijegannathan: Please review http://cr.openjdk.java.net/~sundar/8158250/webrev.00/ for https://bugs.openjdk.java.net/browse/JDK-8158250 Thanks, -Sundar

Re: RFR 8158131: Nashorn should not use jdk.internal.module.Modules API

2016-05-31 Thread Hannes Wallnoefer
+1 for the incremental changes in the last iteration. Hannes Am 2016-05-31 um 09:17 schrieb Sundararajan Athijegannathan: Sorry, I have updated nashorn webrev again: http://cr.openjdk.java.net/~sundar/8158131/nashorn/webrev.02/ Incremental changes: * Missed "nashornModule" name in

Re: RFR 8158131: Nashorn should not use jdk.internal.module.Modules API

2016-05-31 Thread Hannes Wallnoefer
Sorry for the slow review, I needed some explanation from Sundar about the difficulties of dynamic module creation, cross-layer read edges etc. With that knowledge +1 for this change. Hannes Am 2016-05-30 um 19:55 schrieb Alan Bateman: On 30/05/2016 11:43, Sundararajan Athijegannathan wrote:

Re: RFR 8158131: Nashorn should not use jdk.internal.module.Modules API

2016-05-30 Thread Alan Bateman
On 30/05/2016 11:43, Sundararajan Athijegannathan wrote: Updated nashorn webrev: http://cr.openjdk.java.net/~sundar/8158131/nashorn/webrev.01/ * Added @link in ModuleGraphManipulator.java's javadoc * Using Context.class.getModule() in all places where nashorn module is needed * Using all

Re: RFR 8158131: Nashorn should not use jdk.internal.module.Modules API

2016-05-30 Thread Sundararajan Athijegannathan
Updated nashorn webrev: http://cr.openjdk.java.net/~sundar/8158131/nashorn/webrev.01/ * Added @link in ModuleGraphManipulator.java's javadoc * Using Context.class.getModule() in all places where nashorn module is needed * Using all caps names for static final variables - nashornModule,

Re: RFR 8158131: Nashorn should not use jdk.internal.module.Modules API

2016-05-30 Thread Sundararajan Athijegannathan
Hi, Inline replies below. On 5/30/2016 2:16 PM, Michael Haupt wrote: > Hi Sundar, > > lower-case thumbs up for both the jdk and nashorn changes, with remarks: > > == ModuleGraphManipulator.java == > > * please add a @see or @link to the place where the bytes are read and > code is dynamically

Re: RFR 8158131: Nashorn should not use jdk.internal.module.Modules API

2016-05-30 Thread Michael Haupt
Hi Sundar, lower-case thumbs up for both the jdk and nashorn changes, with remarks: == ModuleGraphManipulator.java == * please add a @see or @link to the place where the bytes are read and code is dynamically generated, or used, and vice versa. == JavaAdapterBytecodeGenerator.java == +

Re: RFR 8157680: Callback parameter of any JS builtin implementation should accept any Callable

2016-05-27 Thread Marcus Lagergren
+1 > On 24 May 2016, at 15:52, Sundararajan Athijegannathan > wrote: > > Please review http://cr.openjdk.java.net/~sundar/8157680/webrev.00/ for > https://bugs.openjdk.java.net/browse/JDK-8157680 > > Thanks, > > -Sundar >

Re: RFR 8157789: Nashorn sample/test.js should not use undocumented System property

2016-05-27 Thread Marcus Lagergren
+1 > On 25 May 2016, at 06:38, Sundararajan Athijegannathan > wrote: > > Please review http://cr.openjdk.java.net/~sundar/8157789/webrev.00/ for > https://bugs.openjdk.java.net/browse/JDK-8157789 > > -Sundar >

Re: approval request for JDK-8138906:Test test/script/trusted/JDK-8087292.js intermittently fails

2016-05-27 Thread Marcus Lagergren
+1 > On 26 May 2016, at 18:30, Srinivas Dama wrote: > > Hi, > > Please approve. > > Bug: https://bugs.openjdk.java.net/browse/JDK-8138906 > Jdk9 review thread : > http://mail.openjdk.java.net/pipermail/nashorn-dev/2016-February/005991.html > Jdk8u-webrev :

Re: RFR 8157819: TypeError when a java.util.Comparator object is invoked as a function

2016-05-25 Thread Attila Szegedi
> > On 5/25/2016 5:39 PM, Remi Forax wrote: > > - Mail original - > >> De: "Michael Haupt" <michael.ha...@oracle.com> > >> À: "Sundararajan Athijegannathan" < > sundararajan.athijegannat...@oracle.com> > >> Cc: nashor

Re: [8u] approval request for 8157680: Callback parameter of any JS builtin implementation should accept any Callable

2016-05-25 Thread Seán Coffey
Approved. Regards, Sean. On 25/05/16 15:16, Hannes Wallnoefer wrote: Hi Sundar, the backport looks good. Hannes Am 2016-05-25 um 16:04 schrieb Sundararajan Athijegannathan: Please approve. Bug: https://bugs.openjdk.java.net/browse/JDK-8157680 jdk9 review thread:

Re: [8u] approval request for 8157680: Callback parameter of any JS builtin implementation should accept any Callable

2016-05-25 Thread Hannes Wallnoefer
Hi Sundar, the backport looks good. Hannes Am 2016-05-25 um 16:04 schrieb Sundararajan Athijegannathan: Please approve. Bug: https://bugs.openjdk.java.net/browse/JDK-8157680 jdk9 review thread: http://mail.openjdk.java.net/pipermail/nashorn-dev/2016-May/006225.html jdk8u webrev:

Re: RFR 8157819: TypeError when a java.util.Comparator object is invoked as a function

2016-05-25 Thread Hannes Wallnoefer
chael Haupt" <michael.ha...@oracle.com> À: "Sundararajan Athijegannathan" <sundararajan.athijegannat...@oracle.com> Cc: nashorn-dev@openjdk.java.net Envoyé: Mercredi 25 Mai 2016 13:49:39 Objet: Re: RFR 8157819: TypeError when a java.util.Comparator object is invoked as a

Re: RFR 8157819: TypeError when a java.util.Comparator object is invoked as a function

2016-05-25 Thread Sundararajan Athijegannathan
: "Sundararajan Athijegannathan" <sundararajan.athijegannat...@oracle.com> >> Cc: nashorn-dev@openjdk.java.net >> Envoyé: Mercredi 25 Mai 2016 13:49:39 >> Objet: Re: RFR 8157819: TypeError when a java.util.Comparator object is >> invoked as a function &

Re: RFR 8157819: TypeError when a java.util.Comparator object is invoked as a function

2016-05-25 Thread Remi Forax
- Mail original - > De: "Michael Haupt" <michael.ha...@oracle.com> > À: "Sundararajan Athijegannathan" <sundararajan.athijegannat...@oracle.com> > Cc: nashorn-dev@openjdk.java.net > Envoyé: Mercredi 25 Mai 2016 13:49:39 > Objet: Re: RFR 815781

Re: RFR 8157819: TypeError when a java.util.Comparator object is invoked as a function

2016-05-25 Thread Sundararajan Athijegannathan
Hi, Thanks for the review. Updated : http://cr.openjdk.java.net/~sundar/8157819/webrev.01/ Renamed that method and fixed the article. Leaving MethodType suggestion, as that'd mean unreflect with publicLookup + caching methodTypes of known Object methods.. Thanks, -Sundar On 5/25/2016 5:19 PM,

Re: RFR 8157819: TypeError when a java.util.Comparator object is invoked as a function

2016-05-25 Thread Michael Haupt
Hi Sundar, lower-case thumbs up, with remarks. * "is this a overridable" -> "... an overridable" * My feeling: the name isOverridableObjectMethod would describe the method's intent more clearly. * How about comparing the method types to statically initialised MethodType instances obtained

Re: RFR 8157680: Callback parameter of any JS builtin implementation should accept any Callable

2016-05-25 Thread Michael Haupt
Hi Sundar, lower-case thumbs up. Best, Michael > Am 24.05.2016 um 15:52 schrieb Sundararajan Athijegannathan > : > > Please review http://cr.openjdk.java.net/~sundar/8157680/webrev.00/ for > https://bugs.openjdk.java.net/browse/JDK-8157680 > >

Re: Dynalink and delete

2016-05-24 Thread Sundararajan Athijegannathan
There is a not-so-pretty workaround though. Nashorn routes delete on jdk.nashorn.api.scripting.JSObject instances to removeMember method. If you can wrap your Java objects as JSObjects [ may be just for delete], then you can customize delete on your objects. var myJavaObj = ; // myJavaObj

Re: Dynalink and delete

2016-05-24 Thread Sundararajan Athijegannathan
I recall that we did discuss about the StandardOperation set. "delete" and "iterator" were discussed at one point. But, languages are very different and difficult to generalize to include all operations. For eg. Python's del does not return boolean, ECMAScript delete returns boolean and sometime

Re: RFR 8157680: Callback parameter of any JS builtin implementation should accept any Callable

2016-05-24 Thread Hannes Wallnoefer
This is a nice enhancement! +1 Am 2016-05-24 um 15:52 schrieb Sundararajan Athijegannathan: Please review http://cr.openjdk.java.net/~sundar/8157680/webrev.00/ for https://bugs.openjdk.java.net/browse/JDK-8157680 Thanks, -Sundar

Re: [8u] approval request for 8157160: JSON.stringify does not work on ScriptObjectMirror objects

2016-05-24 Thread Marcus Lagergren
+1 > On 20 May 2016, at 16:51, Sundararajan Athijegannathan > wrote: > > Please approve. > > Bug: https://bugs.openjdk.java.net/browse/JDK-8157160 > > 9 review thread: > http://mail.openjdk.java.net/pipermail/nashorn-dev/2016-May/006179.html > > 8u

Re: [8u] approval request for 8157160: JSON.stringify does not work on ScriptObjectMirror objects

2016-05-20 Thread Jim Laskey (Oracle)
+1 > On May 20, 2016, at 11:51 AM, Sundararajan Athijegannathan > wrote: > > Please approve. > > Bug: https://bugs.openjdk.java.net/browse/JDK-8157160 > > 9 review thread: > http://mail.openjdk.java.net/pipermail/nashorn-dev/2016-May/006179.html > >

Re: RFR(XXS): 8157444: exclude jjs shebang handling test from runs

2016-05-20 Thread Hannes Wallnoefer
+1 Am 2016-05-20 um 15:47 schrieb Michael Haupt: Dear all, please review this fix. Bug: https://bugs.openjdk.java.net/browse/JDK-8157444 Webrev: http://cr.openjdk.java.net/~mhaupt/8157444/webrev.00/ Thanks, Michael

Re: RFR(XXS): 8157444: exclude jjs shebang handling test from runs

2016-05-20 Thread Jim Laskey (Oracle)
+1 > On May 20, 2016, at 10:47 AM, Michael Haupt wrote: > > Dear all, > > please review this fix. > Bug: https://bugs.openjdk.java.net/browse/JDK-8157444 > Webrev: http://cr.openjdk.java.net/~mhaupt/8157444/webrev.00/ > > Thanks, > > Michael > > -- > >

Re: RFR(XXS): 8157444: exclude jjs shebang handling test from runs

2016-05-20 Thread Sundararajan Athijegannathan
+1 On 5/20/2016 7:17 PM, Michael Haupt wrote: > Dear all, > > please review this fix. > Bug: https://bugs.openjdk.java.net/browse/JDK-8157444 > Webrev: http://cr.openjdk.java.net/~mhaupt/8157444/webrev.00/ > > Thanks, > > Michael >

Re: RFR: 8131017: jshell tool: pasting code with tabs invokes tab completion

2016-05-20 Thread Sundararajan Athijegannathan
Looks good. -Sundar On 5/20/2016 4:14 PM, Jan Lahoda wrote: > Please review. > > Problem: when pasting code that contains tab characters into both > jshell and jjs, the tab completion gets invoked on tabs instead of > using the tab character itself. > > The proposed solution is to set

Re: RFR 8157310: jdk.dynalink.linker.support.Lookup should have more checks before adding module read link

2016-05-20 Thread Attila Szegedi
Seconded, this is indeed better. +1 > On 20 May 2016, at 10:38, Hannes Wallnoefer > wrote: > > Hi Sundar, > > I agree methods feel more in their place now. > > Hannes > > Am 2016-05-20 um 09:12 schrieb Sundararajan Athijegannathan: >> Hi, >> >> Thanks for the

Re: RFR 8157310: jdk.dynalink.linker.support.Lookup should have more checks before adding module read link

2016-05-20 Thread Hannes Wallnoefer
Hi Sundar, I agree methods feel more in their place now. Hannes Am 2016-05-20 um 09:12 schrieb Sundararajan Athijegannathan: Hi, Thanks for the review. But, I adjusted the change slightly -- moved addModuleRead and caller-sensitive fallback attempt to CallerSensitiveDynamicMethod itself -

Re: RFR 8157310: jdk.dynalink.linker.support.Lookup should have more checks before adding module read link

2016-05-20 Thread Attila Szegedi
+1. Nice work! > On 19 May 2016, at 15:28, Sundararajan Athijegannathan > wrote: > > Please review http://cr.openjdk.java.net/~sundar/8157310/ for > https://bugs.openjdk.java.net/browse/JDK-8157310 > > What is this fix? > > * When unreflecting caller

Re: RFR 8157310: jdk.dynalink.linker.support.Lookup should have more checks before adding module read link

2016-05-20 Thread Attila Szegedi
+1. Nice work! > On 19 May 2016, at 15:28, Sundararajan Athijegannathan > wrote: > > Please review http://cr.openjdk.java.net/~sundar/8157310/ for > https://bugs.openjdk.java.net/browse/JDK-8157310 > > What is this fix? > > * When unreflecting caller

Re: RFR 8157310: jdk.dynalink.linker.support.Lookup should have more checks before adding module read link

2016-05-19 Thread Hannes Wallnoefer
+1 Am 2016-05-19 um 14:28 schrieb Sundararajan Athijegannathan: Please review http://cr.openjdk.java.net/~sundar/8157310/ for https://bugs.openjdk.java.net/browse/JDK-8157310 What is this fix? * When unreflecting caller sensitive methods, dynalink uses specific Lookup of actual caller

Re: RFR 8157241: Remove javac warnings of Nashorn "ant clean test"

2016-05-19 Thread Marcus Lagergren
+1 > On 18 May 2016, at 14:01, Sundararajan Athijegannathan > wrote: > > Please review http://cr.openjdk.java.net/~sundar/8157241/ for > https://bugs.openjdk.java.net/browse/JDK-8157241 > > Thanks, > > -Sundar >

Re: Review request for JDK-8066229: Fuzzing bug: Can't find scope depth

2016-05-19 Thread Marcus Lagergren
+1 > On 18 May 2016, at 18:36, Hannes Wallnöfer wrote: > > Please review JDK-8066229: Fuzzing bug: Can't find scope depth > > http://cr.openjdk.java.net/~hannesw/8066229/webrev/ > > This adds a test case for bug that is already fixed. It also removes the > EXPECTED file

Re: Review request for JDK-8066229: Fuzzing bug: Can't find scope depth

2016-05-19 Thread Michael Haupt
Hi Hannes, lower-case thumbs up. Best, Michael > Am 18.05.2016 um 18:36 schrieb Hannes Wallnöfer : > > Please review JDK-8066229: Fuzzing bug: Can't find scope depth > > http://cr.openjdk.java.net/~hannesw/8066229/webrev/ > > This adds a test case for bug that is already

Re: Review request for JDK-8157263: Octane svn repository no longer exists

2016-05-19 Thread Michael Haupt
Hi Hannes, lower-case thumbs up. Best, Michael > Am 18.05.2016 um 19:50 schrieb Hannes Wallnoefer > : > > Please review JDK-8157263: Octane svn repository no longer exists: > > http://cr.openjdk.java.net/~hannesw/8157263/webrev/ > > Thanks, > Hannes --

Re: Review request for JDK-8157263: Octane svn repository no longer exists

2016-05-18 Thread Sundararajan Athijegannathan
+1 On 5/18/2016 11:20 PM, Hannes Wallnoefer wrote: > Please review JDK-8157263: Octane svn repository no longer exists: > > http://cr.openjdk.java.net/~hannesw/8157263/webrev/ > > Thanks, > Hannes

Re: Review request for JDK-8066229: Fuzzing bug: Can't find scope depth

2016-05-18 Thread Sundararajan Athijegannathan
+1 On 5/18/2016 10:06 PM, Hannes Wallnöfer wrote: > Please review JDK-8066229: Fuzzing bug: Can't find scope depth > > http://cr.openjdk.java.net/~hannesw/8066229/webrev/ > > This adds a test case for bug that is already fixed. It also removes > the EXPECTED file added in the previous commit

Re: RFR 8154192: Deprivilege java.scripting module

2016-05-18 Thread Mandy Chung
> On May 18, 2016, at 12:55 AM, Sundararajan Athijegannathan > wrote: > > Please review the updated webrevs. > > * Fixed Modules.gmk for order of modules: > > http://cr.openjdk.java.net/~sundar/8154192/top/webrev.01/ > > * From quick reading of

Re: RFR(S): 8157250: BeanLinker assumes fixed array type linkage

2016-05-18 Thread Sundararajan Athijegannathan
+1 On 5/18/2016 9:04 PM, Attila Szegedi wrote: > +1 > >> On 18 May 2016, at 17:31, Michael Haupt wrote: >> >> Dear all, >> >> please review this fix. >> Bug: https://bugs.openjdk.java.net/browse/JDK-8157250 >> Webrev:

Re: RFR(S): 8157250: BeanLinker assumes fixed array type linkage

2016-05-18 Thread Attila Szegedi
+1 > On 18 May 2016, at 17:31, Michael Haupt wrote: > > Dear all, > > please review this fix. > Bug: https://bugs.openjdk.java.net/browse/JDK-8157250 > Webrev: http://cr.openjdk.java.net/~mhaupt/8157250/webrev.00/ > > Thanks, > > Michael > > -- > >

Re: RFR(S): 8157225: adopt method handle for array length getter in BeanLinker

2016-05-18 Thread Michael Haupt
Hi Attila, thanks for your comments. > Am 18.05.2016 um 16:11 schrieb Attila Szegedi : > There’s a bit of an issue with this. Namely, the old GET_ARRAY_LENGTH being > the unreflection of j.l.reflect.Array.getLength() worked for any array type, > so it was always accompanied

Re: RFR 8157241: Remove javac warnings of Nashorn "ant clean test"

2016-05-18 Thread Hannes Wallnoefer
+1 Am 2016-05-18 um 14:01 schrieb Sundararajan Athijegannathan: Please review http://cr.openjdk.java.net/~sundar/8157241/ for https://bugs.openjdk.java.net/browse/JDK-8157241 Thanks, -Sundar

Re: RFR(S): 8157225: adopt method handle for array length getter in BeanLinker

2016-05-18 Thread Attila Szegedi
There’s a bit of an issue with this. Namely, the old GET_ARRAY_LENGTH being the unreflection of j.l.reflect.Array.getLength() worked for any array type, so it was always accompanied with ValidationType.IS_ARRAY. I believe this is wrong now and can cause ClassCastException at runtime (I say

Re: RFR 8157241: Remove javac warnings of Nashorn "ant clean test"

2016-05-18 Thread Michael Haupt
Hi Sundar, lower-case thumbs up. Best, Michael > Am 18.05.2016 um 14:01 schrieb Sundararajan Athijegannathan > : > > Please review http://cr.openjdk.java.net/~sundar/8157241/ for > https://bugs.openjdk.java.net/browse/JDK-8157241 > > Thanks, > >

Re: RFR(S): 8157225: adopt method handle for array length getter in BeanLinker

2016-05-18 Thread Hannes Wallnoefer
+1 Am 2016-05-18 um 11:25 schrieb Michael Haupt: Dear all, please review this change. RFE: https://bugs.openjdk.java.net/browse/JDK-8157225 Webrev: http://cr.openjdk.java.net/~mhaupt/8157225/webrev.00/ Thanks, Michael

Re: RFR(S): 8157225: adopt method handle for array length getter in BeanLinker

2016-05-18 Thread Sundararajan Athijegannathan
+1 Nice! -Sundar On 5/18/2016 2:55 PM, Michael Haupt wrote: > Dear all, > > please review this change. > RFE: https://bugs.openjdk.java.net/browse/JDK-8157225 > Webrev: http://cr.openjdk.java.net/~mhaupt/8157225/webrev.00/ > > Thanks, > > Michael >

Re: RFR 8154192: Deprivilege java.scripting module

2016-05-18 Thread Sundararajan Athijegannathan
Thanks. I'll make that change and push it. -Sundar On 5/18/2016 2:17 PM, Alan Bateman wrote: > On 18/05/2016 08:55, Sundararajan Athijegannathan wrote: >> Please review the updated webrevs. >> >> * Fixed Modules.gmk for order of modules: >> >>

Re: RFR 8154192: Deprivilege java.scripting module

2016-05-18 Thread Alan Bateman
On 18/05/2016 08:55, Sundararajan Athijegannathan wrote: Please review the updated webrevs. * Fixed Modules.gmk for order of modules: http://cr.openjdk.java.net/~sundar/8154192/top/webrev.01/ * From quick reading of j.u.ServiceLoader: AccessControlContext is captured in ServiceLoader

Re: RFR 8157160: JSON.stringify does not work on ScriptObjectMirror objects

2016-05-18 Thread Hannes Wallnoefer
+1 Am 2016-05-18 um 08:50 schrieb Sundararajan Athijegannathan: Please review http://cr.openjdk.java.net/~sundar/8157160/ for https://bugs.openjdk.java.net/browse/JDK-8157160 Thanks, -Sundar

Re: RFR 8154192: Deprivilege java.scripting module

2016-05-18 Thread Sundararajan Athijegannathan
Please review the updated webrevs. * Fixed Modules.gmk for order of modules: http://cr.openjdk.java.net/~sundar/8154192/top/webrev.01/ * From quick reading of j.u.ServiceLoader: AccessControlContext is captured in ServiceLoader constructor & used for iteration (RestrictedIterator). So,

Re: RFR 8157160: JSON.stringify does not work on ScriptObjectMirror objects

2016-05-18 Thread Michael Haupt
Hi Sundar, lower-case thumbs up. Best, Michael > Am 18.05.2016 um 08:50 schrieb Sundararajan Athijegannathan > : > > Please review http://cr.openjdk.java.net/~sundar/8157160/ for > https://bugs.openjdk.java.net/browse/JDK-8157160 > > Thanks, > >

Re: JDK9 features

2016-05-17 Thread Eric Pederson
Thanks! On Tuesday, May 17, 2016, Sundararajan Athijegannathan < sundararajan.athijegannat...@oracle.com> wrote: > Hi, > > Filed a bug: https://bugs.openjdk.java.net/browse/JDK-8157160 > > Thanks, > > -Sundar > > On 5/13/2016 8:16 PM, Eric Pederson wrote: > > Hi Sundar - > > It would be helpful

Re: JDK9 features

2016-05-17 Thread Sundararajan Athijegannathan
Hi, Filed a bug: https://bugs.openjdk.java.net/browse/JDK-8157160 Thanks, -Sundar On 5/13/2016 8:16 PM, Eric Pederson wrote: > Hi Sundar - > > It would be helpful for sure. We use JSON for serialization and > debugging a lot. Also if it worked it could simplify our > custom

Re: RFR 8156847: jdk.dynalink package is shown under "Other Packages" section

2016-05-16 Thread Erik Joelsson
Thanks for the link. I'm happy with the change. /Erik On 2016-05-16 17:28, Sundararajan Athijegannathan wrote: The main dynalink API package "jdk.dynalink" was shown in "Other Packages" section! After this fix, all packages are shown under single section. Please see:

Re: RFR 8156847: jdk.dynalink package is shown under "Other Packages" section

2016-05-16 Thread Sundararajan Athijegannathan
The main dynalink API package "jdk.dynalink" was shown in "Other Packages" section! After this fix, all packages are shown under single section. Please see: http://download.java.net/java/jdk9/docs/jdk/api/dynalink/ Thanks, -Sundar On 5/16/2016 8:55 PM, Erik Joelsson wrote: > As a makefile

Re: RFR 8156847: jdk.dynalink package is shown under "Other Packages" section

2016-05-16 Thread Erik Joelsson
As a makefile change this looks fine, but I have no clue as to what that line actually means for javadoc. /Erik On 2016-05-16 17:08, Sundararajan Athijegannathan wrote: Please review http://cr.openjdk.java.net/~sundar/8156847/webrev.00/ for https://bugs.openjdk.java.net/browse/JDK-8156847

Re: RFR 8156847: jdk.dynalink package is shown under "Other Packages" section

2016-05-16 Thread Jim Laskey (Oracle)
+1 > On May 16, 2016, at 12:08 PM, Sundararajan Athijegannathan > wrote: > > Please review http://cr.openjdk.java.net/~sundar/8156847/webrev.00/ for > https://bugs.openjdk.java.net/browse/JDK-8156847 > > Thanks, > > -Sundar >

Re: [PATCH] 8156896: Script stack trace should display function names

2016-05-13 Thread Vivin Suresh Paliath
Looks like I missed the paragraph in the contribution guidelines that fixes will be backported from 9 to 8 only after 6 weeks. Also, since this isn't critical, I'm guessing that it won't be backported? On Fri, May 13, 2016 at 11:37 AM, Vivin Suresh Paliath < vivin.pali...@gmail.com> wrote: >

Re: Review request for JDK-8156896: Script stack trace should display function names

2016-05-13 Thread Vivin Suresh Paliath
I was able to apply the same changes to jdk8u-dev and it looks like it properly reports the function name in the stacktrace now. I can submit a fix so that this behavior is also available on jdk8. On Fri, May 13, 2016 at 7:16 AM, Vivin Suresh Paliath < vivin.pali...@gmail.com> wrote: > +1 > >

Re: Review request for JDK-8156714: Parsing issue with automatic semicolon insertion

2016-05-13 Thread Sundararajan Athijegannathan
+1 On 5/13/2016 9:22 PM, Hannes Wallnoefer wrote: > Please review JDK-8156714: Parsing issue with automatic semicolon > insertion: > > http://cr.openjdk.java.net/~hannesw/8156714/webrev/ > > Comments are irrelevant for newline detection so we should ignore them > when assigning to

Re: Review request for JDK-8156714: Parsing issue with automatic semicolon insertion

2016-05-13 Thread Jim Laskey (Oracle)
+1 > On May 13, 2016, at 12:52 PM, Hannes Wallnoefer > wrote: > > Please review JDK-8156714: Parsing issue with automatic semicolon insertion: > > http://cr.openjdk.java.net/~hannesw/8156714/webrev/ > > Comments are irrelevant for newline detection so we should

Re: JDK9 features

2016-05-13 Thread Eric Pederson
Hi Sundar - It would be helpful for sure. We use JSON for serialization and debugging a lot. Also if it worked it could simplify our custom ScriptObjectMirror -> native object conversion logic from 30 lines down to JSON.parse(JSON.stringify(scriptObjMirror)) :). Besides the JSON stuff our

Re: Review request for JDK-8156896: Script stack trace should display function names

2016-05-13 Thread Vivin Suresh Paliath
+1 Would it be possible to backport this to jdk8? On May 13, 2016 3:00 AM, "Sundararajan Athijegannathan" < sundararajan.athijegannat...@oracle.com> wrote: > +1 > > > On 5/13/2016 3:14 PM, Hannes Wallnoefer wrote: > > Review request for JDK-8156896: Script stack trace should display > > function

Re: Review request for JDK-8156896: Script stack trace should display function names

2016-05-13 Thread Sundararajan Athijegannathan
+1 On 5/13/2016 3:14 PM, Hannes Wallnoefer wrote: > Review request for JDK-8156896: Script stack trace should display > function names > > http://cr.openjdk.java.net/~hannesw/8156896/webrev/ > > This makes it possible to reliably get the original function name from > Java method names for

Re: Review request for JDK-8156896: Script stack trace should display function names

2016-05-13 Thread Attila Szegedi
+1 > On 13 May 2016, at 11:44, Hannes Wallnoefer > wrote: > > Review request for JDK-8156896: Script stack trace should display function > names > > http://cr.openjdk.java.net/~hannesw/8156896/webrev/ > > This makes it possible to reliably get the original

Re: Tree API: offsets for String fields

2016-05-13 Thread Emilian Bold
Thank you! PS: It would be great if JBS would allow me to register for email updates on issues I care about... --emi On Thu, May 12, 2016 at 8:10 PM, Sundararajan Athijegannathan < sundararajan.athijegannat...@oracle.com> wrote: > Filed: https://bugs.openjdk.java.net/browse/JDK-8156866 > > PS.

Re: Tree API Javadoc down?

2016-05-13 Thread Emilian Bold
Thanks! I guess it would have been too hard for them to add a 301 redirect‽ --emi On Thu, May 12, 2016 at 5:45 PM, Claes Redestad wrote: > Hi, > > I saw elsewhere that docs have moved into /java (sigh): > > >

Re: Stacktraces from dynamically-constructed functions not as expected

2016-05-12 Thread Vivin Suresh Paliath
I have the same preference! That is exactly why I name functions even when they are function expressions. I am developing an API where having the name of the function in the stack trace would be a useful debugging aid as well. Yes the f$foo vs. foo is a different issue, but I would also prefer

Re: JDK9 features

2016-05-12 Thread Sundararajan Athijegannathan
Hi Eric, That commonjs/require loading pattern is not bad [wrapping eval'ed code inside a function]. If your loaded scripts are (somewhat) well-behaved, it does solve the problem of global namespace pollution. Yes, ScriptObjectMirrors are not uniformly treated like ScriptObject everywhere. But,

Re: Stacktraces from dynamically-constructed functions not as expected

2016-05-12 Thread Sundararajan Athijegannathan
Adding to what Hannes said: "stack" property and the format of stack trace are not part of ECMAScript standard. Implementations could differ in this. This is more of debugging aid as Hannes mentioned. I'd personally prefer names being shown in stack traces whenever possible - rather than being

Re: Stacktraces from dynamically-constructed functions not as expected

2016-05-12 Thread Vivin Suresh Paliath
Thanks Hannes. I looked at the issue and it answered another question I had as well; I was wondering about the possibility of using a separator other than $ that is legal in Java, but not in JavaScript. On Thu, May 12, 2016 at 3:08 PM, Hannes Wallnoefer < hannes.wallnoe...@oracle.com> wrote: >

Re: Stacktraces from dynamically-constructed functions not as expected

2016-05-12 Thread Hannes Wallnoefer
Am 2016-05-12 um 23:42 schrieb Vivin Suresh Paliath: Thanks for the explanation Hannes! The issue with $ makes sense; I ran into that some time ago - I can't remember the exact situation, but it was exactly as you described: the $ introduces ambiguity because it is a valid character and so

Re: Stacktraces from dynamically-constructed functions not as expected

2016-05-12 Thread Vivin Suresh Paliath
Thanks for the explanation Hannes! The issue with $ makes sense; I ran into that some time ago - I can't remember the exact situation, but it was exactly as you described: the $ introduces ambiguity because it is a valid character and so could be part of the name of the original function, and not

Re: Stacktraces from dynamically-constructed functions not as expected

2016-05-12 Thread Hannes Wallnoefer
Hi Vivin, What you see is some fuzziness in the translation from JS functions to Java methods and from there to the stack traces you see. When we compile a JS function, we create a Java method with the name of the function concatenated to the names of its parent functions, using '$' as

Re: JDK9 features

2016-05-12 Thread Eric Pederson
Hi Sundar: 1. I investigated loadWithNewGlobal because it looked promising for this use case. Say you use loadWithNewGlobal to load a function. If you call the loaded function and it returns an object or array the caller gets a ScriptObjectMirror. The problems with a ScriptObjectMirror

Re: Tree API: offsets for String fields

2016-05-12 Thread Sundararajan Athijegannathan
Filed: https://bugs.openjdk.java.net/browse/JDK-8156866 PS. Actually, I was wrong about MemberSelectTree - even internally nashorn IR node "AccessNode" maintains 'property' as a String. But, variables, functions have identifier nodes for names. Thanks, -Sundar On 5/12/2016 10:20 PM, Emilian

Re: Tree API: offsets for String fields

2016-05-12 Thread Emilian Bold
Assuming I don't need some special permissions to do that (do I?) I could expand the previous email and post it as an enhancement on https://bugs.openjdk.java.net But I'm also trying to understand the logic behind what you have. The way I see it any AST node has some underlying tokens so its

Re: Tree API: offsets for String fields

2016-05-12 Thread Sundararajan Athijegannathan
Hi, Thanks for your comments. Nashorn Parser API was modeled after similar API provided by javac - for consistency sake and to leverage developer familiarity. For example, ContinueTree, MemberSelectTree of javac Tree API:

Re: Tree API Javadoc down?

2016-05-12 Thread Claes Redestad
Hi, I saw elsewhere that docs have moved into /java (sigh): http://download.java.net/java/jdk9/docs/jdk/api/nashorn/jdk/nashorn/api/tree/IfTree.html /Claes On 2016-05-12 16:44, Emilian Bold wrote: Hello, Google gives me this URL (which I used before)

<    5   6   7   8   9   10   11   12   13   14   >