Re: RFR v4 - 8027634: Support @argfiles for java command-line tool
On Aug 13, 2015, at 9:38 AM, Kumar Srinivasan kumar.x.sriniva...@oracle.com
wrote:
3. void JLI_InitArgProcessing(jboolean isJava, jboolean disableArgFile) {
// No expansion for relaunch
The launcher might re-exec itself, this might happen under certain
circumstances on *nix, is the above logic to prevent subsequent
expansion on relaunch ? However when a re-exec occurs, the
expanded args will be sent to the second invocation of the launcher.
So what does relaunch mean ? in your comment ?
Right, relaunch means that re-exec. As the first time, the arguments has
expanded, we don’t want to do that again for succeeding runs.
For example an argument @@arg, it will become @arg at the second execution
because of escape. That should not be expanded again.
Cheers,
Henry
Re: RFR v4 - 8027634: Support @argfiles for java command-line tool
On Aug 12, 2015, at 7:55 AM, Kumar Srinivasan kumar.x.sriniva...@oracle.com
wrote:
Henry,
Generally looks good here are some comments, on my initial
pass, I am not fully done with args.c I will look at this some
more later today or tomorrow.
4.)
expectingNoDash is expectingMain right ? if so I would rename it so.
Not really, it is expecting a argument without dash, such as -cp or -classpath.
TestHelper.java:
Why add findInOutput method ? this seems to be identical
to matches at line 606, which does exactly that.
To return a string, this is used when trying to match a pattern and get
information from that line, I used to get the starting index for an argument in
verifyOptions.
Guess I can remove that now as each use of verifyOptions all starts from 1.
ArgFileSyntax.java
Nice, but I think the creation and storage of test case can be simplified,
basically loadCases(), also please avoid Xbootclasspath. The trouble
with these tests which operate loops, are very painful to debug
and isolate a failing case, on the night before GA.
I wrote that because I start with testng as a DataProvider, we can change that.
Can we do something like this.
MapString, ListString tests = new ArrayList();
tests.put(testing quotes, Array.asList({{...}, {}});
tests.add(no recurse expansion, Array.asList({{...}, {}});
The execute each test, with a description of what the test is doing.
This way if a test fails its easy to zero in on the failing test.
verifyOutput will print out what line is not matching, so it’s pretty easy to
identify which case failed.
Cheers,
Henry
Re: RFR v4 - 8027634: Support @argfiles for java command-line tool
Hi, Another update for argument file support, this version we added a couple measures to ensure compatibility, 1) Compile time directive ENABLE_ARG_FILES is needed to enable the feature, only java is enabled with this webrev. 2) Escape @argument if so desired: Additional prefixes of @ to a @ prefixed option will act as an escape, ie. the first @ will be removed and the rest of the arguments presented to the launcher literally. For example: java @ foo gets arguments of @ foo java @filename, filename expanded java @@filename will have the leading @ removed and reduced to literal @filename similarly, java @@@filenamewill have the leading @ removed and reduced to literal @@filename 3) The option '-Xdisable-@files' can be used on the command line anywhere to prevent further @filename expansion. 4) Limit the file size must not exceed MAXINT (2, 147, 483, 647) bytes. Webrev at http://cr.openjdk.java.net/~henryjen/jdk9/8027634/v4/webrev/ Cheers, Henry
RFR v2 - 8027634: Support @argfiles for java command-line tool
Thanks Mandy and Magnus for looking into the patch, and feedbacks from Kumar Srinivasan and Michel Trudeau. We have an updated version to be reviewed at, http://cr.openjdk.java.net/~henryjen/jdk9/8027634/v2/webrev/ This version changes the syntax a little bit as following, but pretty much still javac compatible, 1) Whitespace characters are ‘ ‘, ‘\t’. ‘\n’ and ‘\r’, not everything from ‘\0\ to ‘ ‘. 2) Open quote still stop at EOL unless ‘\’ is the last character, which will join the next line by removing leading white spaces. This allows multiple line supports in quote. If leading space is significant, add ‘\’ to escape the white space character. 3) Escape sequences is now only limited to ‘\n’ and ‘\r’. No octal character support. In terms of javac compatibility, unless octal or \a\b\f\t\v escaping sequence are used, they should be completely compatible. I don’t have plan to remove the test as it would be convenient to have it around. Move it into a separate test is probably desirable, though. I am not sure about what is a better place, seems to me test inside implementation file is quite common as in wildcard.c and cmdtoargs.c. Cheers, Henry On Jul 15, 2015, at 7:51 AM, Mandy Chung mandy.ch...@oracle.com wrote: On Jul 10, 2015, at 11:48 AM, Henry Jen henry@oracle.com wrote: Hi, Please review proposed patch for JDK-8027634[1]. This patch is to enable java support command line argument file like javac does. The implementation use the same syntax rule, which is implemented in CommandLine.java[3] with java.io.StreamTokenizer. Some early comment is that we probably don’t need such complexity to support same syntax, also require to quote whole token is a little inconvenient. For example, must be -cp “c:\\foo bar\\lib;c:\\lib” instead of -cp c:\”foo bar”\lib;c:\lib. I am debating if such compatibility is necessary useful, after all, easy and intuitive is more important, and with simpler rule, the implementation will be cleaner as well. I have a slight preference to maintain consistent syntax as javac @argfile support in terms of the quotation. A user could use the same path specified in -cp for both javac @argfile an java @argfile use. I skimmed on the webrev and looks okay to me. I’ll leave it for Kumar to do detailed review. One minor comment: args.c Are you planning to remove the test within #ifdef DEBUG_ARGFILE block? thanks Mandy
Re: RFR - 8027634: Support @argfiles for java command-line tool
Sigh, forgot the link to the webrev again. http://cr.openjdk.java.net/~henryjen/jdk9/8027634/webrev/ Cheers, Henry On Jul 9, 2015, at 8:48 PM, Henry Jen henry@oracle.com wrote: Hi, Please review proposed patch for JDK-8027634[1]. This patch is to enable java support command line argument file like javac does. The implementation use the same syntax rule, which is implemented in CommandLine.java[3] with java.io.StreamTokenizer. Some early comment is that we probably don’t need such complexity to support same syntax, also require to quote whole token is a little inconvenient. For example, must be -cp “c:\\foo bar\\lib;c:\\lib” instead of -cp c:\”foo bar”\lib;c:\lib. I am debating if such compatibility is necessary useful, after all, easy and intuitive is more important, and with simpler rule, the implementation will be cleaner as well. Anyhow, with the patch out, at least developer can build idk and have something to test with to see if this can fulfill their use cases. Also, for tools other than java that use launcher, it’s possible to use -J@argfile to pass arguments. For example, if want to pass -J options to javac, it’s now possible to do so with javac -J@argfile, and put -J options in the argfile. If the consensus is that such syntax compatibility is not important, we will go ahead remove the escaping support(except probably enable escape for quote itself) in quote, and maybe add support of quote within a token. CCing build-dev for build changes, jdk9-dev for wider audience for tools. Cheers, Henry [1] https://bugs.openjdk.java.net/browse/JDK-8027634 [2] http://docs.oracle.com/javase/7/docs/technotes/tools/windows/javac.html#commandlineargfile [3] http://hg.openjdk.java.net/jdk9/jdk9/langtools/file/03e083639ee9/src/jdk.compiler/share/classes/com/sun/tools/javac/main/CommandLine.java
RFR - 8027634: Support @argfiles for java command-line tool
Hi, Please review proposed patch for JDK-8027634[1]. This patch is to enable java support command line argument file like javac does. The implementation use the same syntax rule, which is implemented in CommandLine.java[3] with java.io.StreamTokenizer. Some early comment is that we probably don’t need such complexity to support same syntax, also require to quote whole token is a little inconvenient. For example, must be -cp “c:\\foo bar\\lib;c:\\lib” instead of -cp c:\”foo bar”\lib;c:\lib. I am debating if such compatibility is necessary useful, after all, easy and intuitive is more important, and with simpler rule, the implementation will be cleaner as well. Anyhow, with the patch out, at least developer can build idk and have something to test with to see if this can fulfill their use cases. Also, for tools other than java that use launcher, it’s possible to use -J@argfile to pass arguments. For example, if want to pass -J options to javac, it’s now possible to do so with javac -J@argfile, and put -J options in the argfile. If the consensus is that such syntax compatibility is not important, we will go ahead remove the escaping support(except probably enable escape for quote itself) in quote, and maybe add support of quote within a token. CCing build-dev for build changes, jdk9-dev for wider audience for tools. Cheers, Henry [1] https://bugs.openjdk.java.net/browse/JDK-8027634 [2] http://docs.oracle.com/javase/7/docs/technotes/tools/windows/javac.html#commandlineargfile [3] http://hg.openjdk.java.net/jdk9/jdk9/langtools/file/03e083639ee9/src/jdk.compiler/share/classes/com/sun/tools/javac/main/CommandLine.java
Re: 8076442: Cannot fully read BitSet.stream() if bit Integer.MAX_VALUE is set
Looks fine to me, thanks for taking care of it. Cheers, Henry On 04/03/2015 02:57 AM, Chris Hegarty wrote: This is a request for review for a small change to address 8076442 [1]. Seems to be a day one bug in the internal BitSetIterator, for a boundary condition when the Integer.MAX_VALUE bit is set. The change to the iterator, and test, is in line with the advise in BitSet.nextSetBit(int) ( break/terminate processing after reaching an index of Integer.MAX_VALUE ). http://cr.openjdk.java.net/~chegar/8076442/webrev/ -Chris. [1] https://bugs.openjdk.java.net/browse/JDK-8076442
Re: RFR: 8047724: @since tag cleanup in jaxws
Hi Micoslav, What do you think about the webrev[1]? Does it make sense for upstream maintainers? [1] http://cr.openjdk.java.net/~henryjen/jdk9/8047724/0/webrev/ Cheers, Henry On 06/30/2014 03:14 PM, Alan Bateman wrote: On 30/06/2014 02:30, Henry Jen wrote: Ping. Cheers, Henry Henry - the code in the jaxws repository is maintained upstream. If you change it in the jaxws repository then the changes will likely get overridden at the next sync-up. I'd suggest working with Miroslav (cc'ed) to get the changes pushed upstream first. -Alan.
Re: RFR: 8044862: Fix raw and unchecked lint warnings in macosx specific code
Thanks, Petr. I worked out AquaFileChooserUI and updated the webrev to adapt comments from both you and Joe. The updated webrev is at, http://cr.openjdk.java.net/~henryjen/jdk9/8044862/1/webrev/ Cheers, Henry On 07/09/2014 01:07 AM, Petr Pchelko wrote: Hello, Henry. AquaComboBoxRendererInternal : why ? extends Object and not just ?? AquaComboBoxUI:427, 510, 530 - you can replace JComboBoxObject with JComboBox? and avoid a @SupressWarning annotation. AquaFileChooserUI.createDirectoryComboBoxRenderer actually returns a ListCellRendererFile, DirectoryComboBoxModel is a File model, and a combobox contains only Files. I suggest to try to genereficate this correctly and look where it brings us. Also I didn't look at the KeychainStore.java as I know nothing about this code. With best regards. Petr. On 09 июля 2014 г., at 7:30, Henry Jen henry@oracle.com wrote: Hi, Would someone from swing team care to take a look at this? The change of src/macosx/classes/sun/font/CFontConfiguration.java will be dropped as it had been fixed in JDK-8048980. Cheers, Henry On 06/27/2014 03:00 PM, Joe Darcy wrote: Hi Henry, Your changes look good to me (I assume there was an issue using wildcards in AquaComboBoxButton.java). Thanks, -Joe On 06/27/2014 02:45 PM, Henry Jen wrote: Hi, Please review a webrev for fixing rawtypes and unchecked lint warning for macosx specific java code, this webrev is depending on JDK-8043548[1] where some of the override methods are defined, Also as the src/macosx/classes/apple/launcher/JavaAppLauncher.java to be removed from JDK-8048337, the webrev omits that file. The webrev can be found at, http://cr.openjdk.java.net/~henryjen/jdk9/8044862/0/webrev/ [1] https://bugs.openjdk.java.net/browse/JDK-8043548 Cheers, Henry
Re: RFR: 8042872: Fix raw and unchecked warnings in sun.applet
Thanks Joe for the review. Would someone from deploy team have a quick look so we can push this? Cheers, Henry On 06/23/2014 06:03 PM, Joe Darcy wrote: Hi Henry, The changes look good to me; thanks, -Joe On 06/23/2014 05:31 PM, Henry Jen wrote: Hi, Please review another webrev to clean up rawtypes and unchecked warning, this time for sun.applet. The webrev applied to both jdk9/dev and jdk9/client cleanly, I am not sure which repo should the webrev go once pass review. Webrev can be found at http://cr.openjdk.java.net/~henryjen/jdk9/8042872/0/webrev/ Please also let me know any particular test I should do other than regular jprt. Cheers, Henry
Re: RFR: 8044862: Fix raw and unchecked lint warnings in macosx specific code
Hi, Would someone from swing team care to take a look at this? The change of src/macosx/classes/sun/font/CFontConfiguration.java will be dropped as it had been fixed in JDK-8048980. Cheers, Henry On 06/27/2014 03:00 PM, Joe Darcy wrote: Hi Henry, Your changes look good to me (I assume there was an issue using wildcards in AquaComboBoxButton.java). Thanks, -Joe On 06/27/2014 02:45 PM, Henry Jen wrote: Hi, Please review a webrev for fixing rawtypes and unchecked lint warning for macosx specific java code, this webrev is depending on JDK-8043548[1] where some of the override methods are defined, Also as the src/macosx/classes/apple/launcher/JavaAppLauncher.java to be removed from JDK-8048337, the webrev omits that file. The webrev can be found at, http://cr.openjdk.java.net/~henryjen/jdk9/8044862/0/webrev/ [1] https://bugs.openjdk.java.net/browse/JDK-8043548 Cheers, Henry
RFR: 8049109: Add @since 1.9 to new packages added in jaxp
Hi, Related to JDK-8023276[1] where we added two packages to JDK 9, we should update @since to have 1.9 value as well. http://cr.openjdk.java.net/~henryjen/jdk9/8049109/0/webrev/ [1] https://bugs.openjdk.java.net/browse/JDK-8023276 Cheers, Henry
Re: RFR: 8049109: Add @since 1.9 to new packages added in jaxp
I don't have a strong preference in this case, and I hear you that those classes are public and can be accessed in 1.5 even though it's not part of API specification of JDK. Considered DOM Level 2 is still listed as secondary information, and as the document you pointed out, 1.9 is used because that those APIs will become official in JDK 9. Lets give it a couple days before commit this, hopefuly some dicussion(along with other cases brought up in this thread[1]) will occur and lead to a concensus. Sometime I feel we need a supreme court for Java affairs to come out the final ruling on such stuff. :) [1] http://mail.openjdk.java.net/pipermail/core-libs-dev/2014-July/027503.html Cheers, Henry On 07/02/2014 05:12 PM, huizhe wang wrote: These packages were defined in JDK 1.5 / JAXP 1.3 (JSR 206). In that sense, they were in JDK since 1.5. For reasons I don't know, they were excluded from the Java API Documentation. From the tag conventions http://www.oracle.com/technetwork/java/javase/documentation/index-137868.html#@since [1], you may be right since it Specify the product version when the Java name was added to the API specification. But it can be confusing to users since they've been using them since JDK 1.5 [1] http://www.oracle.com/technetwork/java/javase/documentation/index-137868.html#@since -Joe On 7/2/2014 3:43 PM, Joe Darcy wrote: Looks good Henry; cheers, -Joe On 07/02/2014 02:45 PM, Henry Jen wrote: Hi, Related to JDK-8023276[1] where we added two packages to JDK 9, we should update @since to have 1.9 value as well. http://cr.openjdk.java.net/~henryjen/jdk9/8049109/0/webrev/ [1] https://bugs.openjdk.java.net/browse/JDK-8023276 Cheers, Henry
Re: RFR: 8037948: Improve documentation for org.w3c.dom package
On 06/30/2014 12:26 PM, Lance Andersen wrote: On Jun 30, 2014, at 3:22 PM, huizhe wang huizhe.w...@oracle.com wrote: On 6/30/2014 12:20 PM, Lance Andersen wrote: Looks OK Joe, noticed the @since changed as well? Thanks for review. Yes. The support for DOM L3 was since JDK 1.5. Previously this package file mentioned only DOM L2. OK, thank you. Just seemed strange that the overall version was changed as I suspect L3 is compatible with L2. I might have just indicated that L3 was added in 1.5 as the support was initially in 1.4 I believe. I have no strong preference though I agree with Lance here, the package itself is included in JDK since 1.4. In case classes under this package is not properly labeled with @since tag, they will be considered has the same @since as package.html, then that would be really misleading. Cheers, Henry
Re: RFR: 8047722: @since tag cleanup in corba
Ping. Cheers, Henry On 06/20/2014 02:28 PM, Henry Jen wrote: Hi, Please review a trivial webrev for jdk9/corba that do the same @since tag normalization as in jdk9/jdk. http://cr.openjdk.java.net/~henryjen/jdk9/8047722/0/webrev/ Cheers, Henry
Re: RFR: 8047724: @since tag cleanup in jaxws
Ping. Cheers, Henry On 06/20/2014 02:41 PM, Henry Jen wrote: Hi, Please review a trivial webrev for jdk9/jaxws that do the same @since tag normalization as in jdk9/jdk. http://cr.openjdk.java.net/~henryjen/jdk9/8047724/0/webrev/ I am not sure if there is another mailing list thatis more appropriate for this, if so, please direct me to the right ml. Thanks. Cheers, Henry
RFR: 8044862: Fix raw and unchecked lint warnings in macosx specific code
Hi, Please review a webrev for fixing rawtypes and unchecked lint warning for macosx specific java code, this webrev is depending on JDK-8043548[1] where some of the override methods are defined, Also as the src/macosx/classes/apple/launcher/JavaAppLauncher.java to be removed from JDK-8048337, the webrev omits that file. The webrev can be found at, http://cr.openjdk.java.net/~henryjen/jdk9/8044862/0/webrev/ [1] https://bugs.openjdk.java.net/browse/JDK-8043548 Cheers, Henry
Re: RFR: 8044862: Fix raw and unchecked lint warnings in macosx specific code
On June 27, 2014 at 3:00:13 PM, Joe Darcy (joe.da...@oracle.com(mailto:joe.da...@oracle.com)) wrote: Hi Henry, Your changes look good to me (I assume there was an issue using wildcards in AquaComboBoxButton.java). Cannot make it to work. The problematic code is in line 169, 169 final ListCellRendererObject renderer = comboBox.getRenderer(); 170 171 // fake it out! not renderPressed 172 final Component c = renderer.getListCellRendererComponent(list, comboBox.getSelectedItem(), -1, false, false); The above code involve following two APIs, ListCellRenderer? super E JComboBoxE::getRenderer(); Component ListCellRendererE::getListCellRendererComponent(JList? extends E list, E value, int index, boolean isSelected, boolean cellHasFocus); Feels like ? extends Object should work, but javac still complains, /Users/henryjen/ws/9dev/jdk/src/macosx/classes/com/apple/laf/AquaComboBoxButton.java:169: error: incompatible types: ListCellRendererCAP#1 cannot be converted to ListCellRenderObject final ListCellRendererObject renderer = comboBox.getRenderer(); ^ where CAP#1,CAP#2 are fresh type-variables: CAP#1 extends Object super: CAP#2 from capture of ? super CAP#2 CAP#2 extends Object from capture of ? extends Object Cheers, Henry Thanks, -Joe On 06/27/2014 02:45 PM, Henry Jen wrote: Hi, Please review a webrev for fixing rawtypes and unchecked lint warning for macosx specific java code, this webrev is depending on JDK-8043548[1] where some of the override methods are defined, Also as the src/macosx/classes/apple/launcher/JavaAppLauncher.java to be removed from JDK-8048337, the webrev omits that file. The webrev can be found at, http://cr.openjdk.java.net/~henryjen/jdk9/8044862/0/webrev/ [1] https://bugs.openjdk.java.net/browse/JDK-8043548 Cheers, Henry
RFR: 8048021: Remove @version tag in jaxp repo
Hi, As a follow up task, please review the trivial webrev to remove @version tags used with SCM, which is likely to be meaningless to javadoc readers. In this webrev, we only remove @version with $Id or $Revision for .jave or package.html cause appearance to javadoc. We keep those in .properties files also those @version tags with specific version information. Cheers, Henry On 06/20/2014 05:24 PM, Henry Jen wrote: Looks like it's pretty easy to do a find and replace in files in jaxp folder. If this is desired, we can do it in a separate webrev to be clear on changeset history? Cheers, Henry On 06/20/2014 03:36 PM, huizhe wang wrote: Hi Henry, Is it possible to add to the @since tool/script to remove repository revision notes? They appear in the API document as if they reflect meaningful version. An example in http://cr.openjdk.java.net/~henryjen/jdk9/8047723/0/webrev/src/javax/xml/parsers/DocumentBuilderFactory.java.udiff.html * @version $Revision: 1.9 $, $Date: 2010/05/25 16:19:44 $
Re: RFR: 8047721: @since should have JDK version
OK, I'll remove all @since JCE line, as the class already has @since 1.4
as Joe pointed out earlier.
Uodated webrev at
http://cr.openjdk.java.net/~henryjen/jdk9/8047721/2/webrev/
Cheers,
Henry
On 06/23/2014 10:04 AM, Bradford Wetmore wrote:
I would prefer that JCE1.2 be pulled out completely in the Cipher*
classes. I will be sending you a separate note about JCE logistics.
Thanks for doing this cleanup.
Brad
On 6/20/2014 11:46 AM, Henry Jen wrote:
Hi,
Please review a trivial webrev to add JDK version to @since in a format
as Mark suggested[1].
http://cr.openjdk.java.net/~henryjen/jdk9/8047721/0/webrev/
[1] http://mail.openjdk.java.net/pipermail/jdk9-dev/2014-June/000806.html
Appened is the diff as in the webrev.
Cheers,
Henry
diff --git a/src/share/classes/java/lang/Package.java
b/src/share/classes/java/lang/Package.java
--- a/src/share/classes/java/lang/Package.java
+++ b/src/share/classes/java/lang/Package.java
@@ -107,6 +107,7 @@
* loader to be found.
*
* @see ClassLoader#definePackage
+ * @since 1.2
*/
public class Package implements java.lang.reflect.AnnotatedElement {
/**
diff --git a/src/share/classes/javax/crypto/CipherInputStream.java
b/src/share/classes/javax/crypto/CipherInputStream.java
--- a/src/share/classes/javax/crypto/CipherInputStream.java
+++ b/src/share/classes/javax/crypto/CipherInputStream.java
@@ -170,7 +170,7 @@
* @return the next byte of data, or code-1/code if the end
of the
* stream is reached.
* @exception IOException if an I/O error occurs.
- * @since JCE1.2
+ * @since 1.4, JCE1.2
*/
public int read() throws IOException {
if (ostart = ofinish) {
@@ -196,7 +196,7 @@
* the stream has been reached.
* @exception IOException if an I/O error occurs.
* @seejava.io.InputStream#read(byte[], int, int)
- * @since JCE1.2
+ * @since 1.4, JCE1.2
*/
public int read(byte b[]) throws IOException {
return read(b, 0, b.length);
@@ -217,7 +217,7 @@
* the stream has been reached.
* @exception IOException if an I/O error occurs.
* @seejava.io.InputStream#read()
- * @since JCE1.2
+ * @since 1.4, JCE1.2
*/
public int read(byte b[], int off, int len) throws IOException {
if (ostart = ofinish) {
@@ -254,7 +254,7 @@
* @param n the number of bytes to be skipped.
* @return the actual number of bytes skipped.
* @exception IOException if an I/O error occurs.
- * @since JCE1.2
+ * @since 1.4, JCE1.2
*/
public long skip(long n) throws IOException {
int available = ofinish - ostart;
@@ -277,7 +277,7 @@
* @return the number of bytes that can be read from this
input stream
* without blocking.
* @exception IOException if an I/O error occurs.
- * @since JCE1.2
+ * @since 1.4, JCE1.2
*/
public int available() throws IOException {
return (ofinish - ostart);
@@ -292,7 +292,7 @@
* stream.
*
* @exception IOException if an I/O error occurs.
- * @since JCE1.2
+ * @since 1.4, JCE1.2
*/
public void close() throws IOException {
if (closed) {
@@ -321,7 +321,7 @@
* codemark/code and codereset/code methods.
* @see java.io.InputStream#mark(int)
* @see java.io.InputStream#reset()
- * @since JCE1.2
+ * @since 1.4, JCE1.2
*/
public boolean markSupported() {
return false;
diff --git a/src/share/classes/javax/crypto/CipherOutputStream.java
b/src/share/classes/javax/crypto/CipherOutputStream.java
--- a/src/share/classes/javax/crypto/CipherOutputStream.java
+++ b/src/share/classes/javax/crypto/CipherOutputStream.java
@@ -114,7 +114,7 @@
*
* @param b the codebyte/code.
* @exception IOException if an I/O error occurs.
- * @since JCE1.2
+ * @since 1.4, JCE1.2
*/
public void write(int b) throws IOException {
ibuffer[0] = (byte) b;
@@ -138,7 +138,7 @@
* @exception NullPointerException if codeb/code is null.
* @exception IOException if an I/O error occurs.
* @seejavax.crypto.CipherOutputStream#write(byte[], int,
int)
- * @since JCE1.2
+ * @since 1.4, JCE1.2
*/
public void write(byte b[]) throws IOException {
write(b, 0, b.length);
@@ -152,7 +152,7 @@
* @param off the start offset in the data.
* @param len the number of bytes to write.
* @exception IOException if an I/O error occurs.
- * @since JCE1.2
+ * @since 1.4, JCE1.2
*/
public void write(byte b[], int off, int len) throws IOException {
obuffer = cipher.update(b, off, len);
@@ -174,7 +174,7 @@
* the cipher's block size, no bytes
RFR: 8042872: Fix raw and unchecked warnings in sun.applet
Hi, Please review another webrev to clean up rawtypes and unchecked warning, this time for sun.applet. The webrev applied to both jdk9/dev and jdk9/client cleanly, I am not sure which repo should the webrev go once pass review. Webrev can be found at http://cr.openjdk.java.net/~henryjen/jdk9/8042872/0/webrev/ Please also let me know any particular test I should do other than regular jprt. Cheers, Henry
RFR: 8047721: @since should have JDK version
Hi,
Please review a trivial webrev to add JDK version to @since in a format
as Mark suggested[1].
http://cr.openjdk.java.net/~henryjen/jdk9/8047721/0/webrev/
[1] http://mail.openjdk.java.net/pipermail/jdk9-dev/2014-June/000806.html
Appened is the diff as in the webrev.
Cheers,
Henry
diff --git a/src/share/classes/java/lang/Package.java
b/src/share/classes/java/lang/Package.java
--- a/src/share/classes/java/lang/Package.java
+++ b/src/share/classes/java/lang/Package.java
@@ -107,6 +107,7 @@
* loader to be found.
*
* @see ClassLoader#definePackage
+ * @since 1.2
*/
public class Package implements java.lang.reflect.AnnotatedElement {
/**
diff --git a/src/share/classes/javax/crypto/CipherInputStream.java
b/src/share/classes/javax/crypto/CipherInputStream.java
--- a/src/share/classes/javax/crypto/CipherInputStream.java
+++ b/src/share/classes/javax/crypto/CipherInputStream.java
@@ -170,7 +170,7 @@
* @return the next byte of data, or code-1/code if the end
of the
* stream is reached.
* @exception IOException if an I/O error occurs.
- * @since JCE1.2
+ * @since 1.4, JCE1.2
*/
public int read() throws IOException {
if (ostart = ofinish) {
@@ -196,7 +196,7 @@
* the stream has been reached.
* @exception IOException if an I/O error occurs.
* @seejava.io.InputStream#read(byte[], int, int)
- * @since JCE1.2
+ * @since 1.4, JCE1.2
*/
public int read(byte b[]) throws IOException {
return read(b, 0, b.length);
@@ -217,7 +217,7 @@
* the stream has been reached.
* @exception IOException if an I/O error occurs.
* @seejava.io.InputStream#read()
- * @since JCE1.2
+ * @since 1.4, JCE1.2
*/
public int read(byte b[], int off, int len) throws IOException {
if (ostart = ofinish) {
@@ -254,7 +254,7 @@
* @param n the number of bytes to be skipped.
* @return the actual number of bytes skipped.
* @exception IOException if an I/O error occurs.
- * @since JCE1.2
+ * @since 1.4, JCE1.2
*/
public long skip(long n) throws IOException {
int available = ofinish - ostart;
@@ -277,7 +277,7 @@
* @return the number of bytes that can be read from this
input stream
* without blocking.
* @exception IOException if an I/O error occurs.
- * @since JCE1.2
+ * @since 1.4, JCE1.2
*/
public int available() throws IOException {
return (ofinish - ostart);
@@ -292,7 +292,7 @@
* stream.
*
* @exception IOException if an I/O error occurs.
- * @since JCE1.2
+ * @since 1.4, JCE1.2
*/
public void close() throws IOException {
if (closed) {
@@ -321,7 +321,7 @@
* codemark/code and codereset/code methods.
* @see java.io.InputStream#mark(int)
* @see java.io.InputStream#reset()
- * @since JCE1.2
+ * @since 1.4, JCE1.2
*/
public boolean markSupported() {
return false;
diff --git a/src/share/classes/javax/crypto/CipherOutputStream.java
b/src/share/classes/javax/crypto/CipherOutputStream.java
--- a/src/share/classes/javax/crypto/CipherOutputStream.java
+++ b/src/share/classes/javax/crypto/CipherOutputStream.java
@@ -114,7 +114,7 @@
*
* @param b the codebyte/code.
* @exception IOException if an I/O error occurs.
- * @since JCE1.2
+ * @since 1.4, JCE1.2
*/
public void write(int b) throws IOException {
ibuffer[0] = (byte) b;
@@ -138,7 +138,7 @@
* @exception NullPointerException if codeb/code is null.
* @exception IOException if an I/O error occurs.
* @seejavax.crypto.CipherOutputStream#write(byte[], int, int)
- * @since JCE1.2
+ * @since 1.4, JCE1.2
*/
public void write(byte b[]) throws IOException {
write(b, 0, b.length);
@@ -152,7 +152,7 @@
* @param off the start offset in the data.
* @param len the number of bytes to write.
* @exception IOException if an I/O error occurs.
- * @since JCE1.2
+ * @since 1.4, JCE1.2
*/
public void write(byte b[], int off, int len) throws IOException {
obuffer = cipher.update(b, off, len);
@@ -174,7 +174,7 @@
* the cipher's block size, no bytes will be written out.
*
* @exception IOException if an I/O error occurs.
- * @since JCE1.2
+ * @since 1.4, JCE1.2
*/
public void flush() throws IOException {
if (obuffer != null) {
@@ -198,7 +198,7 @@
* stream.
*
* @exception IOException if an I/O error occurs.
- * @since JCE1.2
+ * @since 1.4, JCE1.2
*/
public void close() throws IOException {
if (closed) {
diff --git
Re: RFR: 8047721: @since should have JDK version
You are absolutely right, I should have fixed it as I did for the rest. Updated webrev can be found at http://cr.openjdk.java.net/~henryjen/jdk9/8047721/1/webrev/ Cheers, Henry On 06/20/2014 01:01 PM, Paul Benedict wrote: Henry, I think JCE1.2 should get the space to be JCE 1.2 so it matches other secondary versions like JNDI 1.1 (last part of your patch). Or, you could make that another cleanup in itself. Cheers, Paul
Re: RFR: 8047721: @since should have JDK version
I assume the reason to have those @since JCE tag is somehow we want to
distinguish those APIs from the rest, thus I chose to preserve them.
If we agree it's not worth to keep, I can remove them.
Cheers,
Henry
On 06/20/2014 01:25 PM, Joe Darcy wrote:
Hi Henry,
For the Cipher{Input, Output}Stream classes, the class itself was added
in 1.4 so it seems redundant to have the @since information on each
method too. In other words, is preserving the @since JCE 1.2 information
useful at this point?
Thanmks,
-Joe
On 06/20/2014 01:20 PM, Henry Jen wrote:
You are absolutely right, I should have fixed it as I did for the rest.
Updated webrev can be found at
http://cr.openjdk.java.net/~henryjen/jdk9/8047721/1/webrev/
Cheers,
Henry
On 06/20/2014 01:01 PM, Paul Benedict wrote:
Henry,
I think JCE1.2 should get the space to be JCE 1.2 so it matches
other
secondary versions like JNDI 1.1 (last part of your patch). Or, you
could
make that another cleanup in itself.
Cheers,
Paul
RFR: 8047722: @since tag cleanup in corba
Hi, Please review a trivial webrev for jdk9/corba that do the same @since tag normalization as in jdk9/jdk. http://cr.openjdk.java.net/~henryjen/jdk9/8047722/0/webrev/ Cheers, Henry
RFR: 8047723: @since tag cleanup in jaxp
Hi, Please review a trivial webrev for jdk9/jaxp that do the same @since tag normalization as in jdk9/jdk. http://cr.openjdk.java.net/~henryjen/jdk9/8047723/0/webrev/ I am not sure if there is another mailing list thatis more appropriate for this, if so, please direct me to the right ml. Thanks. Cheers, Henry
RFR: 8047724: @since tag cleanup in jaxws
Hi, Please review a trivial webrev for jdk9/jaxws that do the same @since tag normalization as in jdk9/jdk. http://cr.openjdk.java.net/~henryjen/jdk9/8047724/0/webrev/ I am not sure if there is another mailing list thatis more appropriate for this, if so, please direct me to the right ml. Thanks. Cheers, Henry
Re: RFR: 8047723: @since tag cleanup in jaxp
Looks like it's pretty easy to do a find and replace in files in jaxp folder. If this is desired, we can do it in a separate webrev to be clear on changeset history? Cheers, Henry On 06/20/2014 03:36 PM, huizhe wang wrote: Hi Henry, Is it possible to add to the @since tool/script to remove repository revision notes? They appear in the API document as if they reflect meaningful version. An example in http://cr.openjdk.java.net/~henryjen/jdk9/8047723/0/webrev/src/javax/xml/parsers/DocumentBuilderFactory.java.udiff.html * @version $Revision: 1.9 $, $Date: 2010/05/25 16:19:44 $ This is also true in packages not in the webrevs (ones that have proper @since tag), for example http://docs.oracle.com/javase/8/docs/api/javax/xml/datatype/DatatypeFactory.html $Revision: 1.13 $, $Date: 2010/03/11 23:10:53 $ It looks good otherwise. Thanks, Joe On 6/20/2014 2:39 PM, Henry Jen wrote: Hi, Please review a trivial webrev for jdk9/jaxp that do the same @since tag normalization as in jdk9/jdk. http://cr.openjdk.java.net/~henryjen/jdk9/8047723/0/webrev/ I am not sure if there is another mailing list thatis more appropriate for this, if so, please direct me to the right ml. Thanks. Cheers, Henry
Re: Swing Dev RFR: 8046485: Add missing @since tag under javax.swing.*
Thanks, Alexander. Cheers, Henry On 06/18/2014 03:44 AM, Alexander Scherbatiy wrote: The fix looks good for me. Thanks, Alexandr. On 6/11/2014 4:00 AM, Henry Jen wrote: Hi, Please review a trivial webrev that provides missing @since tag for elements under javax.swing, Essentially it's simply add @since 1.2 for all classes/interfaces/exception that are missing @since tag, as JDK 1.2 package.html has @since tag but was removed in 1.3. The @since tag is added to *.java instead of package.html so that we can have javac report javadoc via javax.lang.model.util.Elements.getDocComment(Element c). http://cr.openjdk.java.net/~henryjen/jdk9/8046485/0/webrev/ Cheers, Henry
Re: Swing Dev RFR: 8046485: Add missing @since tag under javax.swing.*
On 06/18/2014 10:44 AM, Phil Race wrote: Hello, You say under javax.swing but you don't seem to be touching any sub-packages. No, sub-packages has @since in correponding package.html. I use * to mean entitiy in this package, and ** to signal subpackage as well. If it helps, I can remove .* from the subject. I notice there is at least one @since 1.7 you added to the Painter interface - so they aren't all 1.2 - and yet you didn't add the @since 1.7 to the sole class that implements Painter - perhaps because its in a sub-package. It's used in javax.swing.plaf.nimbus, which has @since 1.7. Also note that whilst @since 1.2 is strictly correct in terms of Java SE but Swing was initially an unbundled library - somewhat analagous to Java FX 2.x, and ran on JDK 1.1.x Understood. We are focus to make sure @since has the first version since the inclusion in J2SE. Other version information, as Mark suggested, can be expressed after comma. In case Java FX is included in JDK9, we would expect it to have have @since 1.9, Java FX x.x. As swing component currenly doesn't have that extra version information, we will simply mark the J2SE debut. Cheers, Henry -phil. On 6/18/2014 9:16 AM, Henry Jen wrote: Thanks, Alexander. Cheers, Henry On 06/18/2014 03:44 AM, Alexander Scherbatiy wrote: The fix looks good for me. Thanks, Alexandr. On 6/11/2014 4:00 AM, Henry Jen wrote: Hi, Please review a trivial webrev that provides missing @since tag for elements under javax.swing, Essentially it's simply add @since 1.2 for all classes/interfaces/exception that are missing @since tag, as JDK 1.2 package.html has @since tag but was removed in 1.3. The @since tag is added to *.java instead of package.html so that we can have javac report javadoc via javax.lang.model.util.Elements.getDocComment(Element c). http://cr.openjdk.java.net/~henryjen/jdk9/8046485/0/webrev/ Cheers, Henry
Re: Swing Dev RFR: 8046485: Add missing @since tag under javax.swing.*
On 06/18/2014 02:42 PM, Phil Race wrote: So for the specific case of the nimbus class I can see that the entire package was new in 1.7 so the individual classes added at that time do not need an @since tag. But my main question was whether you have inspected all classes in all swing packages and none were fouhd in need of an update .. I suppose its likely that after 1.2 people were more diligent in adding these tags. Assuming the answer is yes, then I'm OK. If no, then I'm OK too although at least then we'd know you still had to do those packages .. I inspected all APIs without a @since. @since implies to enclosed entities, so if a package has @since tag, it implies all classes/interfaces has same @since if not otherwise specified, and class implies all member fields or methods. From the analysis by a tool I wrote, all classes in javax.swing now have @since excepted generated javax.swing.*BeanInfo. Cheers, Henry -phil. On 6/18/2014 11:11 AM, Henry Jen wrote: On 06/18/2014 10:44 AM, Phil Race wrote: Hello, You say under javax.swing but you don't seem to be touching any sub-packages. No, sub-packages has @since in correponding package.html. I use * to mean entitiy in this package, and ** to signal subpackage as well. If it helps, I can remove .* from the subject. I notice there is at least one @since 1.7 you added to the Painter interface - so they aren't all 1.2 - and yet you didn't add the @since 1.7 to the sole class that implements Painter - perhaps because its in a sub-package. It's used in javax.swing.plaf.nimbus, which has @since 1.7. Also note that whilst @since 1.2 is strictly correct in terms of Java SE but Swing was initially an unbundled library - somewhat analagous to Java FX 2.x, and ran on JDK 1.1.x Understood. We are focus to make sure @since has the first version since the inclusion in J2SE. Other version information, as Mark suggested, can be expressed after comma. In case Java FX is included in JDK9, we would expect it to have have @since 1.9, Java FX x.x. As swing component currenly doesn't have that extra version information, we will simply mark the J2SE debut. Cheers, Henry -phil. On 6/18/2014 9:16 AM, Henry Jen wrote: Thanks, Alexander. Cheers, Henry On 06/18/2014 03:44 AM, Alexander Scherbatiy wrote: The fix looks good for me. Thanks, Alexandr. On 6/11/2014 4:00 AM, Henry Jen wrote: Hi, Please review a trivial webrev that provides missing @since tag for elements under javax.swing, Essentially it's simply add @since 1.2 for all classes/interfaces/exception that are missing @since tag, as JDK 1.2 package.html has @since tag but was removed in 1.3. The @since tag is added to *.java instead of package.html so that we can have javac report javadoc via javax.lang.model.util.Elements.getDocComment(Element c). http://cr.openjdk.java.net/~henryjen/jdk9/8046485/0/webrev/ Cheers, Henry
Re: Swing Dev RFR: 8046485: Add missing @since tag under javax.swing.*
+1, for any new code, we should definitely do that. It would be nice to add them for existing classes, I can slightly modify the tool and clean it up in a separate round, which I prefer we do. Cheers, Henry On 06/18/2014 05:31 PM, Joe Darcy wrote: FWIW, if I were reviewing code in a new package, I would want to see @since tags in all top-level types in the new package and not just on the package-info.java/package.html file. Cheers, -Joe On 06/18/2014 02:42 PM, Phil Race wrote: So for the specific case of the nimbus class I can see that the entire package was new in 1.7 so the individual classes added at that time do not need an @since tag. But my main question was whether you have inspected all classes in all swing packages and none were fouhd in need of an update .. I suppose its likely that after 1.2 people were more diligent in adding these tags. Assuming the answer is yes, then I'm OK. If no, then I'm OK too although at least then we'd know you still had to do those packages .. -phil. On 6/18/2014 11:11 AM, Henry Jen wrote: On 06/18/2014 10:44 AM, Phil Race wrote: Hello, You say under javax.swing but you don't seem to be touching any sub-packages. No, sub-packages has @since in correponding package.html. I use * to mean entitiy in this package, and ** to signal subpackage as well. If it helps, I can remove .* from the subject. I notice there is at least one @since 1.7 you added to the Painter interface - so they aren't all 1.2 - and yet you didn't add the @since 1.7 to the sole class that implements Painter - perhaps because its in a sub-package. It's used in javax.swing.plaf.nimbus, which has @since 1.7. Also note that whilst @since 1.2 is strictly correct in terms of Java SE but Swing was initially an unbundled library - somewhat analagous to Java FX 2.x, and ran on JDK 1.1.x Understood. We are focus to make sure @since has the first version since the inclusion in J2SE. Other version information, as Mark suggested, can be expressed after comma. In case Java FX is included in JDK9, we would expect it to have have @since 1.9, Java FX x.x. As swing component currenly doesn't have that extra version information, we will simply mark the J2SE debut. Cheers, Henry -phil. On 6/18/2014 9:16 AM, Henry Jen wrote: Thanks, Alexander. Cheers, Henry On 06/18/2014 03:44 AM, Alexander Scherbatiy wrote: The fix looks good for me. Thanks, Alexandr. On 6/11/2014 4:00 AM, Henry Jen wrote: Hi, Please review a trivial webrev that provides missing @since tag for elements under javax.swing, Essentially it's simply add @since 1.2 for all classes/interfaces/exception that are missing @since tag, as JDK 1.2 package.html has @since tag but was removed in 1.3. The @since tag is added to *.java instead of package.html so that we can have javac report javadoc via javax.lang.model.util.Elements.getDocComment(Element c). http://cr.openjdk.java.net/~henryjen/jdk9/8046485/0/webrev/ Cheers, Henry
Re: RFR: 8046389: Add missing @since tag under javax.sql.**
Thanks for the review, Lance. Cheers, Henry On 06/16/2014 04:33 AM, Lance @ Oracle wrote: This is fine Henry Best Lance http://oracle.com/us/design/oracle-email-sig-198324.gifLance Andersen| Principal Member of Technical Staff | +1.781.442.2037 tel:+1.781.442.2037 Oracle Java Engineering 1 Network Drive x-apple-data-detectors://34/0 Burlington, MA 01803 x-apple-data-detectors://34/0 lance.ander...@oracle.com mailto:lance.ander...@oracle.com Sent from my iPad On Jun 9, 2014, at 5:39 PM, Henry Jen henry@oracle.com mailto:henry@oracle.com wrote: Hi, Please review a trivial webrev that provides missing @since tag for elements under javax.sql, javax.sql.CommonDataSource was extracted out since 1.6, methods have @since 1.4 tag because those methods existed in origins since then. http://cr.openjdk.java.net/~henryjen/jdk9/8046389/0/webrev/ Cheers, Henry
RFR: 8046485: Add missing @since tag under javax.swing.*
Hi, Please review a trivial webrev that provides missing @since tag for elements under javax.swing, Essentially it's simply add @since 1.2 for all classes/interfaces/exception that are missing @since tag, as JDK 1.2 package.html has @since tag but was removed in 1.3. The @since tag is added to *.java instead of package.html so that we can have javac report javadoc via javax.lang.model.util.Elements.getDocComment(Element c). http://cr.openjdk.java.net/~henryjen/jdk9/8046485/0/webrev/ Cheers, Henry
RFR: 8046389: Add missing @since tag under javax.sql.**
Hi, Please review a trivial webrev that provides missing @since tag for elements under javax.sql, javax.sql.CommonDataSource was extracted out since 1.6, methods have @since 1.4 tag because those methods existed in origins since then. http://cr.openjdk.java.net/~henryjen/jdk9/8046389/0/webrev/ Cheers, Henry
Re: RFR: 8044740: Convert all JDK versions used in @since tag to 1.n[.n] in jdk repo
Thanks for all reviewing and feedbacks on core-libs-dev[1], I tried to respond to feedbacks with this email and send off to other mailing lists. I am wondering if jdk9-dev is the appropriate list for such a trivious but broad change, so that we can have one instead of many lists, and we still probably miss another. Lets follow up this thread on jdk9-dev. Regarding to whether we should keep JDK, the later convention is 1.#, and as David pointed out the document also list @since that way, I think we should settle on that. For other standards such as SAX or JCE, I propose to convert them to the version of JDK those APIs are included. To retain that information, we can introduce a custom tag, perhaps @standard or @conformingTo? @conformingTo Standard name version[, Standard name version]* For example, @conformingTo SAX 2.0. Repo wise, I think it's best if I can commit to jdk9/dev as a single commit instead of scattering to dev and client. But I can cope if this is absolutely necessary. Some changes to implementation classes, as I mentioned, only when it is straightforward. Essentially, I did a s/(@since *)JDK(.*)/\1\2 against all files. Some changes not obvious are simply remove tailing space, a (positive) side effect of the tools I use so I kept them. Cheers, Henry [1] http://mail.openjdk.java.net/pipermail/core-libs-dev/2014-June/027113.html On 06/03/2014 06:22 PM, Henry Jen wrote: Hi, In an effort to determine APIs availability in a given version, it became obvious that a consistent way to express @since tag would be beneficial. So started with the most obvious ones, where we have various expression for JDK version, this webrev make sure we use @since 1.n[.n] for JDK versions. The main focus is on public APIs, private ones are taken care if it is straightforward, otherwise, we try to keep the information. Some public APIs are using @since STANDARD standard version format, they are also preserved for now, but I think it worth discussion whether we want to change to the version as included in J2SE. There are APIs without @since information, separate webrevs will be coming to complete those information. Bug: https://bugs.openjdk.java.net/browse/JDK-8044740 The webrev can be found at http://cr.openjdk.java.net/~henryjen/jdk9/8044740/0/webrev but it's probably easier just look into the raw diff, http://cr.openjdk.java.net/~henryjen/jdk9/8044740/0/webrev/jdk.changeset Cheers, Henry
RFR: 8044855: Add missing @since tag under java.beans.*
Hi, Please review a trivial webrev that provides missing @since tag for elements under java.beans, This version is determined based on javadoc of 1.0.2, 1.1.8 and 1.2.2. http://cr.openjdk.java.net/~henryjen/jdk9/8044855/0/webrev Cheers, Henry
RFR: 8044740: Convert all JDK versions used in @since tag to 1.n[.n] in jdk repo
Hi, In an effort to determine APIs availability in a given version, it became obvious that a consistent way to express @since tag would be beneficial. So started with the most obvious ones, where we have various expression for JDK version, this webrev make sure we use @since 1.n[.n] for JDK versions. The main focus is on public APIs, private ones are taken care if it is straightforward, otherwise, we try to keep the information. Some public APIs are using @since STANDARD standard version format, they are also preserved for now, but I think it worth discussion whether we want to change to the version as included in J2SE. There are APIs without @since information, separate webrevs will be coming to complete those information. Bug: https://bugs.openjdk.java.net/browse/JDK-8044740 The webrev can be found at http://cr.openjdk.java.net/~henryjen/jdk9/8044740/0/webrev but it's probably easier just look into the raw diff, http://cr.openjdk.java.net/~henryjen/jdk9/8044740/0/webrev/jdk.changeset Cheers, Henry
Re: methods that throws checked exceptions in a lambda block
On 02/12/2014 07:05 AM, Wang Weijun wrote: This line does not compile Files.list(Paths.get(/secret)).forEach(x - Files.write(x, new byte[Files.size(x)]); because Files.write() and Files.size() throw IOE. Then what's the best way to make it work? (I find some old threads on this on lambda-dev. Maybe we have a recommended way now?) I am not sure there is a recommended way. Depends on how you need to handle those exceptions, I'll put the lambda in a private function to take care those exception, and surpress them in a UncheckedIOException. Cheers, Henry
RFR: 8033590: java.util.Comparator thenComparing has unnecessary type restriction
Hi, Please review a simple bug fix that loosen the generic type, the original proposed CCC for JDK-8009736 and webrev/specdiff has a discrepancy, I am not sure if another CCC is needed. https://bugs.openjdk.java.net/browse/JDK-8033590 http://cr.openjdk.java.net/~henryjen/tl/8033590.0/webrev/ Cheers, Henry
hg: jdk8/tl/jdk: 8033590: java.util.Comparator::thenComparing has unnecessary type restriction
Changeset: 7534523b4174 Author:henryjen Date: 2014-02-06 10:30 -0800 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/7534523b4174 8033590: java.util.Comparator::thenComparing has unnecessary type restriction Reviewed-by: psandoz ! src/share/classes/java/util/Comparator.java ! test/java/util/Comparator/TypeTest.java
Re: JDK 9 RFR of JDK-8033527: Fix raw type lint warning in sun.nio.ch
+1.
Cheers,
Henry
On 02/03/2014 10:09 PM, Joe Darcy wrote:
Hello,
Please review this small fix to address
JDK-803352: Fix raw type lint warning in sun.nio.ch
--- a/src/share/classes/sun/nio/ch/Reflect.javaMon Feb 03 16:58:02
2014 -0500
+++ b/src/share/classes/sun/nio/ch/Reflect.javaMon Feb 03 22:07:47
2014 -0800
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 2000, 2011, Oracle and/or its affiliates. All rights
reserved.
+ * Copyright (c) 2000, 2014, Oracle and/or its affiliates. All rights
reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
@@ -75,7 +75,7 @@
static Method lookupMethod(String className,
String methodName,
- Class... paramTypes)
+ Class?... paramTypes)
{
try {
Class? cl = Class.forName(className);
The build succeeds with this change in place.
Thanks,
-Joe
Re: StringJoiner: detect or fix delimiter collision?
On 01/27/2014 12:31 PM, Philip Hodges wrote: I did not predict that it would be a sprintf. It's not going to be consistently misused anything like so frequently. I compared it to the unescaped XML generation antipattern. I have not seen any technical justifications whatsoever so far, just inexplicable enthusiasm. It is like giving young drivers a faster car with no seat belts. As Mike had said, delimiter collision is a valid concern, and I think your metaphore is quite vivid. Anyhow, there is need for fast and simply join operation, and this is what JDK is intend to provide. Nothing prohibit developer to - choose a better delimiter for the data - escape the element before sending to joiner Since we expect more complicate use case is more likely to be used in stream API, such escaping can be easily done by adding a map step. elements.stream() .map(escapeFunction) .collect(joining()) Cheers, Henry
Re: Problem to build jdk on Mac OS X from ppc64 stage after sync with jdk9
I haven't tried this, but Tim Bell mentioned this in an email inquiry I had earlier, This will be cured when the fix for 8021266 is pushed to JDK 9 (see attached). In the meantime, the workaround in JPRT is to submit your JDK 9 jobs with '-bootproduct jdk7u7' since the 7u51 release can not serve as a boot JDK. Cheers, Henry On 01/22/2014 03:53 PM, Vladimir Kozlov wrote: I need help. I am trying to do control build in JPRT after I merged latest jdk9 source: http://hg.openjdk.java.net/jdk9/jdk9 to latest ppc64 sources: http://hg.openjdk.java.net/ppc-aix-port/stage-9 I have build failure on Mac OS X (on other platforms it passed). See the output below. I tried different JPRT queues - the same problem. During sync I had to merge common/autoconf/toolchain.m4 (merged automatically) and regenerate generated-configure.sh. The latest changes to toolchain.m4 was: http://hg.openjdk.java.net/jdk9/jdk9/rev/50669e45cec4 Next jdk files merge was resolved automatically: merging make/CompileDemos.gmk merging src/solaris/bin/java_md_solinux.c merging test/ProblemList.txt merging test/tools/launcher/ExecutionEnvironment.java 291 files updated, 4 files merged, 14 files removed, 0 files unresolved Thanks, Vladimir The build failure on Mac OS X: Cleaning up: /opt/jprt/T/P1/223112.vkozlov/s/build/macosx-x86_64-normal-server-release/jdk/gensrc_jobjc/src Outputting classes to: /opt/jprt/T/P1/223112.vkozlov/s/build/macosx-x86_64-normal-server-release/jdk/gensrc_jobjc/src Searching for bridged frameworks in: /opt/jprt/T/P1/223112.vkozlov/s/build/macosx-x86_64-normal-server-release/jdk/gensrc_jobjc/bridge found 3 frameworks Parsing XML Exception in thread main java.lang.UnsatisfiedLinkError: no JObjC in java.library.path at java.lang.ClassLoader.loadLibrary(ClassLoader.java:1857) at java.lang.Runtime.loadLibrary0(Runtime.java:870) at java.lang.System.loadLibrary(System.java:1119) at com.apple.jobjc.Coder.clinit(Coder.java:60) at com.apple.internal.jobjc.generator.model.coders.CoderDescriptor.clinit(CoderDescriptor.java:38) at com.apple.internal.jobjc.generator.model.types.NType$NPrimitive.init(NType.java:117) at com.apple.internal.jobjc.generator.model.types.Type.clinit(Type.java:57) at com.apple.internal.jobjc.generator.model.Element.init(Element.java:47) at com.apple.internal.jobjc.generator.model.Framework.init(Framework.java:99) at com.apple.internal.jobjc.generator.FrameworkGenerator.parseFrameworksFrom(FrameworkGenerator.java:56) at com.apple.internal.jobjc.generator.Generator.main(Generator.java:57) make[2]: *** [/opt/jprt/T/P1/223112.vkozlov/s/build/macosx-x86_64-normal-server-release/jdk/gensrc_jobjc/_the.generator] Error 1 make[1]: *** [gensrc-only] Error 2 make: *** [jdk-only] Error 2
hg: jdk8/tl/jdk: 8029434: Spliterator of Stream returned by BufferedReader.lines() should have NONNULL characteristic
Changeset: 6d583b9d99e1 Author:henryjen Date: 2013-12-04 08:12 -0800 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/6d583b9d99e1 8029434: Spliterator of Stream returned by BufferedReader.lines() should have NONNULL characteristic Reviewed-by: mduigou ! src/share/classes/java/io/BufferedReader.java ! test/java/io/BufferedReader/Lines.java
RFR: 8029434: Spliterator of Stream returned by BufferedReader.lines() should have NONNULL characteristic
Hi, Please review a small fix that add missing NONNULL characteristic and cleanup in javadoc. Thanks Anthony Vanelverdinghe for reporting of this bug. http://cr.openjdk.java.net/~henryjen/tl/8029434/0/webrev/ Cheers, Henry
hg: jdk8/tl/jdk: 8029483: BufferedReader.lines() javadoc typo should be fixed
Changeset: 1061f4d085b5 Author:henryjen Date: 2013-12-03 11:37 -0800 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/1061f4d085b5 8029483: BufferedReader.lines() javadoc typo should be fixed Reviewed-by: mduigou ! src/share/classes/java/io/BufferedReader.java
Re: RFR: 8029434: Spliterator of Stream returned by BufferedReader.lines() should have NONNULL characteristic
I have separated the fix into two part, 8029434: Spliterator of Stream returned by BufferedReader.lines() should have NONNULL characteristic 8029483: BufferedReader.lines() javadoc typo should be fixed So that we can at least fix the javadoc by jdk8 release if not the characteristic. Cheers, Henry On 12/03/2013 10:52 AM, Mike Duigou wrote: Looks good to me. Mike On Dec 3 2013, at 10:36 , Henry Jen henry@oracle.com wrote: Hi, Please review a small fix that add missing NONNULL characteristic and cleanup in javadoc. Thanks Anthony Vanelverdinghe for reporting of this bug. http://cr.openjdk.java.net/~henryjen/tl/8029434/0/webrev/ Cheers, Henry
Re: Map.merge Javadoc is confusing
On 11/21/2013 06:33 PM, David Holmes wrote: On 22/11/2013 5:02 AM, Louis Wasserman wrote: While I agree that case should be specified, I'm not certain I follow why that's what's going on here. The weird condition is that if oldValue is null, not value; oldValue is the old result of map.get(key). The Javadoc seems not just unspecified, but actively wrong. Here's a worked example: MapString, Integer map = new HashMap(); map.merge(foo, 3, Integer::plus); Integer fooValue = map.get(foo); Going through the Javadoc-specified default implementation: V oldValue = map.get(key); // oldValue is null V newValue = (oldValue == null) ? value : remappingFunction.apply(oldValue, value); // newValue is set to value, which is 3 if (newValue == null) // newValue is nonnull, branch not taken map.remove(key); else if (oldValue == null) // oldValue is null, branch taken map.remove(key); // removes the entry from the map else // else if was already triggered, branch not taken map.put(key, newValue); Seems like a document bug to me, we should fix this @implSpec. Cheers, Henry
Re: RFR: 8029055: Map.merge @implDoc is incorrect. (was: Map.merge Javadoc is confusing)
+1 (Not a reviewer). Cheers, Henry On Fri 22 Nov 2013 04:25:50 PM PST, Mike Duigou wrote: We'll be using https://bugs.openjdk.java.net/browse/JDK-8029055 for this issue. I've posted a webrev here: http://cr.openjdk.java.net/~mduigou/JDK-8029055/0/webrev/ There is an identical change in ConcurrentMap's merge(). Mike On Nov 22 2013, at 16:01 , Henry Jen henry@oracle.com wrote: On 11/21/2013 06:33 PM, David Holmes wrote: On 22/11/2013 5:02 AM, Louis Wasserman wrote: While I agree that case should be specified, I'm not certain I follow why that's what's going on here. The weird condition is that if oldValue is null, not value; oldValue is the old result of map.get(key). The Javadoc seems not just unspecified, but actively wrong. Here's a worked example: MapString, Integer map = new HashMap(); map.merge(foo, 3, Integer::plus); Integer fooValue = map.get(foo); Going through the Javadoc-specified default implementation: V oldValue = map.get(key); // oldValue is null V newValue = (oldValue == null) ? value : remappingFunction.apply(oldValue, value); // newValue is set to value, which is 3 if (newValue == null) // newValue is nonnull, branch not taken map.remove(key); else if (oldValue == null) // oldValue is null, branch taken map.remove(key); // removes the entry from the map else // else if was already triggered, branch not taken map.put(key, newValue); Seems like a document bug to me, we should fix this @implSpec. Cheers, Henry
Re: RFR 8027712 DistinctOpTest fails for unordered test
Change looks good to me(not a reviewer). Cheers, Henry On 11/01/2013 10:57 AM, Paul Sandoz wrote: Hi, This is a eating humble pie and should of correctly listened to Henry in review kind of email :-) The changes to DistinctOpTest recently committed result in a test failure, since one of the tests is incorrectly asserting on a particular element, which is non-determinisitic process [*]. See below for a patch that fixes the test. Paul. [*] Somewhat interesting how this can arise given the sequential nature of the source and the way it is split. Intuitively one would expect that the first F/J leaf task is more likely to complete before the second, but we alternate forking of left/right tasks when splitting. The first leaf task gets forked and the next leaf task gets computed directly on the same thread that forked the first leaf task. Thus setting the common pool parallelism to 0 (or running on a single core machine) will consistently make the test fail, and this probably explains why it was caught so soon. hg qdiff diff -r 18c111c17231 test/java/util/stream/test/org/openjdk/tests/java/util/stream/DistinctOpTest.java --- a/test/java/util/stream/test/org/openjdk/tests/java/util/stream/DistinctOpTest.java Thu Oct 31 11:59:04 2013 +0100 +++ b/test/java/util/stream/test/org/openjdk/tests/java/util/stream/DistinctOpTest.java Fri Nov 01 18:42:08 2013 +0100 @@ -54,10 +54,14 @@ // These tests should short-circuit, otherwise will fail with a time-out // or an OOME -Integer one = Stream.iterate(1, i - i + 1).unordered().parallel().distinct().findAny().get(); -assertEquals(one.intValue(), 1); +// Note that since the streams are unordered and any element is requested +// (a non-deterministic process) the only assertion that can be made is +// that an element should be found -OptionalInteger oi = ThreadLocalRandom.current().ints().boxed().parallel().distinct().findAny(); +OptionalInteger oi = Stream.iterate(1, i - i + 1).unordered().parallel().distinct().findAny(); +assertTrue(oi.isPresent()); + +oi = ThreadLocalRandom.current().ints().boxed().parallel().distinct().findAny(); assertTrue(oi.isPresent()); }
Re: RFR 8027316 Distinct operation on an unordered stream should not be a barrier
Looks good to me. In the test, Integer one = Stream.iterate(1, i - i + 1).unordered().parallel().distinct().findAny().get(); assertEquals(one.intValue(), 1); The implementation is probably make sure this will return 1, but is that what we spec to do? I sort of think it can have various values depends on how many working threads and their scheduling. Cheers, Henry On Oct 30, 2013, at 2:30 AM, Paul Sandoz paul.san...@oracle.com wrote: Hi Please review: http://cr.openjdk.java.net/~psandoz/tl/JDK-8027316-distinct-unordered/webrev/ The fix ensures that the distinct operation, for a parallel stream, is not a full barrier if the upstream is unordered (and not already distinct, otherwise it is a no-op). Thanks, Paul.
RFR(2nd): 8025909: Lambda Library Spec Updates
Hi,
Please review the updated revision at
http://cr.openjdk.java.net/~henryjen/ccc/8025909/1/webrev
- Rewording {Iterable|Map}::forEach from ordering constraint so that
implementation class is allowed to process in parallel ignoring the order.
- fixed typo in Collector as Paul pointed out
Cheers,
Henry
Re: RFR(2nd): 8025909: Lambda Library Spec Updates
Thanks, will fix them for the commit.
Cheers,
Henry
On Oct 22, 2013, at 1:45 PM, Mike Duigou mike.dui...@oracle.com wrote:
Looks fine. Two non-blocking nits:
LambdaMetaFactory:
(-0) : period at the end of @param
(-0) : FLAG_SERIALIZABLE should be in {@code}
On Oct 22 2013, at 10:32 , Henry Jen henry@oracle.com wrote:
Hi,
Please review the updated revision at
http://cr.openjdk.java.net/~henryjen/ccc/8025909/1/webrev
- Rewording {Iterable|Map}::forEach from ordering constraint so that
implementation class is allowed to process in parallel ignoring the order.
- fixed typo in Collector as Paul pointed out
Cheers,
Henry
hg: jdk8/tl/jdk: 8025909: Lambda Library Spec Updates; ...
Changeset: 0913c3bab168 Author:henryjen Date: 2013-10-22 15:12 -0700 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/0913c3bab168 8025909: Lambda Library Spec Updates 8024179: Document limitations and performance characteristics of stream sources and operations 8024138: (Spec clarification) Lambda Metafacory spec should state DMH constraint on implMethod Reviewed-by: mduigou Contributed-by: brian.go...@oracle.com, paul.san...@oracle.com ! src/share/classes/java/lang/Iterable.java ! src/share/classes/java/lang/invoke/LambdaMetafactory.java ! src/share/classes/java/lang/invoke/SerializedLambda.java ! src/share/classes/java/util/DoubleSummaryStatistics.java ! src/share/classes/java/util/Iterator.java ! src/share/classes/java/util/List.java ! src/share/classes/java/util/Map.java ! src/share/classes/java/util/PrimitiveIterator.java ! src/share/classes/java/util/Spliterator.java ! src/share/classes/java/util/function/package-info.java ! src/share/classes/java/util/stream/BaseStream.java ! src/share/classes/java/util/stream/Collectors.java ! src/share/classes/java/util/stream/DoubleStream.java ! src/share/classes/java/util/stream/IntStream.java ! src/share/classes/java/util/stream/LongStream.java ! src/share/classes/java/util/stream/Node.java ! src/share/classes/java/util/stream/SpinedBuffer.java ! src/share/classes/java/util/stream/Stream.java ! src/share/classes/java/util/stream/StreamSupport.java ! src/share/classes/java/util/stream/package-info.java
Re: RFR: 8025909: Lambda Library Spec Updates
Apologize for the multiple copies, Thunderbird had issue with network, and resulting several copies sent. Sorry for the incident. Cheers, Henry On 10/15/2013 10:53 PM, Henry Jen wrote: On 10/15/2013 04:05 PM, Mike Duigou wrote: Where is the null handling for methods like DoubleStream.allMatch() described? I don't see an @throws NullPointerException if predicate is null. The class level functional interface parameters to methods, unless otherwise specified, may not be null instruction is removed with reference to Stream docs. I hope this is sufficient notice. The idea is to avoid duplicate documentation(clutering) by consolidate into reference of Stream and package documentation. It may not be obvious, but should be enough?
RFR: 8026768: java.util.Map.Entry comparingBy methods missing @since 1.8
Hi, A simple webrev to add missing @since 1.8 tag. Cheers, Henry
Re: RFR: 8026768: java.util.Map.Entry comparingBy methods missing @since 1.8
On 10/16/2013 08:18 PM, Henry Jen wrote: Hi, A simple webrev to add missing @since 1.8 tag. Sigh, http://cr.openjdk.java.net/~henryjen/tl/8026768/0/webrev/ Cheers, Henry
hg: jdk8/tl/jdk: 8026768: java.util.Map.Entry comparingBy methods missing @since 1.8
Changeset: e2e3c2c249e2 Author:henryjen Date: 2013-10-16 21:34 -0700 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/e2e3c2c249e2 8026768: java.util.Map.Entry comparingBy methods missing @since 1.8 Reviewed-by: dholmes ! src/share/classes/java/util/Map.java
RFR: 8025909: Lambda Library Spec Updates
Hi, Please kindly review the latest update on lambda library spec, and a few minor code clean up. Webrev and specdiff can be found at, http://cr.openjdk.java.net/~henryjen/ccc/8025909/0/webrev http://cr.openjdk.java.net/~henryjen/ccc/8025909/0/specdiff This webrev also covers update that address JDK-8024179 as the spec update explains why reported scenario is expected. Cheers, Henry
Re: RFR: 8025909: Lambda Library Spec Updates
On 10/15/2013 04:05 PM, Mike Duigou wrote: Where is the null handling for methods like DoubleStream.allMatch() described? I don't see an @throws NullPointerException if predicate is null. The class level functional interface parameters to methods, unless otherwise specified, may not be null instruction is removed with reference to Stream docs. I hope this is sufficient notice. The idea is to avoid duplicate documentation(clutering) by consolidate into reference of Stream and package documentation. It may not be obvious, but should be enough? Note that we state must be non-null unless otherwise specified but don't specify what would happen if null is passed. I think that's is intentionally ambiguous and allows some wiggle room here. Should we be more explicit on NPE? I prefer to leave it as is, so it's unspecified behavior if null is passed. For allMatch/noneMatch I think the case where predicate is null and the stream is empty is covered by and the predicate is not evaluated. Right. That covers the empty case does not evaluate the predicate, and we didn't otherwise specified about null, so in theory it still must be non-null. Cheers, Henry Mike On Oct 15 2013, at 15:34 , Henry Jen henry@oracle.com wrote: Hi, Please kindly review the latest update on lambda library spec, and a few minor code clean up. Webrev and specdiff can be found at, http://cr.openjdk.java.net/~henryjen/ccc/8025909/0/webrev http://cr.openjdk.java.net/~henryjen/ccc/8025909/0/specdiff This webrev also covers update that address JDK-8024179 as the spec update explains why reported scenario is expected. Cheers, Henry
Re: RFR: 8025909: Lambda Library Spec Updates
On 10/15/2013 04:05 PM, Mike Duigou wrote: Where is the null handling for methods like DoubleStream.allMatch() described? I don't see an @throws NullPointerException if predicate is null. The class level functional interface parameters to methods, unless otherwise specified, may not be null instruction is removed with reference to Stream docs. I hope this is sufficient notice. The idea is to avoid duplicate documentation(clutering) by consolidate into reference of Stream and package documentation. It may not as obvious, but should be enough? Note that we state must be non-null unless otherwise specified but don't specify what would happen if null is passed. I think that's is intentionally ambiguous and allows some wiggle room here. Should we be more explicit on NPE? I prefer to leave it as is, so it's unspecified behavior if null is passed. For allMatch/noneMatch I think the case where predicate is null and the stream is empty is covered by and the predicate is not evaluated. Right. That covers the empty case does not evaluate the predicate, and we didn't otherwise specified about null, so in theory it still must be non-null. As we Cheers, Henry Mike On Oct 15 2013, at 15:34 , Henry Jen henry@oracle.com wrote: Hi, Please kindly review the latest update on lambda library spec, and a few minor code clean up. Webrev and specdiff can be found at, http://cr.openjdk.java.net/~henryjen/ccc/8025909/0/webrev http://cr.openjdk.java.net/~henryjen/ccc/8025909/0/specdiff This webrev also covers update that address JDK-8024179 as the spec update explains why reported scenario is expected. Cheers, Henry
RFR: 8026362: java/lang/invoke/lambda/LogGeneratedClassesTest.java failed on windows, jtreg report Fail to org.testng.SkipException
Hi, Please review a quick one line fix for the test, we simply pass(ignore) the test instead of skip the test. Ignore the test is the intention. It would be ideal if we know it's skipped intentionally, but currently jtreg consider a skipped test with a throwable failed test for other scenarios, which certainly can be reviewed separately. In the mean time, we should get rid of this noise. Cheers, Henry http://cr.openjdk.java.net/~henryjen/tl/8026362/0/webrev/
hg: jdk8/tl/jdk: 8023524: Mechanism to dump generated lambda classes / log lambda code generation
Changeset: c070001c4f60 Author:henryjen Date: 2013-10-09 09:41 -0700 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/c070001c4f60 8023524: Mechanism to dump generated lambda classes / log lambda code generation Reviewed-by: plevart, mchung, forax, jjb Contributed-by: brian.go...@oracle.com, henry@oracle.com ! src/share/classes/java/lang/invoke/InnerClassLambdaMetafactory.java + src/share/classes/java/lang/invoke/ProxyClassesDumper.java + test/java/lang/invoke/lambda/LogGeneratedClassesTest.java
Re: RFR: 8023524: Mechanism to dump generated lambda classes / log lambda code generation
Thanks Mandy, and all others have reviewed and commented. Cheers, Henry On 10/09/2013 02:47 AM, Mandy Chung wrote: Hi Henry, On 10/8/2013 10:57 PM, Henry Jen wrote: Hi, Please review updated webrev at http://cr.openjdk.java.net/~henryjen/ccc/8023524/6/webrev ProxyClassesDumper looks simpler after moving the path validation to the static factory method. One minor comment: ProxyClassesDumper.getInstance returns null if the given path is invalid. For a null path, it can simply return null rather than throwing NPE. It's good to see the limited doPrivileged being used here. I can see that listing the limited permissions would be a tradeoff with maintainability and readability. This also leads to the question how one can determine the complete list of permissions required (besides testing) and guideline on when to use limited privileged and when to use the entire ACC (Jeff may have some thoughts). Anyway, just some comments related to limited doPrivileged and not anything related to this fix. Henry - your fix looks good for me. You don't not need to generate a new webrev if you make any change per my comment about null path unless others have any other comment. thanks Mandy
RFR: 8023524: Mechanism to dump generated lambda classes / log lambda code generation
Hi, Please review updated webrev at http://cr.openjdk.java.net/~henryjen/ccc/8023524/6/webrev In this update, - Check the specified directory once, disable dumping if it is invalid. This address the comment of unnecessary complexity, as this is a debug feature and most likely developer would like to restart. - minimum doPrivilege permission - Install security manager in test to verify doPrivilege Cheers, Henry
hg: jdk8/tl/jdk: 8025968: Integrate test improvements made in lambda repo
Changeset: f0ad3e2918b4 Author:henryjen Date: 2013-10-07 11:25 -0700 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/f0ad3e2918b4 8025968: Integrate test improvements made in lambda repo Reviewed-by: psandoz ! test/java/util/stream/bootlib/java/util/stream/OpTestCase.java ! test/java/util/stream/test/org/openjdk/tests/java/util/stream/ExplodeOpTest.java ! test/java/util/stream/test/org/openjdk/tests/java/util/stream/TabulatorsTest.java
RFR: 8026009: Changes for 8025968 break all stream tests
Hi,
May I have a quick review on this left-out change to fix broken test?
Apology for the inconvenience.
http://cr.openjdk.java.net/~henryjen/tl/8026009/0/webrev/
Following is all it is,
diff --git a/test/java/util/stream/bootlib/java/util/stream/OpTestCase.java
b/test/java/util/stream/bootlib/java/util/stream/OpTestCase.java
--- a/test/java/util/stream/bootlib/java/util/stream/OpTestCase.java
+++ b/test/java/util/stream/bootlib/java/util/stream/OpTestCase.java
@@ -591,10 +591,10 @@
// Test data
-private class ShortCircuitOpT implements StatelessTestOpT,T {
+static class ShortCircuitOpT implements StatelessTestOpT,T {
private final StreamShape shape;
-private ShortCircuitOp(StreamShape shape) {
+ShortCircuitOp(StreamShape shape) {
this.shape = shape;
}
Cheers,
Henry
hg: jdk8/tl/jdk: 8026009: Changes for 8025968 break all stream tests
Changeset: 0cffe1dab0bf Author:henryjen Date: 2013-10-07 15:18 -0700 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/0cffe1dab0bf 8026009: Changes for 8025968 break all stream tests Reviewed-by: mduigou ! test/java/util/stream/bootlib/java/util/stream/OpTestCase.java
RFR: 8025968: Integrate test improvements made in lambda repo
Hi, Please review a port from lambda repo to tl, it contains a refactoring of OpTestCase and a small addition to fill test gap. http://cr.openjdk.java.net/~henryjen/tl/8025968/0/webrev/ Cheers, Henry
Re: RFR: 8023524: Mechanism to dump generated lambda classes / log lambda code generation
OK, hopefully we are close to the end here on comments, unless there are security related issues. We still need +1 from a jdk8 reviewer, http://cr.openjdk.java.net/~henryjen/ccc/8023524/5/webrev This update include what Peter's latest suggestion, and since we don't have a reliable way to create read-only directory on Windows, we simply skip the test on non-posix filesystems, that is, Windows. Cheers, Henry
Re: RFR (4th): 8023524: Mechanism to dump generated lambda classes / log lambda code generation
On Oct 2, 2013, at 7:21 AM, Remi Forax fo...@univ-mlv.fr wrote:
On 10/02/2013 04:12 PM, Peter Levart wrote:
On 10/02/2013 12:19 PM, Remi Forax wrote:
final class ProxyClassesDumper {
...
private final Path dumpDir; // null if invalid
public static ProxyClassesDumper create(String dir) {
dir = dir.trim();
Path path = *Paths.get*(dir.isEmpty() ? . : dir);
path = validateDumpDir(path)? path: null;
return new ProxyClassesDumper(path);
}
Hi Remi, Henry,
I think Henry is trying to re-validate the directory on each dump attempt
even if it was invalid on 1st dump attempt. Perhaps in order to be able to
start a VM with dump directory specified and only later create or mount that
directory.
Having a static field in the InnerClassLambdaMetafactory holding an
initialized ProxyClassesDumper is a good idea.
One thing to watch though is that Paths.get(String) can throw
InvalidPathException. This will render InnerClassLambdaMetafactory
non-functional since its static initialization will throw exception. Each
lambda capture will than throw Error. I think the exception should be
caught, logged and null returned from ProxyClassesDumper.create() in that
case...
yes, nice catch.
I had it in there earlier, but need that in exception message. Since it is
called only by InnerClassMetaFactory, which I think won't cause that exception
so I just let it be.
That's not a good idea if consider dumper to be a reusable component.
Cheers,
Henry
RFR: 8023524: Mechanism to dump generated lambda classes / log lambda code generation
Hi, Please review update of the webrev at http://cr.openjdk.java.net/~henryjen/ccc/8023524/4/webrev This update address comments from Remi and Peter, - Remove the unnecessary static field, also take out not needed singleton behavior of ProxyClassesDumper - Ensure InvalidPathException won't stop InnerClassLambdaMetaFactory - Change test code to create readonly directory that didn't work properly on Windows. Cheers, Henry
RFR (4th): 8023524: Mechanism to dump generated lambda classes / log lambda code generation
Hi, Please review the updated webrev at http://cr.openjdk.java.net/~henryjen/ccc/8023524/3/webrev/ This update addressed comments from Mandy with following, - call doPrivileged with specific file permission, ALL FILES, write. - Use nio package to write deal with FS, also create directory hierarchy with package name instead of flat files. - If not specify a value or empty string, the directory is default to current working directory as . is specified. We continue to use local encoding rule as other suggestions doesn't fit. The reason we cannot use something like URLEncoder is that : won't get encoded and it's not valid character for filename on Windows. All the suggestions so far seems to have that same issue. Cheers, Henry
Re: RFR: 8025067: (xs) Arrays.parallelPrefix may fail to throw NPE for null op
Looks good to me. Cheers, Henry On Sep 30, 2013, at 2:27 PM, Mike Duigou mike.dui...@oracle.com wrote: Hello all; This changeset fixes a small corner case where Arrays.parallelPrefix (all variants) failed to throw NPE if the operation was null. If the array or range was empty then op was never examined. The solution in this patch is to add an an explicit Objects.requireNonNull(op) check. http://cr.openjdk.java.net/~mduigou/JDK-8025067/0/webrev/ Cheers, Mike
Re: RFR: 8025067: (xs) Arrays.parallelPrefix may fail to throw NPE for null op
The test itself looks good. Like you said, the cut-n-paste code need some cleanup, you didn't need those no-message version and the array version of assertThrows. Cheers, Henry On Sep 30, 2013, at 3:39 PM, Mike Duigou mike.dui...@oracle.com wrote: After posting the webrev I decided that an update to the regression test was going to be necessary. Here's the updated webrev including additional unit tests. Unfortunately I had to cut-and-paste some library code for exception checking. The library code will need to be cleaned up eventually. http://cr.openjdk.java.net/~mduigou/JDK-8025067/1/webrev/ Mike On Sep 30 2013, at 15:00 , Henry Jen wrote: Looks good to me. Cheers, Henry On Sep 30, 2013, at 2:27 PM, Mike Duigou mike.dui...@oracle.com wrote: Hello all; This changeset fixes a small corner case where Arrays.parallelPrefix (all variants) failed to throw NPE if the operation was null. If the array or range was empty then op was never examined. The solution in this patch is to add an an explicit Objects.requireNonNull(op) check. http://cr.openjdk.java.net/~mduigou/JDK-8025067/0/webrev/ Cheers, Mike
Re: RFR (3rd): 8023524: Mechanism to dump generated lambda classes / log lambda code generation
On Sep 26, 2013, at 4:01 PM, Mandy Chung mandy.ch...@oracle.com wrote: Hi Henry, On 9/25/2013 12:37 AM, Henry Jen wrote: Hi, Please review the update webrev at http://cr.openjdk.java.net/~henryjen/ccc/8023524/2/webrev/ The doPrivileged block looks okay. It'd be good to limit privileges by calling the doPrivileged method with a specific set of Permissions. In this case, it would be FilePermission(ALL FILES, write). Good point, I will do that. If jdk.internal.lambda.dumpProxyClasses property is set with empty value, would it be better to default to CWD? ProxyClassesDumper currently fails with non-existent directory. That sounds reasonable, I will replace empty string with .. Personally I prefer explicit value being specified, I always specify . if I wanted current directory. ProxyClassesDumper - you may want to use java.nio.file.Path and Files. I also suggest to follow the convention of classfile pathname and generate it in the directories reflecting the package name i.e. DIR/com/example/TestLambda$$Lambda$1.class instead of DIR/com.example.TestLambda$$Lambda$1.class, Make sense to me if that's safe enough to do so. I don't have a strong opinion on this. By avoiding create directories, less chance for malicious class to mess with file system. For escaping sequence, as Peter pointed out, I presume you can consider using ParseUtil.encodePath but I am not sure how importance it has to encode the path as this is for debugging purpose. No strong opinion in this one. If I have known this, I could have used that. :) In this latest webrev, a simple encoding is implemented which allows unicode characters to pass through, which is probably good enough? Cheers, Henry
Re: RFR (2nd): 8023524: Mechanism to dump generated lambda classes / log lambda code generation
On 09/24/2013 11:28 PM, Peter Levart wrote: On 09/25/2013 07:56 AM, Peter Levart wrote: Just a thought. How does URLClassLoader do the class name - path to resource translation? Perhaps there's already some code that does this correctly and in a platform-specific way (haven't looked)... Hi, sun.misc.ProxyGenerator has similar capability. It chooses to reject paths that are invalid for the underlying file system (using java.nio.file APIs)... Hi Peter, Thanks for the pointer, I asked similar question but didn't find this. It seems to me simply failed if classname contains invalid characters for the filesystem. I don't know enough to know whether that's appropriate at this moment. Our concern is that if it is possible to create file at bad places, and do our best to save the generated class file for captured lambda. With that, I guess current code is slightly better than previous one. Cheers, Henry
Re: RFR (3rd): 8023524: Mechanism to dump generated lambda classes / log lambda code generation
Hi, Please review the update webrev at http://cr.openjdk.java.net/~henryjen/ccc/8023524/2/webrev/ In this revision, I moved out code for dumping the generated class files into a separate class as Remi suggested, so that the code is only loaded if the property is set. (Verified with -XX:+TraceClassLoading). I also fixed two type as Jon brought to my attention. Cheers, Henry On 09/24/2013 02:59 PM, Henry Jen wrote: Hi, Please review the new webrev at http://cr.openjdk.java.net/~henryjen/ccc/8023524/1/webrev/ The updated code will attempt to escape potentially bad characters(based on our best knowledge on Windows and common systems), it's not likely we can avoid problem for all file systems. Anyhow, we avoid characters that can be used to navigate (known) file system, and if there is any other invalid characters, that should cause an IOException failed to create file and just skip dumping of that class. Let me know if there are other concerns. Cheers, Henry On 09/19/2013 12:27 AM, Florian Weimer wrote: On 09/19/2013 01:00 AM, Henry Jen wrote: Class names can contain '\' and other characters which are problematic on Windows. Thanks for reviewing, I suspect you are pointing out a potential issue to look at, not that the problem exists in current implementation. According to JLS 3.8, the classname(an identifier) can only have letters, digits, plus '_' and '$'. You need to look at the JVM specification, there are only very few characters it excludes. The restrictions come from javac, not the JVM. For example, on Linux, java '\' will load a \.class file and run it (yes, I tried).
Re: Overloads warnings overly agressive?
Hi,
I had reported this issue with attached test with behavior I observed, Brian
and me agreed that SuppressWarnings on either one as in this test case should
be sufficient.
Cheers,
Henry
On Sep 24, 2013, at 10:14 AM, Paul Sandoz paul.san...@oracle.com wrote:
Hi,
There is a new warning about overloads on methods with functional interfaces,
but it appears to be over-agressive as Doug pointed out to me off-list.
If i enable this when compiling tl (make JAVAC_WARNINGS:=-Xlint:overloads)
then one can observe warnings such as on the primitive spliterators:
/Users/sandoz/Projects/jdk8/tl/jdk/src/share/classes/java/util/Spliterator.java:642:
warning: [overloads] tryAdvance(IntConsumer) in OfInt is potentially
ambiguous with tryAdvance(Consumer? super Integer) in OfInt
boolean tryAdvance(IntConsumer action);
^
/Users/sandoz/Projects/jdk8/tl/jdk/src/share/classes/java/util/Spliterator.java:645:
warning: [overloads] forEachRemaining(IntConsumer) in OfInt is potentially
ambiguous with forEachRemaining(Consumer? super Integer) in OfInt
default void forEachRemaining(IntConsumer action) {
The warnings propagate down to implementations, for example:
/Users/sandoz/Projects/jdk8/tl/jdk/src/share/classes/java/util/Random.java:1025:
warning: [overloads] tryAdvance(IntConsumer) in RandomIntsSpliterator is
potentially ambiguous with tryAdvance(Consumer? super T) in Spliterator
public boolean tryAdvance(IntConsumer consumer) {
^
where T is a type-variable:
T extends Object declared in interface Spliterator
/Users/sandoz/Projects/jdk8/tl/jdk/src/share/classes/java/util/Random.java:1036:
warning: [overloads] forEachRemaining(IntConsumer) in RandomIntsSpliterator
is potentially ambiguous with forEachRemaining(Consumer? super T) in
Spliterator
public void forEachRemaining(IntConsumer consumer) {
(Incidentally it does not appear all warnings are reported, warnings for the
double implementations are missing.)
If I write a SuppressWarnings on Spliterator.OfInt:
@SuppressWarnings(overloads)
public interface OfInt extends OfPrimitiveInteger, IntConsumer, OfInt {
then that stops the first set of warnings (above) but the warnings are still
propagated to the second set for Random (or in general implementations or
extensions of). That seems over aggressive and more of an annoyance than
helpful. Can we change this?
Paul.
Re: Overloads warnings overly agressive?
Forgot that attachment is not allowed, the test code is following,
public class TestOverload {
interface FooT {
void m(T arg);
}
interface IntFoo {
void m(int arg);
}
interface BarT {
void bar(FooT arg);
}
interface IntBar extends BarInteger {
@SuppressWarnings(overloads)
default void bar(IntFoo arg) {
System.out.print(Primitive: );
}
@SuppressWarnings(overloads)
default void bar(FooInteger arg) {
System.out.print(Boxed: );
bar((IntFoo) arg::m);
}
}
public static void main(String[] args) {
class Impl implements IntBar {
@Override
public void bar(IntFoo foo) {
IntBar.super.bar(foo);
System.out.print(Overload: );
for (String arg: args) {
Integer n = Integer.valueOf(arg);
foo.m(n.intValue());
}
}
}
Impl impl = new Impl();
impl.bar((int n) - System.out.println(n + 100));
impl.bar((Integer n) - System.out.println(n));
}
}
On Sep 24, 2013, at 11:11 AM, Henry Jen henry@oracle.com wrote:
Hi,
I had reported this issue with attached test with behavior I observed, Brian
and me agreed that SuppressWarnings on either one as in this test case should
be sufficient.
Cheers,
Henry
RFR (2nd): 8023524: Mechanism to dump generated lambda classes / log lambda code generation
Hi, Please review the new webrev at http://cr.openjdk.java.net/~henryjen/ccc/8023524/1/webrev/ The updated code will attempt to escape potentially bad characters(based on our best knowledge on Windows and common systems), it's not likely we can avoid problem for all file systems. Anyhow, we avoid characters that can be used to navigate (known) file system, and if there is any other invalid characters, that should cause an IOException failed to create file and just skip dumping of that class. Let me know if there are other concerns. Cheers, Henry On 09/19/2013 12:27 AM, Florian Weimer wrote: On 09/19/2013 01:00 AM, Henry Jen wrote: Class names can contain '\' and other characters which are problematic on Windows. Thanks for reviewing, I suspect you are pointing out a potential issue to look at, not that the problem exists in current implementation. According to JLS 3.8, the classname(an identifier) can only have letters, digits, plus '_' and '$'. You need to look at the JVM specification, there are only very few characters it excludes. The restrictions come from javac, not the JVM. For example, on Linux, java '\' will load a \.class file and run it (yes, I tried).
RFR: 8023524: Mechanism to dump generated lambda classes / log lambda code generation
Hi, Please review the webrev at http://cr.openjdk.java.net/~henryjen/ccc/8023524/0/webrev/ This webrev enable writing generated classes for lambda to disk at a directory specified with -Djdk.internal.lambda.dumpProxyClasses. The directory has to be an existing writable directory, otherwise, a message will be logged. The validation is done in static block to fail early, and is done each time when lambda capture occurs in case file system is modified(for example, created after seeing error message). As the code using AccessController.doPrivileged to perform file I/O, it would be much appreciated to have extra eyes from security experts to have a look at this. As the directory specified should be existing writable directory, and the generated filename is lambda class name controlled by the program, there should be no security concerns. Cheers, Henry
Re: RFR 8025002 .codePoints().sorted().iterator().hasNext() causes NegativeArraySizeException
Looks good to me. Cheers, Henry On Sep 18, 2013, at 11:27 AM, Paul Sandoz paul.san...@oracle.com wrote: Hi, Please review a fix for CharSequence.codePoints: http://hg.openjdk.java.net/lambda/lambda/jdk/rev/e040a0dda7e4 (That change set should apply cleanly to tl.) The incorrect spliterator characteristics were being reported when constructing the stream. Specifically it should not report SIZED/SUBSIZED. I added a quick test of the characteristics but we should probably add further tests using the stream test framework which would have most likely caught this error much earlier (since parallel evaluation should barf in certain cases). Plus there are more optimal impls that could be provided for String (even IIUC for codePoints if care is taken when splitting). All stuff to consider later on... Paul.
Re: RFR: 8023524: Mechanism to dump generated lambda classes / log lambda code generation
On 09/18/2013 04:56 AM, Florian Weimer wrote:
On 09/18/2013 08:18 AM, Henry Jen wrote:
Hi,
Please review the webrev at
http://cr.openjdk.java.net/~henryjen/ccc/8023524/0/webrev/
This webrev enable writing generated classes for lambda to disk at a
directory specified with -Djdk.internal.lambda.dumpProxyClasses.
+ File out = new File(dirPath, lambdaClassName.replace('/', '.') +
.class);
Class names can contain '\' and other characters which are problematic
on Windows.
Thanks for reviewing, I suspect you are pointing out a potential issue
to look at, not that the problem exists in current implementation.
According to JLS 3.8, the classname(an identifier) can only have
letters, digits, plus '_' and '$'.
The '/' is converted from '.' earlier from, thus there won't be '\'
involved as separator.
Therefore, I think we are safe here.
Cheers,
Henry
hg: jdk8/tl/jdk: 8024874: Copy-paste typo in the spec for j.u.Comparator.thenComparing(Function, Comparator)
Changeset: 86aa8e7503e9 Author:henryjen Date: 2013-09-16 10:28 -0700 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/86aa8e7503e9 8024874: Copy-paste typo in the spec for j.u.Comparator.thenComparing(Function, Comparator) Reviewed-by: mduigou ! src/share/classes/java/util/Comparator.java
RFR: 8024825: Some fixes are missing from java.util.stream spec update
Hi,
Please kindly review webrev at
http://cr.openjdk.java.net/~henryjen/tl/8024825/0/webrev/
This webrev is a typo fix + the cleanup made in lambda repo in following
changeset.
http://hg.openjdk.java.net/lambda/lambda/jdk/rev/a4babe938c8c
Summary of the changeset,
- Typo fix, Short-circuting should be Short-circuiting
- Javadoc format fix, b{@code parallelStream()}/b cause a space
character in front of parallemStream(), use bcode.../code/b instead.
- remove not needed @SuppressWarnings
- formatting cleanup
Cheers,
Henry
hg: jdk8/tl/jdk: 8024825: Some fixes are missing from java.util.stream spec update
Changeset: a7980b099af1 Author:henryjen Date: 2013-09-06 15:36 -0700 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/a7980b099af1 8024825: Some fixes are missing from java.util.stream spec update Reviewed-by: mduigou ! src/share/classes/java/util/stream/ReferencePipeline.java ! src/share/classes/java/util/stream/Stream.java ! src/share/classes/java/util/stream/package-info.java
hg: jdk8/tl/jdk: 8011916: Spec update for java.util.stream; ...
Changeset: be6f5f027bc2 Author:henryjen Date: 2013-09-06 22:20 -0700 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/be6f5f027bc2 8011916: Spec update for java.util.stream 8024339: j.u.s.Stream.reduce(BinaryOperator) throws unexpected NPE Reviewed-by: mduigou Contributed-by: brian.go...@oracle.com ! src/share/classes/java/util/Collection.java ! src/share/classes/java/util/function/package-info.java ! src/share/classes/java/util/stream/BaseStream.java ! src/share/classes/java/util/stream/Collector.java ! src/share/classes/java/util/stream/Collectors.java ! src/share/classes/java/util/stream/DoublePipeline.java ! src/share/classes/java/util/stream/DoubleStream.java ! src/share/classes/java/util/stream/IntPipeline.java ! src/share/classes/java/util/stream/IntStream.java ! src/share/classes/java/util/stream/LongPipeline.java ! src/share/classes/java/util/stream/LongStream.java ! src/share/classes/java/util/stream/PipelineHelper.java ! src/share/classes/java/util/stream/ReferencePipeline.java ! src/share/classes/java/util/stream/Stream.java ! src/share/classes/java/util/stream/StreamSpliterators.java ! src/share/classes/java/util/stream/StreamSupport.java ! src/share/classes/java/util/stream/package-info.java
Re: RFR: 8024500: Missing API coverage for java.util.function.BiFunction andThen
Yes, the alignment is off, will fix. Cheers, Henry On Sep 11, 2013, at 7:36 AM, Alan Bateman alan.bate...@oracle.com wrote: On 11/09/2013 06:03, Henry Jen wrote: Sorry I forgot it again. Copied but not pasted. :( http://cr.openjdk.java.net/~henryjen/tl/8024500/0/webrev/ Thanks for reviewing. A minor comment but you might want to double check SingletonIterator.java as it looks a bit misaligned (might be tabs, might be webrev). -Alan.
Re: RFR: 8024500: Missing API coverage for java.util.function.BiFunction andThen
Thanks, it should be deliver. I'll fix before push it. Cheers, Henry On Sep 10, 2013, at 10:19 PM, Mike Duigou mike.dui...@oracle.com wrote: Tests look fine. In the BiFunction test the unknown word delieve is used. I am not sure what word was intended. Mike On Sep 10 2013, at 22:03 , Henry Jen wrote: Sorry I forgot it again. Copied but not pasted. :( http://cr.openjdk.java.net/~henryjen/tl/8024500/0/webrev/ Thanks for reviewing. Cheers, Henry On Sep 10, 2013, at 8:47 PM, Henry Jen henry@oracle.com wrote: Hi, Please review a simple webrev that adds two tests, one for java.util.function.BiFunction.andThen() another to cover java.util.Collections.SingletonIterator. Cheers, Henry
hg: jdk8/tl/jdk: 8024500: Missing API coverage for java.util.function.BiFunction andThen
Changeset: 70aab3db56de Author:henryjen Date: 2013-09-11 11:25 -0700 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/70aab3db56de 8024500: Missing API coverage for java.util.function.BiFunction andThen Reviewed-by: mduigou, alanb + test/java/util/Collections/SingletonIterator.java + test/java/util/function/BiFunction/BiFunctionTest.java
RFR: 8024500: Missing API coverage for java.util.function.BiFunction andThen
Hi, Please review a simple webrev that adds two tests, one for java.util.function.BiFunction.andThen() another to cover java.util.Collections.SingletonIterator. Cheers, Henry
Re: RFR: 8024500: Missing API coverage for java.util.function.BiFunction andThen
Sorry I forgot it again. Copied but not pasted. :( http://cr.openjdk.java.net/~henryjen/tl/8024500/0/webrev/ Thanks for reviewing. Cheers, Henry On Sep 10, 2013, at 8:47 PM, Henry Jen henry@oracle.com wrote: Hi, Please review a simple webrev that adds two tests, one for java.util.function.BiFunction.andThen() another to cover java.util.Collections.SingletonIterator. Cheers, Henry
RFR: 8011916: Spec update for java.util.stream + 8024339: j.u.s.Stream.reduce(BinaryOperator) throws unexpected NPE
Hi, Please kindly review webrev at http://cr.openjdk.java.net/~henryjen/ccc/8011916/0/webrev/ The webrev brings over the latest javadoc overhaul occurred in lambda repo. The specdiff can be found at http://cr.openjdk.java.net/~henryjen/ccc/8011916/0/specdiff/overview-summary.html There is also some minor cleanup for code, variable renaming, add @Override etc. Bug 8024339 is resolved with spec updated to clarify NPE could be thrown if reduce operation got a null as result. Cheers, Henry
Re: RFR(2nd): 8017513: Support for closeable streams
On 09/04/2013 12:17 AM, Alan Bateman wrote: On 03/09/2013 20:02, Henry Jen wrote: Updated webrev is at http://cr.openjdk.java.net/~henryjen/ccc/8017513/2/webrev/ Addressed comments from Alan and catch IOExpression when close on exception in Files method. Thanks for sorting out the suppressed exception issue. I notice you've added code to catch Throwable if addSuppressed fails. Is this out of concern for OOME (I assume not IAE ias it can't happen here)? Just to be on the safe side to ensure the first exception is thrown without surprise. For Files case, I don't think IAE or NPE could happen. Similar pattern is implemented for Streams.composeXX method, where it is less certain. Cheers, Henry
RFR(2nd): 8017513: Support for closeable streams
Updated webrev is at
http://cr.openjdk.java.net/~henryjen/ccc/8017513/2/webrev/
Addressed comments from Alan and catch IOExpression when close on
exception in Files method.
Cheers,
Henry
On 08/29/2013 02:15 AM, Alan Bateman wrote:
On 29/08/2013 09:51, Paul Sandoz wrote:
On Aug 29, 2013, at 8:58 AM, Alan Batemanalan.bate...@oracle.com
wrote:
:
I think I mentioned this previously but in the Files.list/walk/etc.
methods where you close the resource (on error|runtimeexception) then
it's probably best to catch the IOException and add it as a
suppressed exception.
Can TWR be used?
try (DirectoryStreamPath ds = Files.newDirectoryStream(dir) {
...
}
It should be only closed for the error/exception case (as the Stream
will keep the directory open).
Henry - Paul's comment reminds of that one or more doesn't need to the
javadoc for Files.lines as it only opens one directory.
-Alan.
RFR: 8024718: Difference in Stream.collect(Collector) methods located in jdk8 and jdk8-lambda repos
Hi, Please review a simple webrev at http://cr.openjdk.java.net/~henryjen/tl/8024178/0/webrev/ The reverted change was included as part of JDK-8015318 fix at late stage, after more thoughts, it is not needed. As a collector? super T, A, R returns a R can be assigned to ? super R, so we don't really need ? extends R part. Cheers, Henry
hg: jdk8/tl/jdk: 8024178: Difference in Stream.collect(Collector) methods located in jdk8 and jdk8-lambda repos
Changeset: 3db3ae4e0853 Author:henryjen Date: 2013-09-03 11:44 -0700 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/3db3ae4e0853 8024178: Difference in Stream.collect(Collector) methods located in jdk8 and jdk8-lambda repos Reviewed-by: mduigou ! src/share/classes/java/util/stream/DelegatingStream.java ! src/share/classes/java/util/stream/ReferencePipeline.java ! src/share/classes/java/util/stream/Stream.java
hg: jdk8/tl/jdk: 8022176: Weaken contract of java.lang.AutoCloseable
Changeset: 77a8c4ad516c Author:henryjen Date: 2013-08-28 14:13 -0700 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/77a8c4ad516c 8022176: Weaken contract of java.lang.AutoCloseable Reviewed-by: alanb, martin, mduigou, psandoz Contributed-by: brian.go...@oracle.com ! src/share/classes/java/lang/AutoCloseable.java
