Re: RFR: JDK-8134373: explore potential uses of convenience factories within the JDK

Yes, you're very welcome Stuart. :-) Kind regards, Jonathan On 12 Oct 2016 21:49, "Patrick Reinhart" wrote: You’re welcome :-) -Patrick Am 12.10.2016 um 22:41 schrieb Stuart Marks : Tests passed, spec change approved, changeset pushed:

Re: RFR: JDK-8134373: explore potential uses of convenience factories within the JDK

You’re welcome :-) -Patrick > Am 12.10.2016 um 22:41 schrieb Stuart Marks : > > Tests passed, spec change approved, changeset pushed: > > http://hg.openjdk.java.net/jdk9/dev/jdk/rev/af71f6a36731 > >

Re: RFR: JDK-8134373: explore potential uses of convenience factories within the JDK

Tests passed, spec change approved, changeset pushed: http://hg.openjdk.java.net/jdk9/dev/jdk/rev/af71f6a36731 Jonathan, thanks for your contribution. And Patrick, thanks again for hosting the webrev. s'marks On 10/12/16 6:53 AM, Jonathan Bluett-Duncan wrote: Hi all, Thank you very

Re: RFR: JDK-8134373: explore potential uses of convenience factories within the JDK

Hi Naoto, Great, thanks for confirming this. s'marks On 10/11/16 4:13 PM, Naoto Sato wrote: +1 for removing the link to Collections#unmodifiableList in the spec. I don't see any particular reason to specify in the javadoc and don't believe it would break any existing apps. Naoto On 10/11/16

Re: RFR: JDK-8134373: explore potential uses of convenience factories within the JDK

Hi all, Thank you very much for all reviewing my changeset over the last few days and for finding the bits I forgot to transfer from webrev01 to webrev02. I've been quiet as I'm still busy with university and will be for the foreseeable future. Stuart, many thanks again for sponsoring the change

Re: RFR: JDK-8134373: explore potential uses of convenience factories within the JDK

Hi Stuart, > It appears that the changes to remove the links to > Collections.unmodifiableList() was dropped from webrev.01 to webrev.02. I've > restored them, along with a couple other changes that were also dropped. I > also updated the ModuleFinder.java comment per a request from Alan Bateman.

Re: RFR: JDK-8134373: explore potential uses of convenience factories within the JDK

+1 for removing the link to Collections#unmodifiableList in the spec. I don't see any particular reason to specify in the javadoc and don't believe it would break any existing apps. Naoto On 10/11/16 4:03 PM, Stuart Marks wrote: On 10/10/16 11:26 PM, Andrej Golovnin wrote:

Re: RFR: JDK-8134373: explore potential uses of convenience factories within the JDK

On 10/10/16 11:26 PM, Andrej Golovnin wrote: src/java.base/share/classes/java/util/ResourceBundle.java 2490 public static class Control { 2491 /** 2492 * The default format List, which contains the strings 2493 * "java.class" and "java.properties", in 2494

Re: RFR: JDK-8134373: explore potential uses of convenience factories within the JDK

Hi Jonathan, src/java.base/share/classes/java/util/ResourceBundle.java 2490 public static class Control { 2491 /** 2492 * The default format List, which contains the strings 2493 * "java.class" and "java.properties", in 2494 * this order. This List is

Re: RFR: JDK-8134373: explore potential uses of convenience factories within the JDK

OK, I'll sponsor this. I need to run this through our internal testing system before pushing it. I'll follow up here with results. s'marks On 10/10/16 1:34 AM, Jonathan Bluett-Duncan wrote: Hi all, Would you kindly review the latest webrev now?

Re: RFR: JDK-8134373: explore potential uses of convenience factories within the JDK

Hi all, Would you kindly review the latest webrev now? http://cr.openjdk.java.net/~reinhapa/reviews/8134373/webrev.02 Thanks in advance. Kind regards, Jonathan On 7 October 2016 at 21:59, Patrick Reinhart wrote: > Here is the latest webrev: > >

Re: RFR: JDK-8134373: explore potential uses of convenience factories within the JDK

Here is the latest webrev: http://cr.openjdk.java.net/~reinhapa/reviews/8134373/webrev.02 -Patrick > Am 07.10.2016 um 12:00 schrieb Jonathan Bluett-Duncan > : > > Hi all, > > Sorry for the delayed response, I've been busy lately with university and > other things.

Re: RFR: JDK-8134373: explore potential uses of convenience factories within the JDK

Hi all, Sorry for the delayed response, I've been busy lately with university and other things. Thank you all for your comments. I'll leave the DateTimeFormatter comment in, as you requested Stephen and Roger, and I'll work again with Patrick as soon as I'm ready. Kind regards, Jonathan On 6

Re: RFR: JDK-8134373: explore potential uses of convenience factories within the JDK

On 6 October 2016 at 00:00, Stuart Marks wrote: >> I think you should perform no change to DateTimeFormatter (other than >> a comment) as part of this changeset. The behaviour of that >> DateTimeFormatter method is subtle, and I now suspect that what we >> have there

Re: RFR: JDK-8134373: explore potential uses of convenience factories within the JDK

On 10/5/16 12:32 PM, Stephen Colebourne wrote: On 5 October 2016 at 17:07, Jonathan Bluett-Duncan wrote: Stephen, thank you for getting back about DateTimeFormatter. It's not clear to me what should be done with TCKDateTimeFormatter::test_resolverFields_listOfOneNull

Re: RFR: JDK-8134373: explore potential uses of convenience factories within the JDK

Hi, I would leave the test and code as is for the purposes of 8133273 and leave it to later to resolve 8167222. There is no compelling need to change it. $.02, Roger On 10/5/2016 4:45 PM, Stuart Marks wrote: Stephen, Thanks for the quick followup clarifications. I'm not entirely sure how

Re: RFR: JDK-8134373: explore potential uses of convenience factories within the JDK

Stephen, Thanks for the quick followup clarifications. I'm not entirely sure how you'd like to proceed; see discussion below. Jonathan, also see below. On 10/5/16 9:07 AM, Jonathan Bluett-Duncan wrote: Stuart, thank you very much for your continued review of this changeset, and for finding

Re: RFR: JDK-8134373: explore potential uses of convenience factories within the JDK

On 5 October 2016 at 17:07, Jonathan Bluett-Duncan wrote: > Stephen, thank you for getting back about DateTimeFormatter. It's not clear > to me what should be done with > TCKDateTimeFormatter::test_resolverFields_listOfOneNull in this case. Do I > delete it; or do I

Re: RFR: JDK-8134373: explore potential uses of convenience factories within the JDK

Hi Jonathan, As soon you got all the changes together, we can go thru them and I will recreate the webrev… -Patrick > Am 05.10.2016 um 18:07 schrieb Jonathan Bluett-Duncan > : > > Stuart, thank you very much for your continued review of this changeset, > and for

Re: RFR: JDK-8134373: explore potential uses of convenience factories within the JDK

Stuart, thank you very much for your continued review of this changeset, and for finding those usages of CookieManager::get in Grepcode for me. I've applied the comment you suggested for ModuleFinder and I've also fixed the NetscapeCookieStore typo. Stephen, thank you for getting back about

Re: RFR: JDK-8134373: explore potential uses of convenience factories within the JDK

On 4 October 2016 at 23:27, Stuart Marks wrote: > 4) The 'resolverFields' problem/comment was regarding DateTimeFormatter (see > this part of latest webrev), where I realised I couldn't use Set.of instead > of Collections.unmodifiableSet(new HashSet<>(Arrays.asList(*))),

Re: RFR: JDK-8134373: explore potential uses of convenience factories within the JDK

On 9/30/16 6:57 AM, Jonathan Bluett-Duncan wrote: 1) It actually didn't occur to me that there was a potential TOCTOU problem in ModuleFinder.compose, despite the fact that I found a potential problem in FileTreeIterator - it completely missed me, so thank you for finding it! I'll see if I can

Re: RFR: JDK-8134373: explore potential uses of convenience factories within the JDK

Hi Stuart, Thanks for doing such a thorough review of the parts you've managed to look at so far. I see you had a number of questions in your numbered bullet points, so I'll do my best to answer them below. 1) It actually didn't occur to me that there was a potential TOCTOU problem in

Re: RFR: JDK-8134373: explore potential uses of convenience factories within the JDK

Hi Jonathan, all, I've started to look at this changeset. I'm looking at the one Patrick Reinhart posted a couple weeks ago (! sorry): http://cr.openjdk.java.net/~reinhapa/reviews/8134373/webrev.01/ I looked at only a few files and I'm already starting to have questions. Not because anybody

Re: RFR: JDK-8134373: explore potential uses of convenience factories within the JDK

Hi all, Stuart, thank you very much for your thorough response. Regarding serializability concerns, I've just checked my changes and all non-test code in the JDK which calls it, and it doesn't seem to me that they affect any fields in `Serializable` classes or the return values of methods which

Re: RFR: JDK-8134373: explore potential uses of convenience factories within the JDK

Hi all, Unfortunately I don't have time to work on any of this at the moment, because of JavaOne preparation, and JavaOne next week. Jonathan, thanks for pushing forward with this. I'm glad that others have picked it up. Patrick, thanks for posting the changeset on Jonathan's behalf. This

Re: RFR: JDK-8134373: explore potential uses of convenience factories within the JDK

Patrick, would you kindly line-wrap the TOCTOU comment in http://cr.openjdk.java.net/~reinhapa/reviews/8134373/webrev.01/src/java.base/share/classes/java/nio/file/FileTreeIterator.java.cdiff.html for me, so that each line has 80 characters or less (and maybe insert a `.` at the end)? Kind

Re: RFR: JDK-8134373: explore potential uses of convenience factories within the JDK

Pavel, > 6. I couldn't find any evidence that `resolverFields` might contain `null`. > Am I missing a javadoc or a line of code? Maybe we should talk to java.time > folks to see if it's the case? > When I ran the tests for java.time, one of them failed because `null` was being passed to

Re: RFR: JDK-8134373: explore potential uses of convenience factories within the JDK

Wow, lots of discussion went on since I was busy doing other stuff! Thanks Patrick for doing the work of creating a new webrev for me. Really appreciated! Pavel already mentioned it, but I think List.of instead of Collections.emptyList in ZoneOffsetTransition is the right thing to do for visual

Re: RFR: JDK-8134373: explore potential uses of convenience factories within the JDK

Hello together, I tried to process all suggested change input into the following new webrev: http://cr.openjdk.java.net/~reinhapa/reviews/8134373/webrev.01 Give me feedback if something is missing/wrong -Patrick On 2016-09-15 13:48, Pavel Rappo wrote: Daniel, Claes, List.of() and

Re: RFR: JDK-8134373: explore potential uses of convenience factories within the JDK

Daniel, Claes, List.of() and Collections.emptyList() are not the same. The behaviours are different. Moreover, immutable static factory methods return instances which are value-based. I believe it also means we are not tied with unconditional instantiation, and in case of empty collections/maps

Re: RFR: JDK-8134373: explore potential uses of convenience factories within the JDK

+1 I don't mind List.of() aesthetically, but there are places where startup/footprint is important where Collections.emptyList() is simply superior, e.g., constituting permanent data structures such as the module graph during early bootstrap. I know there are some drawbacks to the singleton

Re: RFR: JDK-8134373: explore potential uses of convenience factories within the JDK

Daniel, if you're referring to 388 List getValidOffsets() { 389 if (isGap()) { > 390 return List.of(); 391 } 392 return List.of(getOffsetBefore(), getOffsetAfter()); 393 } I think in this particular case, List.of is more consistent. > On 15

Re: RFR: JDK-8134373: explore potential uses of convenience factories within the JDK

The date/time changes seem reasonable. Stephen On 14 September 2016 at 21:55, Patrick Reinhart wrote: > Hi Jonathan, > > Here are your changes in a webrev: > > http://cr.openjdk.java.net/~reinhapa/reviews/8134373/webrev.00 >

Re: RFR: JDK-8134373: explore potential uses of convenience factories within the JDK

Hi, I'm not sure I like the replacement of Collections.emptyList() by List.of(); I find emptyList() more expressive (+ it always returns the same instance). best regards, -- daniel On 15/09/16 11:46, Pavel Rappo wrote: Hi, I have had a look at the change. It looks good. Retrofitting

Re: RFR: JDK-8134373: explore potential uses of convenience factories within the JDK

Hi, I have had a look at the change. It looks good. Retrofitting Collections.unmodifiableXXX/Arrays.asList with Convenience Factory Methods requires extra carefulness as the factory methods are stricter than any of those. Not only do they provide unconditional non-modifiability [0], they also do

Re: RFR: JDK-8134373: explore potential uses of convenience factories within the JDK

Thanks Patrick! Stuart, would you be happy to sponsor this change? If anyone else has any thoughts regarding this change, then I'm all ears. Bug link: https://bugs.openjdk.java.net/browse/JDK-8134373 Rationale for changes:

RFR: JDK-8134373: explore potential uses of convenience factories within the JDK

Hi Jonathan, Here are your changes in a webrev: http://cr.openjdk.java.net/~reinhapa/reviews/8134373/webrev.00 -Patrick > Am 13.09.2016 um 14:45 schrieb Patrick Reinhart : > > Hi Jonathan, > > On 2016-09-13