Re: RFR 8152893 : StackWalker#getCallerClass is not filtering hidden/ reflection frames when walker is configured to show hidden /reflection frames

2016-06-03 Thread Brent Christian

On 06/03/2016 04:50 PM, Mandy Chung wrote:



On Jun 3, 2016, at 1:50 PM, Brent Christian  wrote:

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


Looks good.  Nit: copyright end year missing a trailing “,”


Thanks.  Fixed prior to pushing.

-Brent



Re: RFR 8152893 : StackWalker#getCallerClass is not filtering hidden/ reflection frames when walker is configured to show hidden /reflection frames

2016-06-03 Thread Mandy Chung

> On Jun 3, 2016, at 1:50 PM, Brent Christian  
> wrote:
> 
> Updated webrev:
> http://cr.openjdk.java.net/~bchristi/8152893/webrev.01/

Looks good.  Nit: copyright end year missing a trailing “,”

Mandy

Re: RFR JDK-8074819: Resolve disabled warnings for libzip

2016-06-03 Thread Naoto Sato

Looks good to me.

Naoto

On 6/3/16 2:49 PM, Xueming Shen wrote:

Hi,

Please help review the change for JDK-8074819.

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

Thanks,
Sherman


RFR JDK-8074819: Resolve disabled warnings for libzip

2016-06-03 Thread Xueming Shen

Hi,

Please help review the change for JDK-8074819.

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

Thanks,
Sherman


Re: handling the deprecations introduced by early access builds 116 and 118 of jdk 9

2016-06-03 Thread Stuart Marks



On 6/1/16 4:15 PM, Richard Hillegas wrote:

[deprecation warnings]

This was the issue which I faced. The Derby community has spent considerable
effort on maintaining a clean build, one which doesn't swamp real error
indications in a blizzard of diagnostic noise. At the same time, we are
reluctant to wholesale-disable all deprecation warnings because, in general,
they do provide useful advice about best practices. The ameliorations you are
considering do sound useful. I don't have any better suggestions at this time.


Thanks for your followup on this, especially regarding Derby's efforts to 
maintain a clean build. In the JDK we've been working on warnings cleanup for 
several years, so we're sensitive to the issues ourselves. However, we've had 
little visibility into whether other code bases did anything about warnings. 
Some of us speculated that nobody outside the JDK cared about compiler warnings.


We're happy to have been proven wrong about this. But it does mean that we need 
to put more effort into mechanisms to help manage these warnings.


s'marks


Re: RFR 8152893 : StackWalker#getCallerClass is not filtering hidden/ reflection frames when walker is configured to show hidden /reflection frames

2016-06-03 Thread Brent Christian

On 06/02/2016 07:04 PM, Mandy Chung wrote:


Nit: on GetCallerClassTest.java:

178 Class c = (Class) walker.getCallerClass();

The cast should not be needed. Any reason there?


Uh, the reason there is careless copy/paste... :$
Cast removed.


I wonder
if you could move the lambdaRunnable to a different class that the
test will fail without your fix.


Done.  Confirmed that LambdaTest passes with my fix, fails without.

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

Thanks,
-Brent



Re: JDK 9 RFR of JDK-8136738: InputStream documentation for IOException in skip() is unclear or incorrect

2016-06-03 Thread Pavel Rappo
Perfect!

> On 3 Jun 2016, at 20:20, Roger Riggs  wrote:
> 
> +1
> 
> 
> On 6/3/2016 3:15 PM, Brian Burkhalter wrote:
>> So if I make this change to the webrev
>> 
>> --- a/src/java.base/share/classes/java/io/InputStream.java
>> +++ b/src/java.base/share/classes/java/io/InputStream.java
>> @@ -333,8 +333,7 @@
>>  *
>>  * @param n   the number of bytes to be skipped.
>>  * @return the actual number of bytes skipped.
>> - * @throws IOException  if an I/O error occurs, such as attempting 
>> to
>> - * seek to a negative position in a seek-based implementation.
>> + * @throws IOException  if an I/O error occurs.
>>  */
>> public long skip(long n) throws IOException {
>> 
>> do we have consensus?
>> 
>> Thanks,
>> 
>> Brian
>> 
>> On Jun 3, 2016, at 11:34 AM, Pavel Rappo > > wrote:
>> 
>>> 
 On 3 Jun 2016, at 19:30, Bernd Eckenfels > wrote:
 
 It is unclear to me if this is really forbidden in the interface or in
 any implementation. With FileInputStream skip(-5) works.
>>> 
>>> Don't mistake `seek` for `skip`. `skip` can be implemented using `read`, but
>>> may be using `seek`.
>>> 
>>> Here's the API point of view:
>>> 
>>>   * If {@code n} is
>>>   * negative, the {@code skip} method for class {@code InputStream} always
>>>   * returns 0, and no bytes are skipped. Subclasses may handle the negative
>>>   * value differently.
>>> 
>>> Thanks,
>>> -Pavel
>>> 
>>> 
>> 
> 



Re: JDK 9 RFR of JDK-8136738: InputStream documentation for IOException in skip() is unclear or incorrect

2016-06-03 Thread Roger Riggs

+1


On 6/3/2016 3:15 PM, Brian Burkhalter wrote:

So if I make this change to the webrev

--- a/src/java.base/share/classes/java/io/InputStream.java
+++ b/src/java.base/share/classes/java/io/InputStream.java
@@ -333,8 +333,7 @@
  *
  * @param n   the number of bytes to be skipped.
  * @return the actual number of bytes skipped.
- * @throws IOException  if an I/O error occurs, such as 
attempting to

- * seek to a negative position in a seek-based implementation.
+ * @throws IOException  if an I/O error occurs.
  */
 public long skip(long n) throws IOException {

do we have consensus?

Thanks,

Brian

On Jun 3, 2016, at 11:34 AM, Pavel Rappo > wrote:




On 3 Jun 2016, at 19:30, Bernd Eckenfels > wrote:


It is unclear to me if this is really forbidden in the interface or in
any implementation. With FileInputStream skip(-5) works.


Don't mistake `seek` for `skip`. `skip` can be implemented using 
`read`, but

may be using `seek`.

Here's the API point of view:

   * If {@code n} is
   * negative, the {@code skip} method for class {@code InputStream} 
always
   * returns 0, and no bytes are skipped. Subclasses may handle the 
negative

   * value differently.

Thanks,
-Pavel








Re: JDK 9 RFR of JDK-8136738: InputStream documentation for IOException in skip() is unclear or incorrect

2016-06-03 Thread Brian Burkhalter
So if I make this change to the webrev

--- a/src/java.base/share/classes/java/io/InputStream.java
+++ b/src/java.base/share/classes/java/io/InputStream.java
@@ -333,8 +333,7 @@
  *
  * @param  n   the number of bytes to be skipped.
  * @return the actual number of bytes skipped.
- * @throws IOException  if an I/O error occurs, such as attempting to
- * seek to a negative position in a seek-based implementation.
+ * @throws IOException  if an I/O error occurs.
  */
 public long skip(long n) throws IOException {

do we have consensus?

Thanks,

Brian

On Jun 3, 2016, at 11:34 AM, Pavel Rappo  wrote:

> 
>> On 3 Jun 2016, at 19:30, Bernd Eckenfels  wrote:
>> 
>> It is unclear to me if this is really forbidden in the interface or in
>> any implementation. With FileInputStream skip(-5) works.
> 
> Don't mistake `seek` for `skip`. `skip` can be implemented using `read`, but
> may be using `seek`.
> 
> Here's the API point of view:
> 
>* If {@code n} is
>* negative, the {@code skip} method for class {@code InputStream} always
>* returns 0, and no bytes are skipped. Subclasses may handle the negative
>* value differently.
> 
> Thanks,
> -Pavel
> 
> 



Re: RFR (JAXP) 8150187: NPE expected if the system identifier is null for CatalogResolver

2016-06-03 Thread huizhe wang

Thanks Roger, Lance!

Joe

On 6/3/2016 11:26 AM, Lance Andersen wrote:

+1
On Jun 3, 2016, at 1:57 PM, huizhe wang > wrote:


Hi,

Please review a patch that adds null-check to systemId.

JBS: https://bugs.openjdk.java.net/browse/JDK-8150187
webrevs: http://cr.openjdk.java.net/~joehw/jdk9/8150187/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: JDK 9 RFR of JDK-8136738: InputStream documentation for IOException in skip() is unclear or incorrect

2016-06-03 Thread Pavel Rappo

> On 3 Jun 2016, at 19:30, Bernd Eckenfels  wrote:
> 
> It is unclear to me if this is really forbidden in the interface or in
> any implementation. With FileInputStream skip(-5) works.

Don't mistake `seek` for `skip`. `skip` can be implemented using `read`, but
may be using `seek`.

Here's the API point of view:

* If {@code n} is
* negative, the {@code skip} method for class {@code InputStream} always
* returns 0, and no bytes are skipped. Subclasses may handle the negative
* value differently.

Thanks,
-Pavel




Re: JDK 9 RFR of JDK-8136738: InputStream documentation for IOException in skip() is unclear or incorrect

2016-06-03 Thread Bernd Eckenfels
Hello,

I think "seeking negatively" is a bad example for an IO
problem. For this reason alone I would remove it.

It is unclear to me if this is really forbidden in the interface or in
any implementation. With FileInputStream skip(-5) works.

(I would expect it to throw IllegalArgumentException or
UnsupportedOperationExeption instead.)

Gruss
Bernd


 Am Fri, 3 Jun 2016 09:36:11 -0700
schrieb Brian Burkhalter :

> Hi Pavel,
> 
> On Jun 3, 2016, at 3:43 AM, Pavel Rappo 
> wrote:
> 
> > I have a minor question though:
> > 
> > --- a/src/java.base/share/classes/java/io/InputStream.java
> > +++ b/src/java.base/share/classes/java/io/InputStream.java
> > 
> > - * @exception  IOException  if the stream does not support
> > seek,
> > - *  or if some other I/O error occurs.
> > + * @throws IOException  if an I/O error occurs, such as
> > attempting to
> > + * seek to a negative position in a seek-based
> > implementation.
> > 
> > Do we need to mention (even as an example) a case of passing a
> > negative integer position to `seek` function?
> 
> Probably not. Do you or anyone else this this example should be
> removed?
> 
> > The only place where I've found this happens is in
> > java.io.RandomAccessFile#seek
> > 
> > public void seek(long pos) throws IOException {
> >if (pos < 0) {
> >throw new IOException("Negative seek offset");
> >} else {
> >seek0(pos);
> >}
> > }
> > 
> > But this class is not even in the InputStream hierarchy.
> 
> There can be places outside the JDK:
> 
> http://download.java.net/media/jai/javadoc/1.1.3/jai-apidocs/com/sun/media/jai/codec/SeekableStream.html#seek%28long%29
> 
> > But even if someone
> > decides to use seek, can't they choose some other strategy of
> > dealing with negative arguments?
> 
> Yes, they could.
> 
> I concur that the example is not necessary but it is harmless.
> Comments?
> 
> Thanks,
> 
> Brian


Re: RFR (JAXP) 8150187: NPE expected if the system identifier is null for CatalogResolver

2016-06-03 Thread Lance Andersen
+1
> On Jun 3, 2016, at 1:57 PM, huizhe wang  wrote:
> 
> Hi,
> 
> Please review a patch that adds null-check to systemId.
> 
> JBS: https://bugs.openjdk.java.net/browse/JDK-8150187
> webrevs: http://cr.openjdk.java.net/~joehw/jdk9/8150187/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 (JAXP) 8150187: NPE expected if the system identifier is null for CatalogResolver

2016-06-03 Thread Roger Riggs

Looks fine.


On 6/3/2016 1:57 PM, huizhe wang wrote:

Hi,

Please review a patch that adds null-check to systemId.

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

Thanks,
Joe





RFR (JAXP) 8150187: NPE expected if the system identifier is null for CatalogResolver

2016-06-03 Thread huizhe wang

Hi,

Please review a patch that adds null-check to systemId.

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

Thanks,
Joe



Re: JDK 9 RFR of JDK-8136738: InputStream documentation for IOException in skip() is unclear or incorrect

2016-06-03 Thread Brian Burkhalter
Hi Pavel,

On Jun 3, 2016, at 3:43 AM, Pavel Rappo  wrote:

> I have a minor question though:
> 
> --- a/src/java.base/share/classes/java/io/InputStream.java
> +++ b/src/java.base/share/classes/java/io/InputStream.java
> 
> - * @exception  IOException  if the stream does not support seek,
> - *  or if some other I/O error occurs.
> + * @throws IOException  if an I/O error occurs, such as attempting to
> + * seek to a negative position in a seek-based 
> implementation.
> 
> Do we need to mention (even as an example) a case of passing a negative 
> integer
> position to `seek` function?

Probably not. Do you or anyone else this this example should be removed?

> The only place where I've found this happens is in 
> java.io.RandomAccessFile#seek
> 
> public void seek(long pos) throws IOException {
>if (pos < 0) {
>throw new IOException("Negative seek offset");
>} else {
>seek0(pos);
>}
> }
> 
> But this class is not even in the InputStream hierarchy.

There can be places outside the JDK:

http://download.java.net/media/jai/javadoc/1.1.3/jai-apidocs/com/sun/media/jai/codec/SeekableStream.html#seek%28long%29

> But even if someone
> decides to use seek, can't they choose some other strategy of dealing with
> negative arguments?

Yes, they could.

I concur that the example is not necessary but it is harmless. Comments?

Thanks,

Brian

Re: JDK 9 RFR of JDK-8136738: InputStream documentation for IOException in skip() is unclear or incorrect

2016-06-03 Thread Roger Riggs

+1


On 6/2/2016 5:48 PM, Brian Burkhalter wrote:

Please review at your convenience this API doc-only change:

Issue:  https://bugs.openjdk.java.net/browse/JDK-8136738
Patch:  http://cr.openjdk.java.net/~bpb/8136738/webrev.00/

Summary:

Try to make the specification of InputStream.skip() slightly more precise.

Thanks,

Brian




Proposal: java.lang.reflect.Proxy and default methods

2016-06-03 Thread Peter Levart

Hi,

Since Java SE 8 introduced default methods in interfaces there was a 
question what to do with java.lang.reflect.Proxy API. Nothing was done 
to the API at that time, so the default behavior is to proxy default 
methods too. InvocationHandler gets invoked for default methods, but it 
has not provision to forward such calls to the default implementations 
in the interfaces.


I propose a simple API addition that allows calling super default 
methods in proxy instances:


http://cr.openjdk.java.net/~plevart/jdk9-dev/Proxy.invokeSuperDefaults/webrev.02/

With this addition one can simply decide in the InvocationHandler what 
to do with invocations to default methods and can forward such 
invocation to the default implementation:


public class Test {

interface I1 {
default void m() {
System.out.println("  default I1.m() called");
}
}

interface I2 {
default void m() {
System.out.println("  default I2.m() called");
}
}

interface I12 extends I1, I2 {
@Override
void m();

default int sum(int a, int b) {
return a + b;
}

default Object[] concat(Object first, Object... rest) {
Object[] result = new Object[1 + rest.length];
result[0] = first;
System.arraycopy(rest, 0, result, 1, rest.length);
return result;
}
}

public static void main(String[] args) {

InvocationHandler h = (proxy, method, params) -> {
System.out.println("\nInvocationHandler called for: " + 
method +
   " with parameters: " + 
Arrays.toString(params));

if (method.isDefault()) {
try {
return Proxy.invokeSuper(proxy, method, params);
} catch (InvocationTargetException e) {
throw e.getCause();
}
} else {
switch (method.getName()) {
case "m":
System.out.println("  abstract I12.m(): called");
return null;
default:
throw new UnsupportedOperationException(
"Unsupported method: " + method);
}
}
};

I1 i1 = (I1) Proxy.newProxyInstance(
I1.class.getClassLoader(), new Class[]{I1.class}, h);
i1.m();

I2 i2 = (I2) Proxy.newProxyInstance(
I2.class.getClassLoader(), new Class[]{I2.class}, h);
i2.m();

I12 i12 = (I12) Proxy.newProxyInstance(
I12.class.getClassLoader(), new Class[]{I12.class}, h);
i12.m();

System.out.println("  1 + 2 = " + i12.sum(1, 2));
System.out.println("  [1] concat [2, 3, 4] = " +
   Arrays.toString(i12.concat(1, 2, 3, 4)));
}
}


I know FC date is over, but this is really a small change and I have 
heard several people that such feature is missing from the Proxy API.


I'm prepared to create jtreg tests covering the specification if this 
proposal is accepted.


Regards, Peter





Re: RFR : 8073611 : javax.script.ScriptEngineFactory: formatting error in javadoc of getParameter

2016-06-03 Thread Sundararajan Athijegannathan
+1

-Sundar


On 6/3/2016 5:51 PM, Muneer Kolarkunnu wrote:
>
> Hi All,
>
>  
>
> Please review fix for https://bugs.openjdk.java.net/browse/JDK-8073611
>
>  
>
> Webrev : http://cr.openjdk.java.net/~sdama/8073611/webrev.00/
> 
>
>  
>
> Problem: One extra line break has given in the documentation, so
> readers will get confused.
>
> (Ref:
> http://docs.oracle.com/javase/8/docs/api/javax/script/ScriptEngineFactory.html#getParameter-java.lang.String-
> )
>
>  
>
> Solution: Removed that extra line break so that it will come continue
> in the same line.
>
>  
>
> Regards,
>
> Muneer
>
>  
>



RFR : 8073611 : javax.script.ScriptEngineFactory: formatting error in javadoc of getParameter

2016-06-03 Thread Muneer Kolarkunnu
Hi All,

 

Please review fix for https://bugs.openjdk.java.net/browse/JDK-8073611

 

Webrev : http://cr.openjdk.java.net/~sdama/8073611/webrev.00/

 

Problem: One extra line break has given in the documentation, so readers will 
get confused.

(Ref: 
http://docs.oracle.com/javase/8/docs/api/javax/script/ScriptEngineFactory.html#getParameter-java.lang.String-
 )

 

Solution: Removed that extra line break so that it will come continue in the 
same line.

 

Regards,

Muneer

 


Re: JDK 9 RFR of JDK-8136738: InputStream documentation for IOException in skip() is unclear or incorrect

2016-06-03 Thread Pavel Rappo
Hi Brian,

> On 2 Jun 2016, at 22:48, Brian Burkhalter  wrote:
> 
> Please review at your convenience this API doc-only change:
> 
> Issue:https://bugs.openjdk.java.net/browse/JDK-8136738
> Patch:http://cr.openjdk.java.net/~bpb/8136738/webrev.00/
> 
> Summary:
> 
> Try to make the specification of InputStream.skip() slightly more precise.
> 
> Thanks,
> 
> Brian

This looks good. 

I have a minor question though:

--- a/src/java.base/share/classes/java/io/InputStream.java
+++ b/src/java.base/share/classes/java/io/InputStream.java

- * @exception  IOException  if the stream does not support seek,
- *  or if some other I/O error occurs.
+ * @throws IOException  if an I/O error occurs, such as attempting to
+ * seek to a negative position in a seek-based implementation.

Do we need to mention (even as an example) a case of passing a negative integer
position to `seek` function?

The only place where I've found this happens is in java.io.RandomAccessFile#seek

public void seek(long pos) throws IOException {
if (pos < 0) {
throw new IOException("Negative seek offset");
} else {
seek0(pos);
}
}

But this class is not even in the InputStream hierarchy. But even if someone
decides to use seek, can't they choose some other strategy of dealing with
negative arguments?

Thanks,
-Pavel



Re: Review request: JDK-8157892: StackFrame::getFileName returns null when a source file exists for native methods

2016-06-03 Thread Daniel Fuchs

Hi Mandy,

On 6/3/16 12:55 AM, Mandy Chung wrote:

On Jun 2, 2016, at 3:11 PM, Daniel Fuchs  wrote:
>
> Hi Mandy,
>
> Looks good to me. Though I wonder whether a better fix would be
> to not tweak the bci for native methods in the native call that
> sets the value to 0 (I assume this is what happens somewhere
> since bci is initialized to -1 in Java).
>

I think you are suggesting to do the fix in the VM not to set 
StackFrameInfo::bci if it’s a native method and hence it’d be left as -1.  I 
would prefer to do as much work in the java side as possible.


Yes - that's what I was suggesting - as I find it strange
that the VM will set bci=0 if no bci is available.
However your patch as it stands fixes the issue and is good
for me.

best regards,

-- daniel


Re: RFR[9]: 8147585: Annotations with lambda expressions has parameters result in wrong behavior.

2016-06-03 Thread shilpi.rast...@oracle.com

Thanks Vladimir and Joel!!
Please see http://cr.openjdk.java.net/~srastogi/8147585/webrev.04/
Added restriction for synthetic methods as well.

On 6/2/2016 5:04 PM, Vladimir Ivanov wrote:



On 6/2/16 12:02 AM, Joel Borggrén-Franck wrote:

Hi,

I think this was caught by the verifier before 8 since you couldn't have
concrete or private methods in an interface. Now you can (though not in
Java for private ones).

My spider sense tells me there might be something lurking here (though
it was a while since this was in my L1 cache). It is not likely, but I'm
not 100% sure that it is impossible to make javac produce a bridge when
compiling an annotation type for example, so why not remove synthetic
methods as well?


I'm not an expert in annotation processing, but it bothers me as well, 
since current behavior looks too Java-centric. There are valid (in 
JVMS sense) class files which are not produced by javac or from java 
source file.


What is expected behavior in such case? It is unfortunate when 
non-Java languages have to obey to Java rules. It reminds me a problem 
with Class.getSimpleName() (see JDK-8057919 [1]) we fixed some time ago.

Best regards,
Vladimir Ivanov

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



Spending some time with ASM to do a bunch of tests not compilable in
java might be useful, there should also be some frameworks in langtools
to produce invalid classfiles IIRC.
  Raised https://bugs.openjdk.java.net/browse/JDK-8158510 to include 
new test cases.


Regards,
Shilpi


cheers
/Joel

On Tue, 31 May 2016 at 17:49 shilpi.rast...@oracle.com
 > wrote:

Thanks Paul!!
Please see http://cr.openjdk.java.net/~srastogi/8147585/webrev.03/

Thanks,
Shilpi

On 5/31/2016 7:57 PM, Paul Sandoz wrote:
> >"Returns an array containing Method objects reflecting all the
declared methods of the class or interface represented by this Class
object, including public, protected, default (package) access, and
private methods, but excluding inherited methods."