Re: Add getChars to CharSequence

On Mon, May 20, 2013 at 5:30 PM, David Holmes wrote: > On 21/05/2013 3:55 AM, Martin Buchholz wrote: > >> This is __the__ fundamental question of whether to add >> CharSequence.getChars at all. >> >> If none of the objects in the jdk can trust any of the others, they will >> spend all of their tim

Re: Add getChars to CharSequence

On 21/05/2013 3:55 AM, Martin Buchholz wrote: This is __the__ fundamental question of whether to add CharSequence.getChars at all. If none of the objects in the jdk can trust any of the others, they will spend all of their time making defensive copies and unmodifiable wrappers. But you are not

Re: Add getChars to CharSequence

This is __the__ fundamental question of whether to add CharSequence.getChars at all. If none of the objects in the jdk can trust any of the others, they will spend all of their time making defensive copies and unmodifiable wrappers. This is the reverse side of whether you can trust any collection

Re: Add getChars to CharSequence

Hi Martin, I took a look at this in relation to the other StringBuffer bugs. I have a concern. Looking at this: @@ -453,12 +438,11 @@ public AbstractStringBuilder append(CharSequence s) { if (s == null) return appendNull(); -if (s instanceof String) -

Re: Add getChars to CharSequence

On 05/09/2013 02:30 AM, Mike Duigou wrote: Hi Martin; Some notes from a non-exhaustive (ran out of time before dinner) review. Mike AbstractStringBuilder:: - The impls like insert(int dstOffset, CharSequence s) makes me slightly uneasy because the private value field escapes to an unknown cl

Re: Add getChars to CharSequence

On Thu, May 9, 2013 at 5:38 AM, Alan Bateman wrote: > On 08/05/2013 23:05, Martin Buchholz wrote: > >> Alan (et al): Ping. >> >> I've looked through the webrev. > > The scary part is subsequenceRaw where the offsets including the position. > I don't see anything obviously wrong and the tests shou

Re: Add getChars to CharSequence

On Fri, May 10, 2013 at 4:08 PM, Joe Darcy wrote: > On 05/10/2013 03:03 PM, Martin Buchholz wrote: > >> On Wed, May 8, 2013 at 5:30 PM, Mike Duigou >> wrote: >> >> - There's been some informal discussion of packaging commonly used test >>> utils as jtreg @library. This could be a good candidate

Re: Add getChars to CharSequence

On Fri, May 17, 2013 at 5:37 PM, Mike Duigou wrote: > Hi Martin, > > I had failed to notice that you were using 6813523 in your changeset. > Since 6792262 is more specific can we switch to using that one? > > Done.

Re: Add getChars to CharSequence

Hi Martin, I had failed to notice that you were using 6813523 in your changeset. Since 6792262 is more specific can we switch to using that one? Mike On May 17 2013, at 16:42 , Mike Duigou wrote: > Hi Martin; > > There's a bug id we can use for this issue, an old RFE, JDK-6792262. > > I am

Re: Add getChars to CharSequence

Hi Martin; There's a bug id we can use for this issue, an old RFE, JDK-6792262. I am working on the internal CCC case Thanks, Mike On May 10 2013, at 17:15 , Martin Buchholz wrote: > > > > On Wed, May 8, 2013 at 5:30 PM, Mike Duigou wrote: > - Could use a @DataProivder for "CharSequen

Re: Add getChars to CharSequence

On Wed, May 8, 2013 at 5:30 PM, Mike Duigou wrote: > - Could use a @DataProivder for "CharSequence[] seqs = {" style tests. > OK, so I went and learned about DataProvider. Not too painful. Added in some uses of bleeding edge lambda stuff as well to pretty up the GetChars test. Webrev refreshe

Re: Add getChars to CharSequence

On 05/10/2013 03:03 PM, Martin Buchholz wrote: On Wed, May 8, 2013 at 5:30 PM, Mike Duigou wrote: - There's been some informal discussion of packaging commonly used test utils as jtreg @library. This could be a good candidate. ArraysCharSequence is a candidate for testing utils lib. Basic impl

Re: Add getChars to CharSequence

On Wed, May 8, 2013 at 5:30 PM, Mike Duigou wrote: > > - There's been some informal discussion of packaging commonly used test > utils as jtreg @library. This could be a good candidate. ArraysCharSequence > is a candidate for testing utils lib. Basic impls that override no defaults > are going to

Re: Add getChars to CharSequence

On Wed, May 8, 2013 at 5:30 PM, Mike Duigou wrote: > AbstractStringBuilder:: > > - The impls like insert(int dstOffset, CharSequence s) makes me slightly > uneasy because the private value field escapes to an unknown class. Who > knows what evil (or foolishness) could result? > There is an etern

Re: Add getChars to CharSequence

On May 9 2013, at 05:38 , Alan Bateman wrote: > On 08/05/2013 23:05, Martin Buchholz wrote: >> Alan (et al): Ping. >> > I've looked through the webrev. > > The scary part is subsequenceRaw where the offsets including the position. I > don't see anything obviously wrong and the tests should cat

Re: Add getChars to CharSequence

On 08/05/2013 23:05, Martin Buchholz wrote: Alan (et al): Ping. I've looked through the webrev. The scary part is subsequenceRaw where the offsets including the position. I don't see anything obviously wrong and the tests should catch any issues. I don't see any issue conditionally generatin

Re: Add getChars to CharSequence

On Wed, May 8, 2013 at 5:30 PM, Mike Duigou wrote: > > Direct-X-Buffer.java:: > > - +#if[rw] public boolean isDirect() : Why would this be conditionalized > with rw? > > Welcome to the -X- files. Since you have conditional preprocessing and inheritance happening at the same time, all readonly me

Re: Add getChars to CharSequence

Hi Martin; Some notes from a non-exhaustive (ran out of time before dinner) review. Mike AbstractStringBuilder:: - The impls like insert(int dstOffset, CharSequence s) makes me slightly uneasy because the private value field escapes to an unknown class. Who knows what evil (or foolishness) co

Re: Add getChars to CharSequence

Alan (et al): Ping. On Wed, May 1, 2013 at 3:19 PM, Martin Buchholz wrote: > Another version of this change is ready. No longer preliminary. Tests > have been written. > http://cr.openjdk.java.net/~martin/webrevs/openjdk8/getChars/ > > This kind of change is particularly subject to feature cr

Re: Add getChars to CharSequence

Another version of this change is ready. No longer preliminary. Tests have been written. http://cr.openjdk.java.net/~martin/webrevs/openjdk8/getChars/ This kind of change is particularly subject to feature creep, and I am trying to restrain myself. I even addressed some of Ulf's suggestions. Th

Re: Add getChars to CharSequence

Hi Martin, (cc'd core-libs-dev) Am 23.04.2013 21:36, schrieb Martin Buchholz: Hi Ulf! On Mon, Apr 22, 2013 at 9:11 PM, Ulf Zibis mailto:ulf.zi...@cosoco.de>> wrote: Am 22.04.2013 21:45, schrieb Martin Buchholz: Another preliminary webrev is out at http://cr.openjdk.java.

Re: Add getChars to CharSequence

Yes, all-or-nothing behavior on errors is worth something, and we should get it right, especially when it's cheap. On Tue, Apr 23, 2013 at 4:52 AM, Alan Bateman wrote: > One other observation, but as the 2-arg getChars doesn't have explicit > bounds checking then it may have have copied some cha

Re: Add getChars to CharSequence

Hi Ulf! On Mon, Apr 22, 2013 at 9:11 PM, Ulf Zibis wrote: > Am 22.04.2013 21:45, schrieb Martin Buchholz: > >> Another preliminary webrev is out at >> http://cr.openjdk.java.net/~**martin/webrevs/openjdk8/**getChars/< >> http://cr.o

Re: Add getChars to CharSequence

On 23/04/2013 12:17, Alan Bateman wrote: On 22/04/2013 20:45, Martin Buchholz wrote: Another preliminary webrev is out at http://cr.openjdk.java.net/~martin/webrevs/openjdk8/getChars/ Alan et al: Before continuing, can we: Hav

Re: Add getChars to CharSequence

On 22/04/2013 20:45, Martin Buchholz wrote: Another preliminary webrev is out at http://cr.openjdk.java.net/~martin/webrevs/openjdk8/getChars/ Alan et al: Before continuing, can we: Have thumbs up on the changes to out of bound

Re: Add getChars to CharSequence

Am 22.04.2013 21:45, schrieb Martin Buchholz: Another preliminary webrev is out at http://cr.openjdk.java.net/~martin/webrevs/openjdk8/getChars/ Alan et al: Before continuing, can we: Have thumbs up on the changes to out of bou

Re: Add getChars to CharSequence

Another preliminary webrev is out at http://cr.openjdk.java.net/~martin/webrevs/openjdk8/getChars/ Alan et al: Before continuing, can we: Have thumbs up on the changes to out of bounds exceptions? The handling of the rw conditional in the preprocessed sources is very confusing and error-prone.

Re: Add getChars to CharSequence

On Thu, Apr 11, 2013 at 12:55 PM, Ulf Zibis wrote: > > Anyway as those methods all need some CPU time to execute normally, I'm > not sure if it's worth to save 1 comparison by outsourcing. > Saving one comparison is worth doing in any case in these performance-critical methods. "Out-lining" the

Re: Add getChars to CharSequence

On Thu, Apr 11, 2013 at 1:47 PM, Alan Bateman wrote: > On 11/04/2013 18:14, Martin Buchholz wrote: > >> >> : >> >> I thought about this a bit. getChars should be consistent with charAt, >> which does not move position and is relative to position. >> > This is probably right although I think we'll

Re: Add getChars to CharSequence

On 11/04/2013 21:37, Martin Buchholz wrote: Alan, please file an rfe for us: Add default methods CharSequence.getChars to match String and StringBuilder. There are several to choose from, here's one of the long longing requests: 6813523: (str) Add method CharSequence.getChars() as StringBuilde

Re: Add getChars to CharSequence

On 11/04/2013 18:14, Martin Buchholz wrote: : I thought about this a bit. getChars should be consistent with charAt, which does not move position and is relative to position. This is probably right although I think we'll need the spec overridden in CharBuffer to make this clear. You might be

Re: Add getChars to CharSequence

Alan, please file an rfe for us: Add default methods CharSequence.getChars to match String and StringBuilder. Ulf et al: The exception messages for out of bounds checks are maddeningly inconsistent. Are y'all OK with making them consistent and maximally informative? something like "start = %d, e

Re: Add getChars to CharSequence

Hi Martin, great idea! Regarding the exception handling... You should reuse getCharsOutOfBounds() from AbstractStringBuilder in String. Also IMO the exception messages need some "corporate design"; e.g. in some cases the string "srcBegin > srcEnd" is returned, in other cases the int value of s

Re: Add getChars to CharSequence

On Thu, Apr 11, 2013 at 6:36 AM, Alan Bateman wrote: > On 11/04/2013 01:40, Martin Buchholz wrote: > >> I've often wished that CharSequence had getChars methods, as many of the >> concrete implementations already do. >> In jdk8 with default methods, this is possible! >> This will make some of the

Re: Add getChars to CharSequence

On 11/04/2013 01:40, Martin Buchholz wrote: I've often wished that CharSequence had getChars methods, as many of the concrete implementations already do. In jdk8 with default methods, this is possible! This will make some of the String code a little nicer and more efficient. Here's a preliminar

Re: Add getChars to CharSequence

Seems like a nice extension to me. Mike On Apr 10 2013, at 17:40 , Martin Buchholz wrote: > I've often wished that CharSequence had getChars methods, as many of the > concrete implementations already do. > In jdk8 with default methods, this is possible! > This will make some of the String code

Add getChars to CharSequence

I've often wished that CharSequence had getChars methods, as many of the concrete implementations already do. In jdk8 with default methods, this is possible! This will make some of the String code a little nicer and more efficient. Here's a preliminary patch in this direction, that overlaps with s