Re: RFR(s): 8152617 add missing wildcards to Optional or() and flatMap()

[Stuart Marks]

On 10/7/16 3:12 PM, Stefan Zobel wrote:

... After having looked at lots of generic APIs, it seems to me that a
style has emerged where wildcards are used whenever possible, and type
parameters are used only when necessary, ...


Yes, I'm familiar with that kind of reasoning (I think Josh Bloch stated
that principle in "Effective Java"). But, to be honest, I never thought
that it should apply as a strict rule in all circumstances.


Yep, it's in Effective Java, near the end of Item 28:

“As a rule, if a type parameter appears only once in a method
declaration, replace it with a wildcard.”


Rhetorical question: do you really think that a signature employing 3
wildcards is easier to understand for the proverbial "average Joe" than
a bounded type parameter that expresses the sub-type relationship clearly?
I do not.

But anyway, you probably have to follow the established "style".

It's just too bad that most Java programmers I know won't understand the
proposed signature of 'flatMap'.


Turns out that Rémi's example exposes the difference between the wildcard 
approach and the type-parameter approach. Returning to the example,


Optional oi = Optional.empty();
Function fm = n -> Optional.empty();
Optional ocs = oi.flatMap(fm);

If the flatMapper function itself has a wildcard type, for example,

Function fm = n -> 
Optional.empty();

then this will still work with the wildcard approach but fail with the 
type-parameter approach. As Rémi also pointed out, a wildcarded type can result 
from the capture of a type with a wildcarded type parameter.


Based on this, I believe the nested wildcard approach to be the correct one.

s'marks

Re: RFR JDK-8166261: Scanner.nextInt(int) (and similar methods) throws PatternSyntaxException

[Naoto Sato]

+1

Naoto

On 10/7/16 4:08 PM, Xueming Shen wrote:

On 10/07/2016 03:52 PM, Naoto Sato wrote:

The test case now looks good. Using biconsumer makes it more readable.

I see the additional paragraph with regard to IllegalArgumentException
in each method description, however, it contradicts the @throws
clause. Need to add @throws IllegalArgumentException for each method?



thanks. updated. somehow I messed up with two versions there.



Naoto

On 10/7/16 2:38 PM, Xueming Shen wrote:

thanks! updated, with biconsumer as well.

http://cr.openjdk.java.net/~sherman/8166261/webrev/


On 10/05/2016 10:25 AM, Naoto Sato wrote:

Looks good to me.

The test case could use IntStream.rangeClosed(Character.MIN_RADIX,
Character.MAX_RADIX) for the good radixes, instead of hard coding ints.

Naoto

On 10/5/16 8:53 AM, Xueming Shen wrote:

Hi

Please help review

issue:  https://bugs.openjdk.java.net/browse/JDK-8166261
webre: http://cr.openjdk.java.net/~sherman/8166261/webrev

The radix sanity check are missing  from
hasNextByte/Short/Int/Long/BigInteger().
The only method we are doing now is useRadix(). The proposed change
here
is to
add the check into all above methods that take a radix.

Arguably it's an incompatible api change, but I don't expect it really
breaks anyone's
code.  Need go through ccc if approved.

Thanks,
Sherman

Re: RFR JDK-8166261: Scanner.nextInt(int) (and similar methods) throws PatternSyntaxException

[Xueming Shen]

On 10/07/2016 03:52 PM, Naoto Sato wrote:

The test case now looks good. Using biconsumer makes it more readable.

I see the additional paragraph with regard to IllegalArgumentException in each 
method description, however, it contradicts the @throws clause. Need to add 
@throws IllegalArgumentException for each method?



thanks. updated. somehow I messed up with two versions there.



Naoto

On 10/7/16 2:38 PM, Xueming Shen wrote:

thanks! updated, with biconsumer as well.

http://cr.openjdk.java.net/~sherman/8166261/webrev/


On 10/05/2016 10:25 AM, Naoto Sato wrote:

Looks good to me.

The test case could use IntStream.rangeClosed(Character.MIN_RADIX,
Character.MAX_RADIX) for the good radixes, instead of hard coding ints.

Naoto

On 10/5/16 8:53 AM, Xueming Shen wrote:

Hi

Please help review

issue:  https://bugs.openjdk.java.net/browse/JDK-8166261
webre: http://cr.openjdk.java.net/~sherman/8166261/webrev

The radix sanity check are missing  from
hasNextByte/Short/Int/Long/BigInteger().
The only method we are doing now is useRadix(). The proposed change here
is to
add the check into all above methods that take a radix.

Arguably it's an incompatible api change, but I don't expect it really
breaks anyone's
code.  Need go through ccc if approved.

Thanks,
Sherman

Re: RFR JDK-8166261: Scanner.nextInt(int) (and similar methods) throws PatternSyntaxException

[Naoto Sato]

The test case now looks good. Using biconsumer makes it more readable.

I see the additional paragraph with regard to IllegalArgumentException 
in each method description, however, it contradicts the @throws clause. 
Need to add @throws IllegalArgumentException for each method?


Naoto

On 10/7/16 2:38 PM, Xueming Shen wrote:

thanks! updated, with biconsumer as well.

http://cr.openjdk.java.net/~sherman/8166261/webrev/


On 10/05/2016 10:25 AM, Naoto Sato wrote:

Looks good to me.

The test case could use IntStream.rangeClosed(Character.MIN_RADIX,
Character.MAX_RADIX) for the good radixes, instead of hard coding ints.

Naoto

On 10/5/16 8:53 AM, Xueming Shen wrote:

Hi

Please help review

issue:  https://bugs.openjdk.java.net/browse/JDK-8166261
webre: http://cr.openjdk.java.net/~sherman/8166261/webrev

The radix sanity check are missing  from
hasNextByte/Short/Int/Long/BigInteger().
The only method we are doing now is useRadix(). The proposed change here
is to
add the check into all above methods that take a radix.

Arguably it's an incompatible api change, but I don't expect it really
breaks anyone's
code.  Need go through ccc if approved.

Thanks,
Sherman

Re: RFR(s): 8152617 add missing wildcards to Optional or() and flatMap()

[Remi Forax]

- Mail original -
> De: "Stuart Marks" 
> À: "Paul Sandoz" 
> Cc: "core-libs-dev" 
> Envoyé: Vendredi 7 Octobre 2016 21:22:09
> Objet: Re: RFR(s): 8152617 add missing wildcards to Optional or() and 
> flatMap()

> On 10/7/16 11:23 AM, Paul Sandoz wrote:
>>>flatMap(Function> mapper)
>>
>> Optional is final so why do you need to express “? extends Optional” ?
> 
> The short answer is, it doesn't work if you don't have it. :-)
> 
> The theoretical answer is that in this context, "? extends P" means "some
> subtype of P" and not necessarily just a subclass of P.
> 
> (And even though Optional is final, it's not "permanently final" in that a
> hypothetical future version of the class library might change it to non-final,
> allowing subclasses.)
> 
> This issue is covered in Angelika Langer's Generics FAQ entry, "What do
> multi-level (i.e., nested) wildcards mean?" [1] Note that the answer begins, 
> "It
> depends." :-) I also note in passing that I've read this about five times and
> I'm still not quite sure I understand it entirely.
> 
> For me, the best explanation comes from looking at examples. First, the 
> history
> is that the signature in Java 8 is:
> 
>   #1flatMap(Function> mapper)
> 
> I believe Rémi originally proposed something like this (although it was about
> or(), the same issue applies to flatMap()):
> 
>   #2flatMap(Function> mapper)
> 
> The suggested fix that ended up in bug report was this:
> 
>   #3flatMap(Function> mapper)
> 
> But this doesn't work for reasons I explain below. Finally, I'm proposing:
> 
>   #4flatMap(Function> mapper)
> 
> In researching old email threads and chasing links, I ran across an example 
> from
> Stefan Zobel [2] that fails with #3. He had an alternative that didn't seem
> quite right to me, so I ended up with #4.
> 
> I've adapted Stefan's example as follows:
> 
>   Optional oi = Optional.empty();
>   Function fm = n -> Optional.empty();
>   Optional ocs = oi.flatMap(fm);
> 
> The flatmapper function 'fm' returns Optional. In the 
> assignment
> on the last line, U is CharSequence, so we can compare this to the various
> signatures shown above with U filled in.
> 
> Case #1 fails because Optional is incompatible with
> Optional. This is the usual "generics are invariant thing". Even
> though SB is a subtype of CS, Optional isn't a subtype of Optional.
> 
> Case #2 fails because adding a wildcard there doesn't help matters, since
> Optional is still unrelated to Optional.
> 
> Now for the tricky part. :-)
> 
> Surely case #3 should work, because adding an inner wildcard provides a
> subtyping relationship, so Optional is a subtype of Optional CS>.
> 
> That much is true, but these are nested within the Function<> generic type, so
> the "generics are invariant" rule applies again. Thus,
> 
> Function<..., Optional>
> 
> is not a subtype of
> 
> Function<..., Optional>
> 
> To get around this, we have to add the outer wildcard as well, so that
> 
> Function<..., Optional>
> 
> is a subtype of
> 
> Function<..., ? extends Optional>
> 
> So that's what ended up in the signature.
> 
> Similar analysis applies to the or() case.
> 
> Now awaiting a message from Rémi telling me my explanation is incorrect in 
> 3...
> 2... 1... :-) :-)

no i think you are right :)

here is an example that shows that the signature of Optional.flatmap is not 
correct
  public static  Optional first(List list) {
return list.stream().findFirst();
  }
  
  public static void bar(List list) {
Function fun = s -> first(list);
Optional.of("foo").flatMap(fun );
  }
  
  public static void main(String[] args) {
bar(List.of("hello"));
bar(List.of(new StringBuilder("hello")));
  }

while Optional is final, you can still create an Optional when doing a capture, i.e. when you see a wildcard type as a 
parameterized type.
here, first() is called with a List so T = ? extends 
CharSequence so the return type of first() is an Optional.

> 
> s'marks

Rémi

> 
> 
> 
> [1]
> http://angelikalanger.com/GenericsFAQ/FAQSections/TypeArguments.html#What%20do%20multilevel%20wildcards%20mean?
> 
> [2] https://sourceforge.net/p/streamsupport/tickets/125/#2d90

Re: RFR(s): 8152617 add missing wildcards to Optional or() and flatMap()

[Stefan Zobel]

Hi Stuart,

thanks for explaining.

> ... After having looked at lots of generic APIs, it seems to me that a
> style has emerged where wildcards are used whenever possible, and type
> parameters are used only when necessary, ...

Yes, I'm familiar with that kind of reasoning (I think Josh Bloch stated
that principle in "Effective Java"). But, to be honest, I never thought
that it should apply as a strict rule in all circumstances.

Rhetorical question: do you really think that a signature employing 3
wildcards is easier to understand for the proverbial "average Joe" than
a bounded type parameter that expresses the sub-type relationship clearly?
I do not.

But anyway, you probably have to follow the established "style".

It's just too bad that most Java programmers I know won't understand the
proposed signature of 'flatMap'.

Kind regards,
Stefan

Re: RFR: 8159855: Create an SPI for tools

[Mandy Chung]

> On Oct 7, 2016, at 2:40 PM, Jonathan Gibbons  
> wrote:
> 
> Updated webrev with feedback from comments:
> 
> * use doPrivileged within ToolProvider.findFIrst  (includes adding new test)
> * improve whitespace in doc comments
> 
> Webrev: http://cr.openjdk.java.net/~jjg/8159855/webrev.05/

Looks good.  Thanks for adding the new test.

Nit: line 68: s/nonzero/non-zero

Mandy

Re: RFR JDK-8166261: Scanner.nextInt(int) (and similar methods) throws PatternSyntaxException

[Xueming Shen]

thanks! updated, with biconsumer as well.

http://cr.openjdk.java.net/~sherman/8166261/webrev/


On 10/05/2016 10:25 AM, Naoto Sato wrote:

Looks good to me.

The test case could use IntStream.rangeClosed(Character.MIN_RADIX, 
Character.MAX_RADIX) for the good radixes, instead of hard coding ints.

Naoto

On 10/5/16 8:53 AM, Xueming Shen wrote:

Hi

Please help review

issue:  https://bugs.openjdk.java.net/browse/JDK-8166261
webre: http://cr.openjdk.java.net/~sherman/8166261/webrev

The radix sanity check are missing  from
hasNextByte/Short/Int/Long/BigInteger().
The only method we are doing now is useRadix(). The proposed change here
is to
add the check into all above methods that take a radix.

Arguably it's an incompatible api change, but I don't expect it really
breaks anyone's
code.  Need go through ccc if approved.

Thanks,
Sherman

Re: RFR: 8159855: Create an SPI for tools

[Jonathan Gibbons]

Updated webrev with feedback from comments:

* use doPrivileged within ToolProvider.findFIrst  (includes adding new test)
* improve whitespace in doc comments

Webrev: http://cr.openjdk.java.net/~jjg/8159855/webrev.05/

-- Jon



On 10/04/2016 09:19 PM, Mandy Chung wrote:

This SPI is useful and provides as a replacement to existing use of internal 
APIs to launch some of our tools.  We will get jar, jmod, jlink and possibly 
other tools to convert to this SPI.

ToolProvider::findFirst(String name) can find tool providers on classpath.  I 
think it needs to wrap the for-loop (specifically iterating on providers) with 
doPrivileged due to the stack-based permission check.

Otherwise, looks good.

Mandy


On Oct 4, 2016, at 4:46 PM, Jonathan Gibbons  
wrote:

Resend with non-mostly-empty subject line!

-- Jon

On 10/04/2016 04:39 PM, Jonathan Gibbons wrote:

Core-libs folk,

Please review the following change to add a new service provider class
java.util.spi.ToolProvider

which can be used provide simple "command-line" access to select JDK
tools, without starting a new JVM.

The following tools are updated to provide access through the new SPI:
javac, javadoc, javap, jdeps

It is expected that additional tools will also be updated to provide access,
but that will be done separately.

Compiler-dev folk may wish to review the changes to the langtools repository.

JBS: https://bugs.openjdk.java.net/browse/JDK-8159855
Webrev: http://cr.openjdk.java.net/~jjg/8159855/webrev.03/
API: 
http://cr.openjdk.java.net/~jjg/8159855/api.02/java/util/spi/ToolProvider.html

-- Jon

Re: RFR(s): 8152617 add missing wildcards to Optional or() and flatMap()

[Stuart Marks]

On 10/7/16 12:40 PM, Stefan Zobel wrote:

What's wrong with the alternative "additional bounded type parameter" approach?

Couldn't we also get by with something like

 Optional flatMap(Function> mapper)

and

 Optional or(Supplier supplier)

Personally, I find that much easier to digest. But that's only me, of course.


Yes, the additional type parameter seems to work for this example. I'm a bit 
surprised.


I think it's mostly a style thing. To me, a type parameter carries a certain 
amount of semantic weight, implying that it's a concern that the caller needs to 
pay attention to. (Note that type parameters are documented using the javadoc 
@param tag, and by convention every parameter must be documented.) By contrast, 
a wildcard says "anything that satisfies the constraints" but we don't care 
exactly what it is.


If a type parameter is used only once, it can (always, I think) be replaced by a 
wildcard. After having looked at lots of generic APIs, it seems to me that a 
style has emerged where wildcards are used whenever possible, and type 
parameters are used only when necessary, e.g. when the exact same type is 
required in multiple places. I don't know if this is actually written down 
anywhere though.


A difference *would* arise between the multiple-wildcards and 
additional-type-parameter approaches, if it were possible to have subclasses of 
Optional.
Of course, it doesn't apply to this case, since Optional is final, and we have 
no plans to make it non-final!


s'marks

Re: RFR(s): 8152617 add missing wildcards to Optional or() and flatMap()

[Stuart Marks]

On 10/7/16 1:09 PM, Andrej Golovnin wrote:

267 public Optional flatMap(Function> mapper)

I think there should be a space between “public” and “”.


Sure, I'll add this. There's also a space missing at a similar spot at the map() 
declaration; I'll fix that too.


thanks

s'marks

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

[Patrick Reinhart]

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.
> 
> 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 October 2016 at 09:38, Stephen Colebourne  wrote:
> 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 might be the best option.
> >
> > I had recommended removing the comment from DateTimeFormatter, but if
> > Stephen wants the comment in, that's fine with me. In fact I'll defer to
> > what Stephen (and Roger Riggs) want with this code, since they're the
> > maintainers.
> 
> I think it makes sense to leave the new comment in. All further change
> should be under 8167222.
> 
> Stephen
> 

Re: RFR(s): 8152617 add missing wildcards to Optional or() and flatMap()

[Andrej Golovnin]

Hi Stuart,

> Webrev:
> 
>   http://cr.openjdk.java.net/~smarks/reviews/8152617/webrev.0/


267 public Optional flatMap(Function> mapper)

I think there should be a space between “public” and “”.

Best regards,
Andrej Golovnin

Re: RFR(s): 8152617 add missing wildcards to Optional or() and flatMap()

[Stefan Zobel]

2016-10-07 21:42 GMT+02:00 Michael Nascimento :
> Doesn't work, as Stuart has noted (s'marks, as far as I know your
> explanation is 100% correct). Nested generics == pain :-(

Hi Michael,

sorry for being obtrusive. What exactly doesn't work?

Stuart's example

Optional oi = Optional.empty();
Function fm = n -> Optional.empty();
Optional ocs = oi.flatMap(fm);
System.out.println(ocs.orElse("empty"));

work's for me (on 1.8.0_51). Sorry, I'm just trying to understand.

Kind regards,
Stefan


>
> Regards,
> Michael
>

Re: RFR(s): 8152617 add missing wildcards to Optional or() and flatMap()

[Paul Sandoz]

> On 7 Oct 2016, at 12:22, Stuart Marks  wrote:
> 
> 
> 
> On 10/7/16 11:23 AM, Paul Sandoz wrote:
>>>   flatMap(Function> mapper)
>> 
>> Optional is final so why do you need to express “? extends Optional” ?
> 
> The short answer is, it doesn't work if you don't have it. :-)
> 

I thought as much :-)


> The theoretical answer is that in this context, "? extends P" means "some 
> subtype of P" and not necessarily just a subclass of P.
> 

Doh!, yes, that makes sense. Thanks for the detailed explanation. Fix looks 
good.

Paul.

Re: RFR(s): 8152617 add missing wildcards to Optional or() and flatMap()

[Michael Nascimento]

Doesn't work, as Stuart has noted (s'marks, as far as I know your
explanation is 100% correct). Nested generics == pain :-(

Regards,
Michael

On Fri, Oct 7, 2016 at 4:40 PM, Stefan Zobel  wrote:

> 2016-10-07 21:22 GMT+02:00 Stuart Marks :
> >
> >
> > On 10/7/16 11:23 AM, Paul Sandoz wrote:
> >>>
> >>>flatMap(Function> mapper)
> >>
> >>
> >> Optional is final so why do you need to express “? extends Optional” ?
> >
> >
> > The short answer is, it doesn't work if you don't have it. :-)
> >
> > The theoretical answer is that in this context, "? extends P" means
> "some
> > subtype of P" and not necessarily just a subclass of P.
> >
> > (And even though Optional is final, it's not "permanently final" in that
> a
> > hypothetical future version of the class library might change it to
> > non-final, allowing subclasses.)
> >
> > This issue is covered in Angelika Langer's Generics FAQ entry, "What do
> > multi-level (i.e., nested) wildcards mean?" [1] Note that the answer
> begins,
> > "It depends." :-) I also note in passing that I've read this about five
> > times and I'm still not quite sure I understand it entirely.
> >
> > For me, the best explanation comes from looking at examples. First, the
> > history is that the signature in Java 8 is:
> >
> >   #1flatMap(Function> mapper)
> >
> > I believe Rémi originally proposed something like this (although it was
> > about or(), the same issue applies to flatMap()):
> >
> >   #2flatMap(Function> mapper)
> >
> > The suggested fix that ended up in bug report was this:
> >
> >   #3flatMap(Function> mapper)
> >
> > But this doesn't work for reasons I explain below. Finally, I'm
> proposing:
> >
> >   #4flatMap(Function>
> mapper)
> >
> > In researching old email threads and chasing links, I ran across an
> example
> > from Stefan Zobel [2] that fails with #3. He had an alternative that
> didn't
> > seem quite right to me, so I ended up with #4.
> >
> > I've adapted Stefan's example as follows:
> >
> > Optional oi = Optional.empty();
> > Function fm = n ->
> > Optional.empty();
> > Optional ocs = oi.flatMap(fm);
> >
> > The flatmapper function 'fm' returns Optional. In the
> > assignment on the last line, U is CharSequence, so we can compare this to
> > the various signatures shown above with U filled in.
> >
> > Case #1 fails because Optional is incompatible with
> > Optional. This is the usual "generics are invariant thing".
> > Even though SB is a subtype of CS, Optional isn't a subtype of
> > Optional.
> >
> > Case #2 fails because adding a wildcard there doesn't help matters, since
> > Optional is still unrelated to Optional.
> >
> > Now for the tricky part. :-)
> >
> > Surely case #3 should work, because adding an inner wildcard provides a
> > subtyping relationship, so Optional is a subtype of Optional extends
> > CS>.
> >
> > That much is true, but these are nested within the Function<> generic
> type,
> > so the "generics are invariant" rule applies again. Thus,
> >
> > Function<..., Optional>
> >
> > is not a subtype of
> >
> > Function<..., Optional>
> >
> > To get around this, we have to add the outer wildcard as well, so that
> >
> > Function<..., Optional>
> >
> > is a subtype of
> >
> > Function<..., ? extends Optional>
> >
> > So that's what ended up in the signature.
> >
> > Similar analysis applies to the or() case.
> >
> > Now awaiting a message from Rémi telling me my explanation is incorrect
> in
> > 3... 2... 1... :-) :-)
> >
> > s'marks
> >
> >
> >
> > [1]
> > http://angelikalanger.com/GenericsFAQ/FAQSections/
> TypeArguments.html#What%20do%20multilevel%20wildcards%20mean?
> >
> > [2] https://sourceforge.net/p/streamsupport/tickets/125/#2d90
> >
>
>
>
> Hhm, that's really mind-boggling!
>
> What's wrong with the alternative "additional bounded type parameter"
> approach?
>
> Couldn't we also get by with something like
>
>  Optional flatMap(Function>
> mapper)
>
>
> and
>
>
>  Optional or(Supplier supplier)
>
>
> Personally, I find that much easier to digest. But that's only me, of
> course.
>
>
> Regards,
> Stefan
>

Re: RFR(s): 8152617 add missing wildcards to Optional or() and flatMap()

[Stefan Zobel]

2016-10-07 21:22 GMT+02:00 Stuart Marks :
>
>
> On 10/7/16 11:23 AM, Paul Sandoz wrote:
>>>
>>>flatMap(Function> mapper)
>>
>>
>> Optional is final so why do you need to express “? extends Optional” ?
>
>
> The short answer is, it doesn't work if you don't have it. :-)
>
> The theoretical answer is that in this context, "? extends P" means "some
> subtype of P" and not necessarily just a subclass of P.
>
> (And even though Optional is final, it's not "permanently final" in that a
> hypothetical future version of the class library might change it to
> non-final, allowing subclasses.)
>
> This issue is covered in Angelika Langer's Generics FAQ entry, "What do
> multi-level (i.e., nested) wildcards mean?" [1] Note that the answer begins,
> "It depends." :-) I also note in passing that I've read this about five
> times and I'm still not quite sure I understand it entirely.
>
> For me, the best explanation comes from looking at examples. First, the
> history is that the signature in Java 8 is:
>
>   #1flatMap(Function> mapper)
>
> I believe Rémi originally proposed something like this (although it was
> about or(), the same issue applies to flatMap()):
>
>   #2flatMap(Function> mapper)
>
> The suggested fix that ended up in bug report was this:
>
>   #3flatMap(Function> mapper)
>
> But this doesn't work for reasons I explain below. Finally, I'm proposing:
>
>   #4flatMap(Function> mapper)
>
> In researching old email threads and chasing links, I ran across an example
> from Stefan Zobel [2] that fails with #3. He had an alternative that didn't
> seem quite right to me, so I ended up with #4.
>
> I've adapted Stefan's example as follows:
>
> Optional oi = Optional.empty();
> Function fm = n ->
> Optional.empty();
> Optional ocs = oi.flatMap(fm);
>
> The flatmapper function 'fm' returns Optional. In the
> assignment on the last line, U is CharSequence, so we can compare this to
> the various signatures shown above with U filled in.
>
> Case #1 fails because Optional is incompatible with
> Optional. This is the usual "generics are invariant thing".
> Even though SB is a subtype of CS, Optional isn't a subtype of
> Optional.
>
> Case #2 fails because adding a wildcard there doesn't help matters, since
> Optional is still unrelated to Optional.
>
> Now for the tricky part. :-)
>
> Surely case #3 should work, because adding an inner wildcard provides a
> subtyping relationship, so Optional is a subtype of Optional CS>.
>
> That much is true, but these are nested within the Function<> generic type,
> so the "generics are invariant" rule applies again. Thus,
>
> Function<..., Optional>
>
> is not a subtype of
>
> Function<..., Optional>
>
> To get around this, we have to add the outer wildcard as well, so that
>
> Function<..., Optional>
>
> is a subtype of
>
> Function<..., ? extends Optional>
>
> So that's what ended up in the signature.
>
> Similar analysis applies to the or() case.
>
> Now awaiting a message from Rémi telling me my explanation is incorrect in
> 3... 2... 1... :-) :-)
>
> s'marks
>
>
>
> [1]
> http://angelikalanger.com/GenericsFAQ/FAQSections/TypeArguments.html#What%20do%20multilevel%20wildcards%20mean?
>
> [2] https://sourceforge.net/p/streamsupport/tickets/125/#2d90
>



Hhm, that's really mind-boggling!

What's wrong with the alternative "additional bounded type parameter" approach?

Couldn't we also get by with something like

 Optional flatMap(Function> mapper)


and


 Optional or(Supplier supplier)


Personally, I find that much easier to digest. But that's only me, of course.


Regards,
Stefan

Re: RFR(s): 8152617 add missing wildcards to Optional or() and flatMap()

[Stuart Marks]

On 10/7/16 11:23 AM, Paul Sandoz wrote:

   flatMap(Function> mapper)


Optional is final so why do you need to express “? extends Optional” ?


The short answer is, it doesn't work if you don't have it. :-)

The theoretical answer is that in this context, "? extends P" means "some 
subtype of P" and not necessarily just a subclass of P.


(And even though Optional is final, it's not "permanently final" in that a 
hypothetical future version of the class library might change it to non-final, 
allowing subclasses.)


This issue is covered in Angelika Langer's Generics FAQ entry, "What do 
multi-level (i.e., nested) wildcards mean?" [1] Note that the answer begins, "It 
depends." :-) I also note in passing that I've read this about five times and 
I'm still not quite sure I understand it entirely.


For me, the best explanation comes from looking at examples. First, the history 
is that the signature in Java 8 is:


  #1flatMap(Function> mapper)

I believe Rémi originally proposed something like this (although it was about 
or(), the same issue applies to flatMap()):


  #2flatMap(Function> mapper)

The suggested fix that ended up in bug report was this:

  #3flatMap(Function> mapper)

But this doesn't work for reasons I explain below. Finally, I'm proposing:

  #4flatMap(Function> mapper)

In researching old email threads and chasing links, I ran across an example from 
Stefan Zobel [2] that fails with #3. He had an alternative that didn't seem 
quite right to me, so I ended up with #4.


I've adapted Stefan's example as follows:

Optional oi = Optional.empty();
Function fm = n -> Optional.empty();
Optional ocs = oi.flatMap(fm);

The flatmapper function 'fm' returns Optional. In the assignment 
on the last line, U is CharSequence, so we can compare this to the various 
signatures shown above with U filled in.


Case #1 fails because Optional is incompatible with 
Optional. This is the usual "generics are invariant thing". Even 
though SB is a subtype of CS, Optional isn't a subtype of Optional.


Case #2 fails because adding a wildcard there doesn't help matters, since 
Optional is still unrelated to Optional.


Now for the tricky part. :-)

Surely case #3 should work, because adding an inner wildcard provides a 
subtyping relationship, so Optional is a subtype of Optional.


That much is true, but these are nested within the Function<> generic type, so 
the "generics are invariant" rule applies again. Thus,


Function<..., Optional>

is not a subtype of

Function<..., Optional>

To get around this, we have to add the outer wildcard as well, so that

Function<..., Optional>

is a subtype of

Function<..., ? extends Optional>

So that's what ended up in the signature.

Similar analysis applies to the or() case.

Now awaiting a message from Rémi telling me my explanation is incorrect in 3... 
2... 1... :-) :-)


s'marks



[1] 
http://angelikalanger.com/GenericsFAQ/FAQSections/TypeArguments.html#What%20do%20multilevel%20wildcards%20mean?


[2] https://sourceforge.net/p/streamsupport/tickets/125/#2d90

Re: RFR 8151486 : Class.forName causes memory leak

[Mandy Chung]

> On Oct 7, 2016, at 11:25 AM, Brent Christian  
> wrote:
> 
> On 10/5/16 4:43 PM, David Holmes wrote:
>>> 
 Okay but this will still affect the lifecycle of the PDs because
 without the strong reference in L, those weak references in the VM
 will quickly be cleared.
> 
> There's also a strong reference held by the Class object itself (on the VM 
> side [1]).
> 

Yes this is the protection domain of the Class itself (i.e. PD associated with 
its code source) that was set when the class is defined.

> Thanks for having a look, folks.  I've applied Naoto and Mandy's suggestions 
> for the test case and updated the webrev in place with what I plan to push.
> 
> http://cr.openjdk.java.net/~bchristi/8151486/webrev.00/

It might be good to refactor System.getProperty("test.classes", ".”) as a 
static final field.  Otherwise looks good.

No need for a new webrev.

Mandy

Re: RFR 8151486 : Class.forName causes memory leak

[Brent Christian]

On 10/5/16 4:43 PM, David Holmes wrote:



Okay but this will still affect the lifecycle of the PDs because
without the strong reference in L, those weak references in the VM
will quickly be cleared.


There's also a strong reference held by the Class object itself (on the 
VM side [1]).


Thanks for having a look, folks.  I've applied Naoto and Mandy's 
suggestions for the test case and updated the webrev in place with what 
I plan to push.


http://cr.openjdk.java.net/~bchristi/8151486/webrev.00/

-Brent

1. 
http://hg.openjdk.java.net/jdk9/dev/hotspot/file/fec31089c2ef/src/share/vm/classfile/javaClasses.cpp#l917

Re: RFR(s): 8152617 add missing wildcards to Optional or() and flatMap()

[Paul Sandoz]

> On 7 Oct 2016, at 10:20, Stuart Marks  wrote:
> 
> Hi all,
> 
> Please review this small API adjustment to Optional.or and flatMap to add 
> wildcards. This provides a bit more flexibility to callers about the types of 
> functions they can provide to these methods.
> 
> The or() method is new in 9 so there is no compatibility issue.
> 
> The flatMap() method was introduced in 8, so this is a change to an existing 
> API. There shouldn't be a binary compatibility issue, since the method's 
> erasure doesn't change. I *think* it is source compatible, as anything that 
> was accepted by the old signature:

Source computability should be ok IICU because Optional is final and there are 
no overriding methods.

> 
>flatMap(Function> mapper)
> 
> should also be accepted by the new signature:
> 
>flatMap(Function> mapper)
> 
> But there may be some subtle issues of which I'm unaware.
> 

Optional is final so why do you need to express “? extends Optional” ?

Paul.

> Bug report:
> 
>   https://bugs.openjdk.java.net/browse/JDK-8152617
> 
> Webrev:
> 
>   http://cr.openjdk.java.net/~smarks/reviews/8152617/webrev.0/
> 
> Thanks,
> 
> s'marks

Re: RFR (JAXP) 8139584: XMLStreamWriterImpl does not write 'standalone' property

[Joe Wang]

Thanks Lance!

On 10/7/16, 10:07 AM, Lance Andersen wrote:

Hi Joe,

Looks fine

Best
Lance
On Oct 7, 2016, at 12:10 PM, Joe Wang > wrote:


Hi,

Please review a fix for declaring the 'standalone' attribute.

JBS: https://bugs.openjdk.java.net/browse/JDK-8139584
webrev: http://cr.openjdk.java.net/~joehw/jdk9/8139584/webrev/ 



Thanks,
Joe




Lance 
Andersen| Principal Member of Technical Staff | +1.781.442.2037

Oracle Java Engineering
1 Network Drive
Burlington, MA 01803
lance.ander...@oracle.com 

RFR(s): 8152617 add missing wildcards to Optional or() and flatMap()

[Stuart Marks]

Hi all,

Please review this small API adjustment to Optional.or and flatMap to add 
wildcards. This provides a bit more flexibility to callers about the types of 
functions they can provide to these methods.


The or() method is new in 9 so there is no compatibility issue.

The flatMap() method was introduced in 8, so this is a change to an existing 
API. There shouldn't be a binary compatibility issue, since the method's erasure 
doesn't change. I *think* it is source compatible, as anything that was accepted 
by the old signature:


flatMap(Function> mapper)

should also be accepted by the new signature:

flatMap(Function> mapper)

But there may be some subtle issues of which I'm unaware.

Bug report:

https://bugs.openjdk.java.net/browse/JDK-8152617

Webrev:

http://cr.openjdk.java.net/~smarks/reviews/8152617/webrev.0/

Thanks,

s'marks

Re: RFR (JAXP) 8139584: XMLStreamWriterImpl does not write 'standalone' property

[Lance Andersen]

Hi Joe,

Looks fine

Best
Lance
> On Oct 7, 2016, at 12:10 PM, Joe Wang  wrote:
> 
> Hi,
> 
> Please review a fix for declaring the 'standalone' attribute.
> 
> JBS: https://bugs.openjdk.java.net/browse/JDK-8139584
> webrev: http://cr.openjdk.java.net/~joehw/jdk9/8139584/webrev/
> 
> Thanks,
> Joe

 
  

 Lance Andersen| 
Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering 
1 Network Drive 
Burlington, MA 01803
lance.ander...@oracle.com 

RFR (JAXP) 8139584: XMLStreamWriterImpl does not write 'standalone' property

[Joe Wang]

Hi,

Please review a fix for declaring the 'standalone' attribute.

JBS: https://bugs.openjdk.java.net/browse/JDK-8139584
webrev: http://cr.openjdk.java.net/~joehw/jdk9/8139584/webrev/

Thanks,
Joe

Re: RFR: JDK-8166648,,jib make run-test for langtools results in intermittent failures on windows-x86

[Tim Bell]

Again with the review link .. see below:

On 10/07/16 06:40, Tim Bell wrote:

Hello

The change of HotSpot runtimes on 32-bit Windows [1] has been a source
of langtools test failures due to memory space exhaustion.

On our Windows build/test clients, the combination of the C2 runtime and
parallel GC causes about 20 extra threads to be started in each VM.
These added thread stacks, plus heap management in C2, result in too
much memory pressure to fit the workload into 32-bit VMs.

This fix will run langtools jtreg tests using the serial garbage
collector instead of using the default collector (parallel GC).


http://cr.openjdk.java.net/~tbell/8166648/



Testing:
   -5 out of 5 jobs (with the fix) passed langtools testing
   -4 out of 5 control jobs (without the fix) failed testing in
windows_i586_6.3-product-c2-langtools_jtreg


Thanks in advance-

Tim

[1] JDK-8154209 "Remove client VM from default JIB profile on
windows-x86 and linux-x86"
https://bugs.openjdk.java.net/browse/JDK-8154209

Re: RFR: JDK-8166648,,jib make run-test for langtools results in intermittent failures on windows-x86

[Erik Joelsson]

Hello Tim,

Assuming this is the webrev: 
http://cr.openjdk.java.net/~tbell/8166648/webrev/


It looks ok to me.

/Erik

On 2016-10-07 15:40, Tim Bell wrote:

Hello

The change of HotSpot runtimes on 32-bit Windows [1] has been a source 
of langtools test failures due to memory space exhaustion.


On our Windows build/test clients, the combination of the C2 runtime 
and parallel GC causes about 20 extra threads to be started in each 
VM. These added thread stacks, plus heap management in C2, result in 
too much memory pressure to fit the workload into 32-bit VMs.


This fix will run langtools jtreg tests using the serial garbage 
collector instead of using the default collector (parallel GC).


Testing:
   -5 out of 5 jobs (with the fix) passed langtools testing
   -4 out of 5 control jobs (without the fix) failed testing in 
windows_i586_6.3-product-c2-langtools_jtreg



Thanks in advance-

Tim

[1] JDK-8154209 "Remove client VM from default JIB profile on 
windows-x86 and linux-x86"

https://bugs.openjdk.java.net/browse/JDK-8154209

RFR: JDK-8166648,,jib make run-test for langtools results in intermittent failures on windows-x86

[Tim Bell]

Hello

The change of HotSpot runtimes on 32-bit Windows [1] has been a source 
of langtools test failures due to memory space exhaustion.


On our Windows build/test clients, the combination of the C2 runtime and 
parallel GC causes about 20 extra threads to be started in each VM. 
These added thread stacks, plus heap management in C2, result in too 
much memory pressure to fit the workload into 32-bit VMs.


This fix will run langtools jtreg tests using the serial garbage 
collector instead of using the default collector (parallel GC).


Testing:
   -5 out of 5 jobs (with the fix) passed langtools testing
   -4 out of 5 control jobs (without the fix) failed testing in 
windows_i586_6.3-product-c2-langtools_jtreg



Thanks in advance-

Tim

[1] JDK-8154209 "Remove client VM from default JIB profile on 
windows-x86 and linux-x86"

https://bugs.openjdk.java.net/browse/JDK-8154209

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

[Jonathan Bluett-Duncan]

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 October 2016 at 09:38, Stephen Colebourne  wrote:

> 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 might be the best option.
> >
> > I had recommended removing the comment from DateTimeFormatter, but if
> > Stephen wants the comment in, that's fine with me. In fact I'll defer to
> > what Stephen (and Roger Riggs) want with this code, since they're the
> > maintainers.
>
> I think it makes sense to leave the new comment in. All further change
> should be under 8167222.
>
> Stephen
>

RFR:JDK-8163330:HijrahDate aligned day of week incorrect

[Anubhav Meena]

Hi all,

Please review.

Bug Id :  https://bugs.openjdk.java.net/browse/JDK-8163330 


Issue:  The HijrahDate class incorrectly calculates the aligned-day-of-week 
field. It based the calculation on the day-of-week, when it should be based on 
the day-of-month.

Webrev: http://cr.openjdk.java.net/~rchamyal/anmeena/8163330/webrev.02/ 



-- 
Thanks and Regards,
Anubhav