Re: [11] RFR: 4993841: (str) java.lang.Character should have a toString(int) method

[Stuart Marks]

Hi Naoto,

Thanks for fixing the original issue, and for filing the cleanup followup bug.

s'marks


On 3/3/18 9:30 AM, naoto.s...@oracle.com wrote:

Hi Stuart,

Filed an issue for the cleanup:

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

Naoto

On 3/2/18 5:50 PM, Stuart Marks wrote:

Looks good.

I note that other codepoint-consuming methods, such as

 Character.UnicodeBlock.of(cp)
 Character.UnicodeScript.of(cp)
 Character.toChars(cp, char[], int)
 Character.toChars(cp)
 Character.getName(cp)

all throw IAE with no message. It would be nice to add messages to them. It 
would be even nicer to print out the offending value, possibly even in hex. 
Indeed, there are several other places in Character.java where exceptions are 
thrown that lack diagnostic information. Maybe as part of a separate cleanup 
pass?


s'marks

On 3/2/18 3:37 PM, naoto.s...@oracle.com wrote:

Thanks for comments, Martin, Roger. Updated the fix as follows:

http://cr.openjdk.java.net/~naoto/4993841/webrev.04/

Naoto

On 3/1/18 6:47 PM, naoto.s...@oracle.com wrote:

Hi,

Please review the fix to the following issue:

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

The proposed changeset is located at:

http://cr.openjdk.java.net/~naoto/4993841/webrev.03/

This stems from the recent discussion regarding String.repeat().[1] The 
corresponding CSR has already been approved.


Naoto

--
[1] 
http://mail.openjdk.java.net/pipermail/core-libs-dev/2018-February/051568.html

Re: [11] RFR: 4993841: (str) java.lang.Character should have a toString(int) method

[naoto . sato]

Hi Stuart,

Filed an issue for the cleanup:

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

Naoto

On 3/2/18 5:50 PM, Stuart Marks wrote:

Looks good.

I note that other codepoint-consuming methods, such as

     Character.UnicodeBlock.of(cp)
     Character.UnicodeScript.of(cp)
     Character.toChars(cp, char[], int)
     Character.toChars(cp)
     Character.getName(cp)

all throw IAE with no message. It would be nice to add messages to them. 
It would be even nicer to print out the offending value, possibly even 
in hex. Indeed, there are several other places in Character.java where 
exceptions are thrown that lack diagnostic information. Maybe as part of 
a separate cleanup pass?


s'marks

On 3/2/18 3:37 PM, naoto.s...@oracle.com wrote:

Thanks for comments, Martin, Roger. Updated the fix as follows:

http://cr.openjdk.java.net/~naoto/4993841/webrev.04/

Naoto

On 3/1/18 6:47 PM, naoto.s...@oracle.com wrote:

Hi,

Please review the fix to the following issue:

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

The proposed changeset is located at:

http://cr.openjdk.java.net/~naoto/4993841/webrev.03/

This stems from the recent discussion regarding String.repeat().[1] 
The corresponding CSR has already been approved.


Naoto

--
[1] 
http://mail.openjdk.java.net/pipermail/core-libs-dev/2018-February/051568.html 

Re: [11] RFR: 4993841: (str) java.lang.Character should have a toString(int) method

[Martin Buchholz]

Thanks - looks good.


On Fri, Mar 2, 2018 at 3:37 PM,  wrote:

> Thanks for comments, Martin, Roger. Updated the fix as follows:
>
> http://cr.openjdk.java.net/~naoto/4993841/webrev.04/
>
> Naoto
>
>
> On 3/1/18 6:47 PM, naoto.s...@oracle.com wrote:
>
>> Hi,
>>
>> Please review the fix to the following issue:
>>
>> https://bugs.openjdk.java.net/browse/JDK-4993841
>>
>> The proposed changeset is located at:
>>
>> http://cr.openjdk.java.net/~naoto/4993841/webrev.03/
>>
>> This stems from the recent discussion regarding String.repeat().[1] The
>> corresponding CSR has already been approved.
>>
>> Naoto
>>
>> --
>> [1] http://mail.openjdk.java.net/pipermail/core-libs-dev/2018-Fe
>> bruary/051568.html
>>
>

Re: [11] RFR: 4993841: (str) java.lang.Character should have a toString(int) method

[Stuart Marks]

Looks good.

I note that other codepoint-consuming methods, such as

Character.UnicodeBlock.of(cp)
Character.UnicodeScript.of(cp)
Character.toChars(cp, char[], int)
Character.toChars(cp)
Character.getName(cp)

all throw IAE with no message. It would be nice to add messages to them. It 
would be even nicer to print out the offending value, possibly even in hex. 
Indeed, there are several other places in Character.java where exceptions are 
thrown that lack diagnostic information. Maybe as part of a separate cleanup pass?


s'marks

On 3/2/18 3:37 PM, naoto.s...@oracle.com wrote:

Thanks for comments, Martin, Roger. Updated the fix as follows:

http://cr.openjdk.java.net/~naoto/4993841/webrev.04/

Naoto

On 3/1/18 6:47 PM, naoto.s...@oracle.com wrote:

Hi,

Please review the fix to the following issue:

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

The proposed changeset is located at:

http://cr.openjdk.java.net/~naoto/4993841/webrev.03/

This stems from the recent discussion regarding String.repeat().[1] The 
corresponding CSR has already been approved.


Naoto

--
[1] 
http://mail.openjdk.java.net/pipermail/core-libs-dev/2018-February/051568.html

Re: [11] RFR: 4993841: (str) java.lang.Character should have a toString(int) method

[Stuart Marks]

On 3/2/18 4:42 PM, Remi Forax wrote:

Just to be sure, it now means that a code like this will work

   String s = "hello".chars()
 .mapToObj(Character::toString)
 .collect(Collectors.joining());


Yes, this will work, as Naoto mentioned, but I don't recommend it -- this will 
split up surrogate pairs. Simplying joining them back together will work in this 
case, but if any intermediate processing is done, it could be lossy.


I think the more important case is something like this:

String s = "hello\ud83d\ude1d".codePoints()
.mapToObj(Character::toString)
.collect(joining());

Previously, you had to do something like

String s = "hello\ud83d\ude1d".codePoints()
.mapToObj(cp -> new String(Character.toChars(cp)))
.collect(joining());

which is a mouthful and which also creates an extra array.

s'marks






Rémi

- Mail original -

De: "naoto sato" <naoto.s...@oracle.com>
À: "Stuart Marks" <stuart.ma...@oracle.com>, "Xueming Shen" <xueming.s...@gmail.com>, 
"core-libs-dev"
<core-libs-dev@openjdk.java.net>
Envoyé: Vendredi 2 Mars 2018 03:47:51
Objet: [11] RFR: 4993841: (str) java.lang.Character should have a toString(int) 
method



Hi,

Please review the fix to the following issue:

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

The proposed changeset is located at:

http://cr.openjdk.java.net/~naoto/4993841/webrev.03/

This stems from the recent discussion regarding String.repeat().[1] The
corresponding CSR has already been approved.

Naoto

--
[1]
http://mail.openjdk.java.net/pipermail/core-libs-dev/2018-February/051568.html

Re: [11] RFR: 4993841: (str) java.lang.Character should have a toString(int) method

[Jonathan Bluett-Duncan]

Hi Rémi,

I'm sure I've misunderstood something, but AFAIK the code example you gave
is already valid code and was valid as far back as Java 8 (I don't have an
IDE at hand to confirm this).

Did you perhaps mean to ask instead that the code snippet below would work
with the resolution of
https://bugs.openjdk.java.net/browse/JDK-4993841 (difference
being that it uses .codePoints() instead of .chars())? Or have I completely
lost the plot?

String s = "hello".codePoints()
.mapToObj(Character::toString)
.collect(Collectors.joining());

Cheers,
Jonathan

On 3 March 2018 at 00:42, Remi Forax <fo...@univ-mlv.fr> wrote:

> Just to be sure, it now means that a code like this will work
>
>   String s = "hello".chars()
> .mapToObj(Character::toString)
> .collect(Collectors.joining());
>
> Rémi
>
> - Mail original -
> > De: "naoto sato" <naoto.s...@oracle.com>
> > À: "Stuart Marks" <stuart.ma...@oracle.com>, "Xueming Shen" <
> xueming.s...@gmail.com>, "core-libs-dev"
> > <core-libs-dev@openjdk.java.net>
> > Envoyé: Vendredi 2 Mars 2018 03:47:51
> > Objet: [11] RFR: 4993841: (str) java.lang.Character should have a
> toString(int) method
>
> > Hi,
> >
> > Please review the fix to the following issue:
> >
> > https://bugs.openjdk.java.net/browse/JDK-4993841
> >
> > The proposed changeset is located at:
> >
> > http://cr.openjdk.java.net/~naoto/4993841/webrev.03/
> >
> > This stems from the recent discussion regarding String.repeat().[1] The
> > corresponding CSR has already been approved.
> >
> > Naoto
> >
> > --
> > [1]
> > http://mail.openjdk.java.net/pipermail/core-libs-dev/2018-
> February/051568.html
>

Re: [11] RFR: 4993841: (str) java.lang.Character should have a toString(int) method

[Xueming Shen]

+1

On 3/2/18, 3:37 PM, naoto.s...@oracle.com wrote:

Thanks for comments, Martin, Roger. Updated the fix as follows:

http://cr.openjdk.java.net/~naoto/4993841/webrev.04/

Naoto

On 3/1/18 6:47 PM, naoto.s...@oracle.com wrote:

Hi,

Please review the fix to the following issue:

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

The proposed changeset is located at:

http://cr.openjdk.java.net/~naoto/4993841/webrev.03/

This stems from the recent discussion regarding String.repeat().[1] 
The corresponding CSR has already been approved.


Naoto

--
[1] 
http://mail.openjdk.java.net/pipermail/core-libs-dev/2018-February/051568.html 

Re: [11] RFR: 4993841: (str) java.lang.Character should have a toString(int) method

[Naoto Sato]

Yes. With a private build:

jshell> 
"hello".chars().mapToObj(Character::toString).collect(Collectors.joining())

$1 ==> "hello"

Naoto

On 03/02/2018 04:42 PM, Remi Forax wrote:

Just to be sure, it now means that a code like this will work

  String s = "hello".chars()
.mapToObj(Character::toString)
.collect(Collectors.joining());

Rémi

- Mail original -

De: "naoto sato" <naoto.s...@oracle.com>
À: "Stuart Marks" <stuart.ma...@oracle.com>, "Xueming Shen" <xueming.s...@gmail.com>, 
"core-libs-dev"
<core-libs-dev@openjdk.java.net>
Envoyé: Vendredi 2 Mars 2018 03:47:51
Objet: [11] RFR: 4993841: (str) java.lang.Character should have a toString(int) 
method



Hi,

Please review the fix to the following issue:

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

The proposed changeset is located at:

http://cr.openjdk.java.net/~naoto/4993841/webrev.03/

This stems from the recent discussion regarding String.repeat().[1] The
corresponding CSR has already been approved.

Naoto

--
[1]
http://mail.openjdk.java.net/pipermail/core-libs-dev/2018-February/051568.html

Re: [11] RFR: 4993841: (str) java.lang.Character should have a toString(int) method

[Remi Forax]

Just to be sure, it now means that a code like this will work

  String s = "hello".chars()
.mapToObj(Character::toString)
.collect(Collectors.joining());

Rémi

- Mail original -
> De: "naoto sato" <naoto.s...@oracle.com>
> À: "Stuart Marks" <stuart.ma...@oracle.com>, "Xueming Shen" 
> <xueming.s...@gmail.com>, "core-libs-dev"
> <core-libs-dev@openjdk.java.net>
> Envoyé: Vendredi 2 Mars 2018 03:47:51
> Objet: [11] RFR: 4993841: (str) java.lang.Character should have a 
> toString(int) method

> Hi,
> 
> Please review the fix to the following issue:
> 
> https://bugs.openjdk.java.net/browse/JDK-4993841
> 
> The proposed changeset is located at:
> 
> http://cr.openjdk.java.net/~naoto/4993841/webrev.03/
> 
> This stems from the recent discussion regarding String.repeat().[1] The
> corresponding CSR has already been approved.
> 
> Naoto
> 
> --
> [1]
> http://mail.openjdk.java.net/pipermail/core-libs-dev/2018-February/051568.html

Re: [11] RFR: 4993841: (str) java.lang.Character should have a toString(int) method

[naoto . sato]

Thanks for comments, Martin, Roger. Updated the fix as follows:

http://cr.openjdk.java.net/~naoto/4993841/webrev.04/

Naoto

On 3/1/18 6:47 PM, naoto.s...@oracle.com wrote:

Hi,

Please review the fix to the following issue:

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

The proposed changeset is located at:

http://cr.openjdk.java.net/~naoto/4993841/webrev.03/

This stems from the recent discussion regarding String.repeat().[1] The 
corresponding CSR has already been approved.


Naoto

--
[1] 
http://mail.openjdk.java.net/pipermail/core-libs-dev/2018-February/051568.html 

Re: [11] RFR: 4993841: (str) java.lang.Character should have a toString(int) method

[Roger Riggs]

Hi Naoto,

String.java:
3129: Please add a message to the throw IAE.  "Not a valid Unicode code 
point".


Roger



On 3/1/2018 9:47 PM, naoto.s...@oracle.com wrote:

Hi,

Please review the fix to the following issue:

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

The proposed changeset is located at:

http://cr.openjdk.java.net/~naoto/4993841/webrev.03/

This stems from the recent discussion regarding String.repeat().[1] 
The corresponding CSR has already been approved.


Naoto

--
[1] 
http://mail.openjdk.java.net/pipermail/core-libs-dev/2018-February/051568.html

Re: [11] RFR: 4993841: (str) java.lang.Character should have a toString(int) method

[Martin Buchholz]

The multiple redundant bounds checks bother me, but I don't know how to fix
them without abandoning a bit of modularization.

if (cp >= 0) {
  if (COMPACT_STRINGS && cp < 0x100)  return latin1encode()
  if (cp < MIN_SURROGATE || (cp > MAX_SURROGATE && cp <
MIN_SUPPLEMENTARY_CODEPOINT) return bmpencode()
  if (cp >= MIN_SUPPLEMENTARY_CODEPOINT && cp <= MAX_CODEPOINT) return
suppEncode()
}
throw ...


+static byte[] toBytes(int cp) {
+if (Character.isBmpCodePoint(cp)) {
+return toBytes((char)cp);
+} else {
+byte[] result = new byte[4];
+putChar(result, 0, Character.highSurrogate(cp));
+putChar(result, 1, Character.lowSurrogate(cp));
+return result;
+}
+}

We should continue to assume that supplementary characters are very rare,
even in this context, so the supplementary character code could be moved to
a separate cold method.

Re: [11] RFR: 4993841: (str) java.lang.Character should have a toString(int) method

[Martin Buchholz]

+ * @param   codepoint   a {@code codePoint}.


does not match the formal parameter.  javadoc -private should give an error
on that.

[11] RFR: 4993841: (str) java.lang.Character should have a toString(int) method

[naoto . sato]

Hi,

Please review the fix to the following issue:

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

The proposed changeset is located at:

http://cr.openjdk.java.net/~naoto/4993841/webrev.03/

This stems from the recent discussion regarding String.repeat().[1] The 
corresponding CSR has already been approved.


Naoto

--
[1] 
http://mail.openjdk.java.net/pipermail/core-libs-dev/2018-February/051568.html