Re: RFR: 8167237: Jar tool can not correctly find/process the --release option if it occurs before the file list

2016-10-12 Thread Lance Andersen
looks OK Steve

> On Oct 12, 2016, at 8:10 PM, Steve Drach  wrote:
> 
> Hi again,
> 
> Is there anyone willing to review this?  It’s a very simple change.
> 
> Thanks
> Steve
> 
>> Begin forwarded message:
>> 
>> From: Steve Drach 
>> Subject: RFR: 8167237: Jar tool can not correctly find/process the --release 
>> option if it occurs before the file list
>> Date: October 6, 2016 at 1:50:30 PM PDT
>> To: core-libs-dev 
>> 
>> Hi,
>> 
>> Please review the changeset that fixes the problem of not “seeing” the jar 
>> tool —release  option if it is not preceded by anything other than gnu-style 
>> options.  It’s a simple one line change to process —release the same way as 
>> -C.
>> 
>> issue: https://bugs.openjdk.java.net/browse/JDK-8167237 
>> webrev: 
>> http://cr.openjdk.java.net/~sdrach/8167237/webrev.00/ 
>> 
>> 
>> Thanks,
>> Steve

 
  

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





Re: RFR JDK-8166258: Unexpected code conversion by HKSCS converters

2016-10-12 Thread Naoto Sato

Looks good to me.

Naoto

On 10/12/16 3:50 PM, Xueming Shen wrote:

Hi

Please help review the change for #8166258

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

This is an overlook in the code that generates the c2b (encoding) mapping
table from the b2c data. \ufffd is the special code point that used to
indicate
"no mapping" from a native code point to unicode code point. It's wrong to
try to generate a c2b mapping entry from \ufffd.

Thanks,
Sherman


Fwd: RFR: 8167237: Jar tool can not correctly find/process the --release option if it occurs before the file list

2016-10-12 Thread Steve Drach
Hi again,

Is there anyone willing to review this?  It’s a very simple change.

Thanks
Steve

> Begin forwarded message:
> 
> From: Steve Drach 
> Subject: RFR: 8167237: Jar tool can not correctly find/process the --release 
> option if it occurs before the file list
> Date: October 6, 2016 at 1:50:30 PM PDT
> To: core-libs-dev 
> 
> Hi,
> 
> Please review the changeset that fixes the problem of not “seeing” the jar 
> tool —release  option if it is not preceded by anything other than gnu-style 
> options.  It’s a simple one line change to process —release the same way as 
> -C.
> 
> issue: https://bugs.openjdk.java.net/browse/JDK-8167237 
> webrev: 
> http://cr.openjdk.java.net/~sdrach/8167237/webrev.00/ 
> 
> 
> Thanks,
> Steve


RE: RFR(s): 8152617 add missing wildcards to Optional or()andflatMap()

2016-10-12 Thread timo.kinnunen
Hi, 

I know of in/inout/out parameters from GLSL and, with most all conversions in 
GLSL being explicit, they are the easiest thing ever to reason about. In GLSL a 
2D vector and a float array of 2 can both be accessed using array indexing, but 
nobody cares if one should be a subtype of another or vice versa. Not me, not 
the compiler and not the hardware.

Having the caller reason about co-/in-/contra-/use site/declaration site 
variance or existential types (such as subtypes of Optional that will only 
exist in the future, if at all) in exchange for an explicit conversion being 
hidden as an unchecked cast inside a library method (as in this RFR) is a 
lose-lose situation, in my opinion.

If there is no other use case for existential types in Java then Project 
Valhalla needs to start making noises about the imminent deprecation of 
wildcards today.


-- 
Have a nice day, 
Timo

Sent from Mail for Windows 10

From: Remi Forax
Sent: Wednesday, October 12, 2016 14:44
To: timo kinnunen
Cc: Stuart Marks; core-libs-dev
Subject: Re: RFR(s): 8152617 add missing wildcards to Optional or()andflatMap()

Hi Timo,

- Mail original -
> De: "timo kinnunen" 
> À: "Stuart Marks" 
> Cc: "core-libs-dev" 
> Envoyé: Mercredi 12 Octobre 2016 09:33:54
> Objet: RE: RFR(s): 8152617 add missing wildcards to Optional or() 
> andflatMap()

> Hi,
> 
> 
> I’m going to challenge the consensus a little bit. First, Rémi's example can 
> be
> simplified to this one method which fails to compile with the same error
> message[0]:
> 
> private static Optional simple1(
> Optional o, Function f
> ) {return o.flatMap(f);}
> 
> Second, although not in the webrev, Optional::map and Optional::flatMap can be
> implemented without needing any unchecked casts:
> 
> public  Optional flatMap(
> Function mapper
> ) {
> Objects.requireNonNull(mapper);
> return ofNullable(isPresent() ? mapper.apply(get()).orElse(null) : null);
> }
> public  Optional map(
> Function mapper
> ) {
> Objects.requireNonNull(mapper);
> return ofNullable(isPresent() ? mapper.apply(get()) : null);
> }
> 
> These are fully, soundly compile-time typed methods with all types exposed and
> completely under the control of the programmer.

You miss the whole point of wildcards, wildcards are use site variance 
declarations.
When you have a method that takes a Function (with T the type of the 
parameter type and R the return type) as parameter, you can always send a 
function that takes a super type and returns a subtype.
A function is contravariant on its parameter types and covariant on the return 
type.
In a sane language, you should be able to declare something like:

  interface Function {
public R apply(T t);
  }  

but because of the backward compatibility of the collections, in Java, you do 
not use variance annotations when you declare Function but at each time when 
you use it.
So you have to write:
   V foo(Optional optional, Function function) {
...
  }

and to make things less readable, instead of using U+ for U or a subtype of U 
and U- for U and a supertype of U, we use (respectively) ? extends U and ? 
super U, which can be great when your a beginner because you can read it in 
english, by example, ? extends U can be read like that: it's a type i don't 
want to know (?) which is a subtype of U but at the same time because it uses 
the 'extends' keyword like when you specify the bound of a type variable, you 
can easily found that using only type variables is more readable, but it's not. 
A type variable should be used to 'connect' several types in the signature 
together, like the type of the second parameter is the same as the type of the 
return type. So fundamentally, a type variable and a wildcard while denote 
subtyping relationship are two different notations for two different kind of 
usages.  

> 
> Now, to see if the signature of flatMap is truly the problem, it’s possible to
> write the simple1 method as a map followed by a flatMap with the identity
> function, like this:
> 
> private static Optional simple2(
> Optional o, Function f
> ) {return o.map(f).flatMap(Function.identity());}
> 
> In this version, the call to flatMap and its argument don’t use any wildcard
> types, both in the version above and in java.util.Optional[1]. Despite that,
> the compiler from jdk1.8.0_102 still gives an error[2] from the flatMap method
> call. This is quite a curious result as here flatMap and identity are reusing
> types that are already used by type inference. If this isn’t sound but using
> wildcards is then I would really like to see that counterexample!
> 
> Some questions that have arisen (and my answers):
> 
> Should APIs account for types that are not denotable in source code? I’d say 
> no,
> such types are bugs in the language.
> 
> Can non-denotable 

Re: Review request: JDK-8167630 jdeps --generate-module-info forgets to close the resource after checking any unnamed package

2016-10-12 Thread Jonathan Bluett-Duncan
Not a reviewer, but looks good to me. :-)

Kind regards,
Jonathan

On 12 Oct 2016 23:54, "Lance Andersen"  wrote:

> +1
> > On Oct 12, 2016, at 6:52 PM, Mandy Chung  wrote:
> >
> > Simple patch close the ClassFileReader with try-with-resource.
> >
> >
> > diff --git a/src/jdk.jdeps/share/classes/com/sun/tools/jdeps/JdepsTask.java
> b/src/jdk.jdeps/share/classes/com/sun/tools/jdeps/JdepsTask.java
> > --- a/src/jdk.jdeps/share/classes/com/sun/tools/jdeps/JdepsTask.java
> > +++ b/src/jdk.jdeps/share/classes/com/sun/tools/jdeps/JdepsTask.java
> > @@ -680,9 +680,9 @@
> > private boolean genModuleInfo(JdepsConfiguration config) throws
> IOException {
> > // check if any JAR file contains unnamed package
> > for (String arg : inputArgs) {
> > +try (ClassFileReader reader = 
> > ClassFileReader.newInstance(Paths.get(arg)))
> {
> > Optional classInUnnamedPackage =
> > -ClassFileReader.newInstance(Paths.get(arg))
> > -.entries().stream()
> > +reader.entries().stream()
> > .filter(n -> n.endsWith(".class"))
> > .filter(cn -> toPackageName(cn).isEmpty())
> > .findFirst();
> > @@ -696,6 +696,7 @@
> > return false;
> > }
> > }
> > +}
> >
> > ModuleInfoBuilder builder
> > = new ModuleInfoBuilder(config, inputArgs,
> options.genModuleInfo);
> >
> > Thanks
> > Mandy
>
>  
>   <
> http://oracle.com/us/design/oracle-email-sig-198324.gif>
>  Lance Andersen|
> Principal Member of Technical Staff | +1.781.442.2037
> Oracle Java Engineering
> 1 Network Drive
> Burlington, MA 01803
> lance.ander...@oracle.com 
>
>
>
>


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

2016-10-12 Thread Jonathan Bluett-Duncan
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:

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 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 and going through the
whole procedure for me. I look forward to the rest of your results.

Kind regards,
Jonathan


Re: Review request: JDK-8167630 jdeps --generate-module-info forgets to close the resource after checking any unnamed package

2016-10-12 Thread Lance Andersen
+1
> On Oct 12, 2016, at 6:52 PM, Mandy Chung  wrote:
> 
> Simple patch close the ClassFileReader with try-with-resource. 
> 
> 
> diff --git a/src/jdk.jdeps/share/classes/com/sun/tools/jdeps/JdepsTask.java 
> b/src/jdk.jdeps/share/classes/com/sun/tools/jdeps/JdepsTask.java
> --- a/src/jdk.jdeps/share/classes/com/sun/tools/jdeps/JdepsTask.java
> +++ b/src/jdk.jdeps/share/classes/com/sun/tools/jdeps/JdepsTask.java
> @@ -680,9 +680,9 @@
> private boolean genModuleInfo(JdepsConfiguration config) throws 
> IOException {
> // check if any JAR file contains unnamed package
> for (String arg : inputArgs) {
> +try (ClassFileReader reader = 
> ClassFileReader.newInstance(Paths.get(arg))) {
> Optional classInUnnamedPackage =
> -ClassFileReader.newInstance(Paths.get(arg))
> -.entries().stream()
> +reader.entries().stream()
> .filter(n -> n.endsWith(".class"))
> .filter(cn -> toPackageName(cn).isEmpty())
> .findFirst();
> @@ -696,6 +696,7 @@
> return false;
> }
> }
> +}
> 
> ModuleInfoBuilder builder
> = new ModuleInfoBuilder(config, inputArgs, options.genModuleInfo);
> 
> Thanks
> Mandy

 
  

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





Review request: JDK-8167630 jdeps --generate-module-info forgets to close the resource after checking any unnamed package

2016-10-12 Thread Mandy Chung
Simple patch close the ClassFileReader with try-with-resource. 


diff --git a/src/jdk.jdeps/share/classes/com/sun/tools/jdeps/JdepsTask.java 
b/src/jdk.jdeps/share/classes/com/sun/tools/jdeps/JdepsTask.java
--- a/src/jdk.jdeps/share/classes/com/sun/tools/jdeps/JdepsTask.java
+++ b/src/jdk.jdeps/share/classes/com/sun/tools/jdeps/JdepsTask.java
@@ -680,9 +680,9 @@
 private boolean genModuleInfo(JdepsConfiguration config) throws 
IOException {
 // check if any JAR file contains unnamed package
 for (String arg : inputArgs) {
+try (ClassFileReader reader = 
ClassFileReader.newInstance(Paths.get(arg))) {
 Optional classInUnnamedPackage =
-ClassFileReader.newInstance(Paths.get(arg))
-.entries().stream()
+reader.entries().stream()
 .filter(n -> n.endsWith(".class"))
 .filter(cn -> toPackageName(cn).isEmpty())
 .findFirst();
@@ -696,6 +696,7 @@
 return false;
 }
 }
+}
 
 ModuleInfoBuilder builder
 = new ModuleInfoBuilder(config, inputArgs, options.genModuleInfo);

Thanks
Mandy

RFR JDK-8166258: Unexpected code conversion by HKSCS converters

2016-10-12 Thread Xueming Shen

Hi

Please help review the change for #8166258

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

This is an overlook in the code that generates the c2b (encoding) mapping
table from the b2c data. \ufffd is the special code point that used to 
indicate

"no mapping" from a native code point to unicode code point. It's wrong to
try to generate a c2b mapping entry from \ufffd.

Thanks,
Sherman


Re: RFR (JAXP) 8058152: JDK accepts XSLT stylesheet having import element erroneously placed

2016-10-12 Thread Joe Wang

Thanks Naoto, Lance!

I appended _ERR.

-Joe

On 10/12/16, 2:50 PM, Naoto Sato wrote:
+1. You might want to append "_ERR" to the field 
"IMPORT_PRECEDE_OTHERS" to align with other errors (no need for 
further review).


Naoto

On 10/12/16 1:37 PM, Joe Wang wrote:

Hi,

Please review a change to validate that xsl:import element children
/must precede/ all other element children of an 
element, including any  element children as required by
the specification [1].

[1] https://www.w3.org/TR/xslt#import

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

Thanks,
Joe


Re: RFR (JAXP) 8058152: JDK accepts XSLT stylesheet having import element erroneously placed

2016-10-12 Thread Naoto Sato
+1. You might want to append "_ERR" to the field "IMPORT_PRECEDE_OTHERS" 
to align with other errors (no need for further review).


Naoto

On 10/12/16 1:37 PM, Joe Wang wrote:

Hi,

Please review a change to validate that xsl:import element children
/must precede/ all other element children of an 
element, including any  element children as required by
the specification [1].

[1] https://www.w3.org/TR/xslt#import

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

Thanks,
Joe


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

2016-10-12 Thread Roger Riggs

Hi,

Looks ok.

Typically, there is a space after the '//' comment characters, it makes 
it easier to read.
Also, in this case, I don't think the comments help.  "Monday" isn't 
important to the test

and the 'Any Other day' is still 1, not something else.

I would remove those comments.
Otherwise fine, no need for another webrev.

Thanks, Roger


On 10/12/2016 2:16 PM, Anubhav Meena wrote:

Hi Roger,

Thanks for the suggestion, have made the suggested changes and shifted 
the tests to 
/java/time/test/java/time/chrono/TestUmmAlQuraChronology.java.


Updated webrev: 
http://cr.openjdk.java.net/~rchamyal/anmeena/8163330/webrev.04/ 



Thanks,
Anubhav

On Oct 12, 2016, at 8:45 PM, Roger Riggs > wrote:


Hi Anubhav,

In general, I agree with Stephen that the tests should be testing an 
algorithm against facts.
Embedding an algorithm in the test increases the risk that the test 
will just replicate the

implementation code and therefor not be much of a test.

Though in this case, the specification of aligned day of week is of a 
computation.
If the test were to independently compute the correct answer, it 
would be valid as a 'tck' test.


Since the Hijrah calendar is data driven, the tests should be in 
/java/time/*test*/java/time/chrono/TestUmmAlQuraChronology.java.
Tests in java/time/tck/... should correspond directly to specified 
behaviors.
In this case, the algorithm is specified but the test is data 
dependent. (Perhaps a gray area).


Thanks, Roger


On 10/12/2016 11:03 AM, Anubhav Meena wrote:

Hi Stephen,

Have incorporated the changes you suggested. Updated webrev is 
available here
http://cr.openjdk.java.net/~rchamyal/anmeena/8163330/webrev.03/ 



Please review and suggest if anymore changes are required.

Thanks,
Anubhav

On Oct 12, 2016, at 3:21 PM, Anubhav Meena 
> wrote:


Gentle reminder.

On Oct 7, 2016, at 2:12 PM, Anubhav Meena 
> wrote:


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












Re: RFR (JAXP) 8058152: JDK accepts XSLT stylesheet having import element erroneously placed

2016-10-12 Thread Lance Andersen
Looks OK Joe

> On Oct 12, 2016, at 4:37 PM, Joe Wang  wrote:
> 
> Hi,
> 
> Please review a change to validate that xsl:import element children /must 
> precede/ all other element children of an  element, including 
> any  element children as required by the specification [1].
> 
> [1] https://www.w3.org/TR/xslt#import
> 
> JBS: https://bugs.openjdk.java.net/browse/JDK-8058152
> webrev: http://cr.openjdk.java.net/~joehw/jdk9/8058152/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 





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

2016-10-12 Thread Patrick Reinhart
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 
> 
> 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 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 and going through the 
>> whole procedure for me. I look forward to the rest of your results.
>> 
>> Kind regards,
>> Jonathan
>> 
> 



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

2016-10-12 Thread Stuart Marks

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 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 and going through the 
whole procedure for me. I look forward to the rest of your results.


Kind regards,
Jonathan





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

2016-10-12 Thread Stuart Marks

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 4:03 PM, Stuart Marks wrote:



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  * this order. This List is {@linkplain
2495  * Collections#unmodifiableList(List) unmodifiable}.
2496  *
2497  * @see #getFormats(String)
2498  */
2499 public static final List FORMAT_DEFAULT =
List.of("java.class", "java.properties");

I think you should also change the JavaDocs in the
ResourceBundle.Control class for the constants FORMAT_CLASS,
FORMAT_DEFAULT and FORMAT_PROPERTIES, because the JavaDocs for this
constants explicitly mentions, that the lists are created by using
Collections#unmodifiableList(List). Or you cannot change this
constants at all because they are part of the Public API and existed
in this form for a long time. Please ask someone from Oracle for help.
They can explain it better when it is OK to change and when not. Maybe
Stuart can do that.


Hi Andrej,

Thanks for pointing this out.

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. The revised webrev is here:

http://cr.openjdk.java.net/~smarks/reviews/8134373/webrev.03/

In any case, yes, the specifications of the ResourceBundle.Control
fields should be changed to remove the links to
Collections.unmodifiableList(). It's unclear whether having a link this
way implies that it's part of the specification that these fields must
be instances returned from that method. Removing the link makes it clear
that saying the List is unmodifiable is merely a description of the
required behavior.

I spoke with Joe Darcy (our compatibility guru) and we agreed that out
of an abundance of caution it would be wise to file a request to make
this change. (This is the "CCC" - basically an internal change control
process for Java SE specifications.) Doing this is mainly for tracking
purposes, as we believe this to be a compatible change.

I've also included in this request a mention of the change to
CookieManager.get() which we had discussed previously. Even though we
believe this is also a compatible change, it's also a change that should
be tracked.

I'll follow up when this bit of process is finished.

s'marks


RFR (JAXP) 8058152: JDK accepts XSLT stylesheet having import element erroneously placed

2016-10-12 Thread Joe Wang

Hi,

Please review a change to validate that xsl:import element children 
/must precede/ all other element children of an  
element, including any  element children as required by 
the specification [1].


[1] https://www.w3.org/TR/xslt#import

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

Thanks,
Joe


Re: Review Request: JDK-8164689: Retrofit jar, jlink, jmod as a ToolProvider

2016-10-12 Thread Alan Bateman



On 12/10/2016 20:20, Mandy Chung wrote:

Updated webrev:
http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8164689/webrev.01

Mandy


This looks good.

-Alan


Re: RFR 8165793 : Provide an API to query if a ClassLoader is parallel capable

2016-10-12 Thread Mandy Chung

> On Oct 12, 2016, at 11:31 AM, Brent Christian  
> wrote:
> 
> On 10/11/16 8:25 AM, Alan Bateman wrote:
> >
>> One thing that would be good is to beef up
>> the test to cover more scenarios, esp. loader L1 extends loader L2 where
>> you've got 4 combination of capable/non-capable to test.
> 
> I updated the test case to provide better coverage:
> 
> http://cr.openjdk.java.net/~bchristi/8165793/webrev.01/
> 

+1.

Formatting nit: you could break .forEach to the next line

  96 ParaSubCL.class).forEach(IsParallelCapable::testClassLoaderClass);

Mandy

Re: Review Request: JDK-8164689: Retrofit jar, jlink, jmod as a ToolProvider

2016-10-12 Thread Mandy Chung
Updated webrev:
   http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8164689/webrev.01

Mandy

> On Oct 12, 2016, at 9:10 AM, Mandy Chung  wrote:
> 
> 
>> On Oct 11, 2016, at 11:07 PM, Alan Bateman  wrote:
>> 
>> On 11/10/2016 19:03, Mandy Chung wrote:
>> 
>>> This patch updates jar, jlink, jmod tool to be a provider of the new tool 
>>> SPI.  Some tests are also updated to replace the use of internal APIs with 
>>> ToolProvider::findFirst to look up a tool provider.  There are more tests 
>>> that can be updated and something to be cleaned up in the future.
>>> 
>>> http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8164689/webrev.00/index.html
>>> 
>> This update to the tools looks good. For the tests then 
>> ToolProvider.findFirst("jar").get() will draw the attention of the optional 
>> police and might be better to use orElseThrow to supply an exception that 
>> clearly indicates that the tool cannot be found.
> 
> 
> Good suggestion.  Will change that before pushing.
> 
> Thanks
> Mandy



Re: RFR 8165793 : Provide an API to query if a ClassLoader is parallel capable

2016-10-12 Thread Brent Christian

On 10/11/16 8:25 AM, Alan Bateman wrote:
>

One thing that would be good is to beef up
the test to cover more scenarios, esp. loader L1 extends loader L2 where
you've got 4 combination of capable/non-capable to test.


I updated the test case to provide better coverage:

http://cr.openjdk.java.net/~bchristi/8165793/webrev.01/

Thanks,
-Brent


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

2016-10-12 Thread Anubhav Meena
Hi Roger,

Thanks for the suggestion, have made the suggested changes and shifted the 
tests to /java/time/test/java/time/chrono/TestUmmAlQuraChronology.java. 

Updated webrev: http://cr.openjdk.java.net/~rchamyal/anmeena/8163330/webrev.04/

Thanks,
Anubhav

> On Oct 12, 2016, at 8:45 PM, Roger Riggs  wrote:
> 
> Hi Anubhav,
> 
> In general, I agree with Stephen that the tests should be testing an 
> algorithm against facts.
> Embedding an algorithm in the test increases the risk that the test will just 
> replicate the 
> implementation code and therefor not be much of a test.
> 
> Though in this case, the specification of aligned day of week is of a 
> computation.
> If the test were to independently compute the correct answer, it would be 
> valid as a 'tck' test.
> 
> Since the Hijrah calendar is data driven, the tests should be in 
> /java/time/test/java/time/chrono/TestUmmAlQuraChronology.java.
> Tests in java/time/tck/... should correspond directly to specified behaviors.
> In this case, the algorithm is specified but the test is data dependent. 
> (Perhaps a gray area).
> 
> Thanks, Roger
> 
> 
> On 10/12/2016 11:03 AM, Anubhav Meena wrote:
>> Hi Stephen,
>> 
>> Have incorporated the changes you suggested. Updated webrev is available 
>> here 
>> http://cr.openjdk.java.net/~rchamyal/anmeena/8163330/webrev.03/ 
>> 
>> 
>> Please review and suggest if anymore changes are required.
>> 
>> Thanks,
>> Anubhav
>> 
>>> On Oct 12, 2016, at 3:21 PM, Anubhav Meena >> > wrote:
>>> 
>>> Gentle reminder.
>>> 
 On Oct 7, 2016, at 2:12 PM, Anubhav Meena > wrote:
 
 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
>>> 
>> 
> 



Re: Review Request: JDK-8164689: Retrofit jar, jlink, jmod as a ToolProvider

2016-10-12 Thread Mandy Chung

> On Oct 11, 2016, at 11:07 PM, Alan Bateman  wrote:
> 
> On 11/10/2016 19:03, Mandy Chung wrote:
> 
>> This patch updates jar, jlink, jmod tool to be a provider of the new tool 
>> SPI.  Some tests are also updated to replace the use of internal APIs with 
>> ToolProvider::findFirst to look up a tool provider.  There are more tests 
>> that can be updated and something to be cleaned up in the future.
>> 
>> http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8164689/webrev.00/index.html
>> 
> This update to the tools looks good. For the tests then 
> ToolProvider.findFirst("jar").get() will draw the attention of the optional 
> police and might be better to use orElseThrow to supply an exception that 
> clearly indicates that the tool cannot be found.


Good suggestion.  Will change that before pushing.

Thanks
Mandy

Re: DateTimeFormatter.format() uses exceptions for flow control

2016-10-12 Thread Roger Riggs

Created an issue [1] and included the patch.

Thanks, Roger

[1] https://bugs.openjdk.java.net/browse/JDK-8167618


On 10/10/2016 2:53 AM, Clément MATHIEU wrote:

On Mon, 2016-10-10 at 06:47 +1000, David Holmes wrote:

Hi David,


Please note that patches can only be accepted if they are sent
through, or hosted upon OpenJDK infrastructure. If the patch is small
enough can you send it inline in the email (attachments are often
stripped)

Here it is:

--- old/src/java.base/share/classes/java/time/format/DateTimePrintContext.java  
2016-10-09 17:01:30.326739656 +0200
+++ new/src/java.base/share/classes/java/time/format/DateTimePrintContext.java  
2016-10-09 17:01:30.228738595 +0200
@@ -302,13 +302,10 @@
   * @throws DateTimeException if the field is not available and the 
section is not optional
   */
  Long getValue(TemporalField field) {
-try {
+if (optional == 0) {
  return temporal.getLong(field);
-} catch (DateTimeException ex) {
-if (optional > 0) {
-return null;
-}
-throw ex;
+} else {
+return temporal.isSupported(field) ? temporal.getLong(field) : 
null;
  }
  }

Clément MATHIEU




Re: [9] RFR of JDK-8085192: java/rmi/activation/Activatable tests fail intermittently due to "Port already in use"

2016-10-12 Thread Roger Riggs

Hi Chris,


On 10/10/2016 9:43 AM, Chris Hegarty wrote:

Roger,

I addressed all, or most, of your comments in the following
webrev.

 1) Refactored out the use of sun.nio.ch in the test library,
   so that a reduced number of tests need their @modules tag
   updated. ( @modules support with test library usage it a
   pain )
It would be ok to put the modules java.base/sun.nio.ch dependency in the 
test/java/rmi/TEST.properties.

Or perhaps in a new test/java/rmi/activation/TEST.properties

(I would probably move many of the @modules in individual tests there.
Though it is cleaner to have only the ones that are really needed by 
each test.)


Can the @build dependencies also go in TEST.properties using lib.build =



 2) Use Boolean.getBoolean to retrieve the new property

tnx


 3) fix typos and remove stray debugging statements

  http://cr.openjdk.java.net/~chegar/8085192_webrev.03/


test/java/rmi/testlibrary/RMID.java:
 line 60:  ephemeralPort could be removed; (unused)

 line 106: the string 
"java.nio.channels.spi.SelectorProvider=RMIDSelectorProvider" should be

a static and used the JavaVM.  (similar to RMID.EPHEMERAL_MSG)




...
I'm vaguely not very comfortable with scraping the port number off 
stdout

and the inherited channel pieces seem like a lot of moving parts.


Right, I was a little uneasy with this too, to being with, but
it has grown on me ( since it appears stable and reliable in
all my builds and tests ). Also the surface area of the code
change is very small, and the inherit channel mechanism is well
specified and stable.


ok, lets give it a try.

Thanks, Roger




Roger

p.s.  Anyother idea
I assume not all platforms can allow separate processes to open server
sockets to the same port.
If so, we would just have the client allocate a port (0), mark it
non-exclusive and keep it open
while passing the port number to RMID. Only after RMID is started close
the allocating socket.


I believe Stuart did look at this in some detail a while back [1], and
it was somewhat dismissed because of the lack of cross platform support
for SO_REUSEPORT. Maybe things have move on, but I don't think so?

The use of inherit channel is somewhat akin to loading an agent into
the target, but more straightforward.

What do others think, that will have to maintain these tests? I don't
want to make them every harder to maintain. Hamlin's approach is still
on the table too.

-Chris.

[1] 
http://mail.openjdk.java.net/pipermail/serviceability-dev/2014-December/016251.html




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

2016-10-12 Thread Roger Riggs

Hi Anubhav,

In general, I agree with Stephen that the tests should be testing an 
algorithm against facts.
Embedding an algorithm in the test increases the risk that the test will 
just replicate the

implementation code and therefor not be much of a test.

Though in this case, the specification of aligned day of week is of a 
computation.
If the test were to independently compute the correct answer, it would 
be valid as a 'tck' test.


Since the Hijrah calendar is data driven, the tests should be in 
/java/time/*test*/java/time/chrono/TestUmmAlQuraChronology.java.
Tests in java/time/tck/... should correspond directly to specified 
behaviors.
In this case, the algorithm is specified but the test is data dependent. 
(Perhaps a gray area).


Thanks, Roger


On 10/12/2016 11:03 AM, Anubhav Meena wrote:

Hi Stephen,

Have incorporated the changes you suggested. Updated webrev is 
available here
http://cr.openjdk.java.net/~rchamyal/anmeena/8163330/webrev.03/ 



Please review and suggest if anymore changes are required.

Thanks,
Anubhav

On Oct 12, 2016, at 3:21 PM, Anubhav Meena > wrote:


Gentle reminder.

On Oct 7, 2016, at 2:12 PM, Anubhav Meena > wrote:


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








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

2016-10-12 Thread Anubhav Meena
Hi Stephen,

Have incorporated the changes you suggested. Updated webrev is available here 
http://cr.openjdk.java.net/~rchamyal/anmeena/8163330/webrev.03/

Please review and suggest if anymore changes are required.

Thanks,
Anubhav

> On Oct 12, 2016, at 3:21 PM, Anubhav Meena  wrote:
> 
> Gentle reminder.
> 
>> On Oct 7, 2016, at 2:12 PM, Anubhav Meena > > wrote:
>> 
>> 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
> 



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

2016-10-12 Thread Jonathan Bluett-Duncan
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 and going through the
whole procedure for me. I look forward to the rest of your results.

Kind regards,
Jonathan


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

2016-10-12 Thread Remi Forax
Hi Timo,

- Mail original -
> De: "timo kinnunen" 
> À: "Stuart Marks" 
> Cc: "core-libs-dev" 
> Envoyé: Mercredi 12 Octobre 2016 09:33:54
> Objet: RE: RFR(s): 8152617 add missing wildcards to Optional or() 
> andflatMap()

> Hi,
> 
> 
> I’m going to challenge the consensus a little bit. First, Rémi's example can 
> be
> simplified to this one method which fails to compile with the same error
> message[0]:
> 
> private static Optional simple1(
> Optional o, Function f
> ) {return o.flatMap(f);}
> 
> Second, although not in the webrev, Optional::map and Optional::flatMap can be
> implemented without needing any unchecked casts:
> 
> public  Optional flatMap(
> Function mapper
> ) {
> Objects.requireNonNull(mapper);
> return ofNullable(isPresent() ? mapper.apply(get()).orElse(null) : null);
> }
> public  Optional map(
> Function mapper
> ) {
> Objects.requireNonNull(mapper);
> return ofNullable(isPresent() ? mapper.apply(get()) : null);
> }
> 
> These are fully, soundly compile-time typed methods with all types exposed and
> completely under the control of the programmer.

You miss the whole point of wildcards, wildcards are use site variance 
declarations.
When you have a method that takes a Function (with T the type of the 
parameter type and R the return type) as parameter, you can always send a 
function that takes a super type and returns a subtype.
A function is contravariant on its parameter types and covariant on the return 
type.
In a sane language, you should be able to declare something like:

  interface Function {
public R apply(T t);
  }  

but because of the backward compatibility of the collections, in Java, you do 
not use variance annotations when you declare Function but at each time when 
you use it.
So you have to write:
   V foo(Optional optional, Function function) {
...
  }

and to make things less readable, instead of using U+ for U or a subtype of U 
and U- for U and a supertype of U, we use (respectively) ? extends U and ? 
super U, which can be great when your a beginner because you can read it in 
english, by example, ? extends U can be read like that: it's a type i don't 
want to know (?) which is a subtype of U but at the same time because it uses 
the 'extends' keyword like when you specify the bound of a type variable, you 
can easily found that using only type variables is more readable, but it's not. 
A type variable should be used to 'connect' several types in the signature 
together, like the type of the second parameter is the same as the type of the 
return type. So fundamentally, a type variable and a wildcard while denote 
subtyping relationship are two different notations for two different kind of 
usages.  

> 
> Now, to see if the signature of flatMap is truly the problem, it’s possible to
> write the simple1 method as a map followed by a flatMap with the identity
> function, like this:
> 
> private static Optional simple2(
> Optional o, Function f
> ) {return o.map(f).flatMap(Function.identity());}
> 
> In this version, the call to flatMap and its argument don’t use any wildcard
> types, both in the version above and in java.util.Optional[1]. Despite that,
> the compiler from jdk1.8.0_102 still gives an error[2] from the flatMap method
> call. This is quite a curious result as here flatMap and identity are reusing
> types that are already used by type inference. If this isn’t sound but using
> wildcards is then I would really like to see that counterexample!
> 
> Some questions that have arisen (and my answers):
> 
> Should APIs account for types that are not denotable in source code? I’d say 
> no,
> such types are bugs in the language.
> 
> Can non-denotable types be eliminated at the library level by adding more
> wildcards? Unlikely, as such types come from using wildcards to begin with.
> 
> Is there a bug in flatMap or is the bug in the language specification? 
> Compared
> to one simple method, type inference rules are much harder to understand and
> thus more likely to contain undiscovered problems.
> 
> What is gained if these wildcards are added? Although simple1 and simple2 
> would
> compile, you still can’t do anything interesting like calling orElse or
> orElseGet:
> 
> public static void main(String[] args) {
> Optional foo = Optional.ofNullable(args[0]);
> Object o1 = simple1(foo, s -> foo).orElse(o1 = null);
> Object o2 = simple2(foo, s -> foo).orElseGet(() -> null);
> }
> 
> Won’t compile. As far as I can tell, there is no real upside to this change.
> 
> 
> Thanks for your consideration!
> 

Rémi

> 
> 
> 
> [0] Error:(18, 20) java: method flatMap in class java.util.Optional cannot 
> be
> applied to given types;
>  required: java.util.function.Function  java.lang.String,java.util.Optional>
>  found: 

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

2016-10-12 Thread Stephen Colebourne
The indentation in TCKHijrahChronology is not correct.

In TCKHijrahChronology, it would be better to add the expected values
to the provider
ie two additional value for aligned week and aligned day:
{1437, 9, 1, 1, 1}
{1437, 9, 2, 1, 2}
...

That way, the test relies on facts, not on a formula.

Stephen



On 7 October 2016 at 09:42, Anubhav Meena  wrote:
> 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


Re: RFR(s): 8167437: Fix module dependencies for tests that use internal API (java/lang)

2016-10-12 Thread Sergei Kovalev
The fixed version is here: 
http://cr.openjdk.java.net/~skovalev/8167437/webrev.01/


I also modified CR description.


10.10.16 18:15, Sergei Kovalev wrote:

You are right. I'll rollback the change for the file.


10.10.16 18:05, Alan Bateman wrote:
sun.reflect.generics.parser is in java.base, I don't see anything 
using jdk.unsupported/sun.reflect. You can check this quickly by 
running the test without your change.


-Alan




--
With best regards,
Sergei



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

2016-10-12 Thread Anubhav Meena
Gentle reminder.

> On Oct 7, 2016, at 2:12 PM, Anubhav Meena  wrote:
> 
> 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



Reminder

2016-10-12 Thread Anubhav Meena
Hi All,

This is a reminder for the pending 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,
Anubhav


> On Oct 7, 2016, at 2:12 PM, Anubhav Meena  wrote:
> 
> 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



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

2016-10-12 Thread Andrej Golovnin
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.
> The revised webrev is here:
>
> http://cr.openjdk.java.net/~smarks/reviews/8134373/webrev.03/

looks good. Thanks!

Best regards,
Andrej Golovnin

>
> In any case, yes, the specifications of the ResourceBundle.Control fields
> should be changed to remove the links to Collections.unmodifiableList().
> It's unclear whether having a link this way implies that it's part of the
> specification that these fields must be instances returned from that method.
> Removing the link makes it clear that saying the List is unmodifiable is
> merely a description of the required behavior.
>
> I spoke with Joe Darcy (our compatibility guru) and we agreed that out of an
> abundance of caution it would be wise to file a request to make this change.
> (This is the "CCC" - basically an internal change control process for Java
> SE specifications.) Doing this is mainly for tracking purposes, as we
> believe this to be a compatible change.
>
> I've also included in this request a mention of the change to
> CookieManager.get() which we had discussed previously. Even though we
> believe this is also a compatible change, it's also a change that should be
> tracked.
>
> I'll follow up when this bit of process is finished.
>
> s'marks


RE: RFR(s): 8152617 add missing wildcards to Optional or() andflatMap()

2016-10-12 Thread timo.kinnunen
Hi, 


I’m going to challenge the consensus a little bit. First, Rémi's example can be 
simplified to this one method which fails to compile with the same error 
message[0]:

private static Optional simple1(
 Optional o, Function f
) {return o.flatMap(f);} 

Second, although not in the webrev, Optional::map and Optional::flatMap can be 
implemented without needing any unchecked casts:

public  Optional flatMap(
 Function mapper
) {
 Objects.requireNonNull(mapper);
 return ofNullable(isPresent() ? mapper.apply(get()).orElse(null) : null);
}
public  Optional map(
 Function mapper
) {
 Objects.requireNonNull(mapper);
 return ofNullable(isPresent() ? mapper.apply(get()) : null);
}

These are fully, soundly compile-time typed methods with all types exposed and 
completely under the control of the programmer.

Now, to see if the signature of flatMap is truly the problem, it’s possible to 
write the simple1 method as a map followed by a flatMap with the identity 
function, like this:

private static Optional simple2(
 Optional o, Function f
) {return o.map(f).flatMap(Function.identity());}

In this version, the call to flatMap and its argument don’t use any wildcard 
types, both in the version above and in java.util.Optional[1]. Despite that, 
the compiler from jdk1.8.0_102 still gives an error[2] from the flatMap method 
call. This is quite a curious result as here flatMap and identity are reusing 
types that are already used by type inference. If this isn’t sound but using 
wildcards is then I would really like to see that counterexample!

Some questions that have arisen (and my answers):

Should APIs account for types that are not denotable in source code? I’d say 
no, such types are bugs in the language.

Can non-denotable types be eliminated at the library level by adding more 
wildcards? Unlikely, as such types come from using wildcards to begin with.

Is there a bug in flatMap or is the bug in the language specification? Compared 
to one simple method, type inference rules are much harder to understand and 
thus more likely to contain undiscovered problems.

What is gained if these wildcards are added? Although simple1 and simple2 would 
compile, you still can’t do anything interesting like calling orElse or 
orElseGet:

public static void main(String[] args) {
 Optional foo = Optional.ofNullable(args[0]);
 Object o1 = simple1(foo, s -> foo).orElse(o1 = null);
 Object o2 = simple2(foo, s -> foo).orElseGet(() -> null);
}

Won’t compile. As far as I can tell, there is no real upside to this change.


Thanks for your consideration!




[0] Error:(18, 20) java: method flatMap in class java.util.Optional cannot 
be applied to given types;
  required: java.util.function.Function>
  found: java.util.function.Function
  reason: cannot infer type-variable(s) U
(argument mismatch; 
java.util.function.Function cannot be 
converted to java.util.function.Function>)

[1] The  type can be discounted as its presence doesn’t make a 
difference in this case.

[2] Error:(21, 27) java: method flatMap in class wildcards.another.Optional 
cannot be applied to given types;
  required: java.util.function.Function
  found: java.util.function.Function
  reason: inference variable O has incompatible bounds
equality constraints: wildcards.another.Optional,T
upper bounds: wildcards.another.Optional,java.lang.Object


-- 
Have a nice day, 
Timo

Sent from Mail for Windows 10

From: Stuart Marks
Sent: Saturday, October 8, 2016 02:28
To: Stefan Zobel
Cc: core-libs-dev
Subject: Re: RFR(s): 8152617 add missing wildcards to Optional or() andflatMap()



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 

Re: Review Request: JDK-8164689: Retrofit jar, jlink, jmod as a ToolProvider

2016-10-12 Thread Alan Bateman

On 11/10/2016 19:03, Mandy Chung wrote:


This patch updates jar, jlink, jmod tool to be a provider of the new tool SPI.  
Some tests are also updated to replace the use of internal APIs with 
ToolProvider::findFirst to look up a tool provider.  There are more tests that 
can be updated and something to be cleaned up in the future.

http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8164689/webrev.00/index.html

This update to the tools looks good. For the tests then 
ToolProvider.findFirst("jar").get() will draw the attention of the 
optional police and might be better to use orElseThrow to supply an 
exception that clearly indicates that the tool cannot be found.


-Alan