Re: RFR: JDK-8080679: Include jline in JDK for Java and JavaScript REPLs

2015-06-26 Thread Erik Joelsson
Hello Jan, Build changes look good. /Erik On 2015-06-25 18:25, Jan Lahoda wrote: Hello, Based on the feedback I've received so far, I've uploaded an updated version of the patch: http://cr.openjdk.java.net/~jlahoda/8080679/webrev.01/full/ Notable changes: -avoided the dependency on

Re: RFR: JDK-8080679: Include jline in JDK for Java and JavaScript REPLs

2015-06-26 Thread Jan Lahoda
Hi Alan, Thanks for the comments! A question inline. On 25.6.2015 18:38, Alan Bateman wrote: On 25/06/2015 17:25, Jan Lahoda wrote: Hello, Based on the feedback I've received so far, I've uploaded an updated version of the patch: http://cr.openjdk.java.net/~jlahoda/8080679/webrev.01/full/

Re: [9] RFR of 8042377: BufferedWriter and FilteredOutputStream.close throw IAE if flush and close throw equal exceptions

2015-06-26 Thread Alan Bateman
On 25/06/2015 18:58, Brian Burkhalter wrote: Hi Peter, Thanks for the code change suggestion. I have modified the patch to use its logic and to revert to the original boolean instance variable instead of the contentious AtomicBoolean: http://cr.openjdk.java.net/~bpb/8042377/webrev.02/ This

Re: [9] RFR of 8042377: BufferedWriter and FilteredOutputStream.close throw IAE if flush and close throw equal exceptions

2015-06-26 Thread Peter Levart
On 06/26/2015 09:52 AM, Alan Bateman wrote: On 25/06/2015 18:58, Brian Burkhalter wrote: Hi Peter, Thanks for the code change suggestion. I have modified the patch to use its logic and to revert to the original boolean instance variable instead of the contentious AtomicBoolean:

Re: [9] RFR of 8042377: BufferedWriter and FilteredOutputStream.close throw IAE if flush and close throw equal exceptions

2015-06-26 Thread Alan Bateman
On 26/06/2015 09:17, Peter Levart wrote: : Quite the contrary, previously, if flush() threw any unchecked exception (ThreadDeath included), it was ignored if out.close() also threw (IOException or any unchecked exception). Yes, this is what I was trying to say. +

Re: [9] RFR(M): 8076112: Add @HotSpotIntrinsicCandidate annotation to indicate methods for which Java Runtime has intrinsics

2015-06-26 Thread Alan Bateman
On 25/06/2015 12:49, Zoltán Majó wrote: Hi, please review the patch for JDK-8076112. Bug: https://bugs.openjdk.java.net/browse/JDK-8076112 Problem: There is need to indicate Java methods that are potentially intrinsified by JVM. Solution: Mark intrinsified methods with the

Re: RFR: JDK-8080679: Include jline in JDK for Java and JavaScript REPLs

2015-06-26 Thread Alan Bateman
On 26/06/2015 08:58, Jan Lahoda wrote: : The native method readKeyEvent seems to do a FindClass per key event. Maybe this is from the upstream project but I would think it would be more efficient to cache this in a global ref. It would also be more I will work on this. Related to this is

Re: [9] RFR(M): 8076112: Add @HotSpotIntrinsicCandidate annotation to indicate methods for which Java Runtime has intrinsics

2015-06-26 Thread Zoltán Majó
Hi Alan, thank you for the review! Please see my answers/comments below. On 06/26/2015 10:39 AM, Alan Bateman wrote: On 25/06/2015 12:49, Zoltán Majó wrote: Hi, please review the patch for JDK-8076112. Bug: https://bugs.openjdk.java.net/browse/JDK-8076112 Problem: There is need to

Re: Review request: JDK-8080108: [TEST_BUG] The case failed automatically and thrown java.lang.RuntimeException

2015-06-26 Thread Alexander Fomin
reminder On 25.06.2015 11:54, Alexander Fomin wrote: Hello, Please review the test bug fix https://bugs.openjdk.java.net/browse/JDK-8080108 webrev attached. Either you can see it by the link http://stt-13.ru.oracle.com/newroot/tmp/alex/regression/JDK-8080108/JDK9/dev/jdk/webrev/ The

Re: RFR: 8129601: [launcher] test VersionCheck.java fails with new version string (more removal of dead code)

2015-06-26 Thread Kumar Srinivasan
On 6/26/2015 7:31 AM, Alan Bateman wrote: On 26/06/2015 15:20, Kumar Srinivasan wrote: Hi, Please review fix to remove unused dead code, this was needed by mJRE, but with the removal of that, this also is no longer necessary and will make the build infra simpler as individual version numbers

RFR: 8129601: [launcher] test VersionCheck.java fails with new version string (more removal of dead code)

2015-06-26 Thread Kumar Srinivasan
Hi, Please review fix to remove unused dead code, this was needed by mJRE, but with the removal of that, this also is no longer necessary and will make the build infra simpler as individual version numbers info. need not be passed around in the build system. Thanks Kumar Bug:

Re: RFR: 8129601: [launcher] test VersionCheck.java fails with new version string (more removal of dead code)

2015-06-26 Thread Alan Bateman
On 26/06/2015 15:20, Kumar Srinivasan wrote: Hi, Please review fix to remove unused dead code, this was needed by mJRE, but with the removal of that, this also is no longer necessary and will make the build infra simpler as individual version numbers info. need not be passed around in the

Re: RFR - 8129956: jaxp: CodeSource.getLocation() might return null

2015-06-26 Thread Lance Andersen
Hi Daniel, This looks OK to me Best Lance On Jun 26, 2015, at 6:45 AM, Daniel Fuchs daniel.fu...@oracle.com wrote: Hi, It was brought to my attention that CodeSource.getLocation() might return null. The javadoc is not very clear on the subject, leading me to believe it could not. Here

Re: [9] RFR of 8042377: BufferedWriter and FilteredOutputStream.close throw IAE if flush and close throw equal exceptions

2015-06-26 Thread Brian Burkhalter
On Jun 26, 2015, at 1:22 AM, Alan Bateman alan.bate...@oracle.com wrote: That would be better and would deal with the concern with webrev.02. Here’s an updated version incorporating Peter’s ThreadDeath change: http://cr.openjdk.java.net/~bpb/8042377/webrev.03/ Thanks, Brian

Re: RFR: JDK-8080679: Include jline in JDK for Java and JavaScript REPLs

2015-06-26 Thread Alan Bateman
On 26/06/2015 14:38, Jan Lahoda wrote: Uploaded a new iteration of the patch, reflecting the comments so far, here: http://cr.openjdk.java.net/~jlahoda/8080679/webrev.02/full/ A delta patch from previous iteration: http://cr.openjdk.java.net/~jlahoda/8080679/webrev.02/delta/ Any feedback on

Re: [9] RFR(M): 8076112: Add @HotSpotIntrinsicCandidate annotation to indicate methods for which Java Runtime has intrinsics

2015-06-26 Thread james cheng
Hi Zoltán, I am not a reviewer. Just saw some typos in code comments: Here is the updated webrev: - top: http://cr.openjdk.java.net/~zmajo/8076112/top/webrev.06/ - jdk: http://cr.openjdk.java.net/~zmajo/8076112/jdk/webrev.06/ in SHA*.java, 'implCompressImpl', as parameter the method -

Re: JEP 132: More-prompt finalization

2015-06-26 Thread Kim Barrett
On Jun 22, 2015, at 3:33 AM, Peter Levart peter.lev...@gmail.com wrote: Hi Kim, Sorry, I just noticed you'd sent an update a few days ago. I don't have time to look carefully today, but a few quick comments. On 06/09/2015 07:44 AM, Peter Levart wrote: On 06/09/2015 04:03 AM, Kim Barrett

RFR: 8050091: LinkedList has incorrect implementation comment

2015-06-26 Thread Martin Buchholz
Hi Ivan, I'd like you to do a code review. https://bugs.openjdk.java.net/browse/JDK-8050091 http://cr.openjdk.java.net/~martin/webrevs/openjdk9/LinkedList-invariant/

RFR: 6260652: (coll) Arrays.asList(x).toArray().getClass() should be Object[].class

2015-06-26 Thread Martin Buchholz
10 years later ... still asking for approval to commit. https://bugs.openjdk.java.net/browse/JDK-6260652 http://cr.openjdk.java.net/~martin/webrevs/openjdk9/Arrays.asList.toArray/

Re: RFR: 6260652: (coll) Arrays.asList(x).toArray().getClass() should be Object[].class

2015-06-26 Thread Remi Forax
On 06/26/2015 11:54 PM, Martin Buchholz wrote: 10 years later ... still asking for approval to commit. https://bugs.openjdk.java.net/browse/JDK-6260652 http://cr.openjdk.java.net/~martin/webrevs/openjdk9/Arrays.asList.toArray/ +1 I was bitten once by the dubious implementation of toArray(),

Re: RFR: 6260652: (coll) Arrays.asList(x).toArray().getClass() should be Object[].class

2015-06-26 Thread Martin Buchholz
On Fri, Jun 26, 2015 at 3:46 PM, Remi Forax fo...@univ-mlv.fr wrote: as a matter of style, i prefer to always have curly braces after an 'if' Community opinion is moving your way, but I am holding out. and i think that the cast to (ClassT[]) is no longer needed (and false if one day

Re: RFR: 8129601: [launcher] test VersionCheck.java fails with new version string (more removal of dead code)

2015-06-26 Thread Joseph D. Darcy
Looks fine Kumar; cheers, -Joe On 6/26/2015 7:20 AM, Kumar Srinivasan wrote: Hi, Please review fix to remove unused dead code, this was needed by mJRE, but with the removal of that, this also is no longer necessary and will make the build infra simpler as individual version numbers info. need

Re: RFR: JDK-8080679: Include jline in JDK for Java and JavaScript REPLs

2015-06-26 Thread Jan Lahoda
Uploaded a new iteration of the patch, reflecting the comments so far, here: http://cr.openjdk.java.net/~jlahoda/8080679/webrev.02/full/ A delta patch from previous iteration: http://cr.openjdk.java.net/~jlahoda/8080679/webrev.02/delta/ Any feedback on this would be appreciated! Thanks for the