Re: CFR - updated 8001667: Comparator combinators and extension methods

2013-03-06 Thread Paul Sandoz
Hi Henry,

Minor thing.

In Comparator:

 194  * @param other the other comparator used when equals on this.
 195  * @throws NullPointerException if the argument is null.
 196  * @since 1.8
 197  */
 198 default ComparatorT thenComparing(Comparator? super T other) {
 199 return Comparators.compose(this, other);
 200 }

Perhaps:

@param other the other comparator to be used when this comparator compares two 
objects that are equal
@throws NullPointerException if the argument is null.
@since 1.8
@return A lexicographic order comparator composed of this and then the other 
comparator


In Comparators:

 241  * @param T the element type to be compared
 242  * @param first the first comparator
 243  * @param second the secondary comparator used when equals on the first
 244  */
 245 public staticT ComparatorT compose(Comparator? super T first, 
Comparator? super T second) {
 246 Objects.requireNonNull(first);
 247 Objects.requireNonNull(second);
 248 return (ComparatorT  Serializable) (c1, c2) - {
 249 int res = first.compare(c1, c2);
 250 return (res != 0) ? res : second.compare(c1, c2);
 251 };
 252 }

@param second the second comparator to be used when the first comparator 
compares two objects that are equal
@return A lexicographic order comparator composed of the first and then the 
second comparator

Paul.

On Mar 5, 2013, at 8:46 PM, Henry Jen henry@oracle.com wrote:

 Hi,
 
 Another update to reflect functional interface renames involved in the
 API, and a bug fix for a regression found earlier.
 
 CCC had been approved. Can we get it reviewed and pushed?
 
 [1] http://cr.openjdk.java.net/~henryjen/ccc/8001667.4/webrev
 
 Cheers,
 Henry



Re: RFR : JDK-8001642 : Add OptionalT, OptionalDouble, OptionalInt, OptionalLong

2013-03-06 Thread Remi Forax

Ok, let be nuclear on this,
There is no good reason to introduce OptionalT in java.util.

It doen't work like Google's Guava Optional despite having the same 
name, it doesn't work like Scala's Option despite having a similar name, 
moreover the lambda pipeline face a similar issue with the design of 
collectors (see stream.collect()) but solve that similar problem with a 
different design, so the design of Optional is not even consistent with 
the rest of the stream API.


So why do we want something like Optional, we want it to be able to 
represent the fact that as Mike states a returning result can have no 
value by example Colections.emptyList().stream().findFirst() should 
'return' no value.


As Stephen Colebourne said, Optional is a bad name because Scala uses 
Option [1] which can used in the same context, as result of a filter/map 
etc. but Option in Scala is a way to mask null. Given the name 
proximity, people will start to use Optional like Option in Scala and we 
will see methods returning things like OptionalListOptionalString.


Google's Guava, which is a popular library, defines a class named 
Optional, but allow to store null unlike the current proposed 
implementation, this will generate a lot of confusions and frustrations.


In fact, we don't need Optional at all, because we don't need to return 
a value that can represent a value or no value,
the idea is that methods like findFirst should take a lambda as 
parameter letting the user to decide what value should be returned by 
findFirst if there is a value and if there is no value.

So instead of
  stream.findFirst().orElse(null)
you will write
  stream.findFirst(orNull)
with orNull() defined as like that
  public static T Optionalizer orNull() {
return (isPresent, element) - isPresent? element: null;
  }

The whole design is explained here [2] and is similar to the way 
Collectors are defined [3],
it's basically the lambda way of thinking, instead of creating an object 
representing the different states resulting of a call to findFirst,
findFirst takes a lambda as parameter which is fed with the states of a 
call.


cheers,
Rémi

[1] http://www.scala-lang.org/api/current/index.html#scala.Option
[2] 
http://mail.openjdk.java.net/pipermail/lambda-libs-spec-observers/2013-February/001470.html
[3] 
http://hg.openjdk.java.net/lambda/lambda/jdk/file/tip/src/share/classes/java/util/stream/Collectors.java



On 03/04/2013 09:29 PM, Mike Duigou wrote:

Hello All;

This patch introduces Optional container objects to be used by the lambda 
streams libraries for returning results.

The reference Optional type, as defined, intentionally does not allow null 
values. null may be used with the Optional.orElse() method.

All of the Optional types define hashCode() and equals implementations. Use of 
Optional types in collections should be generally discouraged but having useful 
equals() and hashCode() is ever so convenient.

http://cr.openjdk.java.net/~mduigou/JDK-8001642/0/webrev/

Mike





hg: jdk8/tl/jdk: 8009397: test/com/sun/jdi/PrivateTransportTest.sh: ERROR: transport library missing onLoad entry: private_dt_socket

2013-03-06 Thread staffan . larsen
Changeset: 34372bb9115d
Author:sla
Date:  2013-03-05 19:25 +0100
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/34372bb9115d

8009397: test/com/sun/jdi/PrivateTransportTest.sh: ERROR: transport library 
missing onLoad entry: private_dt_socket
Reviewed-by: alanb

! src/share/back/transport.c
! src/share/demo/jvmti/hprof/hprof_init.c
! src/solaris/back/linker_md.c
! src/solaris/demo/jvmti/hprof/hprof_md.c
! src/windows/back/linker_md.c
! src/windows/demo/jvmti/hprof/hprof_md.c



Re: RFR : JDK-8001642 : Add OptionalT, OptionalDouble, OptionalInt, OptionalLong

2013-03-06 Thread Remi Forax

On 03/06/2013 11:54 AM, Jed Wesley-Smith wrote:

Really, this is a lot of fuss over nothing.

There is actually no fundamental difference between Scala's Option, Guava's 
Optional, Fugue's Option, Java's Optional and Haskell's Maybe – they are 
modelling the same thing, the possibility of a value not being present.

The fact that there may be minor differences in api or semantics around whether 
null is a legal value are minor in the scheme of things (and yes, null is a 
pretty stupid legal value of a Some IMHO).

Stephen's example is ludicrous, why have a list of optional values? You'd 
flatten down into just a list – and an optional list only makes sense if the 
enclosed list is guaranteed to be non-empty, otherwise you just return an empty 
list!


People like shooting their own feet.
http://cs.calstatela.edu/wiki/index.php/Courses/CS_460/Fall_2012/Week_8/gamePlay.combat.BattleAnalysis 





If we are going to use potential straw-men as arguments we can stall all 
progress. Please concentrate on the important matters, let's disavow null as a 
valid value and save us all a billion dollars


Also Scala Option is not the only way to solve the null problem.
The JSR308 annotation @Nullable/@NonNull are recognized by Eclipse and 
IntelliJ at least.



.

cheers,
jed.


cheers,
Rémi



On 06/03/2013, at 8:47 PM, Remi Forax fo...@univ-mlv.fr wrote:


Ok, let be nuclear on this,
There is no good reason to introduce OptionalT in java.util.

It doen't work like Google's Guava Optional despite having the same
name, it doesn't work like Scala's Option despite having a similar name,
moreover the lambda pipeline face a similar issue with the design of
collectors (see stream.collect()) but solve that similar problem with a
different design, so the design of Optional is not even consistent with
the rest of the stream API.

So why do we want something like Optional, we want it to be able to
represent the fact that as Mike states a returning result can have no
value by example Colections.emptyList().stream().findFirst() should
'return' no value.

As Stephen Colebourne said, Optional is a bad name because Scala uses
Option [1] which can used in the same context, as result of a filter/map
etc. but Option in Scala is a way to mask null. Given the name
proximity, people will start to use Optional like Option in Scala and we
will see methods returning things like OptionalListOptionalString.

Google's Guava, which is a popular library, defines a class named
Optional, but allow to store null unlike the current proposed
implementation, this will generate a lot of confusions and frustrations.

In fact, we don't need Optional at all, because we don't need to return
a value that can represent a value or no value,
the idea is that methods like findFirst should take a lambda as
parameter letting the user to decide what value should be returned by
findFirst if there is a value and if there is no value.
So instead of
   stream.findFirst().orElse(null)
you will write
   stream.findFirst(orNull)
with orNull() defined as like that
   public static T Optionalizer orNull() {
 return (isPresent, element) - isPresent? element: null;
   }

The whole design is explained here [2] and is similar to the way
Collectors are defined [3],
it's basically the lambda way of thinking, instead of creating an object
representing the different states resulting of a call to findFirst,
findFirst takes a lambda as parameter which is fed with the states of a
call.

cheers,
Rémi

[1] http://www.scala-lang.org/api/current/index.html#scala.Option
[2]
http://mail.openjdk.java.net/pipermail/lambda-libs-spec-observers/2013-February/001470.html
[3]
http://hg.openjdk.java.net/lambda/lambda/jdk/file/tip/src/share/classes/java/util/stream/Collectors.java


On 03/04/2013 09:29 PM, Mike Duigou wrote:

Hello All;

This patch introduces Optional container objects to be used by the lambda 
streams libraries for returning results.

The reference Optional type, as defined, intentionally does not allow null 
values. null may be used with the Optional.orElse() method.

All of the Optional types define hashCode() and equals implementations. Use of 
Optional types in collections should be generally discouraged but having useful 
equals() and hashCode() is ever so convenient.

http://cr.openjdk.java.net/~mduigou/JDK-8001642/0/webrev/

Mike







Re: Request for review- RFE 8005716

2013-03-06 Thread Alan Bateman

On 06/03/2013 04:13, BILL PITTORE wrote:

On 3/5/2013 7:36 PM, Dean Long wrote:
If JNI_ONLOAD_SYMBOLS contains something like _JNI_OnLoad@8 on 
Windows, you can't just

turn that into _JNI_OnLoad@8_ + libname.  I think it needs to be
_JNI_OnLoad_  + libname + @8
I'll look into that. When I built for windows and ran our test, the 
JNI_OnLoad_TestStaticLib was exported without the decoration just 
based on the JNIEXPORT JNICALL specifiers on the function. I didn't do 
anything special to export it. But I recall this problem from another 
project.
10 1014 JNI_OnLoad_TestStaticLib = 
@ILT+15(?JNI_OnLoad_TestStaticLib@@YGHPAUJavaVM_@@PAX@Z)
Dean makes a good point, the @8 need be at the end to match the 
decoration scheme. Also I think it's 32-bit only although it should just 
not be found on 64-bit so it will skip on JNI_OnLoad_L.


-Alan.



Re: Request for review- RFE 8005716

2013-03-06 Thread Alan Bateman

On 05/03/2013 23:05, bill.pitt...@oracle.com wrote:
This request is tied to bugid 8005716 that deals with adding support 
for statically linked JNI libraries.


The JEP is here: http://openjdk.java.net/jeps/178

The bug is here:http://bugs.sun.com/view_bug.do?bug_id=8005716

The webrevs are here:

http://cr.openjdk.java.net/~bpittore/8005716/jdk-webrev.00/
http://cr.openjdk.java.net/~bpittore/8005716/hs-webrev.00/

The main piece of functionality is to check for the presence of 
JNI_OnLoad_libname to determine if the library specified by 'libname' 
has been statically linked into the VM. If the symbol is found, it is 
assumed that the library is linked in and will not be dynamically loaded.
Overall I think this is quite good and follows the proposal in the JEP. 
I don't see anything obvious wrong with the logic and everything should 
just work for shared libraries as it does today. I assume you'll run all 
the appropriate tests.


I see Dean's suggestion to add a JVM function to allow for a lookup 
table when the symbol information is available, If you do that then 
you'll need to get the hotspot changes in first. Alternatively, change 
what you have later once the hotspot changes are in.Just on the


As findBuiltJniFunction can locate a built-in or a JNI_OnLoad/OnUnload 
in a library then the function name is probably not quite right 
(shouldn't have Builtin in the name).


A minor nit in _findBuiltinLib is that if the OOME path should call 
JNU_ReleaseStringPlatformChars before returning.


There are a few commented out statements in jni_util_md.c that I assume 
will be removed. Also you might want to check the indentation in both 
getProcessHandle implementations as they look like 2-spaces whereas the 
libs uses 4 (although this may be mute if you use a JVM function).


Otherwise I think this is good and I can sponsor the jdk part to this 
and help get it into jdk8/tl.


-Alan



RFR: 8007808: Missing method: Executable.getAnnotatedReturnType()

2013-03-06 Thread Joel Borggrén-Franck

Hi,

Can I get a review of this small fix for issue 8007808: Missing method: 
Executable.getAnnotatedReturnType()


http://cr.openjdk.java.net/~jfranck/8007808/webrev.00/

When we added Core Reflection support for type annotations this method got 
left out on Executable, but were included on Method and Constructor.


For Oracle reviewers, CCC is approved.

cheers
/Joel


Re: RFR: 8007808: Missing method: Executable.getAnnotatedReturnType()

2013-03-06 Thread Joe Darcy

Hi Joel,

Look good; approved.

-Joe

On 3/6/2013 6:37 AM, Joel Borggrén-Franck wrote:

Hi,

Can I get a review of this small fix for issue 8007808: Missing 
method: Executable.getAnnotatedReturnType()


http://cr.openjdk.java.net/~jfranck/8007808/webrev.00/

When we added Core Reflection support for type annotations this method 
got left out on Executable, but were included on Method and Constructor.


For Oracle reviewers, CCC is approved.

cheers
/Joel




hg: jdk8/tl/langtools: 3 new changesets

2013-03-06 Thread maurizio . cimadamore
Changeset: d0178bd8125c
Author:mcimadamore
Date:  2013-03-06 15:29 +
URL:   http://hg.openjdk.java.net/jdk8/tl/langtools/rev/d0178bd8125c

8009299: Javac crashes when compiling method reference to static interface 
method
Summary: Assertion in Check.checMethod is too strict
Reviewed-by: jjg

! src/share/classes/com/sun/tools/javac/comp/Attr.java
! src/share/classes/com/sun/tools/javac/jvm/Pool.java
+ test/tools/javac/lambda/MethodReference66.java

Changeset: 8a78243291ef
Author:mcimadamore
Date:  2013-03-06 15:33 +
URL:   http://hg.openjdk.java.net/jdk8/tl/langtools/rev/8a78243291ef

8009459: Wrong behavior of diamond finder with source level 7
Summary: Diamond finder doesn't take into account different inference behaviors
Reviewed-by: jjg

! src/share/classes/com/sun/tools/javac/comp/Attr.java
+ test/tools/javac/generics/diamond/6939780/T6939780.java
+ test/tools/javac/generics/diamond/6939780/T6939780_7.out
+ test/tools/javac/generics/diamond/6939780/T6939780_8.out
- test/tools/javac/generics/diamond/T6939780.java
- test/tools/javac/generics/diamond/T6939780.out

Changeset: c98b3e96c726
Author:mcimadamore
Date:  2013-03-06 15:33 +
URL:   http://hg.openjdk.java.net/jdk8/tl/langtools/rev/c98b3e96c726

8009391: Synthetic name of serializable lambda methods should not contain 
negative numbers
Summary: Use hex representation of method signature hashcode to avoid negative 
numbers
Reviewed-by: jjg

! src/share/classes/com/sun/tools/javac/comp/LambdaToMethod.java



Re: RFR : JDK-8001642 : Add OptionalT, OptionalDouble, OptionalInt, OptionalLong

2013-03-06 Thread Zhong Yu
Just another idea:

findFirst() throws NoSuchElementException if the stream is empty.

Add an intermediary operation that maps an empty stream to a non-empty one

StreamT ifEmpty(T item);

so we can say

stream.ifEmpty(null).findFirst();

stream.ifEmpty(0).reduce(sum);

stream.ifEmpty(-1).min();

Zhong Yu

On Wed, Mar 6, 2013 at 3:47 AM, Remi Forax fo...@univ-mlv.fr wrote:
 Ok, let be nuclear on this,
 There is no good reason to introduce OptionalT in java.util.

 It doen't work like Google's Guava Optional despite having the same
 name, it doesn't work like Scala's Option despite having a similar name,
 moreover the lambda pipeline face a similar issue with the design of
 collectors (see stream.collect()) but solve that similar problem with a
 different design, so the design of Optional is not even consistent with
 the rest of the stream API.

 So why do we want something like Optional, we want it to be able to
 represent the fact that as Mike states a returning result can have no
 value by example Colections.emptyList().stream().findFirst() should
 'return' no value.

 As Stephen Colebourne said, Optional is a bad name because Scala uses
 Option [1] which can used in the same context, as result of a filter/map
 etc. but Option in Scala is a way to mask null. Given the name
 proximity, people will start to use Optional like Option in Scala and we
 will see methods returning things like OptionalListOptionalString.

 Google's Guava, which is a popular library, defines a class named
 Optional, but allow to store null unlike the current proposed
 implementation, this will generate a lot of confusions and frustrations.

 In fact, we don't need Optional at all, because we don't need to return
 a value that can represent a value or no value,
 the idea is that methods like findFirst should take a lambda as
 parameter letting the user to decide what value should be returned by
 findFirst if there is a value and if there is no value.
 So instead of
stream.findFirst().orElse(null)
 you will write
stream.findFirst(orNull)
 with orNull() defined as like that
public static T Optionalizer orNull() {
  return (isPresent, element) - isPresent? element: null;
}

 The whole design is explained here [2] and is similar to the way
 Collectors are defined [3],
 it's basically the lambda way of thinking, instead of creating an object
 representing the different states resulting of a call to findFirst,
 findFirst takes a lambda as parameter which is fed with the states of a
 call.

 cheers,
 Rémi

 [1] http://www.scala-lang.org/api/current/index.html#scala.Option
 [2]
 http://mail.openjdk.java.net/pipermail/lambda-libs-spec-observers/2013-February/001470.html
 [3]
 http://hg.openjdk.java.net/lambda/lambda/jdk/file/tip/src/share/classes/java/util/stream/Collectors.java


 On 03/04/2013 09:29 PM, Mike Duigou wrote:
 Hello All;

 This patch introduces Optional container objects to be used by the lambda 
 streams libraries for returning results.

 The reference Optional type, as defined, intentionally does not allow null 
 values. null may be used with the Optional.orElse() method.

 All of the Optional types define hashCode() and equals implementations. Use 
 of Optional types in collections should be generally discouraged but having 
 useful equals() and hashCode() is ever so convenient.

 http://cr.openjdk.java.net/~mduigou/JDK-8001642/0/webrev/

 Mike





Re: RFR: 8007808: Missing method: Executable.getAnnotatedReturnType()

2013-03-06 Thread Remi Forax

Hi Joel,
looks good for me too :)

Rémi

On 03/06/2013 04:35 PM, Joe Darcy wrote:

Hi Joel,

Look good; approved.

-Joe

On 3/6/2013 6:37 AM, Joel Borggrén-Franck wrote:

Hi,

Can I get a review of this small fix for issue 8007808: Missing 
method: Executable.getAnnotatedReturnType()


http://cr.openjdk.java.net/~jfranck/8007808/webrev.00/

When we added Core Reflection support for type annotations this 
method got left out on Executable, but were included on Method and 
Constructor.


For Oracle reviewers, CCC is approved.

cheers
/Joel






Re: Request for review- RFE 8005716

2013-03-06 Thread Bob Vandette

On Mar 5, 2013, at 7:36 PM, Dean Long wrote:

 If JNI_ONLOAD_SYMBOLS contains something like _JNI_OnLoad@8 on Windows, you 
 can't just
 turn that into _JNI_OnLoad@8_ + libname.  I think it needs to be
 _JNI_OnLoad_  + libname + @8
 
Good catch Dean.


 Looks like onLoadSymbols[] is unused in 
 Java_java_lang_ClassLoader_00024NativeLibrary_findBuiltinLib().
 
 Instead of adding getProcessHandle(), why not add 
 JVM_FindBuiltinLibraryEntry() instead?
 This would make it easier to support table-lookup when runtime symbol 
 information is missing or not
 supported by the platform.

Bill has already factored out the implementation of getProcessHandle.  Although 
your
approach is cleaner, I'm concerned about creating a VM version dependency.   
For a traditional
JRE that doesn't even require static library support, we'd have to make sure to 
run on a VM that
support JNI_VERSION_1_8.  It looks like the JDK maintains their own copy of 
jni.h.  If they didn't 
do that, we would have already gone down that path.   The jdk sources already 
contain several
instances of dlopen, dlysym and the Windows equivalents so I don't see a need 
to add a new
VM function.

Bob.

 
 dl
 
 On 3/5/2013 3:05 PM, bill.pitt...@oracle.com wrote:
 This request is tied to bugid 8005716 that deals with adding support for 
 statically linked JNI libraries.
 
 The JEP is here: http://openjdk.java.net/jeps/178
 
 The bug is here:http://bugs.sun.com/view_bug.do?bug_id=8005716
 
 The webrevs are here:
 
 http://cr.openjdk.java.net/~bpittore/8005716/jdk-webrev.00/
 http://cr.openjdk.java.net/~bpittore/8005716/hs-webrev.00/
 
 The main piece of functionality is to check for the presence of 
 JNI_OnLoad_libname to determine if the library specified by 'libname' has 
 been statically linked into the VM. If the symbol is found, it is assumed 
 that the library is linked in and will not be dynamically loaded.
 
 thanks,
 bill
 



hg: jdk8/tl: 8009162: root repo make test target should run against image

2013-03-06 Thread mike . duigou
Changeset: b35d986ff276
Author:mduigou
Date:  2013-03-06 08:37 -0800
URL:   http://hg.openjdk.java.net/jdk8/tl/rev/b35d986ff276

8009162: root repo make test target should run against image
Reviewed-by: alanb, martin, erikj

! common/makefiles/Main.gmk



Re: RFR: 8006593: Performance and compatibility improvements to hash based Map implementations

2013-03-06 Thread Mike Duigou

On Mar 6 2013, at 09:19 , Alan Bateman wrote:

 On 05/03/2013 22:46, Mike Duigou wrote:
 
 I have updated the webrev to remove the useAltHashing boolean.
 
 http://cr.openjdk.java.net/~mduigou/JDK-8006593/5/webrev/
 
 Mike
 
 Peter's suggestion to remove the useAltHashing field is a great idea.
 
 I've looked at the webrev and it looks good to me. The logical xor in 
 initHashSeedAsNeeded is subtle.
 
 As this is for jdk7u only then I assume you'll get approval on jdk7u-dev once 
 you are done here.

Yes. Per the original review request:
 Once review is completed here this patch will be proposed to JDK7u-dev for 
 integration into the next 7u
 performance/feature release.
I am going to leave this review open for another 24 hours before requesting 
approval from JDK7u-dev.

Mike

Re: RFR: 8002070 Remove the stack search for a resource bundle for Logger to use

2013-03-06 Thread Mandy Chung



On 3/5/2013 9:16 AM, Jim Gish wrote:


On 03/01/2013 05:48 PM, Mandy Chung wrote:

On 3/1/2013 1:25 PM, Jim Gish wrote:
Please review 
http://cr.openjdk.java.net/~jgish/Bug8002070-RemoveResourceBundleStackSearch/ 


This removes the stack search from Logger.findResourceBundle()



It's good to see this stack walk search of resource bundle going away.

In Logger.java, the existing implementation returns the previous 
cached resource bundle if it fails to find one matching the current 
locale but the name matches:


1608 if (name.equals(catalogName)) {
1609 // Return the previous cached value for that name.
1610 // This may be null.
1611 return catalog;
1612 }


Your fix removes these lines which I think is fine.  The 
Logger.getResourceBundle method specifies to return the resource 
bundle for this logger for the current default locale.  I think it'd 
be rare to change the default locale during the execution of an 
application.

The old behavior upon reaching this point in the code was as follows:
To get here, the sought after bundle was not found and (from the 
checks on line 1559,1560), either
(1) the previously cached catalog was null, meaning nothing was yet 
cached and so null was returned, or


correct.

(2) the current locale had changed and no bundle for that locale was 
found, in which case the cached bundle (for the old/wrong locale) was 
returned, or


do you mean it returns the cached bundle only if the name matches?  or 
am I missing the code that you read?


(3) the name was the same but the location of the objects used to 
compare the cached name with that passed in was different, 


Can you elaborate?  Are you referring the if 
(name.equals(catalogName)) statement?


so the previously cached bundle was returned (this is frankly an odd 
case, because it also means that re-searching for a previously found 
bundle is now failing, which only seems possible if the bundle is/was 
(a) not available via the system classloader, (b) the context 
classloader, or (c), the classloaders up the call chain.


The new behavior /does /still allow for a change in the default 
locale.  For example, if you first call findResourceBundleName(foo) 
and the default locale is US, it will search for the foo_US bundle and 
set catalogLocale=US, and catalogName=foo and return the foo_US 
bundle.  If you then search for foo and have changed the default 
locale to DE (if that is indeed a valid locale), it will then search 
for foo_DE and if found set catalogLocale=DE and cache and return the 
foo_DE bundle.   The /only /change in behavior that I see here, is 
that if you change the locale and the bundle is not found, null will 
be returned instead of the previously cached bundle (if one exists), 
for a /different/ locale.




Right.   The behavioral difference that I point out was when foo_DE is 
cached, later the current locale is set to JP where foo_JP and foo don't 
exist, the old behavior returns foo_DE which was a bug too me whereas 
the new behavior returns null which matches the spec of 
Logger.getResourceBundle().


So, the old behavior in effect had a notion of a default catalog, but 
only if you happened to find a bundle on a previous lookup and cache 
it.  However, you wouldn't be guaranteed of acquiring this same bundle 
on a subsequent search if a search for a different name had intervened.


Thus, the new behavior is that if you search for a name/locale and 
it's not found, you will get null (every time). This seems better to 
me because it's consistent, explainable and simple.




This matches what Logger.getResourceBundle() is specified and the 
behavior is correct.  The difference in behavior is a minimal 
compatibility risk.


Comments on the updated webrev:

http://cr.openjdk.java.net/~jgish/Bug8002070-RemoveResourceBundleStackSearch/ 



L122-125: you may want to replace code/code with {@code  }
L129-130: you can use @linkplain instead:

{@linkplain ClassLoader#getSystemClassLoader()system ClassLoader}

L141-142: the bundle associated with the Logger object itself cannot be 
changed - is this statement correct?  The cached catalog object is not 
accessed directly; instead when it finds the resource bundle every time 
it logs a message.


In addition the new paragraph L132-142 doesn't seem to be necessary 
since the spec is pretty clear on using the resource bundle of the 
current locale.


L1575: the @link here is not necessary in a comment

LoggerResourceBundleRace.java: I think what you really want is to add a 
new test that sets a context class loader to a class loader that finds 
the resource bundle for a logger that a system class loader can't 
find.   I suggest to leave this test as it is and then add a new test to 
exercise the context class loader search of a resource bundle as a 
separate RFE that will improve the test coverage.


Mandy


I'd appreciate you verifying this.

Thanks,

Jim


I suggest to document this behavioral 

Re: Request for review- RFE 8005716

2013-03-06 Thread Mike Duigou

On Mar 6 2013, at 08:21 , Bob Vandette wrote:

 For a traditional
 JRE that doesn't even require static library support, we'd have to make sure 
 to run on a VM that
 support JNI_VERSION_1_8.  It looks like the JDK maintains their own copy of 
 jni.h.

In earlier days the jni.h file was copied from hotspot into jdk during the 
compilation process. The current build process doesn't validate that the two 
files match which opens up opportunities for inconsistencies and problems.

  If they didn't 
 do that, we would have already gone down that path.

If it's possible to improve upon having two jni.h instances then we should 
consider doing so.

Mike

hg: jdk8/tl/jdk: 8006853: OCSP timeout set to wrong value if com.sun.security.ocsp.timeout 0

2013-03-06 Thread jason . uh
Changeset: 7246a6e4dd5a
Author:juh
Date:  2013-02-28 16:36 -0800
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/7246a6e4dd5a

8006853: OCSP timeout set to wrong value if com.sun.security.ocsp.timeout  0
Reviewed-by: mullan

! src/share/classes/sun/security/provider/certpath/OCSP.java



Re: Request for review- RFE 8005716

2013-03-06 Thread Dean Long

On 3/5/2013 8:13 PM, BILL PITTORE wrote:

On 3/5/2013 7:36 PM, Dean Long wrote:
If JNI_ONLOAD_SYMBOLS contains something like _JNI_OnLoad@8 on 
Windows, you can't just

turn that into _JNI_OnLoad@8_ + libname.  I think it needs to be
_JNI_OnLoad_  + libname + @8
I'll look into that. When I built for windows and ran our test, the 
JNI_OnLoad_TestStaticLib was exported without the decoration just 
based on the JNIEXPORT JNICALL specifiers on the function. I didn't do 
anything special to export it. But I recall this problem from another 
project.
10 1014 JNI_OnLoad_TestStaticLib = 
@ILT+15(?JNI_OnLoad_TestStaticLib@@YGHPAUJavaVM_@@PAX@Z)


Looks like onLoadSymbols[] is unused in 
Java_java_lang_ClassLoader_00024NativeLibrary_findBuiltinLib().
I'll remove that. I redid the call to findBuiltinJniFunction but 
forgot to remove that.




Instead of adding getProcessHandle(), why not add 
JVM_FindBuiltinLibraryEntry() instead?
This would make it easier to support table-lookup when runtime symbol 
information is missing or not

supported by the platform.
Not sure I'm following you on this. Make JVM_FindBuiltinLibraryEntry() 
an exported function in the VM? How does it find JNI_OnLoad_L? 


For Windows and Linux, it would use basically your same code, just 
refactored a little.
By the way, did you consider using the special RTLD_DEFAULT handle 
instead of

dlopen(NULL)?

Via a table setup by the developer/build system when the library is 
linked in?




Yes, in previous projects we did exactly that for more exotic OSes.

dl


bill



dl

On 3/5/2013 3:05 PM, bill.pitt...@oracle.com wrote:
This request is tied to bugid 8005716 that deals with adding support 
for statically linked JNI libraries.


The JEP is here: http://openjdk.java.net/jeps/178

The bug is here:http://bugs.sun.com/view_bug.do?bug_id=8005716

The webrevs are here:

http://cr.openjdk.java.net/~bpittore/8005716/jdk-webrev.00/
http://cr.openjdk.java.net/~bpittore/8005716/hs-webrev.00/

The main piece of functionality is to check for the presence of 
JNI_OnLoad_libname to determine if the library specified by 
'libname' has been statically linked into the VM. If the symbol is 
found, it is assumed that the library is linked in and will not be 
dynamically loaded.


thanks,
bill








Re: RFR : JDK-8001642 : Add OptionalT, OptionalDouble, OptionalInt, OptionalLong

2013-03-06 Thread Jack Moxley
Whatever happened to the elvis operator solution from project coin, surely that 
solves the majority of issues Optional was meant to solve anyway, without the 
headache of type-mix mud clutering up the code.

http://mail.openjdk.java.net/pipermail/coin-dev/2009-March/47.html

  _  

From: Remi Forax [mailto:fo...@univ-mlv.fr]
To: Jed Wesley-Smith [mailto:jed.wesleysm...@gmail.com]
Cc: lambda-libs-spec-expe...@openjdk.java.net 
[mailto:lambda-libs-spec-expe...@openjdk.java.net], lambda-...@openjdk.java.net 
[mailto:lambda-...@openjdk.java.net], core-libs-dev@openjdk.java.net 
[mailto:core-libs-dev@openjdk.java.net]
Sent: Wed, 06 Mar 2013 11:58:43 +
Subject: Re: RFR : JDK-8001642 : Add OptionalT, OptionalDouble, OptionalInt,  
OptionalLong

On 03/06/2013 11:54 AM, Jed Wesley-Smith wrote:
   Really, this is a lot of fuss over nothing.
  
   There is actually no fundamental difference between Scala's Option, Guava's 
Optional, Fugue's Option, Java's Optional and Haskell's Maybe – they are 
modelling the same thing, the possibility of a value not being present.
  
   The fact that there may be minor differences in api or semantics around 
whether null is a legal value are minor in the scheme of things (and yes, null 
is a pretty stupid legal value of a Some IMHO).
  
   Stephen's example is ludicrous, why have a list of optional values? You'd 
flatten down into just a list – and an optional list only makes sense if the 
enclosed list is guaranteed to be non-empty, otherwise you just return an empty 
list!
  
  People like shooting their own feet.
  
http://cs.calstatela.edu/wiki/index.php/Courses/CS_460/Fall_2012/Week_8/gamePlay.combat.BattleAnalysis
 
  
  
  
   If we are going to use potential straw-men as arguments we can stall all 
progress. Please concentrate on the important matters, let's disavow null as a 
valid value and save us all a billion dollars
  
  Also Scala Option is not the only way to solve the null problem.
  The JSR308 annotation @Nullable/@NonNull are recognized by Eclipse and 
  IntelliJ at least.
  
   .
  
   cheers,
   jed.
  
  cheers,
  Rémi
  
  
   On 06/03/2013, at 8:47 PM, Remi Forax fo...@univ-mlv.fr wrote:
  
   Ok, let be nuclear on this,
   There is no good reason to introduce OptionalT in java.util.
  
   It doen't work like Google's Guava Optional despite having the same
   name, it doesn't work like Scala's Option despite having a similar name,
   moreover the lambda pipeline face a similar issue with the design of
   collectors (see stream.collect()) but solve that similar problem with a
   different design, so the design of Optional is not even consistent with
   the rest of the stream API.
  
   So why do we want something like Optional, we want it to be able to
   represent the fact that as Mike states a returning result can have no
   value by example Colections.emptyList().stream().findFirst() should
   'return' no value.
  
   As Stephen Colebourne said, Optional is a bad name because Scala uses
   Option [1] which can used in the same context, as result of a filter/map
   etc. but Option in Scala is a way to mask null. Given the name
   proximity, people will start to use Optional like Option in Scala and we
   will see methods returning things like OptionalListOptionalString.
  
   Google's Guava, which is a popular library, defines a class named
   Optional, but allow to store null unlike the current proposed
   implementation, this will generate a lot of confusions and frustrations.
  
   In fact, we don't need Optional at all, because we don't need to return
   a value that can represent a value or no value,
   the idea is that methods like findFirst should take a lambda as
   parameter letting the user to decide what value should be returned by
   findFirst if there is a value and if there is no value.
   So instead of
  stream.findFirst().orElse(null)
   you will write
  stream.findFirst(orNull)
   with orNull() defined as like that
  public static T Optionalizer orNull() {
return (isPresent, element) - isPresent? element: null;
  }
  
   The whole design is explained here [2] and is similar to the way
   Collectors are defined [3],
   it's basically the lambda way of thinking, instead of creating an object
   representing the different states resulting of a call to findFirst,
   findFirst takes a lambda as parameter which is fed with the states of a
   call.
  
   cheers,
   Rémi
  
   [1] http://www.scala-lang.org/api/current/index.html#scala.Option
   [2]
   
http://mail.openjdk.java.net/pipermail/lambda-libs-spec-observers/2013-February/001470.html
   [3]
   
http://hg.openjdk.java.net/lambda/lambda/jdk/file/tip/src/share/classes/java/util/stream/Collectors.java
  
  
   On 03/04/2013 09:29 PM, Mike Duigou wrote:
   Hello All;
  
   This patch introduces Optional container objects to be used by the lambda 
streams libraries for returning results.
  
   The reference Optional type, as defined, intentionally does not 

RE: JDK 8 RFR: 8009267: Restore isAnnotationPresent methods in public AnnotatedElement implementations

2013-03-06 Thread Uwe Schindler
My intention was to say implementation removed.

Uwe

-
Uwe Schindler
uschind...@apache.org 
Apache Lucene PMC Member / Committer
Bremen, Germany
http://lucene.apache.org/
 -Original Message-
 From: Joe Darcy [mailto:joe.da...@oracle.com]
 Sent: Monday, March 04, 2013 5:16 PM
 To: Uwe Schindler
 Cc: 'Core-Libs-Dev'; 'Jonathan Gibbons'; 'Alexander Buckley'; 'Maurizio
 Cimadamore'
 Subject: Re: JDK 8 RFR: 8009267: Restore isAnnotationPresent methods in
 public AnnotatedElement implementations
 
 On 03/02/2013 02:34 PM, Uwe Schindler wrote:
  Looks good to me!
  Are this the only methods in the corelib that were removed in favour of a
 default implementation in the interface?
 
 I would not described the method as removed, but to my knowledge
 AnnotatedElement.isAnnotationPresent was the only existing interface
 method in the core libraries replaced by a default method. The intended use
 case for default methods is adding new methods to interface and that
 change has certainly been done in other parts of the API.
 
 Cheers,
 
 -Joe
 
 
  Uwe
 
  -
  Uwe Schindler
  uschind...@apache.org
  Apache Lucene PMC Member / Committer
  Bremen, Germany
  http://lucene.apache.org/
 
  -Original Message-
  From: Joe Darcy [mailto:joe.da...@oracle.com]
  Sent: Saturday, March 02, 2013 3:06 AM
  To: Core-Libs-Dev
  Cc: Jonathan Gibbons; Alexander Buckley; Maurizio Cimadamore;
  uschind...@apache.org
  Subject: JDK 8 RFR: 8009267: Restore isAnnotationPresent methods in
  public AnnotatedElement implementations
 
  Hello,
 
  The changes pushed under
 
8007113: Upgrade AnnotatedElement.isAnnotionPresent to be a
  default method
http://hg.openjdk.java.net/jdk8/tl/jdk/rev/e04467fa13af
 
  combined with how javac currently models default methods under
  pre-JDK-8 source versions can result in some source compatibility
  impacts for those using JDK 8 to compile under older source versions
  if the recommended practice of setting the bootclasspath is not
  followed. [1]
 
  To mitigate these impacts, I'm proposing the changes in
 
8009267: Restore isAnnotationPresent methods in public
  AnnotatedElement implementations
http://cr.openjdk.java.net/~darcy/8009267.1/
 
  which have the effect of restoring a concrete isAnnotationPresent
  method to Class, Package, Field, Method, and Constructor. The
  implementation delegates to the default method on the
 AnnotatedElement interface.
 
  Thanks,
 
  -Joe
 
  [1]
  http://mail.openjdk.java.net/pipermail/compiler-dev/2013-
  February/005738.html



Re: RFR : JDK-8001642 : Add OptionalT, OptionalDouble, OptionalInt, OptionalLong

2013-03-06 Thread Jed Wesley-Smith
There is no need for an Option container to show how nested collections may be 
misused, you could just as easily show an example of a ListListListT that 
is isomorphic to a flattened IterableT . I'd simply point to the utility of 
the monadic bind or flatMap function!

There are several reasons why Option types provide serious usability 
improvements over null, notwithstanding annotation support. The first is that 
the type-system prevents misuse (modulo unsafe API), and the second is that it 
allows operations that produce optional results to be sequenced easily without 
if checks. For mor information see previous posts on Optional forming a Monad.

cheers,
jed.

On 06/03/2013, at 10:58 PM, Remi Forax fo...@univ-mlv.fr wrote:

 On 03/06/2013 11:54 AM, Jed Wesley-Smith wrote:
 Really, this is a lot of fuss over nothing.
 
 There is actually no fundamental difference between Scala's Option, Guava's 
 Optional, Fugue's Option, Java's Optional and Haskell's Maybe – they are 
 modelling the same thing, the possibility of a value not being present.
 
 The fact that there may be minor differences in api or semantics around 
 whether null is a legal value are minor in the scheme of things (and yes, 
 null is a pretty stupid legal value of a Some IMHO).
 
 Stephen's example is ludicrous, why have a list of optional values? You'd 
 flatten down into just a list – and an optional list only makes sense if the 
 enclosed list is guaranteed to be non-empty, otherwise you just return an 
 empty list!
 
 People like shooting their own feet.
 http://cs.calstatela.edu/wiki/index.php/Courses/CS_460/Fall_2012/Week_8/gamePlay.combat.BattleAnalysis
  
 
 
 If we are going to use potential straw-men as arguments we can stall all 
 progress. Please concentrate on the important matters, let's disavow null as 
 a valid value and save us all a billion dollars
 
 Also Scala Option is not the only way to solve the null problem.
 The JSR308 annotation @Nullable/@NonNull are recognized by Eclipse and 
 IntelliJ at least.
 
 .
 
 cheers,
 jed.
 
 cheers,
 Rémi
 
 
 On 06/03/2013, at 8:47 PM, Remi Forax fo...@univ-mlv.fr wrote:
 
 Ok, let be nuclear on this,
 There is no good reason to introduce OptionalT in java.util.
 
 It doen't work like Google's Guava Optional despite having the same
 name, it doesn't work like Scala's Option despite having a similar name,
 moreover the lambda pipeline face a similar issue with the design of
 collectors (see stream.collect()) but solve that similar problem with a
 different design, so the design of Optional is not even consistent with
 the rest of the stream API.
 
 So why do we want something like Optional, we want it to be able to
 represent the fact that as Mike states a returning result can have no
 value by example Colections.emptyList().stream().findFirst() should
 'return' no value.
 
 As Stephen Colebourne said, Optional is a bad name because Scala uses
 Option [1] which can used in the same context, as result of a filter/map
 etc. but Option in Scala is a way to mask null. Given the name
 proximity, people will start to use Optional like Option in Scala and we
 will see methods returning things like OptionalListOptionalString.
 
 Google's Guava, which is a popular library, defines a class named
 Optional, but allow to store null unlike the current proposed
 implementation, this will generate a lot of confusions and frustrations.
 
 In fact, we don't need Optional at all, because we don't need to return
 a value that can represent a value or no value,
 the idea is that methods like findFirst should take a lambda as
 parameter letting the user to decide what value should be returned by
 findFirst if there is a value and if there is no value.
 So instead of
   stream.findFirst().orElse(null)
 you will write
   stream.findFirst(orNull)
 with orNull() defined as like that
   public static T Optionalizer orNull() {
 return (isPresent, element) - isPresent? element: null;
   }
 
 The whole design is explained here [2] and is similar to the way
 Collectors are defined [3],
 it's basically the lambda way of thinking, instead of creating an object
 representing the different states resulting of a call to findFirst,
 findFirst takes a lambda as parameter which is fed with the states of a
 call.
 
 cheers,
 Rémi
 
 [1] http://www.scala-lang.org/api/current/index.html#scala.Option
 [2]
 http://mail.openjdk.java.net/pipermail/lambda-libs-spec-observers/2013-February/001470.html
 [3]
 http://hg.openjdk.java.net/lambda/lambda/jdk/file/tip/src/share/classes/java/util/stream/Collectors.java
 
 
 On 03/04/2013 09:29 PM, Mike Duigou wrote:
 Hello All;
 
 This patch introduces Optional container objects to be used by the lambda 
 streams libraries for returning results.
 
 The reference Optional type, as defined, intentionally does not allow null 
 values. null may be used with the Optional.orElse() method.
 
 All of the Optional types define hashCode() and equals implementations. 
 Use of Optional types 

Re: Request for review- RFE 8005716

2013-03-06 Thread Mike Duigou
Hi Bill;

Some notes from reviewing the JDK side changes.


System.java/Runtime.java:

The example which begins with the name If the filename argument,  needs to 
better identify that L is an example. (Italics?)

java/lang/ClassLoader.java:

NativeLibrary::fromClass could be final.


ClassLoader.c:

In Java_java_lang_ClassLoader_00024NativeLibrary_findBuiltinLib()

These two values are known at compile time. 

int prefixLen = (int) strlen(JNI_LIB_PREFIX); 
int suffixLen = (int) strlen(JNI_LIB_SUFFIX);

Some of the error conditions don't throw exceptions. Such as:

if (cname == NULL) {
   return NULL;
   }

The prefix and suffix are stripped from cname without checking that cname 
actually contains the prefix or suffix.

if (len  prefixLen) is invariant.

src/solaris/native/common/jni_util_md.c:

void* getProcessHandle() {
 static void* procHandle = NULL;
 if (procHandle == NULL) {
procHandle = (void*)dlopen(NULL, RTLD_LAZY);
 }

 return procHandle;
}

Why is the error handling code commented out?

Mike


On Mar 5 2013, at 15:05 , bill.pitt...@oracle.com wrote:

 This request is tied to bugid 8005716 that deals with adding support for 
 statically linked JNI libraries.
 
 The JEP is here: http://openjdk.java.net/jeps/178
 
 The bug is here:http://bugs.sun.com/view_bug.do?bug_id=8005716
 
 The webrevs are here:
 
 http://cr.openjdk.java.net/~bpittore/8005716/jdk-webrev.00/
 http://cr.openjdk.java.net/~bpittore/8005716/hs-webrev.00/
 
 The main piece of functionality is to check for the presence of 
 JNI_OnLoad_libname to determine if the library specified by 'libname' has 
 been statically linked into the VM. If the symbol is found, it is assumed 
 that the library is linked in and will not be dynamically loaded.
 
 thanks,
 bill



Re: RFR : JDK-8001642 : Add OptionalT, OptionalDouble, OptionalInt, OptionalLong

2013-03-06 Thread Jed Wesley-Smith
Really, this is a lot of fuss over nothing.

There is actually no fundamental difference between Scala's Option, Guava's 
Optional, Fugue's Option, Java's Optional and Haskell's Maybe – they are 
modelling the same thing, the possibility of a value not being present.

The fact that there may be minor differences in api or semantics around whether 
null is a legal value are minor in the scheme of things (and yes, null is a 
pretty stupid legal value of a Some IMHO).

Stephen's example is ludicrous, why have a list of optional values? You'd 
flatten down into just a list – and an optional list only makes sense if the 
enclosed list is guaranteed to be non-empty, otherwise you just return an empty 
list!

If we are going to use potential straw-men as arguments we can stall all 
progress. Please concentrate on the important matters, let's disavow null as a 
valid value and save us all a billion dollars.

cheers,
jed.

On 06/03/2013, at 8:47 PM, Remi Forax fo...@univ-mlv.fr wrote:

 Ok, let be nuclear on this,
 There is no good reason to introduce OptionalT in java.util.
 
 It doen't work like Google's Guava Optional despite having the same 
 name, it doesn't work like Scala's Option despite having a similar name, 
 moreover the lambda pipeline face a similar issue with the design of 
 collectors (see stream.collect()) but solve that similar problem with a 
 different design, so the design of Optional is not even consistent with 
 the rest of the stream API.
 
 So why do we want something like Optional, we want it to be able to 
 represent the fact that as Mike states a returning result can have no 
 value by example Colections.emptyList().stream().findFirst() should 
 'return' no value.
 
 As Stephen Colebourne said, Optional is a bad name because Scala uses 
 Option [1] which can used in the same context, as result of a filter/map 
 etc. but Option in Scala is a way to mask null. Given the name 
 proximity, people will start to use Optional like Option in Scala and we 
 will see methods returning things like OptionalListOptionalString.
 
 Google's Guava, which is a popular library, defines a class named 
 Optional, but allow to store null unlike the current proposed 
 implementation, this will generate a lot of confusions and frustrations.
 
 In fact, we don't need Optional at all, because we don't need to return 
 a value that can represent a value or no value,
 the idea is that methods like findFirst should take a lambda as 
 parameter letting the user to decide what value should be returned by 
 findFirst if there is a value and if there is no value.
 So instead of
   stream.findFirst().orElse(null)
 you will write
   stream.findFirst(orNull)
 with orNull() defined as like that
   public static T Optionalizer orNull() {
 return (isPresent, element) - isPresent? element: null;
   }
 
 The whole design is explained here [2] and is similar to the way 
 Collectors are defined [3],
 it's basically the lambda way of thinking, instead of creating an object 
 representing the different states resulting of a call to findFirst,
 findFirst takes a lambda as parameter which is fed with the states of a 
 call.
 
 cheers,
 Rémi
 
 [1] http://www.scala-lang.org/api/current/index.html#scala.Option
 [2] 
 http://mail.openjdk.java.net/pipermail/lambda-libs-spec-observers/2013-February/001470.html
 [3] 
 http://hg.openjdk.java.net/lambda/lambda/jdk/file/tip/src/share/classes/java/util/stream/Collectors.java
 
 
 On 03/04/2013 09:29 PM, Mike Duigou wrote:
 Hello All;
 
 This patch introduces Optional container objects to be used by the lambda 
 streams libraries for returning results.
 
 The reference Optional type, as defined, intentionally does not allow null 
 values. null may be used with the Optional.orElse() method.
 
 All of the Optional types define hashCode() and equals implementations. Use 
 of Optional types in collections should be generally discouraged but having 
 useful equals() and hashCode() is ever so convenient.
 
 http://cr.openjdk.java.net/~mduigou/JDK-8001642/0/webrev/
 
 Mike
 
 
 


Re: CFR - updated 8001667: Comparator combinators and extension methods

2013-03-06 Thread Michael Hixson
Hello,

I'm not one of the people that you're looking at to review this, but I
have to give this feedback anyway.  I tried to give similar feedback
on another mailing list and got no response (maybe I chose the wrong
list).  These changes have been bothering me. :)

1. Why disable the following code even though it is (theoretically) safe?

  ComparatorCharSequence a = comparing(CharSequence::length);
  ComparatorString b = a.thenComparing(CASE_INSENSITIVE_ORDER);

That code would compile if the signatures of the thenComparing methods
had generics like S extends T instead of T.  The example above is
conjured up and ridiculous, but I think real code will have use for
it.  Is there any downside to narrowing the return type this way?

2. There's a thenComparing(Function), but no thenComparing(Function,
Comparator).  This seems like a big omission.  Surely people will want
secondary orderings on fields by something other than natural
(null-unfriendly) ordering.  Also, a Comparators.comparing(Function,
Comparator) method would remove the need for all the Map-centric
methods that are currently in the Comparators class.  For instance:
comparing(Map.Entry::getValue, naturalOrder()).

3. There is no support for sorting of nullable values or fields.
Sorting of nullable values directly perhaps should not be encouraged,
but sorting values by nullable fields within a chained sort is
completely reasonable.  Please add some support for this, either as
chain methods on Comparator, or as factory methods on Comparators.

4. If Comparator had min(a,b) and max(a,b) methods, the
Comparators.lesserOf(c) and greaterOf(c) methods would be unnecessary.
 The min/max methods would be generally more useful than the
BinaryOperators returned from Comparators, and c::min reads better
than Comparators.lesserOf(c).

5. Comparators.reverseOrder(c) is confusing in that it has different
behavior than Collections.reverseOrder(c).  It throws an NPE.  Also,
this new method seems to be useless because one could call
c.reverseOrder() instead.  I suggest removing the method.

6. I don't see why Comparators.compose(c1,c2) is useful given that
users can call c1.thenComparing(c2).  The latter reads better; the
word compose does not naturally fit with comparators and has strange
connotations for those with Math backgrounds.

7. Last I checked, even this simple example did not compile:

  ComparatorString c = comparing(String::length);

It was confused about whether I was implying a ToDoubleFunction or a
ToLongFunction, which were both wrong.  I also ran into a lot of type
inference loop problems when chaining.

Is this simply a problem with lambda evaluation that's going to be
fixed before Java 8 is officially released?  Is there something more
complex going on here that makes statements like this impossible?

If the compilation problems aren't going to be fixed prior to the
release, or if they cannot be fixed, then none of these
Comparator/Comparators additions are useful.  You would be better off
removing them.

-Michael

On Tue, Mar 5, 2013 at 11:46 AM, Henry Jen henry@oracle.com wrote:
 Hi,

 Another update to reflect functional interface renames involved in the
 API, and a bug fix for a regression found earlier.

 CCC had been approved. Can we get it reviewed and pushed?

 [1] http://cr.openjdk.java.net/~henryjen/ccc/8001667.4/webrev

 Cheers,
 Henry



Re: CFR - updated 8001667: Comparator combinators and extension methods

2013-03-06 Thread Ali Ebrahimi
Hi,
just one suggestion:

rename comparing with compareWith

1)

public static T, U extends Comparable? super U ComparatorT
compareWith(Function? super T, ? extends U keyExtractor) {


2)
default ComparatorT thenCompareWith(Comparator? super T other)



Best Regards,
Ali Ebrahimi
On Wed, Mar 6, 2013 at 12:16 AM, Henry Jen henry@oracle.com wrote:

 Hi,

 Another update to reflect functional interface renames involved in the
 API, and a bug fix for a regression found earlier.

 CCC had been approved. Can we get it reviewed and pushed?

 [1] http://cr.openjdk.java.net/~henryjen/ccc/8001667.4/webrev

 Cheers,
 Henry




Re: Request for review- RFE 8005716

2013-03-06 Thread Dean Long
If JNI_ONLOAD_SYMBOLS contains something like _JNI_OnLoad@8 on 
Windows, you can't just

turn that into _JNI_OnLoad@8_ + libname.  I think it needs to be
_JNI_OnLoad_  + libname + @8

Looks like onLoadSymbols[] is unused in 
Java_java_lang_ClassLoader_00024NativeLibrary_findBuiltinLib().


Instead of adding getProcessHandle(), why not add 
JVM_FindBuiltinLibraryEntry() instead?
This would make it easier to support table-lookup when runtime symbol 
information is missing or not

supported by the platform.

dl

On 3/5/2013 3:05 PM, bill.pitt...@oracle.com wrote:
This request is tied to bugid 8005716 that deals with adding support 
for statically linked JNI libraries.


The JEP is here: http://openjdk.java.net/jeps/178

The bug is here:http://bugs.sun.com/view_bug.do?bug_id=8005716

The webrevs are here:

http://cr.openjdk.java.net/~bpittore/8005716/jdk-webrev.00/
http://cr.openjdk.java.net/~bpittore/8005716/hs-webrev.00/

The main piece of functionality is to check for the presence of 
JNI_OnLoad_libname to determine if the library specified by 'libname' 
has been statically linked into the VM. If the symbol is found, it is 
assumed that the library is linked in and will not be dynamically loaded.


thanks,
bill




RE: JDK 8 RFR: 8009267: Restore isAnnotationPresent methods in public AnnotatedElement implementations

2013-03-06 Thread Uwe Schindler
Looks good to me!
Are this the only methods in the corelib that were removed in favour of a 
default implementation in the interface?

Uwe

-
Uwe Schindler
uschind...@apache.org 
Apache Lucene PMC Member / Committer
Bremen, Germany
http://lucene.apache.org/

 -Original Message-
 From: Joe Darcy [mailto:joe.da...@oracle.com]
 Sent: Saturday, March 02, 2013 3:06 AM
 To: Core-Libs-Dev
 Cc: Jonathan Gibbons; Alexander Buckley; Maurizio Cimadamore;
 uschind...@apache.org
 Subject: JDK 8 RFR: 8009267: Restore isAnnotationPresent methods in public
 AnnotatedElement implementations
 
 Hello,
 
 The changes pushed under
 
  8007113: Upgrade AnnotatedElement.isAnnotionPresent to be a default
 method
  http://hg.openjdk.java.net/jdk8/tl/jdk/rev/e04467fa13af
 
 combined with how javac currently models default methods under pre-JDK-8
 source versions can result in some source compatibility impacts for those
 using JDK 8 to compile under older source versions if the recommended
 practice of setting the bootclasspath is not followed. [1]
 
 To mitigate these impacts, I'm proposing the changes in
 
  8009267: Restore isAnnotationPresent methods in public
 AnnotatedElement implementations
  http://cr.openjdk.java.net/~darcy/8009267.1/
 
 which have the effect of restoring a concrete isAnnotationPresent method
 to Class, Package, Field, Method, and Constructor. The implementation
 delegates to the default method on the AnnotatedElement interface.
 
 Thanks,
 
 -Joe
 
 [1]
 http://mail.openjdk.java.net/pipermail/compiler-dev/2013-
 February/005738.html



hg: jdk8/tl: 8009019: Updates to generated-configure.sh required for 8008914

2013-03-06 Thread tim . bell
Changeset: cb0ac8979caa
Author:tbell
Date:  2013-02-26 09:25 -0800
URL:   http://hg.openjdk.java.net/jdk8/tl/rev/cb0ac8979caa

8009019: Updates to generated-configure.sh required for 8008914
Reviewed-by: sundar, jlaskey, jjg

! common/autoconf/generated-configure.sh



Re: RFR: 8007808: Missing method: Executable.getAnnotatedReturnType()

2013-03-06 Thread Joel Borggrén-Franck
Thanks for the reviews Joe and Remi

cheers
/Joel

On 6 mar 2013, at 17:15, Remi Forax fo...@univ-mlv.fr wrote:

 Hi Joel,
 looks good for me too :)
 
 Rémi
 
 On 03/06/2013 04:35 PM, Joe Darcy wrote:
 Hi Joel,
 
 Look good; approved.
 
 -Joe
 
 On 3/6/2013 6:37 AM, Joel Borggrén-Franck wrote:
 Hi,
 
 Can I get a review of this small fix for issue 8007808: Missing method: 
 Executable.getAnnotatedReturnType()
 
 http://cr.openjdk.java.net/~jfranck/8007808/webrev.00/
 
 When we added Core Reflection support for type annotations this method got 
 left out on Executable, but were included on Method and Constructor.
 
 For Oracle reviewers, CCC is approved.
 
 cheers
 /Joel
 
 



Re: Review: 7032154 - Performance tuning of sun.misc.FloatingDecimal/FormattedFloatingDecimal

2013-03-06 Thread Brian Burkhalter
The link below has been updated with a few minor changes, notably to use 
constants from {Double,Float}Consts and to include the link to the OpenJDK 
issue report. A formatting issue resulting from an awk failure during webrev 
script execution was also fixed.

B.

On Feb 28, 2013, at 1:34 PM, Brian Burkhalter wrote:

 I forgot that the attachment is not redistributed and that I can now post on 
 cr.openjdk.java.net so here's the webrev:
 
 http://cr.openjdk.java.net/~bpb/7032154/
 
 Begin forwarded message:
 
 This concerns the issue 
 http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7032154.
 
 The patch described in the issue has been updated to be with respect to the 
 current JDK repository tip. The updated version is attached to this message.



Re: RFR(S): 8009397 test/com/sun/jdi/PrivateTransportTest.sh: ERROR: transport library missing onLoad entry: private_dt_socket

2013-03-06 Thread Staffan Larsen
Nice catch, Serguei!

Unfortunately I have already pushed my change, but I filed JDK-8009558: 
linked_md.c::dll_build_name can get stuck in an infinite loop to track this 
new problem and I am working on a fix.

Thanks,
/Staffan


On 5 mar 2013, at 20:26, serguei.spit...@oracle.com wrote:

 Hi Staffan,
 
 Thank you for this discovery!
 It looks good, but I have a couple of comments.
 
 It seems, there is one more problem in this code:
   68 /* check for NULL path */
   69 if (p == pathname) {
   70 continue;   == Endless loop if we hit this line
   71 }
 
 Do we need to do 'pathname++' before continuing at the line #70?
 It is going to be endless loop in cases there is a PATH_SEPARATOR
 at the beginning of paths or two PATH_SEPARATOR's in a row.
 These would be incorrect path lists but the code above is targeting exactly 
 such cases.
 
 Also, the argument name pathname in the original code is confusing.
 Should we rename it to pathnames?
 
 Thanks,
 Serguei
 
 
 On 3/4/13 10:56 AM, Staffan Larsen wrote:
 I accidentally stepped on this bug the other day. There is a problem in 
 linker_md.c : dll_build_name() where an internal pointer can be moved to 
 point outside the string. The code looks like:
 
   57 static void dll_build_name(char* buffer, size_t buflen,
   58const char* pname, const char* fname) {
   59 // Based on os_solaris.cpp
   60 
   61 char *path_sep = PATH_SEPARATOR;
   62 char *pathname = (char *)pname;
   63 while (strlen(pathname)  0) {
   64 char *p = strchr(pathname, *path_sep);
   65 if (p == NULL) {
   66 p = pathname + strlen(pathname);
   67 }
   68 /* check for NULL path */
   69 if (p == pathname) {
   70 continue;
   71 }
   72 (void)snprintf(buffer, buflen, %.*s/lib%s. LIB_SUFFIX, (p - 
 pathname),
   73pathname, fname);
   74 
   75 if (access(buffer, F_OK) == 0) {
   76 break;
   77 }
   78 pathname = p + 1;
   79 *buffer = '\0';
   80 }
 
 If the supplied pname is a buffer with a simple path without any path 
 separators in it, p will be set to the terminating nul on line 66. Then on 
 line 78 it will be moved outside the buffer. Fixing this also necessitates 
 fixes to the callers to check for an empty return string (in buffer).
 
 The same code show up in both the solaris code and the windows code as well 
 as hprof. 
 
 bug: http://bugs.sun.com/view_bug.do?bug_id=8009397
 webrev: http://cr.openjdk.java.net/~sla/8009397/webrev.00/
 
 Thanks,
 /Staffan
 



Re: RFR(S): 8009397 test/com/sun/jdi/PrivateTransportTest.sh: ERROR: transport library missing onLoad entry: private_dt_socket

2013-03-06 Thread serguei.spit...@oracle.com

Staffan,

Thank you for the confirmation and taking care about this issue!

Thanks,
Serguei


On 3/6/13 11:36 AM, Staffan Larsen wrote:

Nice catch, Serguei!

Unfortunately I have already pushed my change, but I filed 
JDK-8009558: linked_md.c::dll_build_name can get stuck in an infinite 
loop to track this new problem and I am working on a fix.


Thanks,
/Staffan


On 5 mar 2013, at 20:26, serguei.spit...@oracle.com 
mailto:serguei.spit...@oracle.com wrote:



Hi Staffan,

Thank you for this discovery!
It looks good, but I have a couple of comments.

It seems, there is one more problem in this code:
   68 /* check for NULL path */
   69 if (p == pathname) {
   70 continue;== Endless loop if we hit this line
   71 }

Do we need to do 'pathname++' before continuing at the line #70?
It is going to be endless loop in cases there is a PATH_SEPARATOR
at the beginning of paths or two PATH_SEPARATOR's in a row.
These would be incorrect path lists but the code above is targeting 
exactly such cases.


Also, the argument name pathname in the original code is confusing.
Should we rename it to pathnames?

Thanks,
Serguei


On 3/4/13 10:56 AM, Staffan Larsen wrote:

I accidentally stepped on this bug the other day. There is a problem in 
linker_md.c : dll_build_name() where an internal pointer can be moved to point 
outside the string. The code looks like:

   57 static void dll_build_name(char* buffer, size_t buflen,
   58const char* pname, const char* fname) {
   59 // Based on os_solaris.cpp
   60
   61 char *path_sep = PATH_SEPARATOR;
   62 char *pathname = (char *)pname;
   63 while (strlen(pathname)  0) {
   64 char *p = strchr(pathname, *path_sep);
   65 if (p == NULL) {
   66 p = pathname + strlen(pathname);
   67 }
   68 /* check for NULL path */
   69 if (p == pathname) {
   70 continue;
   71 }
   72 (void)snprintf(buffer, buflen, %.*s/lib%s. LIB_SUFFIX, (p - 
pathname),
   73pathname, fname);
   74
   75 if (access(buffer, F_OK) == 0) {
   76 break;
   77 }
   78 pathname = p + 1;
   79 *buffer = '\0';
   80 }

If the supplied pname is a buffer with a simple path without any path 
separators in it, p will be set to the terminating nul on line 66. Then on line 
78 it will be moved outside the buffer. Fixing this also necessitates fixes to 
the callers to check for an empty return string (in buffer).

The same code show up in both the solaris code and the windows code as well as 
hprof.

bug:http://bugs.sun.com/view_bug.do?bug_id=8009397
webrev:http://cr.openjdk.java.net/~sla/8009397/webrev.00/

Thanks,
/Staffan








Re: RFR: 8002070 Remove the stack search for a resource bundle for Logger to use

2013-03-06 Thread Jim Gish


On 03/06/2013 12:58 PM, Mandy Chung wrote:



On 3/5/2013 9:16 AM, Jim Gish wrote:


On 03/01/2013 05:48 PM, Mandy Chung wrote:

On 3/1/2013 1:25 PM, Jim Gish wrote:
Please review 
http://cr.openjdk.java.net/~jgish/Bug8002070-RemoveResourceBundleStackSearch/ 


This removes the stack search from Logger.findResourceBundle()



It's good to see this stack walk search of resource bundle going away.

In Logger.java, the existing implementation returns the previous 
cached resource bundle if it fails to find one matching the current 
locale but the name matches:


1608 if (name.equals(catalogName)) {
1609 // Return the previous cached value for that name.
1610 // This may be null.
1611 return catalog;
1612 }


Your fix removes these lines which I think is fine.  The 
Logger.getResourceBundle method specifies to return the resource 
bundle for this logger for the current default locale.  I think it'd 
be rare to change the default locale during the execution of an 
application.

The old behavior upon reaching this point in the code was as follows:
To get here, the sought after bundle was not found and (from the 
checks on line 1559,1560), either
(1) the previously cached catalog was null, meaning nothing was yet 
cached and so null was returned, or


correct.

(2) the current locale had changed and no bundle for that locale was 
found, in which case the cached bundle (for the old/wrong locale) was 
returned, or


do you mean it returns the cached bundle only if the name matches?  or 
am I missing the code that you read?

Yes.


(3) the name was the same but the location of the objects used to 
compare the cached name with that passed in was different, 


Can you elaborate?  Are you referring the if 
(name.equals(catalogName)) statement?
It used to be:  if (name == catalogName) and now is if 
(name.equals(catalogName))  Therefore if a different object was passed 
in for name than catalogName, that test would fail.  However, even  if 
name.equals(catalogName) was initially true in the old code it could 
proceed to do the search (possibly unnecessarily in the case where it 
would simply return the cached bundle anyway), but even if the search 
failed it would discover the String value of the name was equivalent to 
the String value of catalogName and then and only then return the cached 
catalog.  Now, we simply check for matching String values at the 
beginning and return the cached catalog if the Locale hasn't changed.


so the previously cached bundle was returned (this is frankly an odd 
case, because it also means that re-searching for a previously found 
bundle is now failing, which only seems possible if the bundle is/was 
(a) not available via the system classloader, (b) the context 
classloader, or (c), the classloaders up the call chain.


The new behavior /does /still allow for a change in the default 
locale.  For example, if you first call findResourceBundleName(foo) 
and the default locale is US, it will search for the foo_US bundle 
and set catalogLocale=US, and catalogName=foo and return the foo_US 
bundle.  If you then search for foo and have changed the default 
locale to DE (if that is indeed a valid locale), it will then search 
for foo_DE and if found set catalogLocale=DE and cache and return the 
foo_DE bundle.   The /only /change in behavior that I see here, is 
that if you change the locale and the bundle is not found, null will 
be returned instead of the previously cached bundle (if one exists), 
for a /different/ locale.




Right.   The behavioral difference that I point out was when foo_DE is 
cached, later the current locale is set to JP where foo_JP and foo 
don't exist, the old behavior returns foo_DE which was a bug too me 
whereas the new behavior returns null which matches the spec of 
Logger.getResourceBundle().


So, the old behavior in effect had a notion of a default catalog, but 
only if you happened to find a bundle on a previous lookup and cache 
it.  However, you wouldn't be guaranteed of acquiring this same 
bundle on a subsequent search if a search for a different name had 
intervened.


Thus, the new behavior is that if you search for a name/locale and 
it's not found, you will get null (every time). This seems better to 
me because it's consistent, explainable and simple.




This matches what Logger.getResourceBundle() is specified and the 
behavior is correct.  The difference in behavior is a minimal 
compatibility risk.


Comments on the updated webrev:

http://cr.openjdk.java.net/~jgish/Bug8002070-RemoveResourceBundleStackSearch/ 



L122-125: you may want to replace code/code with {@code  }
I think we'll save that for a later day - and then update all the 
occurrences of code to {@code}

L129-130: you can use @linkplain instead:
 {@linkplain ClassLoader#getSystemClassLoader()system ClassLoader}
All other existing links are using @link.  Why should we use @linkplain 
here?
L141-142: the bundle 

Re: CFR - updated 8001667: Comparator combinators and extension methods

2013-03-06 Thread Henry Jen
On 03/06/2013 03:28 AM, Ali Ebrahimi wrote:
 Hi,
 just one suggestion:
 
 rename comparing with compareWith
 

There was a round of discussion on naming.

http://mail.openjdk.java.net/pipermail/lambda-libs-spec-observers/2012-November/000446.html

I have my personal preference among proposals, but EG seems to have come
to a consensus on this.

I don't feel strongly between thenCompare, thenComparing or
thenCompareWith. But we should be consistent between Comparators and
Comparator, and consider that Comparators methods could be static
interface method on Comparator in the future.

Cheers,
Henry


 1)
 
 public static T, U extends Comparable? super U ComparatorT 
 compareWith(Function? super T, ? extends U keyExtractor) {
 
 
 2)
 default ComparatorT thenCompareWith(Comparator? super T other)
 
 
 
 Best Regards,
 Ali Ebrahimi
 On Wed, Mar 6, 2013 at 12:16 AM, Henry Jen henry@oracle.com
 mailto:henry@oracle.com wrote:
 
 Hi,
 
 Another update to reflect functional interface renames involved in the
 API, and a bug fix for a regression found earlier.
 
 CCC had been approved. Can we get it reviewed and pushed?
 
 [1] http://cr.openjdk.java.net/~henryjen/ccc/8001667.4/webrev
 
 Cheers,
 Henry
 
 



Re: Request for review- RFE 8005716

2013-03-06 Thread BILL PITTORE
Actually for windows I *did* export the undecorated name. I just didn't 
see where I did it in the VS IDE. If you don't export the undecorated 
name it doesn't work of course.


bill

On 3/5/2013 11:13 PM, BILL PITTORE wrote:

On 3/5/2013 7:36 PM, Dean Long wrote:
If JNI_ONLOAD_SYMBOLS contains something like _JNI_OnLoad@8 on 
Windows, you can't just

turn that into _JNI_OnLoad@8_ + libname.  I think it needs to be
_JNI_OnLoad_  + libname + @8
I'll look into that. When I built for windows and ran our test, the 
JNI_OnLoad_TestStaticLib was exported without the decoration just 
based on the JNIEXPORT JNICALL specifiers on the function. I didn't do 
anything special to export it. But I recall this problem from another 
project.
10 1014 JNI_OnLoad_TestStaticLib = 
@ILT+15(?JNI_OnLoad_TestStaticLib@@YGHPAUJavaVM_@@PAX@Z)


Looks like onLoadSymbols[] is unused in 
Java_java_lang_ClassLoader_00024NativeLibrary_findBuiltinLib().
I'll remove that. I redid the call to findBuiltinJniFunction but 
forgot to remove that.




Instead of adding getProcessHandle(), why not add 
JVM_FindBuiltinLibraryEntry() instead?
This would make it easier to support table-lookup when runtime symbol 
information is missing or not

supported by the platform.
Not sure I'm following you on this. Make JVM_FindBuiltinLibraryEntry() 
an exported function in the VM? How does it find JNI_OnLoad_L? Via a 
table setup by the developer/build system when the library is linked in?


bill



dl

On 3/5/2013 3:05 PM, bill.pitt...@oracle.com wrote:
This request is tied to bugid 8005716 that deals with adding support 
for statically linked JNI libraries.


The JEP is here: http://openjdk.java.net/jeps/178

The bug is here:http://bugs.sun.com/view_bug.do?bug_id=8005716

The webrevs are here:

http://cr.openjdk.java.net/~bpittore/8005716/jdk-webrev.00/
http://cr.openjdk.java.net/~bpittore/8005716/hs-webrev.00/

The main piece of functionality is to check for the presence of 
JNI_OnLoad_libname to determine if the library specified by 
'libname' has been statically linked into the VM. If the symbol is 
found, it is assumed that the library is linked in and will not be 
dynamically loaded.


thanks,
bill









Java regex vs. Unicode TR#18 vs. ICU

2013-03-06 Thread Steven R. Loomis

Hello,
 Someone on the ICU team recently compared the use of \w between ICU, 
Java, and Unicode TR#18

http://www.unicode.org/reports/tr18/#Compatibility_Properties .
The results are in the following ICU bug 
http://bugs.icu-project.org/trac/ticket/10006.


A question for core-libs-dev is, does Java plan to change the semantics 
of \w to match TR#18's list?


Thanks,
Steven



Re: CFR - updated 8001667: Comparator combinators and extension methods

2013-03-06 Thread Henry Jen
On 03/06/2013 02:31 AM, Michael Hixson wrote:
 Hello,
 
 I'm not one of the people that you're looking at to review this, but I
 have to give this feedback anyway.  I tried to give similar feedback
 on another mailing list and got no response (maybe I chose the wrong
 list).  These changes have been bothering me. :)
 

I find your earlier posts on lambda-libs-spec-comments archive. I was
not on that list.

 1. Why disable the following code even though it is (theoretically) safe?
 
   ComparatorCharSequence a = comparing(CharSequence::length);
   ComparatorString b = a.thenComparing(CASE_INSENSITIVE_ORDER);
 
 That code would compile if the signatures of the thenComparing methods
 had generics like S extends T instead of T.  The example above is
 conjured up and ridiculous, but I think real code will have use for
 it.  Is there any downside to narrowing the return type this way?
 

I think that make sense, will need to do some experiment to make sure it
won't confuse type system when chaining all together, and I am not sure
how much burden this has on inference engine. I prefer simplicity.

Theoretically, if all function take Comparator carefully allows super
type, which we have, isn't that enough? Your above example can be,

ComparatorString a = comparingCharSequence::length);
a.thenComparing(CASE_INSENSITIVE_ORDER);


 2. There's a thenComparing(Function), but no thenComparing(Function,
 Comparator).  This seems like a big omission.  Surely people will want
 secondary orderings on fields by something other than natural
 (null-unfriendly) ordering.  Also, a Comparators.comparing(Function,
 Comparator) method would remove the need for all the Map-centric
 methods that are currently in the Comparators class.  For instance:
 comparing(Map.Entry::getValue, naturalOrder()).
 

Note that Function form must return a Comparable, which implies an order
already. Combine with Comparator.reverseOrder() method, that would cover
the ground.

If the Comparable is not a fit, probably write a Comparator in lambda is
needed anyway?

 3. There is no support for sorting of nullable values or fields.
 Sorting of nullable values directly perhaps should not be encouraged,
 but sorting values by nullable fields within a chained sort is
 completely reasonable.  Please add some support for this, either as
 chain methods on Comparator, or as factory methods on Comparators.
 

Not sure what do you propose to be added. NULL is a controversial topic,
only application know what it means. Therefore, avoid try to interpret
null is probably a better approach. Write a Comparator if needed.

 4. If Comparator had min(a,b) and max(a,b) methods, the
 Comparators.lesserOf(c) and greaterOf(c) methods would be unnecessary.
  The min/max methods would be generally more useful than the
 BinaryOperators returned from Comparators, and c::min reads better
 than Comparators.lesserOf(c).
 

+1.

 5. Comparators.reverseOrder(c) is confusing in that it has different
 behavior than Collections.reverseOrder(c).  It throws an NPE.  Also,
 this new method seems to be useless because one could call
 c.reverseOrder() instead.  I suggest removing the method.


I agree.

 6. I don't see why Comparators.compose(c1,c2) is useful given that
 users can call c1.thenComparing(c2).  The latter reads better; the
 word compose does not naturally fit with comparators and has strange
 connotations for those with Math backgrounds.
 
 7. Last I checked, even this simple example did not compile:
 
   ComparatorString c = comparing(String::length);
 
 It was confused about whether I was implying a ToDoubleFunction or a
 ToLongFunction, which were both wrong.  I also ran into a lot of type
 inference loop problems when chaining.
 
 Is this simply a problem with lambda evaluation that's going to be
 fixed before Java 8 is officially released?  Is there something more
 complex going on here that makes statements like this impossible?
 
 If the compilation problems aren't going to be fixed prior to the
 release, or if they cannot be fixed, then none of these
 Comparator/Comparators additions are useful.  You would be better off
 removing them.
 

My hope is this to be fixed.

Cheers,
Henry



Re: RFR: 8002070 Remove the stack search for a resource bundle for Logger to use

2013-03-06 Thread Mandy Chung

On 3/6/2013 11:42 AM, Jim Gish wrote:
http://cr.openjdk.java.net/~jgish/Bug8002070-RemoveResourceBundleStackSearch/ 



L122-125: you may want to replace code/code with {@code  }
I think we'll save that for a later day - and then update all the 
occurrences of code to {@code}


Fine with me.


L129-130: you can use @linkplain instead:
 {@linkplain ClassLoader#getSystemClassLoader()system ClassLoader}
All other existing links are using @link.  Why should we use 
@linkplain here?


The javadoc generated in your current change will show:
system ClassLoader, a 
href=ClassLoader.getSystemClassLoader()/a,


If you use @linkplain, the javadoc will show:
a href=system ClassLoader/a



LoggerResourceBundleRace.java: I think what you really want is to add 
a new test that sets a context class loader to a class loader that 
finds the resource bundle for a logger that a system class loader 
can't find.   I suggest to leave this test as it is and then add a 
new test to exercise the context class loader search of a resource 
bundle as a separate RFE that will improve the test coverage.
Leaving the existing test as is not an option unless we change the 
test to run in othervm as I had on my first webrev.  The bundles are 
not found otherwise.  Hence the change to set the context classloader.


I'm confused.  Is the resource bundle 
LoggerResourceBundleRace$MyResources in the classpath as this test 
class is located?  Should the system class loader be able to find them? 
This test shouldn't depend on the stack search.  Shouldn't  the thread 
context classloader be null and then use the system class loader?  Is 
another test set the thread context classloader in the jtreg run? Does 
the test pass if you run it directly (not via jtreg)?


Mandy


Re: Java regex vs. Unicode TR#18 vs. ICU

2013-03-06 Thread Xueming Shen

On 03/06/2013 12:44 PM, Steven R. Loomis wrote:

Hello,
 Someone on the ICU team recently compared the use of \w between ICU, Java, 
and Unicode TR#18
http://www.unicode.org/reports/tr18/#Compatibility_Properties .
The results are in the following ICU bug 
http://bugs.icu-project.org/trac/ticket/10006.

A question for core-libs-dev is, does Java plan to change the semantics of \w 
to match TR#18's list?


It appears the standard has just added one more entry \p{Join_Control}
during their last update :-( I may consider to update the spec/impl to match
that, I would assume there is no any jdk7 application really has dependency
on the updated \w (in jdk7).

-Sherman




Re: Java regex vs. Unicode TR#18 vs. ICU

2013-03-06 Thread Steven R. Loomis

On 3/6/13 1:06 PM, Xueming Shen wrote:

On 03/06/2013 12:44 PM, Steven R. Loomis wrote:

Hello,
 Someone on the ICU team recently compared the use of \w between 
ICU, Java, and Unicode TR#18

http://www.unicode.org/reports/tr18/#Compatibility_Properties .
The results are in the following ICU bug 
http://bugs.icu-project.org/trac/ticket/10006.


A question for core-libs-dev is, does Java plan to change the 
semantics of \w to match TR#18's list?


It appears the standard has just added one more entry \p{Join_Control}
during their last update :-( I may consider to update the spec/impl to 
match
that, I would assume there is no any jdk7 application really has 
dependency

on the updated \w (in jdk7).

-Sherman



Thanks, Sherman

Do you want to open a bug to track this? You can reference the above URLs

Steven



Re: Request for review- RFE 8005716

2013-03-06 Thread BILL PITTORE

On 3/6/2013 12:50 PM, Mike Duigou wrote:

Hi Bill;

Some notes from reviewing the JDK side changes.


System.java/Runtime.java:

The example which begins with the name If the filename argument,  needs to better 
identify that L is an example. (Italics?)

Re-worded that a bit.

java/lang/ClassLoader.java:

NativeLibrary::fromClass could be final.

Ok.


ClassLoader.c:

In Java_java_lang_ClassLoader_00024NativeLibrary_findBuiltinLib()

These two values are known at compile time.

int prefixLen = (int) strlen(JNI_LIB_PREFIX);
int suffixLen = (int) strlen(JNI_LIB_SUFFIX);

Some of the error conditions don't throw exceptions. Such as:

if (cname == NULL) {
return NULL;
}

Fixed.

The prefix and suffix are stripped from cname without checking that cname 
actually contains the prefix or suffix.
..._findBuiltinLib is only called after System.mapLibraryName has added 
prefix and suffix.


if (len  prefixLen) is invariant.
Based on System.mapLibraryName always called, then this could just be an 
assert. Unless JNU_GetStringPlatformChars returns some bogus string.


src/solaris/native/common/jni_util_md.c:

void* getProcessHandle() {
  static void* procHandle = NULL;
  if (procHandle == NULL) {
 procHandle = (void*)dlopen(NULL, RTLD_LAZY);
  }

  return procHandle;
}

Why is the error handling code commented out?

That was just for some debugging. I've removed it.

Thanks for commenting. I'll get a new webrev up shortly.

bill




Mike


On Mar 5 2013, at 15:05 , bill.pitt...@oracle.com wrote:


This request is tied to bugid 8005716 that deals with adding support for 
statically linked JNI libraries.

The JEP is here: http://openjdk.java.net/jeps/178

The bug is here:http://bugs.sun.com/view_bug.do?bug_id=8005716

The webrevs are here:

http://cr.openjdk.java.net/~bpittore/8005716/jdk-webrev.00/
http://cr.openjdk.java.net/~bpittore/8005716/hs-webrev.00/

The main piece of functionality is to check for the presence of 
JNI_OnLoad_libname to determine if the library specified by 'libname' has been 
statically linked into the VM. If the symbol is found, it is assumed that the 
library is linked in and will not be dynamically loaded.

thanks,
bill




hg: jdk8/tl/jdk: 8008759: Do not let internal JDK zlib symbols leak out of fastdebug libzip.so

2013-03-06 Thread martinrb
Changeset: 14e49a70729a
Author:martin
Date:  2013-03-06 17:43 -0800
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/14e49a70729a

8008759: Do not let internal JDK zlib symbols leak out of fastdebug libzip.so
Summary: Define FILES_m to force use of linker script
Reviewed-by: sherman, alanb, ohair

! make/java/zip/Makefile
! src/share/native/java/util/zip/Inflater.c



Re: Do not let internal JDK zlib symbols leak out of fastdebug libzip.so

2013-03-06 Thread Martin Buchholz
Pushed to jdk8/tl/jdk.
I recommend this be backported to jdk7u.


hg: jdk8/tl/jdk: 8009604: old make images failed: JarBASE64Encoder class not found

2013-03-06 Thread weijun . wang
Changeset: cf54f6be3e9e
Author:weijun
Date:  2013-03-07 11:32 +0800
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/cf54f6be3e9e

8009604: old make images failed: JarBASE64Encoder class not found
Reviewed-by: xuelei, wetmore

! make/common/Release.gmk