Re: RFR: 8196750 [Testbug] tools/launcher tests need to tolerate unrelated warnings

[David Holmes]

On 22/03/2018 12:52 PM, Andrei Nazarov wrote:

On Mar 21, 2018 19:13, David Holmes  wrote:

Hi Andrei,

On 22/03/2018 11:12 AM, Andrey Nazarov wrote:
 > Hi,
 >
 > Please review fix in launcher tests.
 >
 > Review: http://cr.openjdk.java.net/~anazarov/JDK-8196750/webrev/
 > Bug: https://bugs.openjdk.java.net/browse/JDK-8196750?filter=-1

test/jdk/tools/launcher/ToolsOpts.java

I don't understand how the change fixes the problem. IIUC the
problem is
that the test expects the output to consist of the command-line passed
to the tool. Instead if it contains a Warning from the VM, it won't
match and so we fail. The new code no longer uses a String[] and no
longer assumes that output[i] == args[i],

output now is List and
output.get(j).equals(arg[j]) has the same sematics.
The difference is that it *searches* args as a line in the output, so it 
would skip any Warnings lines.


So for each arg it searches the entire output.

Okay I see now.

Thanks,
David
-

Output is scanned once so it checks order of args as well.

-- Andrei

it still searches the
current "output" value for a match with one of the args. But that will
still fail if what we actually have is a Warning. ?? I would have
expected to see the Warning strings filtered out more directly.

The other test change seems reasonable.

Thanks,
David

 > —Thanks,
 >   Andrei
 >

RFR 8198882: Add 10 JNDI tests to com/sun/jndi/dns/AttributeTests/

[Chris Yin]

Please review the changes to add 10 JNDI tests to 
com/sun/jndi/dns/AttributeTests/, thanks

bug: https://bugs.openjdk.java.net/browse/JDK-8198882 

webrev: http://cr.openjdk.java.net/~xyin/8198882/webrev.00/ 


Regards,
Chris

Re: RFR: 8196750 [Testbug] tools/launcher tests need to tolerate unrelated warnings

[David Holmes]

Hi Andrei,

On 22/03/2018 11:12 AM, Andrey Nazarov wrote:

Hi,

Please review fix in launcher tests.

Review: http://cr.openjdk.java.net/~anazarov/JDK-8196750/webrev/
Bug: https://bugs.openjdk.java.net/browse/JDK-8196750?filter=-1


test/jdk/tools/launcher/ToolsOpts.java

I don't understand how the change fixes the problem. IIUC the problem is 
that the test expects the output to consist of the command-line passed 
to the tool. Instead if it contains a Warning from the VM, it won't 
match and so we fail. The new code no longer uses a String[] and no 
longer assumes that output[i] == args[i], but it still searches the 
current "output" value for a match with one of the args. But that will 
still fail if what we actually have is a Warning. ?? I would have 
expected to see the Warning strings filtered out more directly.


The other test change seems reasonable.

Thanks,
David


—Thanks,
  Andrei

RFR: 8196750 [Testbug] tools/launcher tests need to tolerate unrelated warnings

[Andrey Nazarov]

Hi,

Please review fix in launcher tests.

Review: http://cr.openjdk.java.net/~anazarov/JDK-8196750/webrev/
Bug: https://bugs.openjdk.java.net/browse/JDK-8196750?filter=-1

—Thanks,
 Andrei

Re: [11] RFR: 8193128: Reduce number of implementation classes returned by List/Set/Map.of()

[Stuart Marks]

Hi Claes,

I'm finally finally getting back to this. I reviewed the current state of the 
patch located here:


http://cr.openjdk.java.net/~redestad/8193128/open.06/

I think this is mostly fine as it is. There are some things about it that could 
be adjusted, but none of them stand in the way of it going in. If you want to 
take care of any of these items before pushing, that'd be great, otherwise they 
can be handled separately later.


* AbstractImmutableCollection and AbstractImmutableSet are in the "List 
Implementations" section of the file. Seems like AIC ought to be moved to the 
top, since it's common to List and Set, and AIS ought to be moved to the "Set 
Implementations" section. This is just location in the file, not nesting level.


* The SubList constructors that are overloaded based on the type of the 1st arg 
(List vs SubList) seems subtle and error-prone. I misread the code the first 
time I saw it. Seems like it would be preferable to have well-named static 
factory methods, each calling a policy-free (private) constructor.


* Should SubList.size be final? Should any of the SubList fields be @Stable?

* Does the SubList class need to nested within AbstractImmutableList? Note that 
it also extends AIL, so I don't think nesting gives it access to anything that 
it doesn't already have. Plus it includes an anonymous inner class based on 
ListIterator, which is a fourth level of nested classes. It'd be good to flatten 
this out a bit if possible.


* The instance returned by SubList.iterator() is also a ListIterator. Hmmm. But 
I'm also wondering if SubList's iterators can be shared somehow with AIL's Itr 
and ListItr.


* Several indexOf tests are added to ListFactories.java. Are these redundant 
with the indexOf tests that were added to MOAT?


Let me know what, if any, you fix up before pushing, and I'll track the rest.

Thanks,

s'marks

Re: Reactive Streams utility API

[James Roper]

Hi all,

We've created a reference implementation. It's been done in such a way that
implementation of new features (stages) is quite straight forward, there is
an abstraction that does the hard work of handling concurrency, the
reactive streams spec conformance, and managing demand and buffering, and
error handling, and then individual stages (eg, map, filter, flatMap) are
implemented using a very easy to use API (note, this API/abstraction is all
private, internal to the reference implementation). Rudimentary tests on
performance show that it's not terrible compared to other reactive streams
implementations, with a number of clear optimization paths identified
should we decide that's necessary. I believe this proposal is now close to
complete - the remaining work is:

* Decide what scope beyond JDK8 streams it should support - while this
decision is not trivial, the amount of work required to actually add these
to the API and implement in the reference implementation is trivial.
* Fill out the TCK with more rigorous verification.
* Create some rigorous benchmarks.

I'm not sure what should be done next. I have talked to a number of people
who are either involved in, or are writing APIs that use Reactive Streams
in private, and interest seems high. Also, there is general consensus in
public discussions in the Jakarta EE/MicroProfile communities that an API
like this would be very valuable in the JDK. The API of course could be
done in Jakarta EE instead, but given that Reactive Streams is part of and
used by the JDK, and given that the JDK8 Streams API is part of the JDK,
Jakarta EE would seem an odd place to put this library - it essentially
would mean that to make effective use of JDK libraries that use Reactive
Streams (eg HTTP client, possibly java.sql2 aka ADBA), you need to use
Jakarta EE (or some third party streaming library).

So unless there's any major feedback coming here on this list, I'd like to
put this forward as a JEP.

Regards,

James

On 15 March 2018 at 12:24, James Roper  wrote:

> 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

Re: [11] [PATCH] 9053056: Anchor name conflict in ClassLoader JavaDoc

[Martin Buchholz]

Hmmm... it's a hard problem because we intentionally have invalid html in
our private javadoc, since it's designed primarily for human readers, e.g.

error: malformed HTML
  [javadoc]  * Precondition and postcondition: 0 <= i < modulus.

jsr166 has:

  [javadoc] 86 errors
  [javadoc] 780 warnings

... the world wants to be able to use markdown in javadoc ...

Re: RFR (JDK11/doc-only) 8199176: Accessibility issues in java.base docs

[Joe Wang]

They look ok. They actually go better with the package name when they 
are just 1 level down instead of 2, that is,  rather than  for 
the titles when the package name is . If we don't want the strong 
emphasis on the titles, we could lower the package name generated by 
javadoc to  instead of . Then we could keep the s.


Most of the package-info pages have already used  for titles, which 
is why we're fixing only a handful of them here. Consider:


*Currently using :*
https://docs.oracle.com/javase/10/docs/api//java/io/package-summary.html
https://docs.oracle.com/javase/10/docs/api//java/lang/invoke/package-summary.html

In the later page, it actually started with  :
 Summary of relevant Java Virtual Machine 
changes


*Currently using :*
https://docs.oracle.com/javase/10/docs/api//javax/xml/transform/package-summary.html

-Joe


On 3/21/2018 2:39 PM, Jonathan Gibbons wrote:



On 3/21/18 2:28 PM, Alan Bateman wrote:

On 21/03/2018 19:08, Joe Wang wrote:

:

*Item 3*: Heading leavels should only increase by one
    This is due to the javadoc tool generates a header with an 
addition of headings, in this particular case, . The fix for 
this particular case is to replace the s with .
I assume this has the effect of increasing the size of the headings. 
Does it look okay? I assume many of these usages of h3 choose that to 
avoid heading in very large font.


-Alan


I know accessibility fixes are important, but usability is important 
too.  The uses of h3 were previously consistent across different pages.


We could remediate the problem either by figuring a design that has 
javadoc put a similar set of headers on every page (thus rendering 
this changeset unnecessary) or else changing the stylesheet so that 
 on a package-summary page looks the same as  on a class page 
(which sounds icky, even as I type it.)


-- Jon

Re: [11] [PATCH] 9053056: Anchor name conflict in ClassLoader JavaDoc

[mandy chung]

On 3/21/18 1:59 PM, Jonathan Gibbons wrote:



On 03/21/2018 01:39 PM, mandy chung wrote:



On 3/21/18 10:26 AM, David Lloyd wrote:

I see it with IntelliJ IDEA when I pop up "quick JavaDoc" on, say,
defineClass and then click on "binary name"; it's actually using the
sources, rather than generated JavaDoc.


I don't object to do this renaming as I favor this more explicit id.  
However you will hit the same issue if another class names an anchor 
conflicts with its private member.   Can you configure IntelliJ to 
generate public/protected elements to avoid this issue?

Mandy


Mandy,

I think that maybe we should establish a convention such that 
user-defined anchors can never conflict with javadoc-generated 
anchors. Using an embedded "-" (as in this proposal) in user-defined 
anchors is a good start.


A secondary proposal would be to have doclint check for potential 
clashes. Even though we may use access filters to restrict the set of 
elements included in any documentation, it should never be good to 
facilitate clashes that may be caused by non-default options or 
non-default tools.




I forgot that you are on this list.

This is a good idea.  This is what I was planning to brainstorm with you 
what we could do to help catch this and generate clean javadoc including 
private elements.


Mandy

Re: RFR (JDK11/doc-only) 8199176: Accessibility issues in java.base docs

[Jonathan Gibbons]

On 3/21/18 2:28 PM, Alan Bateman wrote:

On 21/03/2018 19:08, Joe Wang wrote:

:

*Item 3*: Heading leavels should only increase by one
    This is due to the javadoc tool generates a header with an 
addition of headings, in this particular case, . The fix for this 
particular case is to replace the s with .
I assume this has the effect of increasing the size of the headings. 
Does it look okay? I assume many of these usages of h3 choose that to 
avoid heading in very large font.


-Alan


I know accessibility fixes are important, but usability is important 
too.  The uses of h3 were previously consistent across different pages.


We could remediate the problem either by figuring a design that has 
javadoc put a similar set of headers on every page (thus rendering this 
changeset unnecessary) or else changing the stylesheet so that  on a 
package-summary page looks the same as  on a class page (which 
sounds icky, even as I type it.)


-- Jon

Re: RFR (JDK11/doc-only) 8199176: Accessibility issues in java.base docs

[Alan Bateman]

On 21/03/2018 19:08, Joe Wang wrote:

:

*Item 3*: Heading leavels should only increase by one
    This is due to the javadoc tool generates a header with an 
addition of headings, in this particular case, . The fix for this 
particular case is to replace the s with .
I assume this has the effect of increasing the size of the headings. 
Does it look okay? I assume many of these usages of h3 choose that to 
avoid heading in very large font.


-Alan

Re: [11] [PATCH] 9053056: Anchor name conflict in ClassLoader JavaDoc

[Jonathan Gibbons]

On 3/21/18 2:18 PM, Martin Buchholz wrote:



On Wed, Mar 21, 2018 at 1:59 PM, Jonathan Gibbons 
mailto:jonathan.gibb...@oracle.com>> wrote:



I think that maybe we should establish a convention such that
user-defined anchors can never conflict with javadoc-generated
anchors. Using an embedded "-" (as in this proposal) in
user-defined anchors is a good start.


It never occurred to me that use of "-" in an anchor can prevent 
future conflicts with java identifiers.  This is another reason to use 
multi-word anchors with "-" as separator, which I was already 
personally leaning towards.
If you're (still) using javadoc with -html4, then the set of legal 
characters in an id is limited, and javadoc makes use of '-' to encode 
signatures, so you're still somewhat at risk for a clash. But, if you 
use -html5, the set of characters is much less limited ("no spaces") and 
javadoc does not resort to encoding with `-`. All of which says, "use 
'-' and HTML5".



A secondary proposal would be to have doclint check for potential
clashes. Even though we may use access filters to restrict the set
of elements included in any documentation, it should never be good
to facilitate clashes that may be caused by non-default options or
non-default tools.


Javadoc runs with -private over all the sources needs a lower bar than 
with the default, but certain classes of errors like broken links 
should be made impossible by design of the release process.

I foresee an investigation and potential cleanup :-)

-- Jon

Re: RFR 8180410: ByteArrayOutputStream should not throw IOExceptions

[Brian Burkhalter]

On Mar 21, 2018, at 2:21 PM, Stuart Marks  wrote:

>>> Such a refactoring would be prohibited by an @implSpec.
>> Why then not an @implNote instead of an @apiNote?
> The statement that was proposed:
> This method is equivalent to write(b, 0, b.length).
> is a statement about the semantics of writeBytes(b). Put another way, it 
> states that the requirements on the behavior of writeBytes(b) are equivalent 
> to those on write(b, 0, b.length). Those are statements about the API.
> 
> I don't think anything needs to be said about implementation.

I can see what you are saying now. It would be good to have comments from 
someone else as well.

Thanks,

Brian

Re: RFR 8180410: ByteArrayOutputStream should not throw IOExceptions

[Stuart Marks]

On 3/21/18 1:27 PM, Brian Burkhalter wrote:
On Mar 21, 2018, at 1:23 PM, Stuart Marks > wrote:

Perhaps instead the verbiage just needs to be changed to, e.g.,

“The implementation in this class simply invokes {@link 
#write(byte[],int,int) write(b, 0, b.length)}.”

I don't think you want to specify this detail about the implementation.

Consider a possible future refactoring, where writeBytes() calls some 
internal method instead of calling write(b, 0, b.length). This might be done, 
for example, to leverage an intrinsic, or to avoid unnecessary bounds checks, 
or for some other reason we can't imagine at the moment.


Such a refactoring would be prohibited by an @implSpec.

Why then not an @implNote instead of an @apiNote?

The statement that was proposed:

   This method is equivalent to write(b, 0, b.length).

is a statement about the semantics of writeBytes(b). Put another way, it states 
that the requirements on the behavior of writeBytes(b) are equivalent to those 
on write(b, 0, b.length). Those are statements about the API.


I don't think anything needs to be said about implementation.

s'marks

Re: [11] [PATCH] 9053056: Anchor name conflict in ClassLoader JavaDoc

[Martin Buchholz]

On Wed, Mar 21, 2018 at 1:59 PM, Jonathan Gibbons <
jonathan.gibb...@oracle.com> wrote:

>
> I think that maybe we should establish a convention such that user-defined
> anchors can never conflict with javadoc-generated anchors. Using an
> embedded "-" (as in this proposal) in user-defined anchors is a good start.
>

It never occurred to me that use of "-" in an anchor can prevent future
conflicts with java identifiers.  This is another reason to use multi-word
anchors with "-" as separator, which I was already personally leaning
towards.


> A secondary proposal would be to have doclint check for potential clashes.
> Even though we may use access filters to restrict the set of elements
> included in any documentation, it should never be good to facilitate
> clashes that may be caused by non-default options or non-default tools.
>

Javadoc runs with -private over all the sources needs a lower bar than with
the default, but certain classes of errors like broken links should be made
impossible by design of the release process.

Re: [11] [PATCH] 9053056: Anchor name conflict in ClassLoader JavaDoc

[Martin Buchholz]

On Wed, Mar 21, 2018 at 2:03 PM, David Lloyd  wrote:

> On Wed, Mar 21, 2018 at 3:39 PM, mandy chung 
> wrote:
> > On 3/21/18 10:26 AM, David Lloyd wrote:
> >
> > I see it with IntelliJ IDEA when I pop up "quick JavaDoc" on, say,
> > defineClass and then click on "binary name"; it's actually using the
> > sources, rather than generated JavaDoc.
> >
> >
> > I don't object to do this renaming as I favor this more explicit id.
> > However you will hit the same issue if another class names an anchor
> > conflicts with its private member.   Can you configure IntelliJ to
> generate
> > public/protected elements to avoid this issue?
>
> My impression is that it is generating its indexes and rendering on
> the fly (or at least in some opaque manner) directly from the sources.
> There does not seem to be any setting which governs what is produced
> or displayed; I think it is generally a feature (not a bug) that one
> can browse into dependency and JDK sources and get quick JavaDoc on
> any element, even non-public elements.
>

I agree with David - it's a feature of Intellij to make use of any
documentation available in the sources, including html links in private
javadoc.

Private method javadoc is less important than its public sibling.
Nevertheless, just like with any other documentation, it requires
maintenance.

Re: [11] [PATCH] 9053056: Anchor name conflict in ClassLoader JavaDoc

[David Lloyd]

On Wed, Mar 21, 2018 at 3:39 PM, mandy chung  wrote:
> On 3/21/18 10:26 AM, David Lloyd wrote:
>
> I see it with IntelliJ IDEA when I pop up "quick JavaDoc" on, say,
> defineClass and then click on "binary name"; it's actually using the
> sources, rather than generated JavaDoc.
>
>
> I don't object to do this renaming as I favor this more explicit id.
> However you will hit the same issue if another class names an anchor
> conflicts with its private member.   Can you configure IntelliJ to generate
> public/protected elements to avoid this issue?

My impression is that it is generating its indexes and rendering on
the fly (or at least in some opaque manner) directly from the sources.
There does not seem to be any setting which governs what is produced
or displayed; I think it is generally a feature (not a bug) that one
can browse into dependency and JDK sources and get quick JavaDoc on
any element, even non-public elements.

-- 
- DML

Re: [11] [PATCH] 9053056: Anchor name conflict in ClassLoader JavaDoc

[Jonathan Gibbons]

On 03/21/2018 01:39 PM, mandy chung wrote:



On 3/21/18 10:26 AM, David Lloyd wrote:

I see it with IntelliJ IDEA when I pop up "quick JavaDoc" on, say,
defineClass and then click on "binary name"; it's actually using the
sources, rather than generated JavaDoc.


I don't object to do this renaming as I favor this more explicit id.  
However you will hit the same issue if another class names an anchor 
conflicts with its private member.   Can you configure IntelliJ to 
generate public/protected elements to avoid this issue?

Mandy


Mandy,

I think that maybe we should establish a convention such that 
user-defined anchors can never conflict with javadoc-generated anchors. 
Using an embedded "-" (as in this proposal) in user-defined anchors is a 
good start.


A secondary proposal would be to have doclint check for potential 
clashes. Even though we may use access filters to restrict the set of 
elements included in any documentation, it should never be good to 
facilitate clashes that may be caused by non-default options or 
non-default tools.


-- Jon

Re: [11] [PATCH] 9053056: Anchor name conflict in ClassLoader JavaDoc

[mandy chung]

On 3/21/18 10:26 AM, David Lloyd wrote:

I see it with IntelliJ IDEA when I pop up "quick JavaDoc" on, say,
defineClass and then click on "binary name"; it's actually using the
sources, rather than generated JavaDoc.


I don't object to do this renaming as I favor this more explicit id.  
However you will hit the same issue if another class names an anchor 
conflicts with its private member.   Can you configure IntelliJ to 
generate public/protected elements to avoid this issue?

Mandy

Re: RFR 8180410: ByteArrayOutputStream should not throw IOExceptions

[Brian Burkhalter]

On Mar 21, 2018, at 1:23 PM, Stuart Marks  wrote:

>> Perhaps instead the verbiage just needs to be changed to, e.g.,
>> 
>> “The implementation in this class simply invokes {@link 
>> #write(byte[],int,int) write(b, 0, b.length)}.”
> I don't think you want to specify this detail about the implementation.
> 
> Consider a possible future refactoring, where writeBytes() calls some 
> internal method instead of calling write(b, 0, b.length). This might be done, 
> for example, to leverage an intrinsic, or to avoid unnecessary bounds checks, 
> or for some other reason we can't imagine at the moment.
> 
> Such a refactoring would be prohibited by an @implSpec.

Why then not an @implNote instead of an @apiNote?

Thanks,

Brian

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

[David Lloyd]

Sorry.  This looks OK to me (non-reviewer) now.

On Wed, Mar 21, 2018 at 1:01 PM, Brian Burkhalter
 wrote:
> This is still in need of final approval, assuming it is OK.
>
> Thanks,
>
> Brian
>
> On Mar 14, 2018, at 10:50 AM, Brian Burkhalter 
> wrote:
>
> 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/
>
>



-- 
- DML

Re: RFR 8180410: ByteArrayOutputStream should not throw IOExceptions

[Stuart Marks]

On 3/21/18 1:01 PM, Brian Burkhalter wrote:
On Mar 21, 2018, at 12:34 PM, Stuart Marks > wrote:


Having this as an @implSpec sounds as if the implementation of this method in 
BAOS is *required* to call write(b, 0, b.length). It happens to do that in 
the current webrev, but this is not a requirement on the implementation. (At 
least that doesn't appear to be the intent.)


Perhaps instead the verbiage just needs to be changed to, e.g.,

“The implementation in this class simply invokes {@link #write(byte[],int,int) 
write(b, 0, b.length)}.”

I don't think you want to specify this detail about the implementation.

Consider a possible future refactoring, where writeBytes() calls some internal 
method instead of calling write(b, 0, b.length). This might be done, for 
example, to leverage an intrinsic, or to avoid unnecessary bounds checks, or for 
some other reason we can't imagine at the moment.


Such a refactoring would be prohibited by an @implSpec.

s'marks

Re: RFR 8180410: ByteArrayOutputStream should not throw IOExceptions

[Brian Burkhalter]

On Mar 21, 2018, at 12:34 PM, Stuart Marks  wrote:

> Having this as an @implSpec sounds as if the implementation of this method in 
> BAOS is *required* to call write(b, 0, b.length). It happens to do that in 
> the current webrev, but this is not a requirement on the implementation. (At 
> least that doesn't appear to be the intent.)

Perhaps instead the verbiage just needs to be changed to, e.g.,

“The implementation in this class simply invokes {@link #write(byte[],int,int) 
write(b, 0, b.length)}.”

which appears to correspond to the description

Implementation Specification. This is where the default implementation (or an 
overrideable implementation in a class) is specified. [1]

Thanks,

Brian

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

Re: RFR 8180410: ByteArrayOutputStream should not throw IOExceptions

[Stuart Marks]

On 3/21/18 11:27 AM, Brian Burkhalter wrote:

--- a/src/java.base/share/classes/java/io/ByteArrayOutputStream.java
+++ b/src/java.base/share/classes/java/io/ByteArrayOutputStream.java
@@ -158,15 +158,16 @@
  count += len;
  }
  
  /**

   * Writes the complete contents of the specified byte array
   * to this {@code ByteArrayOutputStream}.
   *
- *  This method is equivalent to {@link #write(byte[],int,int)
+ * @implSpec
+ * This method is equivalent to {@link #write(byte[],int,int)
   * write(b ,0, b.length)}.
   *
   * @param   b the data.
   * @throws  NullPointerException if {@code b} is {@code null}.
   * @since   11
   */
  public void writeBytes(byte b[]) {


Sorry, this is an @apiNote, not an @implSpec.

Having this as an @implSpec sounds as if the implementation of this method in 
BAOS is *required* to call write(b, 0, b.length). It happens to do that in the 
current webrev, but this is not a requirement on the implementation. (At least 
that doesn't appear to be the intent.)


Instead, this statement explains and clarifies, but otherwise doesn't add any 
testable assertions or change any semantics of the contract specified in the 
first sentence of the doc. Thus, it's a note on the API.


Thanks,

s'marks



Thanks,

Brina

On Mar 21, 2018, at 10:00 AM, Brian Burkhalter  
wrote:


I’ll change before pushing.

Thanks,

Brian

On Mar 21, 2018, at 9:58 AM, Roger Riggs  wrote:


An @impSpec for that is fine with me.

Re: RFR (JDK11/doc-only) 8199176: Accessibility issues in java.base docs

[Lance Andersen]

looks fine Joe

> On Mar 21, 2018, at 3:08 PM, Joe Wang  wrote:
> 
> Hi,
> 
> Please an accessibility fix for several package-info files.
> 
> The report contained 4 items. Among them, 1 and 2 are issues in the javadoc 
> tool, and 3 could use an improvement in the tool or documentation. For 
> reference, please see the linked issues in JBS [1].
> 
> 
> For the current issue, we're fixing item 3 manually. Item 4 is a separate 
> issue, but it's bundled with this fix to cut the overhead.
> 
> *Item 3*: Heading leavels should only increase by one
>This is due to the javadoc tool generates a header with an addition of 
> headings, in this particular case, . The fix for this particular case is 
> to replace the s with .
> 
>   Since I know the java.xml packages suffer the same issue, I've included 
> them in this fix as well. For other modules, I'll leave it to the 
> accessibility team.
> 
>   The fix is verified using the accessibility tool, the issue "Heading 
> leavels should only increase by one" no longer showed for the new API doc.
> 
> *Item 4*: Broken inner link
>There were 4: 3 of them missed the "#" sign (as in "FEATURE", "INTERIM", 
> "UPDATE"), the last one was due to capitalization, the inner linked was 
> mistyped in lower case "#feature" when it should have been "#FEATURE".
> 
>You may search the source with
> 
>   
> 
> 
> 
> JBS: https://bugs.openjdk.java.net/browse/JDK-8199176
> webrev: http://cr.openjdk.java.net/~joehw/jdk11/8199176/webrev/
> 
> 
> [1] https://bugs.openjdk.java.net/browse/JDK-8199176
> 
> Thanks,
> Joe
> 

 
  

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

RFR (JDK11/doc-only) 8199176: Accessibility issues in java.base docs

[Joe Wang]

Hi,

Please an accessibility fix for several package-info files.

The report contained 4 items. Among them, 1 and 2 are issues in the 
javadoc tool, and 3 could use an improvement in the tool or 
documentation. For reference, please see the linked issues in JBS [1].



For the current issue, we're fixing item 3 manually. Item 4 is a 
separate issue, but it's bundled with this fix to cut the overhead.


*Item 3*: Heading leavels should only increase by one
This is due to the javadoc tool generates a header with an addition 
of headings, in this particular case, . The fix for this particular 
case is to replace the s with .


   Since I know the java.xml packages suffer the same issue, I've 
included them in this fix as well. For other modules, I'll leave it to 
the accessibility team.


   The fix is verified using the accessibility tool, the issue "Heading 
leavels should only increase by one" no longer showed for the new API doc.


*Item 4*: Broken inner link
There were 4: 3 of them missed the "#" sign (as in "FEATURE", 
"INTERIM", "UPDATE"), the last one was due to capitalization, the inner 
linked was mistyped in lower case "#feature" when it should have been 
"#FEATURE".


You may search the source with

   



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


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

Thanks,
Joe

Re: RFR 8198997: Cache normalized/resolved user.dir property

[Brian Burkhalter]

On Mar 21, 2018, at 11:36 AM, Alan Bateman  wrote:

> That looks right.  I assume there's no test for this as it's never been 
> supported to change user.dir on the command line.

I don’t think a test is called for.

Thanks,

Brian

Re: RFR 8198997: Cache normalized/resolved user.dir property

[Alan Bateman]

On 21/03/2018 18:12, Brian Burkhalter wrote:


That was unclear, at least to me:

--- a/src/java.base/windows/classes/java/io/WinNTFileSystem.java
+++ b/src/java.base/windows/classes/java/io/WinNTFileSystem.java
@@ -48,11 +48,11 @@
  public WinNTFileSystem() {
      Properties props = GetPropertyAction.privilegedGetProperties();
      slash = props.getProperty("file.separator").charAt(0);
      semicolon = props.getProperty("path.separator").charAt(0);
      altSlash = (this.slash == '\\') ? '/' : '\\';
-       userDir = props.getProperty("user.dir");
+       userDir = normalize(props.getProperty("user.dir"));
  }

That looks right.  I assume there's no test for this as it's never been 
supported to change user.dir on the command line.


-Alan

Re: RFR 8180410: ByteArrayOutputStream should not throw IOExceptions

[Brian Burkhalter]

The diff:

--- a/src/java.base/share/classes/java/io/ByteArrayOutputStream.java
+++ b/src/java.base/share/classes/java/io/ByteArrayOutputStream.java
@@ -158,15 +158,16 @@
 count += len;
 }
 
 /**
  * Writes the complete contents of the specified byte array
  * to this {@code ByteArrayOutputStream}.
  *
- *  This method is equivalent to {@link #write(byte[],int,int)
+ * @implSpec
+ * This method is equivalent to {@link #write(byte[],int,int)
  * write(b ,0, b.length)}.
  *
  * @param   b the data.
  * @throws  NullPointerException if {@code b} is {@code null}.
  * @since   11
  */
 public void writeBytes(byte b[]) {

Thanks,

Brina

On Mar 21, 2018, at 10:00 AM, Brian Burkhalter  
wrote:

> I’ll change before pushing.
> 
> Thanks,
> 
> Brian
> 
> On Mar 21, 2018, at 9:58 AM, Roger Riggs  wrote:
> 
>> An @impSpec for that is fine with me.

Re: RFR 8198997: Cache normalized/resolved user.dir property

[Brian Burkhalter]

On Mar 21, 2018, at 11:08 AM, Alan Bateman  wrote:

> On 21/03/2018 18:02, Brian Burkhalter wrote:
>> Ping …
> I think we're waiting for a new version that will normalize the value of 
> user.dir in the WinNTFileSystem constructor.

That was unclear, at least to me:

--- a/src/java.base/windows/classes/java/io/WinNTFileSystem.java
+++ b/src/java.base/windows/classes/java/io/WinNTFileSystem.java
@@ -48,11 +48,11 @@
 public WinNTFileSystem() {
 Properties props = GetPropertyAction.privilegedGetProperties();
 slash = props.getProperty("file.separator").charAt(0);
 semicolon = props.getProperty("path.separator").charAt(0);
 altSlash = (this.slash == '\\') ? '/' : '\\';
-userDir = props.getProperty("user.dir");
+userDir = normalize(props.getProperty("user.dir"));
 }

Thanks,

Brian

Re: RFR 8198997: Cache normalized/resolved user.dir property

[Alan Bateman]

On 21/03/2018 18:02, Brian Burkhalter wrote:

Ping …
I think we're waiting for a new version that will normalize the value of 
user.dir in the WinNTFileSystem constructor.


-Alan

Re: RFR 8198997: Cache normalized/resolved user.dir property

[Brian Burkhalter]

Ping …

On Mar 13, 2018, at 12:33 PM, Brian Burkhalter  
wrote:

> On Mar 13, 2018, at 12:24 PM, Alan Bateman  wrote:
> 
>> As relative paths are command then it might be simpler to just change the 
>> static initializer
> 
> Static initializer or constructor?
> 
>> to initialize userDir to normalize(props.getProperty("user.dir")).
> 
> I was originally going to put that in the constructor but did not know about 
> calling the normalize() instance method from the ctor.

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

[Brian Burkhalter]

This is still in need of final approval, assuming it is OK.

Thanks,

Brian

On Mar 14, 2018, at 10:50 AM, Brian Burkhalter  
wrote:

> 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/

Re: [11] [PATCH] 9053056: Anchor name conflict in ClassLoader JavaDoc

[David Lloyd]

On Wed, Mar 21, 2018 at 12:04 PM, mandy chung  wrote:
>
>
> On 3/21/18 8:58 AM, David Lloyd wrote:
>
> Since adding a field called "name" to java.lang.ClassLoader, the
> "name" anchor which previously referred to the section entitled
> "binary names" has been broken.
>
> The attached doc-only patch changes the name of the anchor to
> "binary-name".  It applies with "patch -p1".
>
> The links are not broken in the published javadoc as it only generates
> javadoc for the public elements.
>
> Are you seeing the broken links from your own javadoc build including
> private elements?

I see it with IntelliJ IDEA when I pop up "quick JavaDoc" on, say,
defineClass and then click on "binary name"; it's actually using the
sources, rather than generated JavaDoc.

-- 
- DML

Re: [11] [PATCH] 9053056: Anchor name conflict in ClassLoader JavaDoc

[mandy chung]

On 3/21/18 8:58 AM, David Lloyd wrote:

Since adding a field called "name" to java.lang.ClassLoader, the
"name" anchor which previously referred to the section entitled
"binary names" has been broken.

The attached doc-only patch changes the name of the anchor to
"binary-name".  It applies with "patch -p1".

The links are not broken in the published javadoc as it only generates 
javadoc for the public elements.


Are you seeing the broken links from your own javadoc build including 
private elements?


Mandy

Re: RFR 8180410: ByteArrayOutputStream should not throw IOExceptions

[Brian Burkhalter]

Hi Roger,

I’ll change before pushing.

Thanks,

Brian

On Mar 21, 2018, at 9:58 AM, Roger Riggs  wrote:

> An @impSpec for that is fine with me.

Re: RFR 8180410: ByteArrayOutputStream should not throw IOExceptions

[Roger Riggs]

Hi Brian,

An @impSpec for that is fine with me.

Roger


On 3/21/2018 12:48 PM, Brian Burkhalter wrote:


On Mar 16, 2018, at 8:04 AM, Brian Burkhalter 
mailto:brian.burkhal...@oracle.com>> wrote:


On Mar 16, 2018, at 7:59 AM, Roger Riggs > wrote:



ByteArrayOutputStream: 166:  Add spaces after "," in the code.


Done: webrev.01 updated in place.


Write:
 Thanks for converting to TestNG.
 There are probably some more testng'ish ways of coding the test but 
its fine as is.


All set as far as I'm concerned.


The CSR for this has been approved with the comment that

 165  *  This method is equivalent to {@link #write(byte[],int,int)
 166  * write(b ,0, b.length)}

looks like an @implSpec tag.

Brian

Re: RFR 8180410: ByteArrayOutputStream should not throw IOExceptions

[Brian Burkhalter]

On Mar 16, 2018, at 8:04 AM, Brian Burkhalter  
wrote:

> On Mar 16, 2018, at 7:59 AM, Roger Riggs  wrote:
> 
>> ByteArrayOutputStream: 166:  Add spaces after "," in the code.
> 
> Done: webrev.01 updated in place.
> 
>> Write:
>>  Thanks for converting to TestNG.
>>  There are probably some more testng'ish ways of coding the test but its 
>> fine as is.
>> 
>> All set as far as I'm concerned.

The CSR for this has been approved with the comment that

 165  *  This method is equivalent to {@link #write(byte[],int,int)
 166  * write(b ,0, b.length)}

looks like an @implSpec tag.

Brian

Re: [11] [PATCH] 9053056: Anchor name conflict in ClassLoader JavaDoc

[Alan Bateman]

On 21/03/2018 16:10, Martin Buchholz wrote:

:

I'm confused by the subject.  There is no such bug
  9053056: Anchor name conflict in ClassLoader JavaDoc


I've just moved it to the jdk project where it is JDK-8199947.

-Alan

Re: [11] [PATCH] 9053056: Anchor name conflict in ClassLoader JavaDoc

[David Lloyd]

Ah, so the submitted bug ID doesn't match the final bug ID.  Excellent.

On Wed, Mar 21, 2018 at 11:24 AM, Martin Buchholz  wrote:
> Found it.
> https://bugs.openjdk.java.net/browse/JDK-8199947
>



-- 
- DML

Re: [11] [PATCH] 9053056: Anchor name conflict in ClassLoader JavaDoc

[Martin Buchholz]

Found it.
https://bugs.openjdk.java.net/browse/JDK-8199947

Re: [11] [PATCH] 9053056: Anchor name conflict in ClassLoader JavaDoc

[David Lloyd]

On Wed, Mar 21, 2018 at 11:10 AM, Martin Buchholz  wrote:
> Hi David.
>
>  I'll take this one.

Thanks.

> - * Returns a {@code Package} of the given name that
> + * Returns a {@code Package} of the given  href="#binary-name">name that
>
> Do Packages (I should really learn about Packages) actually have binary
> names?

Maybe but AFAICT they would be identical to the Java language name.
Rather than try to solve this distinction, in the spirit of solving
one bug at a time (and also laziness), I opted to just replace all
existing references and otherwise change nothing.

> I'm confused by the subject.  There is no such bug

I submitted it via bugreport.java.com; it hasn't been reviewed yet.  I
expect it might show up soon.

> On Wed, Mar 21, 2018 at 9:03 AM David Lloyd  wrote:
>>
>> I discovered there are a couple of references outside of ClassLoader
>> as well.  Here's the updated patch...
>>
>> On Wed, Mar 21, 2018 at 10:58 AM, David Lloyd 
>> wrote:
>> > Since adding a field called "name" to java.lang.ClassLoader, the
>> > "name" anchor which previously referred to the section entitled
>> > "binary names" has been broken.
>> >
>> > The attached doc-only patch changes the name of the anchor to
>> > "binary-name".  It applies with "patch -p1".
>> >
>> > --
>> > - DML
>>
>>
>>
>> --
>> - DML



-- 
- DML

Re: [11] [PATCH] 9053056: Anchor name conflict in ClassLoader JavaDoc

[Martin Buchholz]

Hi David.

 I'll take this one.

- * Returns a {@code Package} of the given name that
+ * Returns a {@code Package} of the given name that

Do Packages (I should really learn about Packages) actually have binary
names?

I'm confused by the subject.  There is no such bug
 9053056: Anchor name conflict in ClassLoader JavaDoc


On Wed, Mar 21, 2018 at 9:03 AM David Lloyd  wrote:

> I discovered there are a couple of references outside of ClassLoader
> as well.  Here's the updated patch...
>
> On Wed, Mar 21, 2018 at 10:58 AM, David Lloyd 
> wrote:
> > Since adding a field called "name" to java.lang.ClassLoader, the
> > "name" anchor which previously referred to the section entitled
> > "binary names" has been broken.
> >
> > The attached doc-only patch changes the name of the anchor to
> > "binary-name".  It applies with "patch -p1".
> >
> > --
> > - DML
>
>
>
> --
> - DML
>

Re: [11] [PATCH] 9053056: Anchor name conflict in ClassLoader JavaDoc

[David Lloyd]

I discovered there are a couple of references outside of ClassLoader
as well.  Here's the updated patch...

On Wed, Mar 21, 2018 at 10:58 AM, David Lloyd  wrote:
> Since adding a field called "name" to java.lang.ClassLoader, the
> "name" anchor which previously referred to the section entitled
> "binary names" has been broken.
>
> The attached doc-only patch changes the name of the anchor to
> "binary-name".  It applies with "patch -p1".
>
> --
> - DML



-- 
- DML
diff --git a/src/java.base/share/classes/java/lang/Class.java 
b/src/java.base/share/classes/java/lang/Class.java
index cf34ff2d86e..adb61e69d35 100644
--- a/src/java.base/share/classes/java/lang/Class.java
+++ b/src/java.base/share/classes/java/lang/Class.java
@@ -382,7 +382,7 @@ public final class Class implements java.io.Serializable,
 
 
 /**
- * Returns the {@code Class} with the given 
+ * Returns the {@code Class} with the given 
  * binary name in the given module.
  *
  *  This method attempts to locate, load, and link the class or 
interface.
@@ -404,7 +404,7 @@ public final class Class implements java.io.Serializable,
  * loads a class in another module.
  *
  * @param  module   A module
- * @param  name The binary name
+ * @param  name The binary 
name
  *  of the class
  * @return {@code Class} object of the given name defined in the given 
module;
  * {@code null} if not found.
diff --git a/src/java.base/share/classes/java/lang/ClassLoader.java 
b/src/java.base/share/classes/java/lang/ClassLoader.java
index f0d41f425dd..cb6193f2b1f 100644
--- a/src/java.base/share/classes/java/lang/ClassLoader.java
+++ b/src/java.base/share/classes/java/lang/ClassLoader.java
@@ -73,7 +73,7 @@ import sun.security.util.SecurityConstants;
 /**
  * A class loader is an object that is responsible for loading classes. The
  * class {@code ClassLoader} is an abstract class.  Given the binary name of a class, a class loader should attempt to
+ * href="#binary-name">binary name of a class, a class loader should 
attempt to
  * locate or generate data that constitutes a definition for the class.  A
  * typical strategy is to transform the name into a file name and then read a
  * "class file" of that name from a file system.
@@ -202,7 +202,7 @@ import sun.security.util.SecurityConstants;
  * }
  * 
  *
- *  Binary names 
+ *  Binary names 
  *
  *  Any class name provided as a {@code String} parameter to methods in
  * {@code ClassLoader} must be a binary name as defined by
@@ -480,7 +480,7 @@ public abstract class ClassLoader {
 // -- Class --
 
 /**
- * Loads the class with the specified binary name.
+ * Loads the class with the specified binary 
name.
  * This method searches for classes in the same manner as the {@link
  * #loadClass(String, boolean)} method.  It is invoked by the Java virtual
  * machine to resolve class references.  Invoking this method is equivalent
@@ -488,7 +488,7 @@ public abstract class ClassLoader {
  * false)}.
  *
  * @param  name
- * The binary name of the class
+ * The binary name of the class
  *
  * @return  The resulting {@code Class} object
  *
@@ -500,7 +500,7 @@ public abstract class ClassLoader {
 }
 
 /**
- * Loads the class with the specified binary name.  The
+ * Loads the class with the specified binary 
name.  The
  * default implementation of this method searches for classes in the
  * following order:
  *
@@ -530,7 +530,7 @@ public abstract class ClassLoader {
  * during the entire class loading process.
  *
  * @param  name
- * The binary name of the class
+ * The binary name of the class
  *
  * @param  resolve
  * If {@code true} then resolve the class
@@ -579,7 +579,7 @@ public abstract class ClassLoader {
 }
 
 /**
- * Loads the class with the specified binary name
+ * Loads the class with the specified binary 
name
  * in a module defined to this class loader.  This method returns {@code 
null}
  * if the class could not be found.
  *
@@ -598,7 +598,7 @@ public abstract class ClassLoader {
  * @param  module
  * The module
  * @param  name
- * The binary name of the class
+ * The binary name of the class
  *
  * @return The resulting {@code Class} object in a module defined by
  * this class loader, or {@code null} if the class could not be 
found.
@@ -674,7 +674,7 @@ public abstract class ClassLoader {
 }
 
 /**
- * Finds the class with the specified binary name.
+ * Finds the class with the specified binary 
name.
  * This method should be overridden by class loader implementations that
  * follow the delegation model for loading classes, and will be invoked by
  * the {@link #loadClass loadClass} method after checking the
@@ -683,7 +683,7

[11] [PATCH] 9053056: Anchor name conflict in ClassLoader JavaDoc

[David Lloyd]

Since adding a field called "name" to java.lang.ClassLoader, the
"name" anchor which previously referred to the section entitled
"binary names" has been broken.

The attached doc-only patch changes the name of the anchor to
"binary-name".  It applies with "patch -p1".

-- 
- DML
diff --git a/src/java.base/share/classes/java/lang/ClassLoader.java 
b/src/java.base/share/classes/java/lang/ClassLoader.java
index f0d41f425dd..cb6193f2b1f 100644
--- a/src/java.base/share/classes/java/lang/ClassLoader.java
+++ b/src/java.base/share/classes/java/lang/ClassLoader.java
@@ -73,7 +73,7 @@ import sun.security.util.SecurityConstants;
 /**
  * A class loader is an object that is responsible for loading classes. The
  * class {@code ClassLoader} is an abstract class.  Given the binary name of a class, a class loader should attempt to
+ * href="#binary-name">binary name of a class, a class loader should 
attempt to
  * locate or generate data that constitutes a definition for the class.  A
  * typical strategy is to transform the name into a file name and then read a
  * "class file" of that name from a file system.
@@ -202,7 +202,7 @@ import sun.security.util.SecurityConstants;
  * }
  * 
  *
- *  Binary names 
+ *  Binary names 
  *
  *  Any class name provided as a {@code String} parameter to methods in
  * {@code ClassLoader} must be a binary name as defined by
@@ -480,7 +480,7 @@ public abstract class ClassLoader {
 // -- Class --
 
 /**
- * Loads the class with the specified binary name.
+ * Loads the class with the specified binary 
name.
  * This method searches for classes in the same manner as the {@link
  * #loadClass(String, boolean)} method.  It is invoked by the Java virtual
  * machine to resolve class references.  Invoking this method is equivalent
@@ -488,7 +488,7 @@ public abstract class ClassLoader {
  * false)}.
  *
  * @param  name
- * The binary name of the class
+ * The binary name of the class
  *
  * @return  The resulting {@code Class} object
  *
@@ -500,7 +500,7 @@ public abstract class ClassLoader {
 }
 
 /**
- * Loads the class with the specified binary name.  The
+ * Loads the class with the specified binary 
name.  The
  * default implementation of this method searches for classes in the
  * following order:
  *
@@ -530,7 +530,7 @@ public abstract class ClassLoader {
  * during the entire class loading process.
  *
  * @param  name
- * The binary name of the class
+ * The binary name of the class
  *
  * @param  resolve
  * If {@code true} then resolve the class
@@ -579,7 +579,7 @@ public abstract class ClassLoader {
 }
 
 /**
- * Loads the class with the specified binary name
+ * Loads the class with the specified binary 
name
  * in a module defined to this class loader.  This method returns {@code 
null}
  * if the class could not be found.
  *
@@ -598,7 +598,7 @@ public abstract class ClassLoader {
  * @param  module
  * The module
  * @param  name
- * The binary name of the class
+ * The binary name of the class
  *
  * @return The resulting {@code Class} object in a module defined by
  * this class loader, or {@code null} if the class could not be 
found.
@@ -674,7 +674,7 @@ public abstract class ClassLoader {
 }
 
 /**
- * Finds the class with the specified binary name.
+ * Finds the class with the specified binary 
name.
  * This method should be overridden by class loader implementations that
  * follow the delegation model for loading classes, and will be invoked by
  * the {@link #loadClass loadClass} method after checking the
@@ -683,7 +683,7 @@ public abstract class ClassLoader {
  * @implSpec The default implementation throws {@code 
ClassNotFoundException}.
  *
  * @param  name
- * The binary name of the class
+ * The binary name of the class
  *
  * @return  The resulting {@code Class} object
  *
@@ -697,7 +697,7 @@ public abstract class ClassLoader {
 }
 
 /**
- * Finds the class with the given binary name
+ * Finds the class with the given binary name
  * in a module defined to this class loader.
  * Class loader implementations that support the loading from modules
  * should override this method.
@@ -715,7 +715,7 @@ public abstract class ClassLoader {
  * class loader
 
  * @param  name
- * The binary name of the class
+ * The binary name of the class
  *
  * @return The resulting {@code Class} object, or {@code null}
  * if the class could not be found.
@@ -737,7 +737,7 @@ public abstract class ClassLoader {
  * Converts an array of bytes into an instance of class {@code Class}.
  * Before the {@code Class} can be used it must be resolved.  This method

Re: Raw String Literal Library Support

[Jim Laskey]

One more change set.

trimIndent -> stripIndent
trimMarkers -> stripMarkers



> On Mar 20, 2018, at 10:35 AM, Jim Laskey  wrote:
> 
> Summary.
> 
> A. Line support.
> 
> - Supporting a mix of line terminators `\n|\r\n|\r` is already a well 
> established pattern in language parsers, in the JDK (ex. see  
> java.nio.file.FileChannelLinesSpliterator) and RegEx (ex. see `\R`). The 
> performance difference between checking one vs the three is negligible.
> 
> - Yes, Stream stream = 
> Pattern.compile("\n|\r\n|\r").splitAsStream(string); is very useful 
> (Spliterators rule), but is cumbersome in this expected to be common use 
> case. Only so-so streamy. :-)
> 
> - BufferedRead.lines() vs. String.lines() is a tricky discussion. It comes 
> down to whether the new line is a terminator or a separator.  In the i/o 
> case, it seems terminator is the right answer. A well formed text file will 
> have a new line at the end of every line.  However, I think you’ll find when 
> people work with multi-line strings they think of new line as a separator. 
> Hence, the common use of split(“\n”) and “”.split(“\n”).length == 1. 
> Indentation, the position of closing delimiter and margin trimming makes that 
> last line very fluid.
> 
> What clinches the deal is that  
> string.lines().collect(joining(“\n”)).equals(string). I’ll ensure both 
> versions of lines() have the difference well javadocumented.
> 
> - The current Spliterator implementation makes 
> String.lines().toArray(String[]::new) an order of magnitude faster than 
> split(`\n|\r\n|\r`). That’s why I implemented it for margin management. 
> Faster still if no collection/array is constructed.
> 
> BTW: split(`\R`) is 2x-3x faster than split(`\n|\r\n|\r`). Nice.
> 
> B. Additions to basic trim methods.
> 
> - Revamped to become strip, stripLeading, stripTrailing using 
> Character.isWhiteSpace(codepoint) as the test (optimized using ch == ‘ ' || 
> ch == ‘\t’ || Character.isWhiteSpace(ch)).
> 
> - No strong feeling about it, but String.trim() could be recommended for 
> deprecation.
> 
> C. Margin management.
> 
> - String.trimMarkers() as a default to String.trimMarkers(“|”, “|”) is 
> reasonable.  Will put it in the CSR for broader discussion.
> 
> - Re use of patterns. I think the Stream lines() method will make it 
> very easy enough to create custom trim margin lambdas.
> 
> D. Escape management.
> 
> - Good
> 
> Cheers,
> 
> — Jim
> 
> 
> 
> 
>> On Mar 13, 2018, at 10: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