Re: JDK 9 RFR Update String.join sample code to use List convenience factory methods

[Stuart Marks]

On 5/31/16 6:31 PM, Joseph D. Darcy wrote:

The String.join javadoc contains some sample code to demonstrate how to use the
method. The sample code can be improved with the new-in-JDK-9 List convenience
factory method. Please review this patch to update the sample code:

--- a/src/java.base/share/classes/java/lang/String.javaTue May 31 17:54:41
2016 -0700
+++ b/src/java.base/share/classes/java/lang/String.javaTue May 31 18:27:45
2016 -0700
@@ -2424,9 +2424,7 @@
  *
  * For example,
  * {@code
- * List strings = new LinkedList<>();
- * strings.add("Java");strings.add("is");
- * strings.add("cool");
+ * List strings = List.of("Java", "is", "cool");
  * String message = String.join(" ", strings);
  * //message returned is: "Java is cool"
  *


This change looks good: +1 for removing a usage of LinkedList, +1 for removing 
multiple statements on the same line, and +1 for using the new JEP 269 API!



(A corresponding update should *not* be made to the String.join sample using a
Set since the Set convenience factory methods do not guarantee ordering.)


Correct.

But the Set example is:


 Set strings = new LinkedHashSet<>();
 strings.add("Java"); strings.add("is");
 strings.add("very"); strings.add("cool");
 String message = String.join("-", strings);


It would be good if you could make the add() calls one per line.

Alternatively, you could do

Set strings =
new LinkedHashSet<>(List.of("Java", "is", "very", "cool"));
String message = String.join("-", strings);

Up to you.

s'marks

"very-cool-Java-is" -- Yoda

Re: JDK 9 RFR Update String.join sample code to use List convenience factory methods

[Lance Andersen]

looks fine joe
> On May 31, 2016, at 9:31 PM, Joseph D. Darcy  wrote:
> 
> Hello,
> 
> The String.join javadoc contains some sample code to demonstrate how to use 
> the method. The sample code can be improved with the new-in-JDK-9 List 
> convenience factory method. Please review this patch to update the sample 
> code:
> 
> --- a/src/java.base/share/classes/java/lang/String.javaTue May 31 
> 17:54:41 2016 -0700
> +++ b/src/java.base/share/classes/java/lang/String.javaTue May 31 
> 18:27:45 2016 -0700
> @@ -2424,9 +2424,7 @@
>  *
>  * For example,
>  * {@code
> - * List strings = new LinkedList<>();
> - * strings.add("Java");strings.add("is");
> - * strings.add("cool");
> + * List strings = List.of("Java", "is", "cool");
>  * String message = String.join(" ", strings);
>  * //message returned is: "Java is cool"
>  *
> 
> (A corresponding update should *not* be made to the String.join sample using 
> a Set since the Set convenience factory methods do not guarantee ordering.)
> 
> 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 

JDK 9 RFR Update String.join sample code to use List convenience factory methods

[Joseph D. Darcy]

Hello,

The String.join javadoc contains some sample code to demonstrate how to 
use the method. The sample code can be improved with the new-in-JDK-9 
List convenience factory method. Please review this patch to update the 
sample code:


--- a/src/java.base/share/classes/java/lang/String.javaTue May 31 
17:54:41 2016 -0700
+++ b/src/java.base/share/classes/java/lang/String.javaTue May 31 
18:27:45 2016 -0700

@@ -2424,9 +2424,7 @@
  *
  * For example,
  * {@code
- * List strings = new LinkedList<>();
- * strings.add("Java");strings.add("is");
- * strings.add("cool");
+ * List strings = List.of("Java", "is", "cool");
  * String message = String.join(" ", strings);
  * //message returned is: "Java is cool"
  *

(A corresponding update should *not* be made to the String.join sample 
using a Set since the Set convenience factory methods do not guarantee 
ordering.)


Thanks,

-Joe

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

[Stuart Marks]

On 5/30/16 11:48 AM, Richard Hillegas wrote:

Dalibor Topic recommended that I post this feedback on core-libs-dev. This is my
feedback after ameliorating the deprecation warnings which surfaced when I
compiled and tested Apache Derby with early access builds 116 and 118 of JDK 9.
Derby is a pure Java relational database whose original code goes back almost 20
years. Other large, old code bases (like Weblogic) may have similar experiences.
More detail on my experience can be found on the JIRA issue which tracks the
Derby community's attempt to keep our code evergreen against JDK 9:
https://issues.apache.org/jira/browse/DERBY-6856


Hi Rick,

Thanks for your feedback on the API deprecations.

A couple notes on deprecation. First, the deprecation JEP (JEP 277) [1] has 
clarified the definition of deprecation so that by default it no longer means 
that the API will be removed. In the absence of forRemoval=true, deprecation is 
merely a recommendation for code to migrate away from the annotated API. Only 
when the forRemoval=true element is present does it mean that the API is 
actually going to be removed. None of these deprecations has forRemoval=true, 
this means that there's no great urgency for anyone to migrate away from them.


Now, they will generate compilation warnings, which is quite possibly a problem. 
There are some existing mechanisms for disabling warnings, such as 
-Xlint:-deprecation and the @SuppressWarnings annotation. These might not be 
sufficient. We're considering adding some finer-grained mechanisms. Ideally, for 
deprecated APIs that aren't being removed, it should be possible to manage the 
warnings so that migration of any code base can proceed at whatever pace its 
maintainers feel is appropriate, without it being forced by any particular JDK 
release.


If you have any thoughts on how to better manage deprecation warnings, I'd love 
to hear them.



o Deprecating autoboxing constructors - Deprecating the autoboxing constructors
for primitive wrapper objects caused a large rototill of Derby code. That
rototill was comparable in size to the changes made necessary by Java 5's
introduction of generics. Hopefully, IDEs can automate much of this chore.


The boxing constructors -- e.g., new Integer(432) -- are the ones being 
deprecated. The preferred alternative is Integer.valueOf(432). Note that 
*auto*boxing ends calling valueOf() under the covers. Autoboxing is generally 
preferable, although not without pitfalls, such as the overloading of 
List.remove(int) vs List.remove(Object), as you stumbled across in the 
referenced bug report. Using valueOf() instead of autoboxing would have avoided 
the error.



o Deprecating Class.newInstance() - The deprecation of Class.newInstance()
forced a similarly large rototill. The code became more verbose. Additional
exceptions had to be caught and propagated up the call stack. For reasons which
I don't understand, I had better luck using Class.getConstructor().newInstance()
than Class.getDeclaredConstructor().newInstance(). But the former replacement
code requires you to make constructors public. For some code bases, that may
introduce security problems which are worse than the security problem being
addressed by this deprecation. I hope that IDEs and the release notes for JDK 9
will provide some guidance for how to handle these issues.


It would be good to understand why getDeclaredConstructor() didn't work. Clearly 
requiring a public no-arg constructor is a non-starter.



o Deprecating java.util.Observable and java.util.Observer - Two ameliorations
are recommended at
http://mail.openjdk.java.net/pipermail/core-libs-dev/2016-April/040436.html. The
first suggestion (use the awt event model) runs very much counter to the whole
intent of Jigsaw. That is because pulling in awt can bloat up an application
with large, otherwise unneeded libraries. Using awt was out of the question for
Derby, given that the community had already invested a great deal of effort in
paring back Derby's dependencies in order to let the code run on JDK 8 compact
profile 2. That left us with the other option: write your own replacement
classes. If a lot of people end up having to write the same replacement code,
then that argues for leaving this small but useful functionality in the JDK. I
think that the people who advocated for this deprecation did not have good
visibility into how widely these classes are being used in the wild. I recommend
that this deprecation be re-evaluated.


Observable and Observer have a long history of problem reports and enhancement 
requests that show that people want them to be something other than what they 
are. This includes: Observable should be an interface, not an abstract class; 
there is only one "changed" bit, without any notion of what has changed; there 
is no control over what thread calls observers; there is no ability to control 
sequence of calls to observers; change notifications aren't in one-for-one 
correspondence with 

Re: RFR(s): 8157777: RMI test DeadCachedConnection doesn't wait for registry to die

[Joseph D. Darcy]

Hi Stuart,

Looks good; thanks,

-Joe

On 5/31/2016 5:01 PM, Stuart Marks wrote:

Hi all,

Please review this small test fix to improve the reliability of an RMI 
test. Basically this waits for a subprocess to exit instead of 
proceeding immediately.


webrev:

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

bug:

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

Thanks,

s'marks

RFR(s): 8157777: RMI test DeadCachedConnection doesn't wait for registry to die

[Stuart Marks]

Hi all,

Please review this small test fix to improve the reliability of an RMI test. 
Basically this waits for a subprocess to exit instead of proceeding immediately.


webrev:

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

bug:

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

Thanks,

s'marks

Re: RFR: JDK-8157850 Jar tests should pass through VM options

[Martin Buchholz]

Pushed.

Can someone give Andrey some commit bits?

On Tue, May 31, 2016 at 10:23 AM, Martin Buchholz  wrote:
> I approve this change.
>
> On Mon, May 30, 2016 at 4:59 PM, David Holmes  wrote:
>> On 27/05/2016 2:20 AM, Andrey Nazarov wrote:
>>>
>>> Thanks for feedback guys.
>>>
>>> I've updated review
>>> http://cr.openjdk.java.net/~anazarov/8157850/webrev.02/
>>
>>
>> Using test.tool.vm.opts seems reasonable for jar and javac.
>>
>>> Please sponsor this patch if you are OK.
>>
>>
>> Sorry not my area.
>>
>> Thanks,
>> David
>>
>>
>>> My use case is to run tests with different -Xms and -Xmx options. Mostly
>>> due to I need to increase heap size to gather code coverage by jcov.
>>>
>>> --Andrey
>>>
>>> On 25.05.2016 23:42, Jonathan Gibbons wrote:



 On 05/25/2016 01:33 PM, David Holmes wrote:
>
> On 26/05/2016 6:04 AM, Jonathan Gibbons wrote:
>>
>> Using JAVA_OPTIONS for tools is conceptually wrong.
>>
>> JAVA_OPTIONS is specifically intended to pass options to VM instances,
>> as created by a "java" command.  It is not intended that you should
>> prefix the options with -J and use them for arbitrary tools.
>
>
> I have to agree. There is no guarantee the options being passed for
> the VM under test make any sense for the running of the jar tool in
> the jar tests. I think a number of hotspot related test options could
> cause problems here.
>
> Are there specific VM options that people think should be passed
> through? The bug report has no detail at all.
>
> David


 Generally, I think that blindly passing through all the options
 regardless is as bad a programming practice as never passing them
 though.  There are some that make sense to all VMs, like -ea, -esa
 etc, and maybe system properties, but for those, the VM options
 mechanism is generally good enough.  (i.e. system properties
 test.vm.opts, test.tool.vm.opts)

 From a jtreg point of view, I'd be interested to know uses cases for
 passing additional more specific options to the VMs used to run tools
 like jar, jlink, javac, etc

 -- Jon.



>
>> The code in the webrev is also perverse for taking the trouble to split
>> the string to a stream, collect the results into a list, convert the
>> list back into a stream again and use .forEach.
>>
>> You could do better, and much simpler, with something like
>>
>> if (!option.isEmpty()) {
>> commands.addAll(Arrays.asList(option.split("\\s+",-1)));
>> }
>>
>> -- Jon
>>
>>
>> On 05/25/2016 10:48 AM, Martin Buchholz wrote:
>>>
>>> Hi Andrey,
>>>
>>> Looking at http://openjdk.java.net/jtreg/vmoptions.html,
>>> I see we have both test.vm.opts and test.tool.vm.opts
>>> and the latter is supposed to take care of adding "-J".
>>>
>>> +VM_OPTIONS.stream().map(opt -> "-J" +
>>> opt).forEach(commands::add);
>>> +JAVA_OPTIONS.stream().map(opt -> "-J" +
>>> opt).forEach(commands::add);
>>>
>>> ---
>>>
>>> Maybe splitting on "\\s+" would be better.
>>>
>>> ---
>>>
>>> I think we should have test library methods to return the List
>>> for java subprocesses, one that could try hard to get the option
>>> tokenization correct.
>>>
>>> ---
>>>
>>>
>>> On Wed, May 25, 2016 at 9:07 AM, Andrey Nazarov
>>>  wrote:

 Some jar tests start VMs without passing vmoptions from jtreg.

 This fix pass jtreg's vmoptions and javaoptions to processes(java,
 jar,
 javac) started by tests.

 webrev: http://cr.openjdk.java.net/~anazarov/8157850/webrev.01/

 jbs: https://bugs.openjdk.java.net/browse/JDK-8157850

 --Andrey

>>

>>>
>>

Re: JDK 9 RFR of problem listing of java_sql_Timestamp.java

[Sergey Bylokhov]

I think that we can fix the bug itself since the fix is trivial and 
already proposed by Mandy, no?


On 01.06.16 1:17, Joseph D. Darcy wrote:

Hello,

After the push for , the test

java/beans/XMLEncoder/java_sql_Timestamp.java

is failing across platforms. I'd like to problem list the test until the
fix for JDK-8158281: "java_sql_Timestamp.java fails with AssertionError"
is ready.

Patch below.

Thanks,

-Joe

--- a/test/ProblemList.txtTue May 31 13:15:48 2016 -0700
+++ b/test/ProblemList.txtTue May 31 15:16:18 2016 -0700
@@ -122,6 +122,8 @@
 java/beans/Introspector/8132566/OverridePropertyInfoTest.java 8132565
generic-all
 java/beans/Introspector/8132566/OverrideUserDefPropertyInfoTest.java
8132565 generic-all

+java/beans/XMLEncoder/java_sql_Timestamp.java 8158281 generic-all
+
 


 # jdk_lang




--
Best regards, Sergey.

JDK 9 RFR of problem listing of java_sql_Timestamp.java

[Joseph D. Darcy]

Hello,

After the push for , the test

java/beans/XMLEncoder/java_sql_Timestamp.java

is failing across platforms. I'd like to problem list the test until the 
fix for JDK-8158281: "java_sql_Timestamp.java fails with AssertionError" 
is ready.


Patch below.

Thanks,

-Joe

--- a/test/ProblemList.txtTue May 31 13:15:48 2016 -0700
+++ b/test/ProblemList.txtTue May 31 15:16:18 2016 -0700
@@ -122,6 +122,8 @@
 java/beans/Introspector/8132566/OverridePropertyInfoTest.java 8132565 
generic-all
 java/beans/Introspector/8132566/OverrideUserDefPropertyInfoTest.java 
8132565 generic-all


+java/beans/XMLEncoder/java_sql_Timestamp.java 8158281 generic-all
+
 

 # jdk_lang

RFR (JAXP) 8158246: Several api/org_xml/sax/helpers/XMLReader tests failed due to no SAXException occurs

[huizhe wang]

Hi,

This issue was caused by the change 8152912 where the logic was changed 
so that the parser was directly instantiated when the className was 
within the DEFAULT_PACKAGE. The problem is that the className can be 
wrong even if it contains the DEFAULT_PACKAGE.


The patch reverts the change 8152912 and adds a check for the default 
class name so that only if the specified className is the default 
parser, an instance is instantiated.


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

8152912 webrev:
http://cr.openjdk.java.net/~joehw/jdk9/8152912/webrev01/src/java.xml/share/classes/org/xml/sax/helpers/NewInstance.java.udiff.html

Thanks,
Joe

JDK 9 RFR of JDK-5040830: (ann) please improve toString() for annotations containing exception proxies

[joe darcy]

Hello,

Some background, when everything is going well, the toString form of an 
annotation looks something like


// Old non-erroneous annotation

@DangerousAnnotation(utopia=BRIGADOON,
thirtyTwoBitsAreNotEnough=42,
classy=interface Fleeting,
classies=[class java.lang.Object, int],
moreClassies=[])

with newlines added for clarity. However, there are various kinds of 
problems that can occur with the data in an annotation and in those 
cases an exception proxy is created so that if the corresponding method 
is called an exception is thrown rather than the data being returned. In 
these cases the string form of an annotation with 
exceptions-that-would-be-thrown-if-methods-are-called looks like:


// Current erroneous annotation

@DangerousAnnotation(utopia=sun.reflect.annotation.EnumConstantNotPresentExceptionProxy@766a8c91,
thirtyTwoBitsAreNotEnough=sun.reflect.annotation.AnnotationTypeMismatchExceptionProxy@1f40866a,
classy=sun.reflect.annotation.TypeNotPresentExceptionProxy@3de4f72b,
classies=[class java.lang.Object, int],
moreClassies=[])

Having the proxy implementation leak through to the string 
representation in this case is not helpful and the string can be made 
more informative. In addition, for Class-related values, the current 
form doesn't use the syntax which is legal in source code for 
annotations. Class-valued annotations are set using Class literal 
syntax, "Foo.class" rather than "class Foo", and arrays of Class-valued 
item should use braces ("{}") rather than brackets, ("[]").


For example, it would be better to have annotation string 
representations which looked like:


// New non-erroneous annotation

@DangerousAnnotation(utopia=BRIGADOON,
thirtyTwoBitsAreNotEnough=42,
classy=Fleeting.class,
classies={java.lang.Object.class, int.class},
moreClassies={})

// New erroneous annotation

@DangerousAnnotation(utopia=BRIGADOON /* Warning: constant not present! */,
thirtyTwoBitsAreNotEnough=/* Warning type mismatch! "class 
java.lang.Integer[42]" */,

classy=Fleeting.class /* Warning: type not present! */,
classies={java.lang.Object.class, int.class},
moreClassies={})

Please review the code to implement these behavioral changes:

5040830: (ann) please improve toString() for annotations containing 
exception proxies

http://cr.openjdk.java.net/~darcy/5040830.2

(If you have erroneous Class-value items in an array of Class values, 
then this throws an exception since the exception proxy cannot be stored 
into the Class[].)


Thanks,

-Joe

Re: RFR: 8151832: Improve exception messages in exceptions thrown by jigsaw code

[Seán Coffey]

Thanks Paul. Best to have consistent formatting. Webrev updated in place.

On the beginning quote observation, I wasn't too such myself. Presumably 
a typo. I've removed for now.


Regards,
Sean.

On 31/05/2016 19:48, Paul Benedict wrote:

Hi Sean,

I just have a few minor comments.

Nearly all the new messages follow the message/colon/space/details 
format. There are a few missing the space between the colon and details:

*) ImageHeader:
"jimage header not the correct size:"

*) JrtPath
throw new ProviderMismatchException("path class:"

*) ConstructorFinder
all new messages

Some other observations:
*) ImageLocation: why the beginning quote?
throw new InternalError("\"Missing jimage attribute data");

*) JrtFileSystem: extra space before colon
"option class : " + option.getClass().getName());

Cheers,
Paul

On Tue, May 31, 2016 at 12:57 PM, Seán Coffey > wrote:


I've gone ahead with a trimmed down webrev as per Alan's request.

new webrev :
http://cr.openjdk.java.net/~coffeys/webrev.8151832.v2/webrev/


Regards,
Sean.

On 16/05/2016 15:10, Alan Bateman wrote:



On 16/05/2016 14:45, Seán Coffey wrote:


On 16/05/16 13:51, Alan Bateman wrote:

On 16/05/2016 13:44, Seán Coffey wrote:

Some extra capturing of context in exception
handling. I've re-based the original suggested
patch and added some minor edits.

https://bugs.openjdk.java.net/browse/JDK-8151832
webrev :

http://cr.openjdk.java.net/~coffeys/webrev.8151832/webrev/index.html



Would it be possible to leave out the changes to the
source files in the module and loader directories for
now? We have many changes to this code coming that
replace parts of this code and I think would be better
to do a pass over the exceptions in a few months once
the code is more stable.

Yes, I can hold off. I figured such improvements might
help people while they adopt and set up modules but let's
cancel until the code stabilizes some more!

Thanks. The rest mostly look okay although I think several of
these exceptions in the jimage code need to re-examined - for
example IOOBE is thrown in several places where the root cause
must be a corrupt or truncated jimage file.

-Alan

Re: RFR: 8151832: Improve exception messages in exceptions thrown by jigsaw code

[Paul Benedict]

Hi Sean,

I just have a few minor comments.

Nearly all the new messages follow the message/colon/space/details format.
There are a few missing the space between the colon and details:
*) ImageHeader:
"jimage header not the correct size:"

*) JrtPath
throw new ProviderMismatchException("path class:"

*) ConstructorFinder
all new messages

Some other observations:
*) ImageLocation: why the beginning quote?
throw new InternalError("\"Missing jimage attribute data");

*) JrtFileSystem: extra space before colon
"option class : " + option.getClass().getName());

Cheers,
Paul

On Tue, May 31, 2016 at 12:57 PM, Seán Coffey 
wrote:

> I've gone ahead with a trimmed down webrev as per Alan's request.
>
> new webrev : http://cr.openjdk.java.net/~coffeys/webrev.8151832.v2/webrev/
>
> Regards,
> Sean.
>
> On 16/05/2016 15:10, Alan Bateman wrote:
>
>>
>>
>> On 16/05/2016 14:45, Seán Coffey wrote:
>>
>>>
>>> On 16/05/16 13:51, Alan Bateman wrote:
>>>
 On 16/05/2016 13:44, Seán Coffey wrote:

> Some extra capturing of context in exception handling. I've re-based
> the original suggested patch and added some minor edits.
>
> https://bugs.openjdk.java.net/browse/JDK-8151832
> webrev :
> http://cr.openjdk.java.net/~coffeys/webrev.8151832/webrev/index.html
>
> Would it be possible to leave out the changes to the source files in
 the module and loader directories for now? We have many changes to this
 code coming that replace parts of this code and I think would be better to
 do a pass over the exceptions in a few months once the code is more stable.

>>> Yes, I can hold off. I figured such improvements might help people while
>>> they adopt and set up modules but let's cancel until the code stabilizes
>>> some more!
>>>
>>> Thanks. The rest mostly look okay although I think several of these
>> exceptions in the jimage code need to re-examined - for example IOOBE is
>> thrown in several places where the root cause must be a corrupt or
>> truncated jimage file.
>>
>> -Alan
>>
>>
>>
>

Re: RFR 9: 8158254 : Put java/time/test/java/time/TestClock_System on the problem list for Solaris

[Roger Riggs]

Thanks Joe, Lance,

Corrected to use solaris-all.

Roger



On 5/31/2016 2:16 PM, joe darcy wrote:
The syntax used elsewhere in the file is "solaris-all"; I'm not sure 
just "solaris" would have the right semantics.


+1 if "solaris-all" were used.

Thanks,

-Joe


On 5/31/2016 11:11 AM, Lance Andersen wrote:

+1
On May 31, 2016, at 2:09 PM, Roger Riggs  
wrote:


Please review this change to add a java.time test to the 
ProblemList.txt, only for Solaris.


A change[1] was tried for a more efficient way to read the current 
time but it didn't work out
and had to be fixed.  The original change propagated to the 
jdk9-master but the fix [2] has not yet.


Jira:

  8158128: regression: 
java/time/test/java/time/TestClock_System.java fails at 
test_OffsetRegular()


-- old/test/ProblemList.txt2016-05-31 12:31:46.530188722 -0400
+++ new/test/ProblemList.txt2016-05-31 12:31:46.210188722 -0400
@@ -346,6 +346,13 @@

 



+# jdk_time
+
+java/time/test/java/time/TestClock_System.java 8158128 solaris
+
+
+ 


+
# jdk_util

Thanks, Roger


[1] JDK-8154710 Investigate use of in-memory low-resolution 
timestamps for Java and internal time API


[2] 8157175 GetNanoTimeAdjustment.java fails with excessive 
adjustment error






 

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 9: 8158254 : Put java/time/test/java/time/TestClock_System on the problem list for Solaris

[joe darcy]

The syntax used elsewhere in the file is "solaris-all"; I'm not sure 
just "solaris" would have the right semantics.


+1 if "solaris-all" were used.

Thanks,

-Joe


On 5/31/2016 11:11 AM, Lance Andersen wrote:

+1

On May 31, 2016, at 2:09 PM, Roger Riggs  wrote:

Please review this change to add a java.time test to the ProblemList.txt, only 
for Solaris.

A change[1] was tried for a more efficient way to read the current time but it 
didn't work out
and had to be fixed.  The original change propagated to the jdk9-master but the 
fix [2] has not yet.

Jira:

  8158128: regression: java/time/test/java/time/TestClock_System.java fails at 
test_OffsetRegular()

-- old/test/ProblemList.txt2016-05-31 12:31:46.530188722 -0400
+++ new/test/ProblemList.txt2016-05-31 12:31:46.210188722 -0400
@@ -346,6 +346,13 @@



+# jdk_time
+
+java/time/test/java/time/TestClock_System.java 8158128 solaris
+
+
+
+
# jdk_util

Thanks, Roger


[1] JDK-8154710 Investigate use of in-memory low-resolution timestamps for Java 
and internal time API

[2] 8157175 GetNanoTimeAdjustment.java fails with excessive adjustment error




  
   

  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 9: 8158254 : Put java/time/test/java/time/TestClock_System on the problem list for Solaris

[Lance Andersen]

+1
> On May 31, 2016, at 2:09 PM, Roger Riggs  wrote:
> 
> Please review this change to add a java.time test to the ProblemList.txt, 
> only for Solaris.
> 
> A change[1] was tried for a more efficient way to read the current time but 
> it didn't work out
> and had to be fixed.  The original change propagated to the jdk9-master but 
> the fix [2] has not yet.
> 
> Jira:
> 
>  8158128: regression: java/time/test/java/time/TestClock_System.java fails at 
> test_OffsetRegular()
> 
> -- old/test/ProblemList.txt2016-05-31 12:31:46.530188722 -0400
> +++ new/test/ProblemList.txt2016-05-31 12:31:46.210188722 -0400
> @@ -346,6 +346,13 @@
> 
> 
> 
> +# jdk_time
> +
> +java/time/test/java/time/TestClock_System.java 8158128 solaris
> +
> +
> +
> +
> # jdk_util
> 
> Thanks, Roger
> 
> 
> [1] JDK-8154710 Investigate use of in-memory low-resolution timestamps for 
> Java and internal time API
> 
> [2] 8157175 GetNanoTimeAdjustment.java fails with excessive adjustment error
> 
> 
> 

 
  

 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 9: 8158254 : Put java/time/test/java/time/TestClock_System on the problem list for Solaris

[Roger Riggs]

Please review this change to add a java.time test to the 
ProblemList.txt, only for Solaris.


A change[1] was tried for a more efficient way to read the current time 
but it didn't work out
and had to be fixed.  The original change propagated to the jdk9-master 
but the fix [2] has not yet.


Jira:

  8158128: regression: java/time/test/java/time/TestClock_System.java 
fails at test_OffsetRegular()


-- old/test/ProblemList.txt2016-05-31 12:31:46.530188722 -0400
+++ new/test/ProblemList.txt2016-05-31 12:31:46.210188722 -0400
@@ -346,6 +346,13 @@

 

+# jdk_time
+
+java/time/test/java/time/TestClock_System.java 8158128 solaris
+
+
+
+
 # jdk_util

Thanks, Roger


[1] JDK-8154710 Investigate use of in-memory low-resolution timestamps 
for Java and internal time API


[2] 8157175 GetNanoTimeAdjustment.java fails with excessive adjustment 
error

Fwd: JDK 9 r-team mtg notes, 5/26/16

[Sandeep Konchady]

> Begin forwarded message:
> 
> From: Sandeep Konchady 
> Subject: Fwd: JDK 9 r-team mtg notes, 5/26/16
> Date: May 31, 2016 at 11:07:48 AM PDT
> To: javase-sqe-staff_ww_grp staff 
> 
> 
> 
>> Begin forwarded message:
>> 
>> From: Barbara O'connor > >
>> Subject: Fwd: JDK 9 r-team mtg notes, 5/26/16
>> Date: May 31, 2016 at 10:46:21 AM PDT
>> To: jdk_confidential_ww_grp > >
>> 
>> 
>> 
>> 
>> JDK 9 Release Team Meeting Minutes 5-26-2016
>> 
>> Attendees:  Stephen, Sandeep, Andrey, Joe, Paul, Dave, Mark, Ken, Georges, 
>> Jeannette, Vladimir K., Barb
>> 
>> Agenda
>> 
>> Propose to Target JEPs 
>> JEP 288/JDK-8149555 
>> FC Exception Requests
>> JEP 223/JDK-8061493  - New 
>> Version-String Scheme
>> JEP C123/JDK-8061494  - 
>> New Version-String Scheme (closed)
>> JEP 287/JDK-8064399  - 
>> SHA-3 Hash Algorithms 
>> JEP 273/JDK-8051408  - 
>> DRBG-Based SecureRandom Implementations
>> JEC C153/JDK-8065162  - 
>> Deterministic GC
>> JEP C169/JDK-8150020  - 
>> JDK 9 Platform Limitations (PM)
>> JEP C168/JDK-8134633 - JDK 
>> 9 Platform Support (PM)
>> JDK 9 Feature Complete Status
>> Opening JDK 10 Forest
>> Enhancement FC Exception Reviews
>> Ops Review - June 16
>> VPAT/Accessibility - Previous Outages 
>> 
>> JDK 9 Group Status - see detailed status pages for details
>> Dev Status 
>> : 
>> SQE Status :
>> Perf Status 
>> : 
>> JCK Status :  
>> Doc Status 
>> :
>> PM: 
>> L10N Status 
>> : 
>> JDK 9 Status Summary
>> 
>> JDK 9 Status - GREEN
>> GA - Target, March 23, 2017 
>> JDK 9 over-all status is GREEN 
>> FC with noted exceptions, including Jigsaw related work was achieved on 5/26
>> SE 9 JCP milestone dates and Umbrella JSR dates finalized. Format for MR and 
>> spec leads mtg has been agreed on and kick off meeting will be planned for 
>> end June/early July.
>> TOI process update - on hold till post FC. Slides finalized and being 
>> reviewed with key stakeholders prior to review at dev staff, r-team
>> Bug Projection data is being collected and will be rolled up week of June 13.
>> Regular drops of Jigsaw EA builds 
>>  on going. Jigsaw integration 
>> into master occurred week of March 21 (b111)
>> L10n Preparatory drop has been delivered by WPTG. Integration back to master 
>> should occur by next week. No risk/impact to schedule. 
>> Planning for JDK 9 Ops review underway. Expect to have ops review presented 
>> in JDK 9 r-team June 16.
>> JMC 6.0:  Green
>> JMC 6.0 is Green, on track for JMC milestone dates, although concern raised 
>> that Jigsaw FC has pushed out there could be impact downstream to JMC.  
>> JCK 9 monthly drops to licensees on-going. First drop with Jigsaw delivered 
>> on April 14. 
>> JEP summary: Note that JEPs not in Completed or Closed have either rec'd FC 
>> exception or do not impact FC status.
>> Total number of JEPs (This week/Last week): 116/115
>> Closed/Delivered: 25/25
>> Proposed to Drop: 0/0
>> Completed: 61/47
>> Integrated: 15/10
>> Targeted: 9/16
>> Proposed To Target: 1/0
>> Candidate: 0/3
>> Submitted: 3/3
>> Draft: 2/1
>> Outstanding checklists:
>> Initial export approval was completed before first EA drop. Re-approvals 
>> will be done throughout the project.
>> Third party approvals are in progress. Existing third party approvals are 
>> being reviewed for cleanup + ensuring using secure versions and are being 
>> "mapped" to JDK 9.
>> Security checklists placeholders have been created for functional areas
>> VPAT draft in progress, backlog of accessibility bugs to be scrubbed
>> Meeting Notes
>> 
>> Propose to Target JEPs 
>> JEP 288/JDK-8149555 
>>  Mark will send out notice to openjdk
>> FC Exception Requests
>> JEP 223/JDK-8061493  - New 
>> Version-String Scheme
>> JEP C123/JDK-8061494  - 
>> New Version-String Scheme (closed)
>> JDK 9 R-Team FC Exception Approved
>> JEP 287/JDK-8064399 

Re: RFR: 8151832: Improve exception messages in exceptions thrown by jigsaw code

[Seán Coffey]

I've gone ahead with a trimmed down webrev as per Alan's request.

new webrev : http://cr.openjdk.java.net/~coffeys/webrev.8151832.v2/webrev/

Regards,
Sean.

On 16/05/2016 15:10, Alan Bateman wrote:



On 16/05/2016 14:45, Seán Coffey wrote:


On 16/05/16 13:51, Alan Bateman wrote:

On 16/05/2016 13:44, Seán Coffey wrote:
Some extra capturing of context in exception handling. I've 
re-based the original suggested patch and added some minor edits.


https://bugs.openjdk.java.net/browse/JDK-8151832
webrev : 
http://cr.openjdk.java.net/~coffeys/webrev.8151832/webrev/index.html


Would it be possible to leave out the changes to the source files in 
the module and loader directories for now? We have many changes to 
this code coming that replace parts of this code and I think would 
be better to do a pass over the exceptions in a few months once the 
code is more stable.
Yes, I can hold off. I figured such improvements might help people 
while they adopt and set up modules but let's cancel until the code 
stabilizes some more!


Thanks. The rest mostly look okay although I think several of these 
exceptions in the jimage code need to re-examined - for example IOOBE 
is thrown in several places where the root cause must be a corrupt or 
truncated jimage file.


-Alan

Re: RFR:JDK-8066806:java.time.format.DateTimeFormatter cannot parse an offset with single digit hour

[nadeesh tv]

Hi Stephen,

Thanks for the suggestions and the code.

Regards,
Nadeesh

On 5/31/2016 7:15 PM, Stephen Colebourne wrote:

Where the new patterns are described in Javadoc, there is no
discussion of the difference between "H" and "HH".

Add after 

"Patterns containing "HH" will format and parse a two digit hour,
zero-padded if necessary. Patterns containing "H" will format with no
zero-padding, and parse either one or two digits."

"with colo" should be "with colon"

As for the main code, I've had a go at a rewrite:
https://gist.github.com/jodastephen/68857dd344e33bd6c0b3b4d24279d2e4

It is completely untested, and surely has mistakes, however as a
design it seems reasonable.

I agree that the tests need to cover these cases:

- offset at end of line
- offset followed by letters
- offset followed by numbers

Stephen


On 26 May 2016 at 08:49, nadeesh tv  wrote:

Hi all,

Please review

BugId : https://bugs.openjdk.java.net/browse/JDK-8066806

Issue: java.time.format.DateTimeFormatter cannot parse an offset with single
digit hour

webrev:  http://cr.openjdk.java.net/~ntv/8066806/webrev.03/

Solution: Added the suggested patterns but the parsing logic became too
complex.
  Appreciate any suggestion to make the  parsing less complicated

--
Thanks and Regards,
Nadeesh TV



--
Thanks and Regards,
Nadeesh TV

Re: RFR: JDK-8157850 Jar tests should pass through VM options

[Martin Buchholz]

I approve this change.

On Mon, May 30, 2016 at 4:59 PM, David Holmes  wrote:
> On 27/05/2016 2:20 AM, Andrey Nazarov wrote:
>>
>> Thanks for feedback guys.
>>
>> I've updated review
>> http://cr.openjdk.java.net/~anazarov/8157850/webrev.02/
>
>
> Using test.tool.vm.opts seems reasonable for jar and javac.
>
>> Please sponsor this patch if you are OK.
>
>
> Sorry not my area.
>
> Thanks,
> David
>
>
>> My use case is to run tests with different -Xms and -Xmx options. Mostly
>> due to I need to increase heap size to gather code coverage by jcov.
>>
>> --Andrey
>>
>> On 25.05.2016 23:42, Jonathan Gibbons wrote:
>>>
>>>
>>>
>>> On 05/25/2016 01:33 PM, David Holmes wrote:

 On 26/05/2016 6:04 AM, Jonathan Gibbons wrote:
>
> Using JAVA_OPTIONS for tools is conceptually wrong.
>
> JAVA_OPTIONS is specifically intended to pass options to VM instances,
> as created by a "java" command.  It is not intended that you should
> prefix the options with -J and use them for arbitrary tools.


 I have to agree. There is no guarantee the options being passed for
 the VM under test make any sense for the running of the jar tool in
 the jar tests. I think a number of hotspot related test options could
 cause problems here.

 Are there specific VM options that people think should be passed
 through? The bug report has no detail at all.

 David
>>>
>>>
>>> Generally, I think that blindly passing through all the options
>>> regardless is as bad a programming practice as never passing them
>>> though.  There are some that make sense to all VMs, like -ea, -esa
>>> etc, and maybe system properties, but for those, the VM options
>>> mechanism is generally good enough.  (i.e. system properties
>>> test.vm.opts, test.tool.vm.opts)
>>>
>>> From a jtreg point of view, I'd be interested to know uses cases for
>>> passing additional more specific options to the VMs used to run tools
>>> like jar, jlink, javac, etc
>>>
>>> -- Jon.
>>>
>>>
>>>

> The code in the webrev is also perverse for taking the trouble to split
> the string to a stream, collect the results into a list, convert the
> list back into a stream again and use .forEach.
>
> You could do better, and much simpler, with something like
>
> if (!option.isEmpty()) {
> commands.addAll(Arrays.asList(option.split("\\s+",-1)));
> }
>
> -- Jon
>
>
> On 05/25/2016 10:48 AM, Martin Buchholz wrote:
>>
>> Hi Andrey,
>>
>> Looking at http://openjdk.java.net/jtreg/vmoptions.html,
>> I see we have both test.vm.opts and test.tool.vm.opts
>> and the latter is supposed to take care of adding "-J".
>>
>> +VM_OPTIONS.stream().map(opt -> "-J" +
>> opt).forEach(commands::add);
>> +JAVA_OPTIONS.stream().map(opt -> "-J" +
>> opt).forEach(commands::add);
>>
>> ---
>>
>> Maybe splitting on "\\s+" would be better.
>>
>> ---
>>
>> I think we should have test library methods to return the List
>> for java subprocesses, one that could try hard to get the option
>> tokenization correct.
>>
>> ---
>>
>>
>> On Wed, May 25, 2016 at 9:07 AM, Andrey Nazarov
>>  wrote:
>>>
>>> Some jar tests start VMs without passing vmoptions from jtreg.
>>>
>>> This fix pass jtreg's vmoptions and javaoptions to processes(java,
>>> jar,
>>> javac) started by tests.
>>>
>>> webrev: http://cr.openjdk.java.net/~anazarov/8157850/webrev.01/
>>>
>>> jbs: https://bugs.openjdk.java.net/browse/JDK-8157850
>>>
>>> --Andrey
>>>
>
>>>
>>
>

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

[Richard Hillegas]

Dalibor Topic recommended that I post this feedback on core-libs-dev. 
This is my feedback after ameliorating the deprecation warnings which 
surfaced when I compiled and tested Apache Derby with early access 
builds 116 and 118 of JDK 9. Derby is a pure Java relational database 
whose original code goes back almost 20 years. Other large, old code 
bases (like Weblogic) may have similar experiences. More detail on my 
experience can be found on the JIRA issue which tracks the Derby 
community's attempt to keep our code evergreen against JDK 9: 
https://issues.apache.org/jira/browse/DERBY-6856


o Deprecating autoboxing constructors - Deprecating the autoboxing 
constructors for primitive wrapper objects caused a large rototill of 
Derby code. That rototill was comparable in size to the changes made 
necessary by Java 5's introduction of generics. Hopefully, IDEs can 
automate much of this chore.


o Deprecating Class.newInstance() - The deprecation of 
Class.newInstance() forced a similarly large rototill. The code became 
more verbose. Additional exceptions had to be caught and propagated up 
the call stack. For reasons which I don't understand, I had better luck 
using Class.getConstructor().newInstance() than 
Class.getDeclaredConstructor().newInstance(). But the former replacement 
code requires you to make constructors public. For some code bases, that 
may introduce security problems which are worse than the security 
problem being addressed by this deprecation. I hope that IDEs and the 
release notes for JDK 9 will provide some guidance for how to handle 
these issues.


o Deprecating java.util.Observable and java.util.Observer - Two 
ameliorations are recommended at 
http://mail.openjdk.java.net/pipermail/core-libs-dev/2016-April/040436.html. 
The first suggestion (use the awt event model) runs very much counter to 
the whole intent of Jigsaw. That is because pulling in awt can bloat up 
an application with large, otherwise unneeded libraries. Using awt was 
out of the question for Derby, given that the community had already 
invested a great deal of effort in paring back Derby's dependencies in 
order to let the code run on JDK 8 compact profile 2. That left us with 
the other option: write your own replacement classes. If a lot of people 
end up having to write the same replacement code, then that argues for 
leaving this small but useful functionality in the JDK. I think that the 
people who advocated for this deprecation did not have good visibility 
into how widely these classes are being used in the wild. I recommend 
that this deprecation be re-evaluated.


Thanks,
-Rick

Re: Review Request JDK-8152721: Java Web Start splash mechanism is not working in JDK9

[Kumar Srinivasan]

Hi Mandy,

I am ok with the changes.

Thanks
Kumar





On 31/05/2016 03:59, Mandy Chung wrote:
SelectVersion is called for launching any application.  It processes 
-splash option separately from the launcher argument processing.  To 
hit this bug, it will need to launch with the new module-system 
option together with -splash. I have verified with my modified 
version of a client test verifying the -splash option.


I prefer to file a JBS issue for the client team to create a test for 
this if that’s okay with you.



That should be okay as we should have tests for this option.

-Alan

Re: RFR: JDK-8061777, , (zipfs) IllegalArgumentException in ZipCoder.toString when using Shitft_JIS

[Xueming Shen]

Ulf, thanks for the suggestions!

On 5/31/16 6:27 AM, Ulf Zibis wrote:

Hi Sherman,

1.) wouldn't it be better to have a public getBytes() in 
AbstractStringBuilder?

Then you could save the array copy with sb.toString() here:
 178 return new ZipPath(this, sb.toString(), zc.isUTF8());
 525 return zfs.getBytes(to.toString());




One single use case here probably is not a strong enough reason to
add such a utility method into ASB.


You could simplify even more.

2.) No need for fork on isUTF8 in ZFS.iteratorOf() and for parameter 
isUTF8:

  55 ZipPath(ZipFileSystem zfs, byte[] path, boolean normalized)
  56 {
  57 this.zfs = zfs;
  58 if (normalized)
  59 this.path = path;
  60 // if zfs is NOT in utf8, normalize the path as "String"
  61 // to avoid incorrectly normalizing byte '0x5c' (as '\')
  62 // to '/'.
  63 else if (zfs.zc.isUTF8())
  64 this.path = normalize(path);
  65 else
  66 this.path = normalize(zfs.getString(path));
  67 }

  64 ZipPath(ZipFileSystem zfs, String path) {
  65 this.zfs = zfs;
  66 // if zfs is NOT in utf8, normalize the path as "String"
  67 // to avoid incorrectly normalizing byte '0x5c' (as '\')
  68 // to '/'.
  69 if (zfs.zc.isUTF8()) {
  70 this.path = normalize(zfs.getBytes(path));
  71 } else {
  72 this.path = normalize(path);
  73 }
  74 }
  75


I was hesitated to exposure zfs.zc to be package private for some 
reason, that
was why have the pair of zfs.getBytes/String(). But sure I can do it if 
it makes

the communication clearer.

webrev has been updated accordingly.

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



6.) Avoid String instatiation especially when called from child paths 
iterator *loop*.

Instead:
 500 if (len > 1 && prevC == '/')
 501 path = path.substring(0, len - 1);
 502 return zfs.getBytes(path);
provide:
 500 return zfs.getBytes(path, 0, len - (len > 1 && prevC == 
'/' ? 1 : 0 ));




This can be a candidate for further optimization. Currently my 
assumption is that
non-utf8 jar/zip files are not the main use cases for zipfs (assume most 
of them
are the result of either the jar tool or j.u.zip apis), so I would wait 
to add a new

method into zc. Hope this is fine with you.

Sherman

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

[shilpi.rast...@oracle.com]

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."

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

[Paul Sandoz]

> On 31 May 2016, at 14:17, shilpi.rast...@oracle.com wrote:
> 
> Hi All,
> 
> Please see updated webrev 
> http://cr.openjdk.java.net/~srastogi/8147585/webrev.02/
> 

I meant do something like this:

  static class MethodsWithAnnotations {
@LambdaWithParameter
public void testAnnotationLambdaWithParameter() {
}

@LambdaWithoutParameter
public void testAnnotationLambdaWithoutParameter() {
}
  }

Then you don’t need to do:

  49 if(!method.getName().equals("testAnnotationWithLambda")) {



> On 5/31/2016 2:21 PM, Paul Sandoz wrote:
>>> On 31 May 2016, at 10:35, shilpi.rast...@oracle.com
>>>  wrote:
>>> 
>>> Thanks Paul for comments.
>>> 
>>> Please see
>>> http://cr.openjdk.java.net/~srastogi/8147585/webrev.01/
>>> 
>>> 
>>> Now processing only public abstract methods of interface.
>>> 
>>> 
>> Thanks. It would be good to get some got feedback from those wiser than I in 
>> this regard.
>> 
>> Have you looked at the existing annotation-based tests to see if they test 
>> edge cases e.g. annotation classes generated with incorrect methods? that 
>> might give us some clues.
>> 
>   I saw existing annotation-based test, valid modifier for annotations, valid 
> method for annotation tests we are checking in javac code.
>   default, static, private modifier we are restricting at compile time so we 
> can not add test cases for this  (compilation will fail).
> 

We may be getting our wires crossed. I was wondering if there were some 
existing tests that generate annotation classes by directly producing byte code 
that would otherwise not be possible with javac. If there are it might give us 
some clues.

I was not suggesting as part of this fix you write some such tests.


> So only scenario i can add is for synthetic methods. ( according to my 
> assumption)
> In AnnotationType.java
> 
> public Method[] run() {
>  // Initialize memberTypes and defaultValues
>  return annotationClass.getDeclaredMethods();
>   }
> });
> 
> As here, calling getDeclaredMethods() on annotationclass and 
> getDeclaredMethods() doc says
> 
> https://docs.oracle.com/javase/8/docs/api/java/lang/Class.html#getDeclaredMethods--
> "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."
> 
> Doc did not mention anything about synthetic methods so i am not sure this is 
> expected behavior or not.

A synthetic method is one that must one of public, protected, default (package) 
access, and private (i don’t actually know if synthetic methods are stricter in 
their scope).

But i think we are deviating off topic, which is the processing of declared 
methods by AnnotationType to build up the annotation property metadata.

I just want to be sure that skipping all non-public non-abstract methods when 
processing is not gonna cause issues. It seems obvious it should skip such 
methods, but it has not been implemented that way.

Perhaps this is just an oversight, i dunno, but sometimes things in a JDK are 
there for a reason, and it’s often hard to know. We can try making an offering 
to the Java gods and looking out for the eagle that flies straight and narrow 
to the right, or alternatively ask someone more expert in this area :-)

AFAICT from debugging a little in this area the additional information added by 
processing never gets exposed, so it’s just wasted space/time.

Paul.



> If yes, Could you please suggest how to add testcases to test synthetic 
> method?
> Shall I use ASM?
> 
> Regards,
> Shilp

Re: RFR:JDK-8066806:java.time.format.DateTimeFormatter cannot parse an offset with single digit hour

[Stephen Colebourne]

Where the new patterns are described in Javadoc, there is no
discussion of the difference between "H" and "HH".

Add after 

"Patterns containing "HH" will format and parse a two digit hour,
zero-padded if necessary. Patterns containing "H" will format with no
zero-padding, and parse either one or two digits."

"with colo" should be "with colon"

As for the main code, I've had a go at a rewrite:
https://gist.github.com/jodastephen/68857dd344e33bd6c0b3b4d24279d2e4

It is completely untested, and surely has mistakes, however as a
design it seems reasonable.

I agree that the tests need to cover these cases:

- offset at end of line
- offset followed by letters
- offset followed by numbers

Stephen


On 26 May 2016 at 08:49, nadeesh tv  wrote:
> Hi all,
>
> Please review
>
> BugId : https://bugs.openjdk.java.net/browse/JDK-8066806
>
> Issue: java.time.format.DateTimeFormatter cannot parse an offset with single
> digit hour
>
> webrev:  http://cr.openjdk.java.net/~ntv/8066806/webrev.03/
>
> Solution: Added the suggested patterns but the parsing logic became too
> complex.
>  Appreciate any suggestion to make the  parsing less complicated
>
> --
> Thanks and Regards,
> Nadeesh TV
>

Re: RFR: JDK-8061777, , (zipfs) IllegalArgumentException in ZipCoder.toString when using Shitft_JIS

[Ulf Zibis]

Hi Sherman,

1.) wouldn't it be better to have a public getBytes() in AbstractStringBuilder?
Then you could save the array copy with sb.toString() here:
 178 return new ZipPath(this, sb.toString(), zc.isUTF8());
 525 return zfs.getBytes(to.toString());


You could simplify even more.

2.) No need for fork on isUTF8 in ZFS.iteratorOf() and for parameter isUTF8:
  55 ZipPath(ZipFileSystem zfs, byte[] path, boolean normalized)
  56 {
  57 this.zfs = zfs;
  58 if (normalized)
  59 this.path = path;
  60 // if zfs is NOT in utf8, normalize the path as "String"
  61 // to avoid incorrectly normalizing byte '0x5c' (as '\')
  62 // to '/'.
  63 else if (zfs.zc.isUTF8())
  64 this.path = normalize(path);
  65 else
  66 this.path = normalize(zfs.getString(path));
  67 }

  64 ZipPath(ZipFileSystem zfs, String path) {
  65 this.zfs = zfs;
  66 // if zfs is NOT in utf8, normalize the path as "String"
  67 // to avoid incorrectly normalizing byte '0x5c' (as '\')
  68 // to '/'.
  69 if (zfs.zc.isUTF8()) {
  70 this.path = normalize(zfs.getBytes(path));
  71 } else {
  72 this.path = normalize(path);
  73 }
  74 }
  75

3.) We could benefit from better byte array performance for all one byte charsets, e.g. common 
Windows-1252:

With:
if (zfs.zc.isOneByte())
this.path = normalize(path, zfs.zc.backSlashCode, zfs.zc.slashCode);

4.) We could do the same for all double byte charsets and benefit from the new intrinsic accessor 
methods on byte arrays/buffers with VarHandles.


5.) As alternative to 2.) consider moving the string concatenation to ZipPath constructor. I 
believe, this would make things more simple and less redundant:

ZipPath(ZipFileSystem zfs, String first, String... more)

6.) Avoid String instatiation especially when called from child paths iterator 
*loop*.
Instead:
 500 if (len > 1 && prevC == '/')
 501 path = path.substring(0, len - 1);
 502 return zfs.getBytes(path);
provide:
 500 return zfs.getBytes(path, 0, len - (len > 1 && prevC == '/' ? 1 : 
0 ));

7.) Last but not least, the normalize algorithm could be an additional usecase 
for
http://bugs.java.com/bugdatabase/view_bug.do?bug_id=6695386
with '\' as terminator.

-Ulf


Am 31.05.2016 um 07:07 schrieb Xueming Shen:

Thanks Paul,

updated accordingly.

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

-sherman

On 5/30/16 2:31 AM, Paul Sandoz wrote:

Hi Sherman,

Do you consider modifying the new ZipPath constructor you added to accept a boolean value for 
UTF-8 encoding?


If so you can more clearly document the behaviour and avoid duplication of the operators in 
ZipFileSystem e.g.:


   return new ZipPath(this, first, zc.isUTF8());

Paul.

Re: RFR 8154189: Deprivilege java.sql and java.sql.rowset module

[Lance Andersen]

Hi Sean,

Yes I will and add you to the watch list…

Best
Lance
> On May 31, 2016, at 8:16 AM, Sean Mullan  wrote:
> 
> On 05/27/2016 02:10 PM, Lance Andersen wrote:
>>> The change looks fine.
>> Thank you
>>> >
>>> >It’s okay to grants AllPermission to get started.  It’d be nice if we 
>>> >could grant fine-grained permissions in the future.
>> Agree, it is something to try and work towards.
> 
> Hi Lance,
> 
> Could you file a separate RFE for doing that (unless you have already)? 
> Please add my name to the watch list.
> 
> Thanks,
> Sean

 
  

 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[9]: 8147585: Annotations with lambda expressions has parameters result in wrong behavior.

[shilpi.rast...@oracle.com]

Hi All,

Please see updated webrev 
http://cr.openjdk.java.net/~srastogi/8147585/webrev.02/


On 5/31/2016 2:21 PM, Paul Sandoz wrote:

On 31 May 2016, at 10:35, shilpi.rast...@oracle.com wrote:

Thanks Paul for comments.

Please see http://cr.openjdk.java.net/~srastogi/8147585/webrev.01/

Now processing only public abstract methods of interface.


Thanks. It would be good to get some got feedback from those wiser than I in 
this regard.

Have you looked at the existing annotation-based tests to see if they test edge 
cases e.g. annotation classes generated with incorrect methods? that might give 
us some clues.
  I saw existing annotation-based test, valid modifier for annotations, 
valid method for annotation tests we are checking in javac code.
  default, static, private modifier we are restricting at compile time 
so we can not add test cases for this  (compilation will fail).


So only scenario i can add is for synthetic methods. ( according to my 
assumption)

In AnnotationType.java

public Method[] run() {
 // Initialize memberTypes and defaultValues
 return annotationClass.getDeclaredMethods();
  }
});

As here, calling getDeclaredMethods() on annotationclass and 
getDeclaredMethods() doc says


https://docs.oracle.com/javase/8/docs/api/java/lang/Class.html#getDeclaredMethods--
"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."


Doc did not mention anything about synthetic methods so i am not sure 
this is expected behavior or not.
If yes, Could you please suggest how to add testcases to test synthetic 
method?

Shall I use ASM?

Regards,
Shilpi



Testing wise:

- you can avoid the filtering of the test method by placing the annotations in 
another auxiliary class (my preference is to nest rather than using auxiliary 
classes, up to you)

- there is a couple of minor style inconsistencies e.g. a space here "@ interface 
LambdaWithoutParameter"

- 30  * @run testng/othervm -ea -esa AnnotationWithLambda

You can just do:

   @run testng AnnotationWithLambda

?

Thanks,
Paul.





Thanks,
Shilpi

On 5/30/2016 6:35 PM, Paul Sandoz wrote:

Hi Shilpi,

You have found the right place but i am not sure your fix is entirely correct.

(Tip: if you use  -Xlog:exceptions=info you can observe the IAE exception when 
the annotation is processed)

In your test you have:

@Target(value = ElementType.METHOD)
@Retention(RetentionPolicy.RUNTIME)
@ interface LambdaWithParameter {
 Consumer f1 = a -> {
 System.out.println("lambda has parameter");
 };
}

@Target(value = ElementType.METHOD)
@Retention(RetentionPolicy.RUNTIME)
@ interface LambdaWithoutParameter {
 Runnable r = () -> System.out.println("lambda without parameter");
}


Both of those annotations will have static synthetic methods generated by the 
compiler that the indy resolves and links to (look at the javap output). The 
former will have a method with one parameter.


The code in sun/reflect/annotation/AnnotationType.java:

for (Method method : methods) {
 if (method.getParameterTypes().length != 0)
 throw new IllegalArgumentException(method + " has params");

has thrown the IAE since 2004, but it’s not clear why it was added as opposed 
to something more general (see below).


The correct fix appears to be to skip over any non-abstract/non-public methods. 
Thus only public abstract methods get processed.

Your current fix now processes synthetic methods with parameters, in addition 
to those which were already processed such as synthetic methods without 
parameters, or even private methods that could have been generated by some 
tool. I dunno how much say the verifier has in all this, perhaps little or no 
say.

Thus non-public/non-abstract methods could add inconsistent information to the 
data structures of AnnotationType. Perhaps this is mostly harmless?

Perhaps Joel (CC’ed) can she some more light on this?

Paul.



On 30 May 2016, at 08:00, shilpi.rast...@oracle.com wrote:

Hi All,

Please review fix for https://bugs.openjdk.java.net/browse/JDK-8147585
http://cr.openjdk.java.net/~srastogi/8147585/webrev.00/

Problem: Annotation with Lamda has parameters results into wrong behavior ( not 
considered as valid annotation) because
According to JLS "By virtue of the AnnotationTypeElementDeclaration production, a 
method declaration in an annotation type declaration cannot have formal parameters, type 
parameters, or a throws clause. The following production from §4.3 is shown here for 
convenience:"
(Ref: https://docs.oracle.com/javase/specs/jls/se8/html/jls-9.html#jls-9.6.1)


Solution: We should skip synthetic methods during Annotation parsing. According to JLS 
"An annotation type has no elements other than those defined by the methods it 
explicitly declares."
(Ref https://docs.oracle.com/javase/specs/jls/se8

Re: RFR 8154189: Deprivilege java.sql and java.sql.rowset module

[Sean Mullan]

On 05/27/2016 02:10 PM, Lance Andersen wrote:

The change looks fine.

Thank you

>
>It’s okay to grants AllPermission to get started.  It’d be nice if we could 
grant fine-grained permissions in the future.

Agree, it is something to try and work towards.


Hi Lance,

Could you file a separate RFE for doing that (unless you have already)? 
Please add my name to the watch list.


Thanks,
Sean

Re: RFR: 8151876: (tz) Support tzdata2016d

[Seán Coffey]

Masayoshi,

I still think the test adds value. At minimum it identifies timezones 
which don't have a localised string in the JRE provider.


We need to start another discussion about how best to represent timezone 
names for newly added timezones. At the moment, short and long formats 
will be of "GMT±hh:mm" format. I suggest we use the IANA timezone name 
for the long format name in future (e.g. "Asia/Tomsk" instead of 
"GMT+06:00")


For the sake of getting this into the JDK code lines, I suggest we go 
ahead with your suggestion to remove the test for now. I've also 
reviewed this Ramanand. Your edits look fine (including test removal for 
now)


Regards,
Sean.

On 31/05/2016 07:06, Masayoshi Okutsu wrote:


The CheckDisplayNames.java change is different from what I suggested 
and doesn't make sense. But we no longer need the test. I'd suggest 
CheckDisplayNames.java be removed. Otherwise, the fix looks OK to me.


Masayoshi


On 5/31/2016 3:03 AM, Ramanand Patil wrote:


Hi Masayoshi and All,

Here is the updated Webrev: 
http://cr.openjdk.java.net/~rpatil/8151876/webrev.01/ 



As suggested by Masayoshi, I have kept the existing names unchanged.


Also, this patch contains extra test case fixes for


1./java/util/TimeZone/CheckDisplayNames.java/


2.java/util/TimeZone/Bug8149452.java

The exclusion for the *newly* added TimeZones is added in these test 
cases where the entries are not present in the resources and the 
Short/Long display names fallback to the GMT±hh:mm format.


Regards,

Ramanand.

*From:*Masayoshi Okutsu
*Sent:* Saturday, May 28, 2016 10:58 AM
*To:* Seán Coffey; Ramanand Patil; i18n-...@openjdk.java.net; 
core-libs-dev@openjdk.java.net

*Subject:* Re: RFR: 8151876: (tz) Support tzdata2016d

CLDR locale data defines time zone names, like this (in en.xml):


 
 Almaty Time
 Almaty Standard 
Time
 Almaty Summer Time
 
 

The CLDR converter tool tries to fill in the missing short names from 
the legacy TimeZoneNames data. Removing existing names causes some 
unexpected behavior. I think JDK-8157814 is an example of the 
unexpected behavior. And the suggested fix in JDK-8157814 looks to me 
like a workaround.


I still think the existing names should be kept unchanged for this fix.

Thanks,
Masayoshi

On 5/28/2016 12:04 AM, Seán Coffey wrote:

I guess there's a low risk of timezone not being identified if
being parsed in through a formatter. Isn't such an approach
discouraged though ? short IDs were already subject to change in
tzdata releases. Ramanand found one issue by removal of these
resource strings (so far) and that's captured in JDK-8157814

There's a decision to be made about how to use the GMT±hh:mm
format for new releases given IANA's new short ID identifier
mechanism. I think that could be discussed as a separate issue.
I'd like to see us follow a similar approach to IANA and use GMT
identifiers on new timezones and perhaps even consider using the
IANA long name format also for the getDisplayName(..) calls that
can be made. e.g. "Asia/Almaty" instead of "Alma-Ata Time"

Regards,

Sean.

On 27/05/16 15:24, Masayoshi Okutsu wrote:

This change seems to beyond my proposal that the "GMT±hh:mm"
format is used for *new* zones with the "±hh" format. But
this change removes *existing* zones which have changed to
use the "±hh" format in tzdata. Can this cause any
compatibility issues?

And have we agreed to use the "GMT±hh:mm" format?

Thanks,
Masayoshi


On 5/27/2016 10:19 PM, Seán Coffey wrote:

Looks fine to me Ramanand. the recent 2016d changes have
introduced some boundary issues for JDK rule parsing and
those issues can be followed up in separate issues like you say.

Regards,
Sean.

On 26/05/16 14:22, Ramanand Patil wrote:

HI all,

Please review the latest TZDATA integration (tzdata2016d) to
JDK9.

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

Webrev: http://cr.openjdk.java.net/~rpatil/8151876/webrev.00/


Patch Contains:

1.   IANA tzdata2016d integration into JDK. [It also
includes tzdata2016b and tzdata2016c which was not integrated].

2.   "GMT[+ -]hh:mm" is used for formatting of the
modified or newly added TimeZones in tzdata2016d.

[This is done to accommodate the IANA's new system where the
zones use numeric time zone abbreviations like "+04" instead
of invented abbreviations like "ASTT".]

3.   Test c

RE: RFR: 8151876: (tz) Support tzdata2016d

[Ramanand Patil]

Hi Masayoshi,

 

Thank you, I will delete this test before pushing the patch.

 

 

Regards,

Ramanand.

 

From: Masayoshi Okutsu 
Sent: Tuesday, May 31, 2016 11:37 AM
To: Ramanand Patil; Seán Coffey; i18n-...@openjdk.java.net; 
core-libs-dev@openjdk.java.net
Subject: Re: RFR: 8151876: (tz) Support tzdata2016d

 

The CheckDisplayNames.java change is different from what I suggested and 
doesn't make sense. But we no longer need the test. I'd suggest 
CheckDisplayNames.java be removed. Otherwise, the fix looks OK to me.

Masayoshi

 

On 5/31/2016 3:03 AM, Ramanand Patil wrote:

Hi Masayoshi and All,

 

Here is the updated Webrev: HYPERLINK 
"http://cr.openjdk.java.net/%7Erpatil/8151876/webrev.01/"http://cr.openjdk.java.net/~rpatil/8151876/webrev.01/

 

As suggested by Masayoshi, I have kept the existing names unchanged.


Also, this patch contains extra test case fixes for


1.    java/util/TimeZone/CheckDisplayNames.java 


2.   java/util/TimeZone/Bug8149452.java


The exclusion for the newly added TimeZones is added in these test cases where 
the entries are not present in the resources and the Short/Long display names 
fallback to the GMT±hh:mm format.

 

 

Regards,

Ramanand.

 

From: Masayoshi Okutsu 
Sent: Saturday, May 28, 2016 10:58 AM
To: Seán Coffey; Ramanand Patil; HYPERLINK 
"mailto:i18n-...@openjdk.java.net"i18n-...@openjdk.java.net; HYPERLINK 
"mailto:core-libs-dev@openjdk.java.net"core-libs-dev@openjdk.java.net
Subject: Re: RFR: 8151876: (tz) Support tzdata2016d

 

CLDR locale data defines time zone names, like this (in en.xml):

   
    
    Almaty Time
    Almaty Standard 
Time
    Almaty Summer Time
    
    
 

The CLDR converter tool tries to fill in the missing short names from the 
legacy TimeZoneNames data. Removing existing names causes some unexpected 
behavior. I think JDK-8157814 is an example of the unexpected behavior. And the 
suggested fix in JDK-8157814 looks to me like a workaround.

I still think the existing names should be kept unchanged for this fix.

Thanks,
Masayoshi

On 5/28/2016 12:04 AM, Seán Coffey wrote:

I guess there's a low risk of timezone not being identified if being parsed in 
through a formatter. Isn't such an approach discouraged though ? short IDs were 
already subject to change in tzdata releases. Ramanand found one issue by 
removal of these resource strings (so far) and that's captured in JDK-8157814

There's a decision to be made about how to use the GMT±hh:mm format for new 
releases given IANA's new short ID identifier mechanism. I think that could be 
discussed as a separate issue. I'd like to see us follow a similar approach to 
IANA and use GMT identifiers on new timezones and perhaps even consider using 
the IANA long name format also for the getDisplayName(..) calls that can be 
made. e.g. "Asia/Almaty" instead of "Alma-Ata Time" 

Regards,
Sean.

On 27/05/16 15:24, Masayoshi Okutsu wrote:

This change seems to beyond my proposal that the "GMT±hh:mm" format is used for 
*new* zones with the "±hh" format. But this change removes *existing* zones 
which have changed to use the "±hh" format in tzdata. Can this cause any 
compatibility issues? 

And have we agreed to use the "GMT±hh:mm" format? 

Thanks, 
Masayoshi 


On 5/27/2016 10:19 PM, Seán Coffey wrote: 




Looks fine to me Ramanand. the recent 2016d changes have introduced some 
boundary issues for JDK rule parsing and those issues can be followed up in 
separate issues like you say. 

Regards, 
Sean. 

On 26/05/16 14:22, Ramanand Patil wrote: 




HI all, 

Please review the latest TZDATA integration (tzdata2016d) to JDK9. 

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

Webrev: HYPERLINK 
"http://cr.openjdk.java.net/%7Erpatil/8151876/webrev.00/"http://cr.openjdk.java.net/~rpatil/8151876/webrev.00/
 

Patch Contains: 

1.   IANA tzdata2016d integration into JDK. [It also includes tzdata2016b 
and tzdata2016c which was not integrated]. 

2.   "GMT[+ -]hh:mm" is used for formatting of the modified or newly added 
TimeZones in tzdata2016d. 

[This is done to accommodate the IANA's new system where the zones use numeric 
time zone abbreviations like "+04" instead of invented abbreviations like 
"ASTT".] 

3.   Test case: 
java/time/test/java/time/format/TestZoneTextPrinterParser.java is updated to 
include the failures because of GMT[+ -]hh:mm format names. 

4.   Few other failing tests: For few other failing tests, new linked bugs 
are created and will be addressed in a separate patch. 


Regards, 

Ramanand. 

 

 

 

 

 

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

[Paul Sandoz]

> On 31 May 2016, at 10:35, shilpi.rast...@oracle.com wrote:
> 
> Thanks Paul for comments.
> 
> Please see http://cr.openjdk.java.net/~srastogi/8147585/webrev.01/
> 
> Now processing only public abstract methods of interface.
> 

Thanks. It would be good to get some got feedback from those wiser than I in 
this regard.

Have you looked at the existing annotation-based tests to see if they test edge 
cases e.g. annotation classes generated with incorrect methods? that might give 
us some clues.

Testing wise:

- you can avoid the filtering of the test method by placing the annotations in 
another auxiliary class (my preference is to nest rather than using auxiliary 
classes, up to you)

- there is a couple of minor style inconsistencies e.g. a space here "@ 
interface LambdaWithoutParameter"

- 30  * @run testng/othervm -ea -esa AnnotationWithLambda

You can just do:

  @run testng AnnotationWithLambda

?

Thanks,
Paul.




> Thanks,
> Shilpi
> 
> On 5/30/2016 6:35 PM, Paul Sandoz wrote:
>> Hi Shilpi,
>> 
>> You have found the right place but i am not sure your fix is entirely 
>> correct.
>> 
>> (Tip: if you use  -Xlog:exceptions=info you can observe the IAE exception 
>> when the annotation is processed)
>> 
>> In your test you have:
>> 
>> @Target(value = ElementType.METHOD)
>> @Retention(RetentionPolicy.RUNTIME)
>> @ interface LambdaWithParameter {
>> Consumer f1 = a -> {
>> System.out.println("lambda has parameter");
>> };
>> }
>> 
>> @Target(value = ElementType.METHOD)
>> @Retention(RetentionPolicy.RUNTIME)
>> @ interface LambdaWithoutParameter {
>> Runnable r = () -> System.out.println("lambda without parameter");
>> }
>> 
>> 
>> Both of those annotations will have static synthetic methods generated by 
>> the compiler that the indy resolves and links to (look at the javap output). 
>> The former will have a method with one parameter.
>> 
>> 
>> The code in sun/reflect/annotation/AnnotationType.java:
>> 
>> for (Method method : methods) {
>> if (method.getParameterTypes().length != 0)
>> throw new IllegalArgumentException(method + " has params");
>> 
>> has thrown the IAE since 2004, but it’s not clear why it was added as 
>> opposed to something more general (see below).
>> 
>> 
>> The correct fix appears to be to skip over any non-abstract/non-public 
>> methods. Thus only public abstract methods get processed.
>> 
>> Your current fix now processes synthetic methods with parameters, in 
>> addition to those which were already processed such as synthetic methods 
>> without parameters, or even private methods that could have been generated 
>> by some tool. I dunno how much say the verifier has in all this, perhaps 
>> little or no say.
>> 
>> Thus non-public/non-abstract methods could add inconsistent information to 
>> the data structures of AnnotationType. Perhaps this is mostly harmless?
>> 
>> Perhaps Joel (CC’ed) can she some more light on this?
>> 
>> Paul.
>> 
>> 
>>> On 30 May 2016, at 08:00, shilpi.rast...@oracle.com wrote:
>>> 
>>> Hi All,
>>> 
>>> Please review fix for https://bugs.openjdk.java.net/browse/JDK-8147585
>>> http://cr.openjdk.java.net/~srastogi/8147585/webrev.00/
>>> 
>>> Problem: Annotation with Lamda has parameters results into wrong behavior ( 
>>> not considered as valid annotation) because
>>> According to JLS "By virtue of the AnnotationTypeElementDeclaration 
>>> production, a method declaration in an annotation type declaration cannot 
>>> have formal parameters, type parameters, or a throws clause. The following 
>>> production from §4.3 is shown here for convenience:"
>>> (Ref: 
>>> https://docs.oracle.com/javase/specs/jls/se8/html/jls-9.html#jls-9.6.1)
>>> 
>>> 
>>> Solution: We should skip synthetic methods during Annotation parsing. 
>>> According to JLS "An annotation type has no elements other than those 
>>> defined by the methods it explicitly declares."
>>> (Ref https://docs.oracle.com/javase/specs/jls/se8/html/.html#jls-9jls-9.6.1)
>>> 
>>> 
>>> Thanks,
>>> Shilpi
> 

Re: RFR: JDK-8061777, , (zipfs) IllegalArgumentException in ZipCoder.toString when using Shitft_JIS

[Paul Sandoz]

> On 31 May 2016, at 07:07, Xueming Shen  wrote:
> 
> Thanks Paul,
> 
> updated accordingly.
> 
> http://cr.openjdk.java.net/~sherman/8061777/webrev
> 

+1

Alas it’s awkward to do the converse for constructor accepting byte[] that may 
or may not be utf-8 given the "boolean normalized” parameter.

Paul.

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

[shilpi.rast...@oracle.com]

Thanks Paul for comments.

Please see http://cr.openjdk.java.net/~srastogi/8147585/webrev.01/

Now processing only public abstract methods of interface.

Thanks,
Shilpi

On 5/30/2016 6:35 PM, Paul Sandoz wrote:

Hi Shilpi,

You have found the right place but i am not sure your fix is entirely correct.

(Tip: if you use  -Xlog:exceptions=info you can observe the IAE exception when 
the annotation is processed)

In your test you have:

@Target(value = ElementType.METHOD)
@Retention(RetentionPolicy.RUNTIME)
@ interface LambdaWithParameter {
 Consumer f1 = a -> {
 System.out.println("lambda has parameter");
 };
}

@Target(value = ElementType.METHOD)
@Retention(RetentionPolicy.RUNTIME)
@ interface LambdaWithoutParameter {
 Runnable r = () -> System.out.println("lambda without parameter");
}


Both of those annotations will have static synthetic methods generated by the 
compiler that the indy resolves and links to (look at the javap output). The 
former will have a method with one parameter.


The code in sun/reflect/annotation/AnnotationType.java:

for (Method method : methods) {
 if (method.getParameterTypes().length != 0)
 throw new IllegalArgumentException(method + " has params");

has thrown the IAE since 2004, but it’s not clear why it was added as opposed 
to something more general (see below).


The correct fix appears to be to skip over any non-abstract/non-public methods. 
Thus only public abstract methods get processed.

Your current fix now processes synthetic methods with parameters, in addition 
to those which were already processed such as synthetic methods without 
parameters, or even private methods that could have been generated by some 
tool. I dunno how much say the verifier has in all this, perhaps little or no 
say.

Thus non-public/non-abstract methods could add inconsistent information to the 
data structures of AnnotationType. Perhaps this is mostly harmless?

Perhaps Joel (CC’ed) can she some more light on this?

Paul.



On 30 May 2016, at 08:00, shilpi.rast...@oracle.com wrote:

Hi All,

Please review fix for https://bugs.openjdk.java.net/browse/JDK-8147585
http://cr.openjdk.java.net/~srastogi/8147585/webrev.00/

Problem: Annotation with Lamda has parameters results into wrong behavior ( not 
considered as valid annotation) because
According to JLS "By virtue of the AnnotationTypeElementDeclaration production, a 
method declaration in an annotation type declaration cannot have formal parameters, type 
parameters, or a throws clause. The following production from §4.3 is shown here for 
convenience:"
(Ref: https://docs.oracle.com/javase/specs/jls/se8/html/jls-9.html#jls-9.6.1)


Solution: We should skip synthetic methods during Annotation parsing. According to JLS 
"An annotation type has no elements other than those defined by the methods it 
explicitly declares."
(Ref https://docs.oracle.com/javase/specs/jls/se8/html/.html#jls-9jls-9.6.1)


Thanks,
Shilpi