Re: [JDK-6341887] RFR: Patch V3: java.util.zip: Add ByteBuffer methods to Inflater/Deflater

[Xueming Shen]

Hi David,

https://github.com/dmlloyd/openjdk/commit/zlib-bytebuffer-v12

Should we start to review the changes included in above link, or we 
should wait ? It appears
the API is being updated but some implementation have not been updated 
to follow the spec
yet, especially the piece that deals with the output buffer/byteWritten 
when DataFormatException

is raised, for example

(1) the "outputConsumedID" is defined but never used to update the 
corresponding java field

 in Inflater.c and

(2) the "outputConsumed" is used to update the output ByteBuffer when 
DFE raised (in Java), but
 the corresponding "byteWritten" is not being updated before the 
exception is thrown.


-Sherman



On 3/13/18, 10:46 AM, David Lloyd wrote:

Sorry all, it looks like GMail doesn't know how to keep replies with
the thread when you change the subject line.  The follow-up to this
thread is 
http://mail.openjdk.java.net/pipermail/core-libs-dev/2018-March/051960.html
with only a few small changes as discussed above.

On Fri, Mar 2, 2018 at 2:36 PM, David Lloyd  wrote:

On Fri, Mar 2, 2018 at 2:34 PM, David Lloyd  wrote:

On Fri, Mar 2, 2018 at 12:49 PM, Xueming Shen  wrote:

Hi David,

(1) Deflater.deflate(Bytebuffer)
  the api doc regarding "no_flush" appears to be the copy/paste of the
byte[] version
  without being updated to the corresponding ByteBuffer?

You're right, I missed that one.  I've incorporated this fix locally:

Oops, this should have been:

--- 8<  --- cut here --- 8<  ---

diff --git a/src/java.base/share/classes/java/util/zip/Deflater.java
b/src/java.base/share/classes/java/util/zip/Deflater.java
index 524125787a8..40f0d9736e2 100644
--- a/src/java.base/share/classes/java/util/zip/Deflater.java
+++ b/src/java.base/share/classes/java/util/zip/Deflater.java
@@ -481,9 +481,9 @@ public class Deflater {
   * in order to determine if more input data is required.
   *
   *This method uses {@link #NO_FLUSH} as its compression flush mode.
- * An invocation of this method of the form {@code deflater.deflate(b)}
+ * An invocation of this method of the form {@code
deflater.deflate(output)}
   * yields the same result as the invocation of
- * {@code deflater.deflate(b, 0, b.length, Deflater.NO_FLUSH)}.
+ * {@code deflater.deflate(output, Deflater.NO_FLUSH)}.
   *
   * @param output the buffer for the compressed data
   * @return the actual number of bytes of compressed data written to the

--- 8<  --- cut here --- 8<  ---

--
- DML

Re: Reactive Streams utility API

[James Roper]

Hi all,

An update on this. We've now filled out the API with feature parity with
the JDK8 Streams API - for operators that make sense in Reactive Streams.
We've provided example implementations of the API backed by both Akka
Streams and rxjava, showing that it can be widely implemented. The TCK
still needs some work, but covers the major features, and comprehensively
covers all publishers/subscribers with the Reactive Streams TCK (so
comprehensive that we actually found two Reactive Streams TCK violations in
Akka Streams with this, and a couple in rxjava too).

There are two major areas of work left to get something that would be ready
to be a candidate for a final API. The first is to produce a zero
dependency reference implementation of the API. This is what I plan on
starting on next.

The second is to decide what additional operators and generators the API
should provide. So far, the scope has been mostly limited to a subset of
the JDK8 Streams scope, only a few additional API pieces have been created,
such as a few variations on flatMap (one that supports CompletionStage, and
one that supports Iterable). There are a number of other features for
consideration to provide basic necessary functionality for this API, here's
some examples off the top of my head (some of these can already be
implemented in terms of other stages already provided):

* A cancelled subscriber (useful for cleaning up hot publishers)
* An ignoring subscriber (useful when you do the actual work in a previous
stage of the graph, such as mapping to completion stages)
* Error handling subscribers and/or processors
* Termination listening subscribers and/or processors
* A processor that wraps a subscriber and publisher
* The ability to merge streams - so far only concat is provided, and all
flatMaps are essentially concatenations, merge variants may be useful
(though introduce a lot of complexity, such as specifying breadth)
* The ability to split streams into sub streams - a use case for this is in
parsing a stream that contains potentially large sub streams, like parsing
a multipart/form-data body
* Batching of elements in a stream, based on predicate, influenced by
backpressure or based on a scheduled tick
* Scheduled features such as emitting ticks, rate limiting, etc
* The ability to control buffering and asynchronous boundaries within a
graph
* Naming of stages for debug/error reporting/monitoring purposes

Not all of the above may be absolutely necessary, but should be considered,
and there may be other features as well that would be useful to consider.

Please visit the repo and any feedback would be much appreciated:

https://github.com/lightbend/reactive-streams-utils

Regards,

James

On 8 March 2018 at 03:59, Brian Goetz  wrote:

> To answer the questions at the bottom: the next step is to start working
> on this and get folks excited about contributing.  There's plenty of time
> for process later, but filing a JEP or creating a project shouldn't be a
> barrier to innovating.
>
>
> On 2/28/2018 10:33 PM, James Roper wrote:
>
>> Hi all,
>>
>> We've put together a simple proposal for this. Please read the README for
>> an introduction to this proposal.
>>
>> https://github.com/lightbend/reactive-streams-utils
>>
>> Regards,
>>
>> James
>>
>> On 22 February 2018 at 11:47, James Roper  wrote:
>>
>> Hi all,
>>>
>>> This is an email to give people a heads up that we'd like to look at
>>> creating an API, in the same vein as the JDK8 Streams API, for building
>>> reactive streams (a la JDK9 juc.Flow). Our goals for this are:
>>>
>>> * To fill a gap in the JDK where if a developer wants to do even the
>>> simplest of things with a JDK9 juc.Flow, such as map or filter, they need
>>> to bring in a third party library that implements that.
>>> * To produce an API that can build Publishers, Subscribers, Processors,
>>> and complete graphs, for the purposes of consuming APIs that use reactive
>>> streams (for example, JDK9 Http Client).
>>> * To produce an API that aligns closely with ju.stream.Stream, using it
>>> for inspiration for naming, scope, general API shape, and other aspects.
>>> The purpose of this goal is to ensure familiarity of Java developers with
>>> the new API, and to limit the number of concepts Java developers need to
>>> understand to do the different types of streaming offered by the JDK.
>>> * To produce an API that can be implemented by multiple providers
>>> (including an RI in the JDK itself), using the ServiceLoader mechanism to
>>> provide and load a default implementation (while allowing custom
>>> implementations to be manually provided). There are a lot of concerns
>>> that
>>> each different streams implementation provides and implements, beyond
>>> streaming, for example monitoring/tracing, concurrency modelling,
>>> buffering
>>> strategies, performance aspects of the streams handling including fusing,
>>> and context (eg thread local) propagation. This 

RFR 8180410: ByteArrayOutputStream should not throw IOExceptions

[Brian Burkhalter]

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

This proposed patch would make the following changes:

1. Add a new method writeBytes(byte[]) which writes all supplied bytes but does 
not throw IOE.
2. Document some previously undocumented exceptions which can be thrown by 
write(byte[],int,int) and writeTo(OutputStream).
3. s/@exception/@throws/.

The test is renamed from WriteBounds to Write with the addition of a test of 
write() and writeBytes().

A CSR will be filed later.

Thanks,

Brian

Re: Raw String Literal Library Support

[Stuart Marks]

Hi Jim,

Some comments (really, mainly just quibbles) about string trimming. First,

* String.trim trims characters <= \u0020 from each end of a string. I agree that 
String.trim should be preserved unchanged for compatibility purposes.


* The trimLeft, trimRight, and trimWhitespace (which trims both ends) methods 
make sense. These three should all use the same definition of whitespace.


* My issue concerns what definition of whitespace they use.

What you outlined in the quoted section below doesn't line up with the 
definitions in the API spec.


The existing methods Character.isSpaceChar(codepoint) and 
Character.isWhitespace(codepoint) are well-defined but somewhat different 
notions of whitespace.


**

The Character.isSpaceChar method returns true if the code point is a member of 
any of these categories:


SPACE_SEPARATOR
LINE_SEPARATOR
PARAGRAPH_SEPARATOR

In JDK 10, which conforms to Unicode 8.0.0, the SPACE_SEPARATOR category 
includes the following characters:


U+0020 SPACE
U+00A0 NO-BREAK SPACE
U+1680 OGHAM SPACE MARK
U+2000 EN QUAD
U+2001 EM QUAD
U+2002 EN SPACE
U+2003 EM SPACE
U+2004 THREE-PER-EM SPACE
U+2005 FOUR-PER-EM SPACE
U+2006 SIX-PER-EM SPACE
U+2007 FIGURE SPACE
U+2008 PUNCTUATION SPACE
U+2009 THIN SPACE
U+200A HAIR SPACE
U+202F NARROW NO-BREAK SPACE
U+205F MEDIUM MATHEMATICAL SPACE
U+3000 IDEOGRAPHIC SPACE

The LINE_SEPARATOR category contains this one character:

U+2028 LINE SEPARATOR

And the PARAGRAPH_SEPARATOR category contains just this one character:

U+2029 PARAGRAPH SEPARATOR

**

Meanwhile, the Character.isWhitespace method returns true if the code point is 
in one of these categories:


SPACE_SEPARATOR, excluding
U+00A0 NO-BREAK SPACE
U+2007 FIGURE SPACE
U+202F NARROW NO-BREAK SPACE
LINE_SEPARATOR
PARAGRAPH_SEPARATOR

or if it is one of these characters:

U+0009 HORIZONTAL TABULATION.
U+000A LINE FEED.
U+000B VERTICAL TABULATION.
U+000C FORM FEED.
U+000D CARRIAGE RETURN.
U+001C FILE SEPARATOR.
U+001D GROUP SEPARATOR.
U+001E RECORD SEPARATOR.
U+001F UNIT SEPARATOR.

**

You mentioned several different definitions of whitespace:

 - trim's whitespace (TWS): chars <= U+0020
 - Character's whitespace (CWS): I'm not sure what you meant by this
 - union whitespace (UWS): union of TWS and CWS

I don't think we should be creating a new definition of whitespace, such as UWS, 
if at all possible. TWS is strange in that it contains a bunch of control 
characters that aren't necessarily whitespace, and it omits Unicode whitespace. 
Character.isSpaceChar includes various no-break spaces, which I don't think 
should be trimmed away, and it also omits various ASCII white space characters, 
which I think most programmers would find surprising.


Finally, Character.isWhitespace includes the ASCII whitespace characters and 
Unicode space separators, but excludes no-break spaces. This makes the most 
sense to me. So, how about we define trimLeft, trimRight, and trimWhitespace all 
in terms of Character.isWhitespace?


s'marks




On 3/13/18 6:47 AM, Jim Laskey wrote:

B. Additions to basic trim methods. In addition to margin methods trimIndent 
and trimMarkers described below in Section C, it would be worth introducing 
trimLeft and trimRight to augment the longstanding trim method. A key question 
is how trimLeft and trimRight should detect whitespace, because different 
definitions of whitespace exist in the library.

trim itself uses the simple test less than or equal to the space character, a 
fast test but not Unicode friendly.

Character.isWhitespace(codepoint) returns true if codepoint one of the 
following;

SPACE_SEPARATOR.
LINE_SEPARATOR.
PARAGRAPH_SEPARATOR.
'\t', U+0009 HORIZONTAL TABULATION.
'\n', U+000A LINE FEED.
'\u000B', U+000B VERTICAL TABULATION.
'\f', U+000C FORM FEED.
'\r', U+000D CARRIAGE RETURN.
'\u001C', U+001C FILE SEPARATOR.
'\u001D', U+001D GROUP SEPARATOR.
'\u001E', U+001E RECORD SEPARATOR.
'\u001F', U+001F UNIT SEPARATOR.
' ',  U+0020 SPACE.
(Note: that non-breaking space (\u00A0) is excluded)

Character.isSpaceChar(codepoint) returns true if codepoint one of the following;

SPACE_SEPARATOR.
LINE_SEPARATOR.
PARAGRAPH_SEPARATOR.
' ',  U+0020 SPACE.
'\u00A0', U+00A0 NON-BREAKING SPACE.
That sets up several kinds of whitespace; trim's whitespace (TWS), Character 
whitespace (CWS) and the union of the two (UWS). TWS is a fast test. CWS is a 
slow test. UWS is fast for Latin1 and slow-ish for UTF-16.

We are recommending that trimLeft and trimRight use UWS, leave trim alone to 
avoid breaking the world and then possibly introduce trimWhitespace that uses 
UWS.

public String trim()
Removes characters less than equal to space from the beginning and end of the 
string. No, change except spec clarification and 

Re: Raw String Literal Library Support

[Michael Hixson]

Hi Jim,

Does string.lines() agree with new BufferedReader(new
StringReader(string)).lines() on what the lines are for all inputs?
For example, does ``.lines() produce an empty stream?

-Michael

On Tue, Mar 13, 2018 at 6:47 AM, Jim Laskey  wrote:
> With the announcement of JEP 326 Raw String Literals, we would like to open 
> up a discussion with regards to RSL library support. Below are several 
> implemented String methods that are believed to be appropriate. Please 
> comment on those mentioned below including recommending alternate names or 
> signatures. Additional methods can be considered if warranted, but as always, 
> the bar for inclusion in String is high.
>
> You should keep a couple things in mind when reviewing these methods.
>
> Methods should be applicable to all strings, not just Raw String Literals.
>
> The number of additional methods should be minimized, not adding every 
> possible method.
>
> Don't put any emphasis on performance. That is a separate discussion.
>
> Cheers,
>
> -- Jim
>
> A. Line support.
>
> public Stream lines()
> Returns a stream of substrings extracted from this string partitioned by line 
> terminators. Internally, the stream is implemented using a Spliteratorthat 
> extracts one line at a time. The line terminators recognized are \n, \r\n and 
> \r. This method provides versatility for the developer working with 
> multi-line strings.
>  Example:
>
> String string = "abc\ndef\nghi";
> Stream stream = string.lines();
> List list = stream.collect(Collectors.toList());
>
>  Result:
>
>  [abc, def, ghi]
>
>
>  Example:
>
> String string = "abc\ndef\nghi";
> String[] array = string.lines().toArray(String[]::new);
>
>  Result:
>
>  [Ljava.lang.String;@33e5ccce // [abc, def, ghi]
>
>
>  Example:
>
> String string = "abc\ndef\r\nghi\rjkl";
> String platformString =
> string.lines().collect(joining(System.lineSeparator()));
>
>  Result:
>
>  abc
>  def
>  ghi
>  jkl
>
>
>  Example:
>
> String string = " abc  \n   def  \n ghi   ";
> String trimmedString =
>  string.lines().map(s -> s.trim()).collect(joining("\n"));
>
>  Result:
>
>  abc
>  def
>  ghi
>
>
>  Example:
>
> String table = `First Name  SurnamePhone
> Al  Albert 555-
> Bob Roberts555-
> Cal Calvin 555-
>`;
>
> // Extract headers
> String firstLine = table.lines().findFirst().orElse("");
> List headings = List.of(firstLine.trim().split(`\s{2,}`));
>
> // Build stream of maps
> Stream> stream =
> table.lines().skip(1)
>  .map(line -> line.trim())
>  .filter(line -> !line.isEmpty())
>  .map(line -> line.split(`\s{2,}`))
>  .map(columns -> {
>  List values = List.of(columns);
>  return IntStream.range(0, headings.size()).boxed()
>  .collect(toMap(headings::get, 
> values::get));
>  });
>
> // print all "First Name"
> stream.map(row -> row.get("First Name"))
>   .forEach(name -> System.out.println(name));
>
>  Result:
>
>  Al
>  Bob
>  Cal
> B. Additions to basic trim methods. In addition to margin methods trimIndent 
> and trimMarkers described below in Section C, it would be worth introducing 
> trimLeft and trimRight to augment the longstanding trim method. A key 
> question is how trimLeft and trimRight should detect whitespace, because 
> different definitions of whitespace exist in the library.
>
> trim itself uses the simple test less than or equal to the space character, a 
> fast test but not Unicode friendly.
>
> Character.isWhitespace(codepoint) returns true if codepoint one of the 
> following;
>
>SPACE_SEPARATOR.
>LINE_SEPARATOR.
>PARAGRAPH_SEPARATOR.
>'\t', U+0009 HORIZONTAL TABULATION.
>'\n', U+000A LINE FEED.
>'\u000B', U+000B VERTICAL TABULATION.
>'\f', U+000C FORM FEED.
>'\r', U+000D CARRIAGE RETURN.
>'\u001C', U+001C FILE SEPARATOR.
>'\u001D', U+001D GROUP SEPARATOR.
>'\u001E', U+001E RECORD SEPARATOR.
>'\u001F', U+001F UNIT SEPARATOR.
>' ',  U+0020 SPACE.
> (Note: that non-breaking space (\u00A0) is excluded)
>
> Character.isSpaceChar(codepoint) returns true if codepoint one of the 
> following;
>
>SPACE_SEPARATOR.
>LINE_SEPARATOR.
>PARAGRAPH_SEPARATOR.
>' ',  U+0020 SPACE.
>'\u00A0', U+00A0 NON-BREAKING SPACE.
> That sets up several kinds of whitespace; trim's whitespace (TWS), Character 
> whitespace (CWS) and the union of the two (UWS). TWS is a fast test. 

Re: [PATCH] 8188240: Reflection Proxy should skip static methods

[Aleksey Shipilev]

On 03/14/2018 10:44 PM, mandy chung wrote:
> David - I think the test fails even in your first version.
> 
> It should use ProxyClashTest.class.getClassLoader() to define the proxy class 
> as the test is running
> in agent vm mode.

Right. This passes local testing:
 http://cr.openjdk.java.net/~shade/8188240/webrev.02/

...and I am going to redo jdk/submit.

-Aleksey

Re: [PATCH] 8188240: Reflection Proxy should skip static methods

[David Lloyd]

On Wed, Mar 14, 2018 at 4:31 PM, Aleksey Shipilev  wrote:
> On 03/14/2018 07:09 PM, David Lloyd wrote:
>> On Wed, Mar 14, 2018 at 1:00 PM, mandy chung  wrote:
>>> Thanks for adding the new test.   Looks okay and some minor comment.
>>>
>>> +try {
>>>:
>>> +} catch (Throwable e) {
>>> +System.err.println("\nTEST FAILED:");
>>> +e.printStackTrace();
>>> +throw new RuntimeException("TEST FAILED: " + e.toString());
>>> +}
>>>
>>>
>>> You can take out this try-catch (Basic1.java isn't be the best example to
>>> reference that is old and needs update).
>>
>> Fixed & attached.
>
> Have you tried to run the test?
>
> Because it fails:
>
> $ make images run-test TEST=jdk/java/lang/reflect/Proxy/ProxyClashTest.java
>
> Dynamic proxy API static method clash test
>
> java.lang.IllegalArgumentException: ProxyClashTest$ClashWithRunnable 
> referenced from a method is not
> visible from class loader
> at 
> java.base/java.lang.reflect.Proxy$ProxyBuilder.ensureVisible(Proxy.java:851)
> at 
> java.base/java.lang.reflect.Proxy$ProxyBuilder.validateProxyInterfaces(Proxy.java:682)
> at 
> java.base/java.lang.reflect.Proxy$ProxyBuilder.(Proxy.java:628)
> at 
> java.base/java.lang.reflect.Proxy.lambda$getProxyConstructor$1(Proxy.java:426)
> at
> java.base/jdk.internal.loader.AbstractClassLoaderValue$Memoizer.get(AbstractClassLoaderValue.java:329)
> at
> java.base/jdk.internal.loader.AbstractClassLoaderValue.computeIfAbsent(AbstractClassLoaderValue.java:205)
> at 
> java.base/java.lang.reflect.Proxy.getProxyConstructor(Proxy.java:424)
> at java.base/java.lang.reflect.Proxy.getProxyClass(Proxy.java:384)
> at ProxyClashTest.main(ProxyClashTest.java:56)
> at 
> java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
> at 
> java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
> at
> java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
> at java.base/java.lang.reflect.Method.invoke(Method.java:564)
> at 
> com.sun.javatest.regtest.agent.MainActionHelper$SameVMRunnable.run(MainActionHelper.java:229)
> at java.base/java.lang.Thread.run(Thread.java:841)
>
> JavaTest Message: Test threw exception: java.lang.IllegalArgumentException
> JavaTest Message: shutting down test

I did not run it with jtreg, just on the class path after compiling by
hand.  I guess it is failing because the nested class is not visible
from the system class loader when running within JTReg; maybe the fix
would be to use the class loader of the test class or the nested
class?

-- 
- DML

Re: [PATCH] 8188240: Reflection Proxy should skip static methods

[mandy chung]

David - I think the test fails even in your first version.

It should use ProxyClashTest.class.getClassLoader() to define the proxy 
class as the test is running in agent vm mode.


Mandy

On 3/14/18 2:31 PM, Aleksey Shipilev wrote:

Have you tried to run the test
Because it fails:

$ make images run-test TEST=jdk/java/lang/reflect/Proxy/ProxyClashTest.java

Dynamic proxy API static method clash test

java.lang.IllegalArgumentException: ProxyClashTest$ClashWithRunnable referenced 
from a method is not
visible from class loader
at 
java.base/java.lang.reflect.Proxy$ProxyBuilder.ensureVisible(Proxy.java:851)
at 
java.base/java.lang.reflect.Proxy$ProxyBuilder.validateProxyInterfaces(Proxy.java:682)
at java.base/java.lang.reflect.Proxy$ProxyBuilder.(Proxy.java:628)
at 
java.base/java.lang.reflect.Proxy.lambda$getProxyConstructor$1(Proxy.java:426)
at
java.base/jdk.internal.loader.AbstractClassLoaderValue$Memoizer.get(AbstractClassLoaderValue.java:329)
at
java.base/jdk.internal.loader.AbstractClassLoaderValue.computeIfAbsent(AbstractClassLoaderValue.java:205)
at java.base/java.lang.reflect.Proxy.getProxyConstructor(Proxy.java:424)
at java.base/java.lang.reflect.Proxy.getProxyClass(Proxy.java:384)
at ProxyClashTest.main(ProxyClashTest.java:56)
at 
java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
at 
java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
at
java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
at java.base/java.lang.reflect.Method.invoke(Method.java:564)
at 
com.sun.javatest.regtest.agent.MainActionHelper$SameVMRunnable.run(MainActionHelper.java:229)
at java.base/java.lang.Thread.run(Thread.java:841)

JavaTest Message: Test threw exception: java.lang.IllegalArgumentException
JavaTest Message: shutting down test


Thanks,
-Aleksey

Re: [PATCH] 8188240: Reflection Proxy should skip static methods

[Aleksey Shipilev]

On 03/14/2018 07:09 PM, David Lloyd wrote:
> On Wed, Mar 14, 2018 at 1:00 PM, mandy chung  wrote:
>> Thanks for adding the new test.   Looks okay and some minor comment.
>>
>> +try {
>>:
>> +} catch (Throwable e) {
>> +System.err.println("\nTEST FAILED:");
>> +e.printStackTrace();
>> +throw new RuntimeException("TEST FAILED: " + e.toString());
>> +}
>>
>>
>> You can take out this try-catch (Basic1.java isn't be the best example to
>> reference that is old and needs update).
> 
> Fixed & attached.

Have you tried to run the test?

Because it fails:

$ make images run-test TEST=jdk/java/lang/reflect/Proxy/ProxyClashTest.java

Dynamic proxy API static method clash test

java.lang.IllegalArgumentException: ProxyClashTest$ClashWithRunnable referenced 
from a method is not
visible from class loader
at 
java.base/java.lang.reflect.Proxy$ProxyBuilder.ensureVisible(Proxy.java:851)
at 
java.base/java.lang.reflect.Proxy$ProxyBuilder.validateProxyInterfaces(Proxy.java:682)
at java.base/java.lang.reflect.Proxy$ProxyBuilder.(Proxy.java:628)
at 
java.base/java.lang.reflect.Proxy.lambda$getProxyConstructor$1(Proxy.java:426)
at
java.base/jdk.internal.loader.AbstractClassLoaderValue$Memoizer.get(AbstractClassLoaderValue.java:329)
at
java.base/jdk.internal.loader.AbstractClassLoaderValue.computeIfAbsent(AbstractClassLoaderValue.java:205)
at java.base/java.lang.reflect.Proxy.getProxyConstructor(Proxy.java:424)
at java.base/java.lang.reflect.Proxy.getProxyClass(Proxy.java:384)
at ProxyClashTest.main(ProxyClashTest.java:56)
at 
java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
at 
java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
at
java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
at java.base/java.lang.reflect.Method.invoke(Method.java:564)
at 
com.sun.javatest.regtest.agent.MainActionHelper$SameVMRunnable.run(MainActionHelper.java:229)
at java.base/java.lang.Thread.run(Thread.java:841)

JavaTest Message: Test threw exception: java.lang.IllegalArgumentException
JavaTest Message: shutting down test


Thanks,
-Aleksey

Re: [PATCH] 8188240: Reflection Proxy should skip static methods

[Aleksey Shipilev]

On 03/14/2018 07:59 PM, Andrej Golovnin wrote:
> Hi David,
> 
> +if (! Modifier.isStatic(m.getModifiers())) {
> 
> I think the whitespace after the ‘!’-sign should be removed.

I agree. No problem, I will remove this space in my patch queue.

-Aleksey

Re: [PATCH] 8188240: Reflection Proxy should skip static methods

[Andrej Golovnin]

Hi David,

+if (! Modifier.isStatic(m.getModifiers())) {

I think the whitespace after the ‘!’-sign should be removed.

Best regards,
Andrej Golovnin

> On 14. Mar 2018, at 19:09, David Lloyd  wrote:
> 
> On Wed, Mar 14, 2018 at 1:00 PM, mandy chung  wrote:
>> Thanks for adding the new test.   Looks okay and some minor comment.
>> 
>> +try {
>>   :
>> +} catch (Throwable e) {
>> +System.err.println("\nTEST FAILED:");
>> +e.printStackTrace();
>> +throw new RuntimeException("TEST FAILED: " + e.toString());
>> +}
>> 
>> 
>> You can take out this try-catch (Basic1.java isn't be the best example to
>> reference that is old and needs update).
> 
> Fixed & attached.
> 
>> I assume your colleague at Red Hat will sponsor it for you?
> 
> I will find out.
> 
> -- 
> - DML
diff --git a/src/java.base/share/classes/java/lang/reflect/ProxyGenerator.java 
b/src/java.base/share/classes/java/lang/reflect/ProxyGenerator.java
index cc8dbbfffab..465a5b938e3 100644
--- a/src/java.base/share/classes/java/lang/reflect/ProxyGenerator.java
+++ b/src/java.base/share/classes/java/lang/reflect/ProxyGenerator.java
@@ -449,7 +449,9 @@ class ProxyGenerator {
  */
 for (Class intf : interfaces) {
 for (Method m : intf.getMethods()) {
-addProxyMethod(m, intf);
+if (! Modifier.isStatic(m.getModifiers())) {
+addProxyMethod(m, intf);
+}
 }
 }
 
diff --git a/test/jdk/java/lang/reflect/Proxy/ProxyClashTest.java 
b/test/jdk/java/lang/reflect/Proxy/ProxyClashTest.java
new file mode 100644
index 000..604ddf6a930
--- /dev/null
+++ b/test/jdk/java/lang/reflect/Proxy/ProxyClashTest.java
@@ -0,0 +1,70 @@
+/*
+ * Copyright (c) 2018, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+/* @test
+ * @bug 8188240
+ * @summary This is a test to ensure that proxies do not inherit static 
methods.
+ *
+ * @build ProxyClashTest
+ * @run main ProxyClashTest
+ */
+
+import java.lang.reflect.Method;
+import java.lang.reflect.Proxy;
+import java.util.Observer;
+
+public class ProxyClashTest {
+
+public interface ClashWithRunnable {
+static int run() { return 123; }
+
+static void foo() {}
+}
+
+public static void main(String[] args) {
+System.err.println(
+"\nDynamic proxy API static method clash test\n");
+
+Class[] interfaces =
+new Class[] { ClashWithRunnable.class, Runnable.class, 
Observer.class };
+
+ClassLoader loader = ClassLoader.getSystemClassLoader();
+
+/*
+ * Generate a proxy class.
+ */
+Class proxyClass = Proxy.getProxyClass(loader, interfaces);
+System.err.println("+ generated proxy class: " + proxyClass);
+
+for (Method method : proxyClass.getDeclaredMethods()) {
+if (method.getName().equals("run") && method.getReturnType() == 
int.class) {
+throw new RuntimeException("proxy inherited a static method");
+}
+if (method.getName().equals("foo")) {
+throw new RuntimeException("proxy inherited a static method");
+}
+}
+
+System.err.println("\nTEST PASSED");
+}
+}
> 

Re: [PATCH] 8188240: Reflection Proxy should skip static methods

[Aleksey Shipilev]

On 03/14/2018 07:44 PM, mandy chung wrote:
 I assume your colleague at Red Hat will sponsor it for you?
>>> I will find out.
>> I can do it, but I need to get updated on two things:
>>  a) Are we pushing JDK changes directly to jdk/jdk now?
>>  b) Do we need to run it through JDK Submit first, or that is optional for 
>> this change?
> 
> While this patch is trivial, I recommend to run through JDK submit since 
> proxies are used by other
> areas.  I did find surprises in the past via testing on trivial proxy class 
> changes.

Makes sense, submitted for testing.

-Aleksey

Re: [PATCH] 8188240: Reflection Proxy should skip static methods

[mandy chung]

On 3/14/18 11:37 AM, Aleksey Shipilev wrote:

On 03/14/2018 07:09 PM, David Lloyd wrote:

On Wed, Mar 14, 2018 at 1:00 PM, mandy chung  wrote:

Thanks for adding the new test.   Looks okay and some minor comment.

+try {
:
+} catch (Throwable e) {
+System.err.println("\nTEST FAILED:");
+e.printStackTrace();
+throw new RuntimeException("TEST FAILED: " + e.toString());
+}


You can take out this try-catch (Basic1.java isn't be the best example to
reference that is old and needs update).

Fixed & attached.


I assume your colleague at Red Hat will sponsor it for you?

I will find out.

I can do it, but I need to get updated on two things:
  a) Are we pushing JDK changes directly to jdk/jdk now?
  b) Do we need to run it through JDK Submit first, or that is optional for 
this change?


While this patch is trivial, I recommend to run through JDK submit since 
proxies are used by other areas.  I did find surprises in the past via 
testing on trivial proxy class changes.


Mandy

Re: [PATCH] 8188240: Reflection Proxy should skip static methods

[Aleksey Shipilev]

On 03/14/2018 07:09 PM, David Lloyd wrote:
> On Wed, Mar 14, 2018 at 1:00 PM, mandy chung  wrote:
>> Thanks for adding the new test.   Looks okay and some minor comment.
>>
>> +try {
>>:
>> +} catch (Throwable e) {
>> +System.err.println("\nTEST FAILED:");
>> +e.printStackTrace();
>> +throw new RuntimeException("TEST FAILED: " + e.toString());
>> +}
>>
>>
>> You can take out this try-catch (Basic1.java isn't be the best example to
>> reference that is old and needs update).
> 
> Fixed & attached.
> 
>> I assume your colleague at Red Hat will sponsor it for you?
> 
> I will find out.

I can do it, but I need to get updated on two things:
 a) Are we pushing JDK changes directly to jdk/jdk now?
 b) Do we need to run it through JDK Submit first, or that is optional for this 
change?

Thanks,
-Aleksey

Re: [PATCH] 8188240: Reflection Proxy should skip static methods

[mandy chung]

On 3/14/18 11:09 AM, David Lloyd wrote:

On Wed, Mar 14, 2018 at 1:00 PM, mandy chung  wrote:

Thanks for adding the new test.   Looks okay and some minor comment.

+try {
:
+} catch (Throwable e) {
+System.err.println("\nTEST FAILED:");
+e.printStackTrace();
+throw new RuntimeException("TEST FAILED: " + e.toString());
+}


You can take out this try-catch (Basic1.java isn't be the best example to
reference that is old and needs update).

Fixed & attached.

+1

Mandy

Re: [PATCH] 8188240: Reflection Proxy should skip static methods

[David Lloyd]

On Wed, Mar 14, 2018 at 1:00 PM, mandy chung  wrote:
> Thanks for adding the new test.   Looks okay and some minor comment.
>
> +try {
>:
> +} catch (Throwable e) {
> +System.err.println("\nTEST FAILED:");
> +e.printStackTrace();
> +throw new RuntimeException("TEST FAILED: " + e.toString());
> +}
>
>
> You can take out this try-catch (Basic1.java isn't be the best example to
> reference that is old and needs update).

Fixed & attached.

> I assume your colleague at Red Hat will sponsor it for you?

I will find out.

-- 
- DML
diff --git a/src/java.base/share/classes/java/lang/reflect/ProxyGenerator.java 
b/src/java.base/share/classes/java/lang/reflect/ProxyGenerator.java
index cc8dbbfffab..465a5b938e3 100644
--- a/src/java.base/share/classes/java/lang/reflect/ProxyGenerator.java
+++ b/src/java.base/share/classes/java/lang/reflect/ProxyGenerator.java
@@ -449,7 +449,9 @@ class ProxyGenerator {
  */
 for (Class intf : interfaces) {
 for (Method m : intf.getMethods()) {
-addProxyMethod(m, intf);
+if (! Modifier.isStatic(m.getModifiers())) {
+addProxyMethod(m, intf);
+}
 }
 }
 
diff --git a/test/jdk/java/lang/reflect/Proxy/ProxyClashTest.java 
b/test/jdk/java/lang/reflect/Proxy/ProxyClashTest.java
new file mode 100644
index 000..604ddf6a930
--- /dev/null
+++ b/test/jdk/java/lang/reflect/Proxy/ProxyClashTest.java
@@ -0,0 +1,70 @@
+/*
+ * Copyright (c) 2018, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+/* @test
+ * @bug 8188240
+ * @summary This is a test to ensure that proxies do not inherit static 
methods.
+ *
+ * @build ProxyClashTest
+ * @run main ProxyClashTest
+ */
+
+import java.lang.reflect.Method;
+import java.lang.reflect.Proxy;
+import java.util.Observer;
+
+public class ProxyClashTest {
+
+public interface ClashWithRunnable {
+static int run() { return 123; }
+
+static void foo() {}
+}
+
+public static void main(String[] args) {
+System.err.println(
+"\nDynamic proxy API static method clash test\n");
+
+Class[] interfaces =
+new Class[] { ClashWithRunnable.class, Runnable.class, 
Observer.class };
+
+ClassLoader loader = ClassLoader.getSystemClassLoader();
+
+/*
+ * Generate a proxy class.
+ */
+Class proxyClass = Proxy.getProxyClass(loader, interfaces);
+System.err.println("+ generated proxy class: " + proxyClass);
+
+for (Method method : proxyClass.getDeclaredMethods()) {
+if (method.getName().equals("run") && method.getReturnType() == 
int.class) {
+throw new RuntimeException("proxy inherited a static method");
+}
+if (method.getName().equals("foo")) {
+throw new RuntimeException("proxy inherited a static method");
+}
+}
+
+System.err.println("\nTEST PASSED");
+}
+}

Re: [PATCH] 8188240: Reflection Proxy should skip static methods

[mandy chung]

On 3/13/18 5:16 PM, David Lloyd wrote:

OK, done.  It's a little bigger now so I'm attaching it.


Thanks for adding the new test.   Looks okay and some minor comment.

+try {
   :
+} catch (Throwable e) {
+System.err.println("\nTEST FAILED:");
+e.printStackTrace();
+throw new RuntimeException("TEST FAILED: " + e.toString());
+}


You can take out this try-catch (Basic1.java isn't be the best example to 
reference that is old and needs update).

I assume your colleague at Red Hat will sponsor it for you?

Mandy

Re: RFR of 8180451: ByteArrayInputStream should override readAllBytes, readNBytes, and transferTo

[Brian Burkhalter]

On Mar 14, 2018, at 9:27 AM, David Lloyd  wrote:

> @@ -196,14 +194,32 @@
> return len;
> }
> 
> +public synchronized byte[] readAllBytes() {
> +byte[] result = Arrays.copyOfRange(buf, pos, count);
> +pos = count;
> +return result;
> +}
> +
> +public synchronized int readNBytes(byte[] b, int off, int len) {
> +int n = read(b, off, len);
> +return n == -1 ? 0 : n;
> +}
> 
> This probably doesn't need to be synchronized, though I imagine the
> difference would be minimal.

You are correct, it does not.

> +public synchronized long transferTo(OutputStream out) throws IOException 
> {
> +int len = count - pos
> +out.write(but, pos, len);
> 
> s/but/buf/ I guess?

Webrevs corrected in place:

http://cr.openjdk.java.net/~bpb/8180451/webrev.00-01/
http://cr.openjdk.java.net/~bpb/8180451/webrev.01/

Thanks,

Brian

Re: RFR of 8180451: ByteArrayInputStream should override readAllBytes, readNBytes, and transferTo

[Brian Burkhalter]

On Mar 14, 2018, at 9:27 AM, David Lloyd  wrote:

> +public synchronized long transferTo(OutputStream out) throws IOException 
> {
> +int len = count - pos
> +out.write(but, pos, len);
> 
> s/but/buf/ I guess?

Yes, I already caught that myself. I think I generated the webrev before 
running tests, i.e., out of sequence.

Thanks,

Brian

> 
> +pos = count;
> +return len;
> +}
> +

Re: RFR of 8180451: ByteArrayInputStream should override readAllBytes, readNBytes, and transferTo

[David Lloyd]

In:

@@ -196,14 +194,32 @@
 return len;
 }

+public synchronized byte[] readAllBytes() {
+byte[] result = Arrays.copyOfRange(buf, pos, count);
+pos = count;
+return result;
+}
+
+public synchronized int readNBytes(byte[] b, int off, int len) {
+int n = read(b, off, len);
+return n == -1 ? 0 : n;
+}

This probably doesn't need to be synchronized, though I imagine the
difference would be minimal.

+public synchronized long transferTo(OutputStream out) throws IOException {
+int len = count - pos
+out.write(but, pos, len);

s/but/buf/ I guess?

+pos = count;
+return len;
+}
+

On Wed, Mar 14, 2018 at 10:31 AM, Brian Burkhalter
 wrote:
> Reprising this thread from three months ago [1].
>
> A patch including the changes suggested below is at
>
> http://cr.openjdk.java.net/~bpb/8180451/webrev.01/
>
> with the differences between this and the prior version at
>
> http://cr.openjdk.java.net/~bpb/8180451/webrev.00-01/
>
> Thanks,
>
> Brian
>
> [1] 
> http://mail.openjdk.java.net/pipermail/core-libs-dev/2017-December/050550.html
>
> On Dec 12, 2017, at 11:42 AM, Paul Sandoz  wrote:
>
>> 208 public synchronized long transferTo(OutputStream out) throws 
>> IOException {
>> 209 int pos0 = pos;
>> 210 out.write(buf, pos, count - pos);
>> 211 pos = count;
>> 212 return count - pos0;
>> 213 }
>>
>>  int len = count - pos
>>  out.write(but, pos, len);
>>  pos = count;
>>  return len;
>
> On Dec 12, 2017, at 12:23 PM, Brent Christian  
> wrote:
>
>> The changes look fine to me.
>> I would have found the test case a little easier to follow if "off"/"len" 
>> weren't named so similarly to "offset"/"length", but it's not a big deal.
>
>



-- 
- DML

Re: RFR: 8199471: Enable generation of callSiteForms at link time

[Claes Redestad]

On 2018-03-14 16:55, mandy chung wrote:



On 3/14/18 7:09 AM, Claes Redestad wrote:


http://cr.openjdk.java.net/~redestad/8199471/open.01/



This looks fine.


Thanks!


  The following comment needs update before you push.

src/java.base/share/classes/jdk/internal/misc/JavaLangInvokeAccess.java

 101  * Returns a {@code byte[]} representation of a class 
implementing

 102  * the invoker forms for the set of supplied {@code methodTypes}.
 103  */
 104 byte[] generateInvokersHolderClassBytes(String className,
 105 MethodType[] invokerMethodTypes,
 106 MethodType[] callSiteFormsMethodTypes);


Will do!

/Claes

Re: RFR: 8199471: Enable generation of callSiteForms at link time

[mandy chung]

On 3/14/18 7:09 AM, Claes Redestad wrote:


http://cr.openjdk.java.net/~redestad/8199471/open.01/



This looks fine.  The following comment needs update before you push.

src/java.base/share/classes/jdk/internal/misc/JavaLangInvokeAccess.java

 101  * Returns a {@code byte[]} representation of a class implementing
 102  * the invoker forms for the set of supplied {@code methodTypes}.
 103  */
 104 byte[] generateInvokersHolderClassBytes(String className,
 105 MethodType[] invokerMethodTypes,
 106 MethodType[] callSiteFormsMethodTypes);


Mandy

Re: RFR 8199464 [11] Remove remaining vestiges of Java_sun_reflect_Reflection_getCallerClass

[mandy chung]

Looks okay.

Mandy

On 3/14/18 5:56 AM, Chris Hegarty wrote:

This is a review request to remove remaining vestiges of
Java_sun_reflect_Reflection_getCallerClass.

JDK-8179424 removed terminally deprecated 
jdk.unsupported/sun.reflect.Reflection.getCallerClass(int), these 
references are to the no-args getCallerClass that was removed a long 
time ago. These should be cleaned up anyway.



diff --git a/make/mapfiles/libjava/reorder-sparc 
b/make/mapfiles/libjava/reorder-sparc

--- a/make/mapfiles/libjava/reorder-sparc
+++ b/make/mapfiles/libjava/reorder-sparc
@@ -26,7 +26,6 @@
 text: .text%Java_java_io_FileDescriptor_initIDs;
 text: .text%Java_java_io_FileOutputStream_initIDs;
 text: .text%Java_java_lang_System_setIn0;
-text: .text%Java_sun_reflect_Reflection_getCallerClass__;
 text: .text%Java_java_lang_Class_forName0;
 text: .text%Java_java_lang_Object_getClass;
 text: .text%Java_sun_reflect_Reflection_getClassAccessFlags;
diff --git a/make/mapfiles/libjava/reorder-sparcv9 
b/make/mapfiles/libjava/reorder-sparcv9

--- a/make/mapfiles/libjava/reorder-sparcv9
+++ b/make/mapfiles/libjava/reorder-sparcv9
@@ -25,7 +25,6 @@
 text: .text%Java_java_io_FileDescriptor_initIDs;
 text: .text%Java_java_io_FileOutputStream_initIDs;
 text: .text%Java_java_lang_System_setIn0;
-text: .text%Java_sun_reflect_Reflection_getCallerClass__;
 text: .text%Java_java_lang_Class_forName0;
 text: .text%Java_java_lang_String_intern;
 text: .text%Java_java_lang_StringUTF16_isBigEndian;
diff --git a/make/mapfiles/libjava/reorder-x86 
b/make/mapfiles/libjava/reorder-x86

--- a/make/mapfiles/libjava/reorder-x86
+++ b/make/mapfiles/libjava/reorder-x86
@@ -26,7 +26,6 @@
 text: .text%Java_java_io_FileDescriptor_initIDs;
 text: .text%Java_java_io_FileOutputStream_initIDs;
 text: .text%Java_java_lang_System_setIn0;
-text: .text%Java_sun_reflect_Reflection_getCallerClass__;
 text: .text%Java_java_lang_Class_forName0;
 text: .text%Java_java_lang_String_intern;
 text: .text%Java_java_lang_StringUTF16_isBigEndian;


-Chris.

Re: Raw String Literal Library Support

[Remi Forax]

doh,
sorry for the tangential comment, it was the only comment i had, all other 
methods are fine.

Rémi

- Mail original -
> De: "Brian Goetz" 
> À: "Peter Levart" , "Xueming Shen" 
> , "core-libs-dev"
> 
> Envoyé: Mercredi 14 Mars 2018 16:26:30
> Objet: Re: Raw String Literal Library Support

> Perhaps we can "split" this discussion on splitting into a separate
> thread.  What's happened here is what always happens, which is:
> 
>  - Jim spent a lot of time and effort writing a comprehensive and clear
> proposal;
>  - Someone made a tangential comment on one aspect of it;
>  - Flood of deep-dive responses on that aspect;
>  - Everyone chimes in designing their favorite method not proposed;
>  - No one ever comes back to the substance of the proposal.
> 
> Hitting the reset button...
> 
> 
> On 3/14/2018 9:11 AM, Peter Levart wrote:
>> I think that:
>>
>> String delim = ...;
>> String r =
>> s.splits(Pattern.quote(delim)).collect(Collectors.joining(delim));
>>
>> ... should always produce a result such that r.equals(s);
>>
>>
>> Otherwise, is it wise to add methods that take a regex as a String? It
>> is rarely needed for a regex parameter to be dynamic. Usually a
>> constant is specified. Are there any plans for Java to support Pattern
>> constants? With constant dynamic support they would be trivial to
>> implement in bytecode. If there are any such plans, then the methods
>> should perhaps take a Pattern instead.
>>
>> syntax suggestion:
>>
>> '~' is an unary operator for bit-wise negation of integer values. It
>> could be overloaded for String(s) such that the following two were
>> equivalent:
>>
>> ~ string
>> Pattern.compile(string)
>>
>> Now if 'string' above is a constant, '~ string' could be a constant
>> too. Combined with raw string literals, Pattern constants could be
>> very compact.
>>
>>
>> What do you think?
>>
>> Regards, Peter
>>
>> On 03/14/2018 02:35 AM, Xueming Shen wrote:
>>> On 3/13/18, 5:12 PM, Jonathan Bluett-Duncan wrote:
 Paul,

 AFAICT, one sort of behaviour which String.split() allows which
 Pattern.splitAsStream() and the proposed String.splits() don't is
 allowing
 a negative limit, e.g. String.split(string, -1).

 Over at http://errorprone.info/bugpattern/StringSplitter, they argue
 that a
 limit of -1 has less surprising behaviour than the default of 0,
 because
 e.g. "".split(":") produces [] (empty array), whereas ":".split(":")
 produces [""] (array with an empty string), which IMO is not
 consistent.

 This compares with ":".split(":", -1) and "".split(":", -1) which
 produce
 ["", ""] (array with two empty strings, each representing ends of
 `:`) and
 [] (empty array) respectively - more consistent IMO.

 Should String.splits(`\n|\r\n?`) follow the behaviour of
 String.split(...,
 0) or String.split(..., -1)?  I'd personally argue for the latter.
>>>
>>> While these look really confusing, but ":".split(":", n) and
>>> "".split(":", n) are really two
>>> different scenario. One is for a matched delimiter and the other is a
>>> split with no
>>> matched delimiter, in which the spec specifies clearly that it
>>> returns the original string,
>>> in this case the empty string "". Arguably these two don't have to be
>>> "consistent".
>>>
>>> Personally I think the returned list/array from string.split(regex,
>>> -1) might be kinda of
>>> "surprising" for end user, in which it has a "trailing" empty string,
>>> as it appears to be
>>> useless in most use scenario and you probably have to do some special
>>> deal with it.
>>>
>>> -Sherman
>>>
>>>
>>>

 Cheers,
 Jonathan

 On 13 March 2018 at 23:22, Paul Sandoz  wrote:

>
>> On Mar 13, 2018, at 3:49 PM, John Rose
>> wrote:
>>
>> On Mar 13, 2018, at 6:47 AM, Jim Laskey
>> wrote:
>>> …
>>> A. Line support.
>>>
>>> public Stream  lines()
>>>
>> Suggest factoring this as:
>>
>> public Stream  splits(String regex) { }
> +1
>
> This is a natural companion to the existing array returning method
> (as it
> was the case on Pattern when we added splitAsStream), where one can
> use a
> limit() operation to achieve the same effect as the limit parameter
> on the
> array returning method.
>
>
>> public Stream  lines() { return splits(`\n|\r\n?`); }
>>
> See also Files/BufferedReader.lines. (Without going into details
> Files.lines has some interesting optimizations.)
>
> Paul.
>>>

Re: RFR of 8180451: ByteArrayInputStream should override readAllBytes, readNBytes, and transferTo

[Brian Burkhalter]

Reprising this thread from three months ago [1].

A patch including the changes suggested below is at

http://cr.openjdk.java.net/~bpb/8180451/webrev.01/

with the differences between this and the prior version at

http://cr.openjdk.java.net/~bpb/8180451/webrev.00-01/

Thanks,

Brian

[1] 
http://mail.openjdk.java.net/pipermail/core-libs-dev/2017-December/050550.html

On Dec 12, 2017, at 11:42 AM, Paul Sandoz  wrote:

> 208 public synchronized long transferTo(OutputStream out) throws 
> IOException {
> 209 int pos0 = pos;
> 210 out.write(buf, pos, count - pos);
> 211 pos = count;
> 212 return count - pos0;
> 213 }
> 
>  int len = count - pos
>  out.write(but, pos, len);
>  pos = count;
>  return len;

On Dec 12, 2017, at 12:23 PM, Brent Christian  
wrote:

> The changes look fine to me.
> I would have found the test case a little easier to follow if "off"/"len" 
> weren't named so similarly to "offset"/"length", but it's not a big deal.

Re: RFR: 8199471: Enable generation of callSiteForms at link time

[Paul Sandoz]

> On Mar 14, 2018, at 7:09 AM, Claes Redestad  wrote:
> 
> Hi Paul,
> 
> On 2018-03-13 20:32, Paul Sandoz wrote:
>> Invokers.java
>> —
>> 
>> Looks good.
> 
> Thanks!
> 
>> 
>> Minor comment:
>> 
>>  664 /* Placeholder class for Invokers generated ahead of time */
>>  665 final class Holder {}
>>  666
>>  667 /* Placeholder class for callSiteForms generated ahead of time */
>>  668 final class CSHolder {}
>> 
>> is it easy for you to change, for clarity, Holder to InvokersHolder and 
>> CSHolder to CallSiteHolder?
> 
> I instead took a stab at consolidating these two holder classes into one,
> and instead of unnecessarily complicating the code (as I had feared), it
> actually reduced duplicated code, only slightly complicated one of
> the helper methods and made the patch smaller overall:
> 
> http://cr.openjdk.java.net/~redestad/8199471/open.01/
> 

Better!

Paul.

Re: Raw String Literal Library Support

[Brian Goetz]

Perhaps we can "split" this discussion on splitting into a separate 
thread.  What's happened here is what always happens, which is:


 - Jim spent a lot of time and effort writing a comprehensive and clear 
proposal;

 - Someone made a tangential comment on one aspect of it;
 - Flood of deep-dive responses on that aspect;
 - Everyone chimes in designing their favorite method not proposed;
 - No one ever comes back to the substance of the proposal.

Hitting the reset button...


On 3/14/2018 9:11 AM, Peter Levart wrote:

I think that:

String delim = ...;
String r = 
s.splits(Pattern.quote(delim)).collect(Collectors.joining(delim));


... should always produce a result such that r.equals(s);


Otherwise, is it wise to add methods that take a regex as a String? It 
is rarely needed for a regex parameter to be dynamic. Usually a 
constant is specified. Are there any plans for Java to support Pattern 
constants? With constant dynamic support they would be trivial to 
implement in bytecode. If there are any such plans, then the methods 
should perhaps take a Pattern instead.


syntax suggestion:

'~' is an unary operator for bit-wise negation of integer values. It 
could be overloaded for String(s) such that the following two were 
equivalent:


~ string
Pattern.compile(string)

Now if 'string' above is a constant, '~ string' could be a constant 
too. Combined with raw string literals, Pattern constants could be 
very compact.



What do you think?

Regards, Peter

On 03/14/2018 02:35 AM, Xueming Shen wrote:

On 3/13/18, 5:12 PM, Jonathan Bluett-Duncan wrote:

Paul,

AFAICT, one sort of behaviour which String.split() allows which
Pattern.splitAsStream() and the proposed String.splits() don't is 
allowing

a negative limit, e.g. String.split(string, -1).

Over at http://errorprone.info/bugpattern/StringSplitter, they argue 
that a
limit of -1 has less surprising behaviour than the default of 0, 
because

e.g. "".split(":") produces [] (empty array), whereas ":".split(":")
produces [""] (array with an empty string), which IMO is not 
consistent.


This compares with ":".split(":", -1) and "".split(":", -1) which 
produce
["", ""] (array with two empty strings, each representing ends of 
`:`) and

[] (empty array) respectively - more consistent IMO.

Should String.splits(`\n|\r\n?`) follow the behaviour of 
String.split(...,

0) or String.split(..., -1)?  I'd personally argue for the latter.


While these look really confusing, but ":".split(":", n) and 
"".split(":", n) are really two
different scenario. One is for a matched delimiter and the other is a 
split with no
matched delimiter, in which the spec specifies clearly that it 
returns the original string,
in this case the empty string "". Arguably these two don't have to be 
"consistent".


Personally I think the returned list/array from string.split(regex, 
-1) might be kinda of
"surprising" for end user, in which it has a "trailing" empty string, 
as it appears to be
useless in most use scenario and you probably have to do some special 
deal with it.


-Sherman





Cheers,
Jonathan

On 13 March 2018 at 23:22, Paul Sandoz  wrote:



On Mar 13, 2018, at 3:49 PM, John Rose  
wrote:


On Mar 13, 2018, at 6:47 AM, Jim Laskey  
wrote:

…
A. Line support.

public Stream  lines()


Suggest factoring this as:

public Stream  splits(String regex) { }

+1

This is a natural companion to the existing array returning method 
(as it
was the case on Pattern when we added splitAsStream), where one can 
use a
limit() operation to achieve the same effect as the limit parameter 
on the

array returning method.



public Stream  lines() { return splits(`\n|\r\n?`); }


See also Files/BufferedReader.lines. (Without going into details
Files.lines has some interesting optimizations.)

Paul.

Re: Raw String Literal Library Support

[John Rose]

On Mar 14, 2018, at 6:11 AM, Peter Levart  wrote:
> 
> Pattern.compile(string)
> 
> Now if 'string' above is a constant, '~ string' could be a constant too. 
> Combined with raw string literals, Pattern constants could be very compact.
> 
> 
> What do you think?

There's no need to introduce syntax in order to gain constant folding.

It's enough to ensure that Pattern.compiler(constant) reduces to
the ldc of a dynamic constant.  We are experimenting on such
ideas here:
  http://hg.openjdk.java.net/amber/amber/shortlog/condy-folding 


(This is very vaguely similar to constexpr in C++, but less static.
It's early days, but enough to show that syntax isn't necessary.)

— John

Re: RFR: 8199471: Enable generation of callSiteForms at link time

[Claes Redestad]

Hi Paul,

On 2018-03-13 20:32, Paul Sandoz wrote:

Invokers.java
—

Looks good.


Thanks!



Minor comment:

  664 /* Placeholder class for Invokers generated ahead of time */
  665 final class Holder {}
  666
  667 /* Placeholder class for callSiteForms generated ahead of time */
  668 final class CSHolder {}

is it easy for you to change, for clarity, Holder to InvokersHolder and 
CSHolder to CallSiteHolder?


I instead took a stab at consolidating these two holder classes into one,
and instead of unnecessarily complicating the code (as I had feared), it
actually reduced duplicated code, only slightly complicated one of
the helper methods and made the patch smaller overall:

http://cr.openjdk.java.net/~redestad/8199471/open.01/

/Claes

Re: RFR 8199464 [11] Remove remaining vestiges of Java_sun_reflect_Reflection_getCallerClass

[Chris Hegarty]

On 14/03/18 13:08, Alan Bateman wrote:

On 14/03/2018 12:56, Chris Hegarty wrote:

This is a review request to remove remaining vestiges of
Java_sun_reflect_Reflection_getCallerClass.

JDK-8179424 removed terminally deprecated 
jdk.unsupported/sun.reflect.Reflection.getCallerClass(int), these 
references are to the no-args getCallerClass that was removed a long 
time ago. These should be cleaned up anyway.
Looks okay to me. AFAIK, the reorder- files are essentially 
obsolete, at least I haven't seen any updates promoted by startup 
benchmarks in a long time.


The following warning was noted recently in 8179424, so I
decided to just remove these. Somebody must be still using
them :-(

 ld: warning: mapfile: text segment: section 
'.text%Java_sun_reflect_Reflection_getCallerClass__' does not appear in 
any input file


-Chris.

Re: Raw String Literal Library Support

[Peter Levart]

I think that:

String delim = ...;
String r = 
s.splits(Pattern.quote(delim)).collect(Collectors.joining(delim));


... should always produce a result such that r.equals(s);


Otherwise, is it wise to add methods that take a regex as a String? It 
is rarely needed for a regex parameter to be dynamic. Usually a constant 
is specified. Are there any plans for Java to support Pattern constants? 
With constant dynamic support they would be trivial to implement in 
bytecode. If there are any such plans, then the methods should perhaps 
take a Pattern instead.


syntax suggestion:

'~' is an unary operator for bit-wise negation of integer values. It 
could be overloaded for String(s) such that the following two were 
equivalent:


~ string
Pattern.compile(string)

Now if 'string' above is a constant, '~ string' could be a constant too. 
Combined with raw string literals, Pattern constants could be very compact.



What do you think?

Regards, Peter

On 03/14/2018 02:35 AM, Xueming Shen wrote:

On 3/13/18, 5:12 PM, Jonathan Bluett-Duncan wrote:

Paul,

AFAICT, one sort of behaviour which String.split() allows which
Pattern.splitAsStream() and the proposed String.splits() don't is 
allowing

a negative limit, e.g. String.split(string, -1).

Over at http://errorprone.info/bugpattern/StringSplitter, they argue 
that a

limit of -1 has less surprising behaviour than the default of 0, because
e.g. "".split(":") produces [] (empty array), whereas ":".split(":")
produces [""] (array with an empty string), which IMO is not consistent.

This compares with ":".split(":", -1) and "".split(":", -1) which 
produce
["", ""] (array with two empty strings, each representing ends of 
`:`) and

[] (empty array) respectively - more consistent IMO.

Should String.splits(`\n|\r\n?`) follow the behaviour of 
String.split(...,

0) or String.split(..., -1)?  I'd personally argue for the latter.


While these look really confusing, but ":".split(":", n) and 
"".split(":", n) are really two
different scenario. One is for a matched delimiter and the other is a 
split with no
matched delimiter, in which the spec specifies clearly that it returns 
the original string,
in this case the empty string "". Arguably these two don't have to be 
"consistent".


Personally I think the returned list/array from string.split(regex, 
-1) might be kinda of
"surprising" for end user, in which it has a "trailing" empty string, 
as it appears to be
useless in most use scenario and you probably have to do some special 
deal with it.


-Sherman





Cheers,
Jonathan

On 13 March 2018 at 23:22, Paul Sandoz  wrote:




On Mar 13, 2018, at 3:49 PM, John Rose  wrote:

On Mar 13, 2018, at 6:47 AM, Jim Laskey  
wrote:

…
A. Line support.

public Stream  lines()


Suggest factoring this as:

public Stream  splits(String regex) { }

+1

This is a natural companion to the existing array returning method 
(as it
was the case on Pattern when we added splitAsStream), where one can 
use a
limit() operation to achieve the same effect as the limit parameter 
on the

array returning method.



public Stream  lines() { return splits(`\n|\r\n?`); }


See also Files/BufferedReader.lines. (Without going into details
Files.lines has some interesting optimizations.)

Paul.

Re: RFR 8199464 [11] Remove remaining vestiges of Java_sun_reflect_Reflection_getCallerClass

[Alan Bateman]

On 14/03/2018 12:56, Chris Hegarty wrote:

This is a review request to remove remaining vestiges of
Java_sun_reflect_Reflection_getCallerClass.

JDK-8179424 removed terminally deprecated 
jdk.unsupported/sun.reflect.Reflection.getCallerClass(int), these 
references are to the no-args getCallerClass that was removed a long 
time ago. These should be cleaned up anyway.
Looks okay to me. AFAIK, the reorder- files are essentially 
obsolete, at least I haven't seen any updates promoted by startup 
benchmarks in a long time.


-Alan

RE: RFR 8199464 [11] Remove remaining vestiges of Java_sun_reflect_Reflection_getCallerClass

[Langer, Christoph]

Looks good, Chris.

> -Original Message-
> From: core-libs-dev [mailto:core-libs-dev-boun...@openjdk.java.net] On
> Behalf Of Chris Hegarty
> Sent: Mittwoch, 14. März 2018 13:57
> To: build-dev ; Core-Libs-Dev  d...@openjdk.java.net>
> Subject: RFR 8199464 [11] Remove remaining vestiges of
> Java_sun_reflect_Reflection_getCallerClass
> 
> This is a review request to remove remaining vestiges of
> Java_sun_reflect_Reflection_getCallerClass.
> 
> JDK-8179424 removed terminally deprecated
> jdk.unsupported/sun.reflect.Reflection.getCallerClass(int), these
> references are to the no-args getCallerClass that was removed a long
> time ago. These should be cleaned up anyway.
> 
> 
> diff --git a/make/mapfiles/libjava/reorder-sparc
> b/make/mapfiles/libjava/reorder-sparc
> --- a/make/mapfiles/libjava/reorder-sparc
> +++ b/make/mapfiles/libjava/reorder-sparc
> @@ -26,7 +26,6 @@
>   text: .text%Java_java_io_FileDescriptor_initIDs;
>   text: .text%Java_java_io_FileOutputStream_initIDs;
>   text: .text%Java_java_lang_System_setIn0;
> -text: .text%Java_sun_reflect_Reflection_getCallerClass__;
>   text: .text%Java_java_lang_Class_forName0;
>   text: .text%Java_java_lang_Object_getClass;
>   text: .text%Java_sun_reflect_Reflection_getClassAccessFlags;
> diff --git a/make/mapfiles/libjava/reorder-sparcv9
> b/make/mapfiles/libjava/reorder-sparcv9
> --- a/make/mapfiles/libjava/reorder-sparcv9
> +++ b/make/mapfiles/libjava/reorder-sparcv9
> @@ -25,7 +25,6 @@
>   text: .text%Java_java_io_FileDescriptor_initIDs;
>   text: .text%Java_java_io_FileOutputStream_initIDs;
>   text: .text%Java_java_lang_System_setIn0;
> -text: .text%Java_sun_reflect_Reflection_getCallerClass__;
>   text: .text%Java_java_lang_Class_forName0;
>   text: .text%Java_java_lang_String_intern;
>   text: .text%Java_java_lang_StringUTF16_isBigEndian;
> diff --git a/make/mapfiles/libjava/reorder-x86
> b/make/mapfiles/libjava/reorder-x86
> --- a/make/mapfiles/libjava/reorder-x86
> +++ b/make/mapfiles/libjava/reorder-x86
> @@ -26,7 +26,6 @@
>   text: .text%Java_java_io_FileDescriptor_initIDs;
>   text: .text%Java_java_io_FileOutputStream_initIDs;
>   text: .text%Java_java_lang_System_setIn0;
> -text: .text%Java_sun_reflect_Reflection_getCallerClass__;
>   text: .text%Java_java_lang_Class_forName0;
>   text: .text%Java_java_lang_String_intern;
>   text: .text%Java_java_lang_StringUTF16_isBigEndian;
> 
> 
> -Chris.

RFR 8199464 [11] Remove remaining vestiges of Java_sun_reflect_Reflection_getCallerClass

[Chris Hegarty]

This is a review request to remove remaining vestiges of
Java_sun_reflect_Reflection_getCallerClass.

JDK-8179424 removed terminally deprecated 
jdk.unsupported/sun.reflect.Reflection.getCallerClass(int), these 
references are to the no-args getCallerClass that was removed a long 
time ago. These should be cleaned up anyway.



diff --git a/make/mapfiles/libjava/reorder-sparc 
b/make/mapfiles/libjava/reorder-sparc

--- a/make/mapfiles/libjava/reorder-sparc
+++ b/make/mapfiles/libjava/reorder-sparc
@@ -26,7 +26,6 @@
 text: .text%Java_java_io_FileDescriptor_initIDs;
 text: .text%Java_java_io_FileOutputStream_initIDs;
 text: .text%Java_java_lang_System_setIn0;
-text: .text%Java_sun_reflect_Reflection_getCallerClass__;
 text: .text%Java_java_lang_Class_forName0;
 text: .text%Java_java_lang_Object_getClass;
 text: .text%Java_sun_reflect_Reflection_getClassAccessFlags;
diff --git a/make/mapfiles/libjava/reorder-sparcv9 
b/make/mapfiles/libjava/reorder-sparcv9

--- a/make/mapfiles/libjava/reorder-sparcv9
+++ b/make/mapfiles/libjava/reorder-sparcv9
@@ -25,7 +25,6 @@
 text: .text%Java_java_io_FileDescriptor_initIDs;
 text: .text%Java_java_io_FileOutputStream_initIDs;
 text: .text%Java_java_lang_System_setIn0;
-text: .text%Java_sun_reflect_Reflection_getCallerClass__;
 text: .text%Java_java_lang_Class_forName0;
 text: .text%Java_java_lang_String_intern;
 text: .text%Java_java_lang_StringUTF16_isBigEndian;
diff --git a/make/mapfiles/libjava/reorder-x86 
b/make/mapfiles/libjava/reorder-x86

--- a/make/mapfiles/libjava/reorder-x86
+++ b/make/mapfiles/libjava/reorder-x86
@@ -26,7 +26,6 @@
 text: .text%Java_java_io_FileDescriptor_initIDs;
 text: .text%Java_java_io_FileOutputStream_initIDs;
 text: .text%Java_java_lang_System_setIn0;
-text: .text%Java_sun_reflect_Reflection_getCallerClass__;
 text: .text%Java_java_lang_Class_forName0;
 text: .text%Java_java_lang_String_intern;
 text: .text%Java_java_lang_StringUTF16_isBigEndian;


-Chris.