RE: approval request for JDK-8138906:Test test/script/trusted/JDK-8087292.js intermittently fails
Hi, There are some 8u specific changes, Please do a backport review. Here are the links. Bug: https://bugs.openjdk.java.net/browse/JDK-8138906 Jdk9 review thread: http://mail.openjdk.java.net/pipermail/nashorn-dev/2016-March/006022.html Jdk9 webrev : http://cr.openjdk.java.net/~sdama/8138906/webrev.00/ Jdk8u webrev: http://cr.openjdk.java.net/~sdama/8138906/8u/webrev.01/ Regards, Srinivas -Original Message- From: Seán Coffey Sent: Monday, May 30, 2016 6:56 PM To: Srinivas Dama Subject: Re: approval request for JDK-8138906:Test test/script/trusted/JDK-8087292.js intermittently fails Approved for jdk8u-dev. If the code differs from the JDK 9 fix, please obtain a code review before pushing. Regards, Sean. On 26/05/16 17: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/00599 > 1.html Jdk8u-webrev : > http://cr.openjdk.java.net/~sdama/8138906/8u/webrev.00/ > > Regards, > Srinivas
RFR:DK-8148457(Remove jdk.nashorn.tools.FXShell class and associated build.xml cruft)
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 targets. Regards, Srinivas
RFR:8158817
Hi, Please review http://cr.openjdk.java.net/~sdama/8158817/webrev.00/ for https://bugs.openjdk.java.net/browse/JDK-8158817 Regards, Srinivas
approval request for JDK-8138906:Test test/script/trusted/JDK-8087292.js intermittently fails
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 : http://cr.openjdk.java.net/~sdama/8138906/8u/webrev.00/ Regards, Srinivas
Review request for JDK-8138906
Hello All, Please review : Bug : https://bugs.openjdk.java.net/browse/JDK-8138906 Webrev : http://cr.openjdk.java.net/~sdama/8138906/webrev.01 Fix is to pick 'java' from specified JAVA_HOME folder, along with this, I have added some utility functions(can be added more) which can be used in all nashorn test case to make them work Independent of underlying shell.(removed dependency on shell commands like : which) Regards, Srinivas
Review request for JDK-8138906
Hi, Please review http://cr.openjdk.java.net/~sdama/8138906/webrev.00/ for https://bugs.openjdk.java.net/browse/JDK-8138906 I did testing with recent $EXEC changes from Michael and very small code cleanup. Regards, Srinivas
Review request for JDK-8147613
Hi, Please review : Webrev : http://cr.openjdk.java.net/~sdama/8147613/webrev.00/ It covers fix for both: https://bugs.openjdk.java.net/browse/JDK-8147613 https://bugs.openjdk.java.net/browse/JDK-8147788 Regards, Srinivas
RE: Review request for JDK-8138906
Hi Michael, Thank you . Here is the latest webrev with all modifications. Bug : https://bugs.openjdk.java.net/browse/JDK-8138906 Webrev : http://cr.openjdk.java.net/~sdama/8138906/webrev.02/ Regards, Srinivas From: Michael Haupt Sent: Tuesday, March 01, 2016 3:16 PM To: Srinivas Dama Cc: nashorn-dev@openjdk.java.net Subject: Re: Review request for JDK-8138906 Hi Srinivas, Am 29.02.2016 um 17:28 schrieb Srinivas Dama mailto:srinivas.d...@oracle.com"srinivas.d...@oracle.com>: Please review : Bug : https://bugs.openjdk.java.net/browse/JDK-8138906 Webrev : http://cr.openjdk.java.net/~sdama/8138906/webrev.01 I've verified this works on Windows. Lower-case thumbs up, with these remarks: == JDK-util.js == Lines 64/65: typos, please use "Unix cygpath implementation. Supports only two outputs, windows (C:\dir\) and mixed (C:/dir/)." == JDK-8087292.js == Copyright: convention requires a space after the comma in "2015,2016", and a comma after "2016", i.e., please use "2015, 2016, ". As a matter of personal preference, how about using semicolons consistently, i.e., either at the ends of all lines, or never? I have added some utility functions(can be added more) which can be used in all nashorn test case to make them work Independent of underlying shell.(removed dependency on shell commands like : which) These look useful and should be adopted by more tests (that's a future RFE). Best, Michael -- HYPERLINK "http://www.oracle.com/; \nOracle Dr. Michael Haupt | Principal Member of Technical Staff Phone: +49 331 200 7277 | Fax: +49 331 200 7561 Oracle Java Platform Group | LangTools Team | Nashorn Oracle Deutschland B.V. & Co. KG | Schiffbauergasse 14 | 14467 Potsdam, Germany ORACLE Deutschland B.V. & Co. KG | Hauptverwaltung: Riesstraße 25, D-80992 München Registergericht: Amtsgericht München, HRA 95603 Komplementärin: ORACLE Deutschland Verwaltung B.V. | Hertogswetering 163/167, 3543 AS Utrecht, Niederlande Handelsregister der Handelskammer Midden-Nederland, Nr. 30143697 Geschäftsführer: Alexander van der Ven, Jan Schultheiss, Val Maher HYPERLINK "http://www.oracle.com/commitment; \nGreen Oracle Oracle is committed to developing practices and products that help protect the environment
RE: RFR(S): 8151291: $EXEC yields "unknown command" on Cygwin
Hi Michael,
Looks good.
Few limitations with absolute paths.
On cmd.exe:
jjs -scripting
$EXEC("D:\work\nashorn-dev\dev\build\windows-x86_64-normal-server-release\images\jdk\bin\java.exe")
Good thing is on cygwin if we give mixed path like:
$EXEC("D:/work/nashorn-dev/dev/build/windows-x86_64-normal-server-release/images/jdk/bin/java.exe"),it
works.
But
$EXEC("/cygdrive/d/work/nashorn-dev/dev/build/windows-x86_64-normal-server-release/images/jdk/bin/java.exe")
fails.
So if we handle only cmd.exe is enough I guess.
Absolute paths should be handled because many test cases pass absolute paths
taken from JAVA_HOME.
Regards,
Srinivas
-Original Message-
From: Michael Haupt
Sent: Monday, March 07, 2016 5:58 PM
To: Nashorn-dev
Subject: RFR(S): 8151291: $EXEC yields "unknown command" on Cygwin
Dear all,
please review this change.
Bug: https://bugs.openjdk.java.net/browse/JDK-8151291
Webrev: http://cr.openjdk.java.net/~mhaupt/8151291/webrev.00/
For Cygwin, paths need special attention as the /cygdrive/x/... format is not
recognised by Windows.
Thanks,
Michael
--
Oracle is committed to developing
practices and products that help protect the environment
RE: RFR 8160801 :add documentation for NativeString
Hi Michael, Thank you for comments . Please find patch with your suggestions added at http://cr.openjdk.java.net/~sdama/8160801/webrev.00/ Regards, Srinivas From: Michael Haupt Sent: Tuesday, July 26, 2016 6:06 PM To: Srinivas Dama Cc: Nashorn-dev Subject: Re: RFR 8160801 :add documentation for NativeString Hi Srini, thumbs up, with conditions (see below). I'll be happy to sponsor the push. +String.prototype.concat=returns string value which is mix of calling object string value and string value given as argument A mix would be fun. :-) How about this: returns the string resulting from appending the argument string value to the calling object +String.prototype.indexOf=returns the index of first occurrence of specified string, starting the search from position given as argument.returns -1 if the string is not found + +String.prototype.lastIndexOf=returns the index of last occurrence of specified string, searching backwards from position given as argument.returns -1 if the string is not found Please replace ".returns" with ", returns" (note the extra space). Best, Michael Am 26.07.2016 um 13:57 schrieb Srinivas Dama mailto:srinivas.d...@oracle.com"srinivas.d...@oracle.com>: Hello, Please review http://cr.openjdk.java.net/~sdama/8160801/webrev.00/ for https://bugs.openjdk.java.net/browse/JDK-8160801 Regards, Srinivas -- HYPERLINK "http://www.oracle.com/; \nOracle Dr. Michael Haupt | Principal Member of Technical Staff Phone: +49 331 200 7277 | Fax: +49 331 200 7561 Oracle Java Platform Group | LangTools Team | Nashorn Oracle Deutschland B.V. & Co. KG | Schiffbauergasse 14 | 14467 Potsdam, Germany ORACLE Deutschland B.V. & Co. KG | Hauptverwaltung: Riesstraße 25, D-80992 München Registergericht: Amtsgericht München, HRA 95603 Komplementärin: ORACLE Deutschland Verwaltung B.V. | Hertogswetering 163/167, 3543 AS Utrecht, Niederlande Handelsregister der Handelskammer Midden-Nederland, Nr. 30143697 Geschäftsführer: Alexander van der Ven, Jan Schultheiss, Val Maher HYPERLINK "http://www.oracle.com/commitment; \nGreen Oracle Oracle is committed to developing practices and products that help protect the environment
RFR 8160801 :add documentation for NativeString
Hello, Please review http://cr.openjdk.java.net/~sdama/8160801/webrev.00/ for https://bugs.openjdk.java.net/browse/JDK-8160801 Regards, Srinivas
RE: RFR: 8134304: NPE in initialization of OptimisticTypesPersistence
Hi, Please review http://cr.openjdk.java.net/~sdama/8134304/webrev.01/ for https://bugs.openjdk.java.net/browse/JDK-8134304 Localized fix only to OptimisticTypesPersistence.java. Regards, Srinivas -Original Message- From: Srinivas Dama Sent: Wednesday, August 10, 2016 2:40 PM To: Nashorn-dev Subject: RFR: 8134304: NPE in initialization of OptimisticTypesPersistence Hi, Please review http://cr.openjdk.java.net/~sdama/8134304/webrev.00/ for https://bugs.openjdk.java.net/browse/JDK-8134304 Regards, Srinivas
RFR 8142969:Nashorn logging API requires testing
Hi, Please review http://cr.openjdk.java.net/~sdama/8142969/webrev.00/ for https://bugs.openjdk.java.net/browse/JDK-8142969 Fixed the existing test case and moved it from test/script/currently-failing to test/script/nosecurity directory. Regards, Srinivas
RFR: 8156743: ES6 for..of should work for Java Maps and Sets
Hello, Please review http://cr.openjdk.java.net/~sdama/8156743/webrev.00/ for https://bugs.openjdk.java.net/browse/JDK-8156743 Regards, Srinivas
RFR 8166296:add documentation for Date,RegExp,Error,JSON objects
Hello, Please review http://cr.openjdk.java.net/~sdama/8166296/webrev.00/ for https://bugs.openjdk.java.net/browse/JDK-8166296 Regards, Srinivas
RFR:8164618: add documentation for NativeNumber and NativeBoolean
Hi, Please review http://cr.openjdk.java.net/~sdama/8164618/webrev.00/ for https://bugs.openjdk.java.net/browse/JDK-8164618 Regards, Srinivas
RE: RFR: 8156615:Catch parameter can be a BindingPattern in ES6 mode
Hi,
Please review
webrev: http://cr.openjdk.java.net/~sdama/8156615/webrev.03/
Bug : https://bugs.openjdk.java.net/browse/JDK-8156615
@Attila, Thank you for the comments.
Regards,
Srinivas
-Original Message-
From: Attila Szegedi [mailto:szege...@gmail.com]
Sent: Thursday, November 10, 2016 5:38 PM
To: Nashorn-dev
Subject: Re: RFR: 8156615:Catch parameter can be a BindingPattern in ES6 mode
I’m still not happy with this, although we’re getting there :-)
- you changed getException() to return Expression, even though Sundar asked you
to add a different method. I don’t like the fact that getException() returns
Expression, since it can’t be an arbitrary expression; it’s limited to
identifier and binding pattern, yet you have unchecked casts on its return
value on invocation everywhere now. Maybe keep your “Expression getException()”
change, but add “IdentNode getExceptionIdentifier() { return
(IdentNode)exception; }” and have the callers use that now instead. That way,
you encapsulate the cast and don’t force the users of the API to do it
everywhere, and it’s easier to track with call hierarchy in the IDEs which
callers presume the exception is an identifier.
- the JavaDoc for getException() and setException() should be updated to
reflect the value can be a binding pattern. Please use the wording “binding
pattern” instead of just “pattern” in the documentation (including the field
declaration) to avoid ambiguity.
- Maybe throw an IllegalArgumentException if either the constructor or
setException receive something that isn’t either an ident node or a binding
pattern. That way, you could enforce an invariant in the class that the types
of the expression are only legitimate ones. I’m unsure if it’s easy to
recognize the binding pattern or not (e.g. if it just looks like an
object/array literal).
Attila.
> On 10 Nov 2016, at 12:46, Srinivas Dama <srinivas.d...@oracle.com> wrote:
>
> Hi,
>
> Please review
> Bug: https://bugs.openjdk.java.net/browse/JDK-8156615
> webrev: http://cr.openjdk.java.net/~sdama/8156615/webrev.02/
>
> Included all changes suggested with some modifications.
>
> Regards,
> Srinivas
>
> -Original Message-
> From: Attila Szegedi [mailto:szege...@gmail.com]
> Sent: Tuesday, November 08, 2016 7:40 PM
> To: Nashorn-dev
> Subject: Re: RFR: 8156615:Catch parameter can be a BindingPattern in
> ES6 mode
>
> What Sundar said. Additionally:
>
> + if (exception instanceof IdentNode) {
> + this.exception = ((IdentNode) exception == null) ? null :
> + ((IdentNode) exception).setIsInitializedHere();
>
> First, casting an expression before a null comparison is silly :-).
> Second, you don't even need to check for exception == null as "exception
> instanceof IdentNode" implies that exception != null. So that line should
> just be:
>
> + if (exception instanceof IdentNode) {
> + this.exception = ((IdentNode) exception).setIsInitializedHere();
>
> Attila.
>
>> On 08 Nov 2016, at 04:45, Sundararajan Athijegannathan
>> <sundararajan.athijegannat...@oracle.com> wrote:
>>
>> You need to update Parser API implementation to handle catch
>> parameter being expression - rather than an IdentNode.
>>
>> => you need to
>>
>> * add another getter in CatchNode to expose Expression
>>
>> * Use it in IRTranslator (of Parser API impl.) to translate catch
>> param as expression
>>
>> * You need to check for expression param in codegen pipeline to throw
>> "not yet implemented" error.
>>
>> -Sundar
>>
>> On 11/7/2016 9:40 PM, Srinivas Dama wrote:
>>> Please review:
>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8156615
>>> Webrev: http://cr.openjdk.java.net/~sdama/8156615/webrev.00/
>>>
>>> Please note that, this contains only parsing support for catch parameter as
>>> BindingPattern or BindingIdentifier.
>>>
>>> Regards,
>>> Srinivas
>>
>
RFR 8168663: Nashorn: ant testng tests doesn't support external java options
Hi, Please review http://cr.openjdk.java.net/~sdama/8168663/webrev.00/ for https://bugs.openjdk.java.net/browse/JDK-8168663 Added run.test.jvmargs.external property so that we can pass jvm options for complete test run as below. ant -Drun.test.jvmargs.external="-XX:+UseSerialGC -XX:-TieredCompilation" test Regards, Srinivas
RFR: 8151994 (test/script/basic/JDK-8141209.js fails)
Hi, Please review http://cr.openjdk.java.net/~sdama/8151994/webrev.00/ for https://bugs.openjdk.java.net/browse/JDK-8151994 Added @fork tag in test case since test runner without fork uses customized output stream to compare output, whereas test case uses System.out. Regards, Srinivas
RE: RFR: 8151994 (test/script/basic/JDK-8141209.js fails)
Hi, Please review http://cr.openjdk.java.net/~sdama/8151994/webrev.01/ for https://bugs.openjdk.java.net/browse/JDK-8151994 Moved earlier fix to test/script/trusted directory. Regards, Srinivas -Original Message- From: Hannes Wallnöfer Sent: Wednesday, December 21, 2016 5:15 PM To: Srinivas Dama Cc: Nashorn-dev Subject: Re: RFR: 8151994 (test/script/basic/JDK-8141209.js fails) +1 Hannes > Am 20.12.2016 um 16:01 schrieb Srinivas Dama <srinivas.d...@oracle.com>: > > Hi, > > Please review http://cr.openjdk.java.net/~sdama/8151994/webrev.00/ > for https://bugs.openjdk.java.net/browse/JDK-8151994 > > Added @fork tag in test case since test runner without fork uses customized > output stream to compare output, whereas test case uses System.out. > > Regards, > Srinivas
RFR: 8178315: nashorn ant build failure with @moduleGraph javadoc tag
Hi, Please review http://cr.openjdk.java.net/~bgopularam/sdama/8178315/webrev.00/ for https://bugs.openjdk.java.net/browse/JDK-8178315 . Regards, Srinivas
RE: RFR: 8156743: ES6 for..of should work for Java Maps and Sets
Hi Attila, Thank you for the detailed review. Regards, Srinivas -Original Message- From: Attila Szegedi [mailto:szege...@gmail.com] Sent: Thursday, March 02, 2017 9:01 PM To: Srinivas Dama Cc: Nashorn-dev Subject: Re: RFR: 8156743: ES6 for..of should work for Java Maps and Sets Contrary to the issue name, I don’t see anything for handling Set in the patch, but I guess that’s because Set is a Collection and those already work? As an aside… I was looking into your use of NativeJava.from to convert a Java array to a NativeArray, and realized that we have at least 3 ways to do it, each with different drawbacks: 1. NativeJava.from as used in this patch. Its drawback is that it defensively always clones the array (here we woulnd’t need to clone it, although it doesn’t hurt) 2. Global.instance.wrapAsObject. Its drawback is that its Object[] case is further down the if/else list than NativeJava.from (5th instanceof vs. 2nd) 3. Global.instance.allocate. (This one is primarily used from generated bytecode.) Its drawback is that it iterates over the elements scanning for any ScriptRuntime.EMPTY values. I guess we could pass in a boolean flag to indicate there are no empty values to skip this scan (compiler could set the flag when it can prove the array literal it’s constructing has no empties). This doesn’t affect your patch - I think NativeJava.from() is a decent choice. So, +1. Attila. > On 02 Mar 2017, at 15:36, Srinivas Dama <srinivas.d...@oracle.com> wrote: > > Hello, > > Please review http://cr.openjdk.java.net/~sdama/8156743/webrev.00/ > for https://bugs.openjdk.java.net/browse/JDK-8156743 > > Regards, > Srinivas >
RFR: 8184239(Fix broken nashorn/samples)
Hi, Please review http://cr.openjdk.java.net/~sdama/8184239/webrev.00/ for https://bugs.openjdk.java.net/browse/JDK-8184239 Regards, Srinivas
RE: RFR: 8184241(Fix nashorn/samples/filebrowser.js)
Hi Hannes, Thank you for the comments. Here is the new patch using our own base class instead of TreeItem in test case. http://cr.openjdk.java.net/~sdama/8184241/webrev.02/ Regards, Srinivas -Original Message- From: Hannes Wallnöfer Sent: Wednesday, July 19, 2017 2:51 PM To: Srinivas Dama Cc: Nashorn-Dev Subject: Re: RFR: 8184241(Fix nashorn/samples/filebrowser.js) Hi Srini, The fix looks good, but what for is the arguments handling in the test? Also, I would prefer to use our own base class (constructor must invoke overridable method, what if TreeItem is refactored in the future?) Hannes > Am 19.07.2017 um 10:45 schrieb Srinivas Dama <srinivas.d...@oracle.com>: > > Hi, > > Please review http://cr.openjdk.java.net/~sdama/8184241/webrev.00/ for > https://bugs.openjdk.java.net/browse/JDK-8184241. > > Regards, > Srinivas
RFR: 8184241(Fix nashorn/samples/filebrowser.js)
Hi, Please review http://cr.openjdk.java.net/~sdama/8184241/webrev.00/ for https://bugs.openjdk.java.net/browse/JDK-8184241. Regards, Srinivas
RFR: 8180727(Use jdk.editpad to replace jdk.nashorn.tools.jjs.EditPad duplicated class)
Hi, Please review http://cr.openjdk.java.net/~sdama/8180727/webrev.00/ for https://bugs.openjdk.java.net/browse/JDK-8180727 Regards, Srinivas
RE: [10] RFR: 8184893: jdk8u152 b06 : issues with nashorn when running kraken benchmarks
Hi Hannes, Lower-case thumbs up. -Original Message- From: Hannes Wallnöfer Sent: Tuesday, July 25, 2017 7:14 PM To: Nashorn-dev Subject: [10] RFR: 8184893: jdk8u152 b06 : issues with nashorn when running kraken benchmarks Please review 8184893: jdk8u152 b06 : issues with nashorn when running kraken benchmarks Kraken benchmark contains some files that invokes the Array constructor with lots of number literal arguments. We assume too low code footprint for these number literals in WeighNodes.java, resulting in the function not being split. Bug: https://bugs.openjdk.java.net/browse/JDK-8184893 Webrev: http://cr.openjdk.java.net/~hannesw/8184893/webrev/ Thanks, Hannes
RE: RFR: 8184239(Fix broken nashorn/samples)
Hi, Please review the updated patch. Sorry I missed one sample earlier. http://cr.openjdk.java.net/~sdama/8184239/webrev.01/ Regards, Srinivas -Original Message- From: Jim Laskey (Oracle) Sent: Wednesday, July 12, 2017 5:34 PM To: Srinivas Dama Cc: Nashorn-Dev Subject: Re: RFR: 8184239(Fix broken nashorn/samples) +1 > On Jul 12, 2017, at 6:00 AM, Srinivas Dama <srinivas.d...@oracle.com> wrote: > > Hi, > > Please review http://cr.openjdk.java.net/~sdama/8184239/webrev.00/ for > https://bugs.openjdk.java.net/browse/JDK-8184239 > > Regards, > Srinivas
RFR: 8179891: JavaDoc for for..in is incorrect
Hi, Please review http://cr.openjdk.java.net/~sdama/8179891/webrev.00/index.html for https://bugs.openjdk.java.net/browse/JDK-8179891 Regards, Srinivas
RFR:8186011(Fix samples/java_completion.js and samples/disassemble.js)
Hi, Please review http://cr.openjdk.java.net/~sdama/8186011/webrev.00/ for https://bugs.openjdk.java.net/browse/JDK-8186011 Regards, Srinivas
RFR: 8185257(Nashorn AST is missing nodes when a for-loop contains a VariableDeclarationList)
Please review http://cr.openjdk.java.net/~sdama/8185257/webrev.00/ for https://bugs.openjdk.java.net/browse/JDK-8185257 Regards, Srinivas
Re: RFR: 8185257(Nashorn AST is missing nodes when a for-loop contains a VariableDeclarationList)
Thank you for the comments sundar. Here is the revised patch with test case modified. http://cr.openjdk.java.net/~sdama/8185257/webrev.01/ Regards, Srinivas - Original Message - From: sundararajan.athijegannat...@oracle.com To: nashorn-dev@openjdk.java.net Sent: Tuesday, September 12, 2017 6:49:09 PM GMT +05:30 Chennai, Kolkata, Mumbai, New Delhi Subject: Re: RFR: 8185257(Nashorn AST is missing nodes when a for-loop contains a VariableDeclarationList) You may want to print Tree kind in the test rather than using implementation class name (and using that it .EXPECTED file). Other than that, +1 -Sundar On 12/09/17, 4:58 PM, Srinivas Dama wrote: > Please review http://cr.openjdk.java.net/~sdama/8185257/webrev.00/ > for https://bugs.openjdk.java.net/browse/JDK-8185257 > > Regards, > Srinivas
Re: RFR:8186011(Fix samples/java_completion.js and samples/disassemble.js)
Hi, Please review migrated patch to jdk10/master repo. http://cr.openjdk.java.net/~sdama/8186011/webrev.01/ Regards, Srinivas - Original Message - From: hannes.wallnoe...@oracle.com To: srinivas.d...@oracle.com Cc: nashorn-dev@openjdk.java.net Sent: Tuesday, September 19, 2017 4:10:46 PM GMT +05:30 Chennai, Kolkata, Mumbai, New Delhi Subject: Re: RFR:8186011(Fix samples/java_completion.js and samples/disassemble.js) +1 Hannes > Am 19.09.2017 um 12:42 schrieb Sundararajan Athijegannathan > <sundararajan.athijegannat...@oracle.com>: > > +1 > > -Sundar > > On 19/09/17, 4:03 PM, Srinivas Dama wrote: >> Hi, >> >> Please review http://cr.openjdk.java.net/~sdama/8186011/webrev.00/ >> for https://bugs.openjdk.java.net/browse/JDK-8186011 >> >> Regards, >> Srinivas
RFR:8147076(LinkerCallSite.ARGLIMIT is used incorrectly)
Hi, Please review http://cr.openjdk.java.net/~sdama/8147076/webrev.00/ for https://bugs.openjdk.java.net/browse/JDK-8147076 Regards, Srinivas
Re: RFR:8147076(LinkerCallSite.ARGLIMIT is used incorrectly)
Hi, here is the revised patch http://cr.openjdk.java.net/~sdama/8147076/webrev.01/ Regards, Srinivas - Original Message - From: hannes.wallnoe...@oracle.com To: srinivas.d...@oracle.com Cc: nashorn-dev@openjdk.java.net Sent: Friday, September 29, 2017 7:33:18 PM GMT +05:30 Chennai, Kolkata, Mumbai, New Delhi Subject: Re: RFR:8147076(LinkerCallSite.ARGLIMIT is used incorrectly) Hi Srini, a few notes: - Shouldn’t the new test include an invocation with > ARGLIMIT double arguments to test it works? - No need to use Number(1.1), a number with a fractional part will result in a double. - I assume eval() in the test is to force this-object and callee to be sent, maybe add a comment about that? - there’s a typo in the test: arguements Hannes > Am 29.09.2017 um 15:34 schrieb Srinivas Dama <srinivas.d...@oracle.com>: > > Hi, > > Please review http://cr.openjdk.java.net/~sdama/8147076/webrev.00/ > for https://bugs.openjdk.java.net/browse/JDK-8147076 > > Regards, > Srinivas
Re: RFR:8147076(LinkerCallSite.ARGLIMIT is used incorrectly)
Hi Hannes, Thank you for the comments -Modified test case already covers both > ARGLIMIT and ARGLIMIT double arguments to test it works? - No need to use Number(1.1), a number with a fractional part will result in a double. - I assume eval() in the test is to force this-object and callee to be sent, maybe add a comment about that? - there’s a typo in the test: arguements Hannes > Am 29.09.2017 um 15:34 schrieb Srinivas Dama <srinivas.d...@oracle.com>: > > Hi, > > Please review http://cr.openjdk.java.net/~sdama/8147076/webrev.00/ > for https://bugs.openjdk.java.net/browse/JDK-8147076 > > Regards, > Srinivas
RE: RFR: 8185257(Nashorn AST is missing nodes when a for-loop contains a VariableDeclarationList)
Hi, Please review revised patch after migrating to jdk10/master repo. http://cr.openjdk.java.net/~sdama/8185257/webrev.02/ Regards, Srinivas -Original Message- From: Hannes Wallnöfer Sent: Wednesday, September 13, 2017 1:49 AM To: Srinivas Dama Cc: Nashorn-dev Subject: Re: RFR: 8185257(Nashorn AST is missing nodes when a for-loop contains a VariableDeclarationList) +1 Hannes > Am 12.09.2017 um 19:38 schrieb Sundararajan Athijegannathan > <sundararajan.athijegannat...@oracle.com>: > > +1 > > -Sundar > > On 12/09/17, 11:02 PM, Srinivas Dama wrote: >> Thank you for the comments sundar. >> >> Here is the revised patch with test case modified. >> http://cr.openjdk.java.net/~sdama/8185257/webrev.01/ >> >> Regards, >> Srinivas >> - Original Message - >> From: sundararajan.athijegannat...@oracle.com >> To: nashorn-dev@openjdk.java.net >> Sent: Tuesday, September 12, 2017 6:49:09 PM GMT +05:30 Chennai, >> Kolkata, Mumbai, New Delhi >> Subject: Re: RFR: 8185257(Nashorn AST is missing nodes when a >> for-loop contains a VariableDeclarationList) >> >> You may want to print Tree kind in the test rather than using >> implementation class name (and using that it .EXPECTED file). >> >> Other than that, +1 >> >> -Sundar >> >> On 12/09/17, 4:58 PM, Srinivas Dama wrote: >>> Please review http://cr.openjdk.java.net/~sdama/8185257/webrev.00/ >>> for https://bugs.openjdk.java.net/browse/JDK-8185257 >>> >>> Regards, >>> Srinivas
RE: RFR: 8184720(Nashorn engine in strict mode throws a java.lang.ClassCastException when calling apply() and passing the arguments object)
Thank you Hannes for the comments. Here is the revised patch at http://cr.openjdk.java.net/~sdama/8184720/webrev.01/ . Note: This includes additional fix from Hannes on top of existing fix to resolve assertion error while running tests from 'ant' test runner without 'scripting' option. Regards, Srinivas -Original Message- From: Hannes Wallnöfer Sent: Wednesday, August 23, 2017 12:15 PM To: Srinivas Dama Cc: Nashorn-Dev Subject: Re: RFR: 8184720(Nashorn engine in strict mode throws a java.lang.ClassCastException when calling apply() and passing the arguments object) Hi Srini, I don’t think the -scripting option is needed in the test, and you could just call the function without try-catch and printing messages. Otherwise +1 Hannes > Am 23.08.2017 um 06:38 schrieb Srinivas Dama <srinivas.d...@oracle.com>: > > Hi, > > Please review http://cr.openjdk.java.net/~sdama/8184720/webrev.00/ for > https://bugs.openjdk.java.net/browse/JDK-8184720 > > Regards, > Srinivas
RFR:8073640:Nashorn scripting: here document with only whitespace gives error
Hi, Please review http://cr.openjdk.java.net/~sdama/8073640/webrev.00/ for https://bugs.openjdk.java.net/browse/JDK-8073640 Regards, srinivas
RFR:8177691:Labeled break in catch and finally works wrongly, when invoked through nashorn
Hi, Please review http://cr.openjdk.java.net/~sdama/8177691/webrev.00/ for https://bugs.openjdk.java.net/browse/JDK-8177691 Regards, Srinivas
RFR: 8134516(Move getInvokeByName and getDynamicInvoker methods from Global to Context)
Hi, Please review http://cr.openjdk.java.net/~sdama/8134516/webrev.01/ for https://bugs.openjdk.java.net/browse/JDK-8134516 Regards, Srinivas
Re: RFR: 8134516(Move getInvokeByName and getDynamicInvoker methods from Global to Context)
Hi Attila, Thank you for the comments. I have resolved the bug as "will-not-fix" and added appropriate comments in the bug. So I am taking back this RFR. Regards, Srinivas - Original Message - From: szege...@gmail.com To: srinivas.d...@oracle.com Cc: nashorn-dev@openjdk.java.net Sent: Monday, December 11, 2017 4:45:56 PM GMT +05:30 Chennai, Kolkata, Mumbai, New Delhi Subject: Re: RFR: 8134516(Move getInvokeByName and getDynamicInvoker methods from Global to Context) You shouldn't make Global.getContext() public. Context is a security-sensitive class, and the only public access to it is supposed to be through Context.getContext(), which includes a security check. I see you have also simply disabled that security check in your patch. I’m afraid that’s unacceptable. You could make the patch much smaller (and avoid security concerns altogether) if you kept Global.getInvokeByName and Global.getDynamicInvoker methods as they are and have them internally delegate to their new equivalents in Context, adding “this” as the last argument as they do so. This is all in addition to Hannes’ observation with regard to the performance implications in his previous e-mail. Attila. > On Dec 6, 2017, at 4:47 PM, Srinivas Dama <srinivas.d...@oracle.com> wrote: > > Hi, > > Please review http://cr.openjdk.java.net/~sdama/8134516/webrev.01/ for > https://bugs.openjdk.java.net/browse/JDK-8134516 > > Regards, > Srinivas
