Re: [11] RFR: 4993841: (str) java.lang.Character should have a toString(int) method
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
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
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
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
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" À: "Stuart Marks" , "Xueming Shen" , "core-libs-dev" 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
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 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" > > À: "Stuart Marks" , "Xueming Shen" < > xueming.s...@gmail.com>, "core-libs-dev" > > > > 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
+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
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" À: "Stuart Marks" , "Xueming Shen" , "core-libs-dev" 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
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" > À: "Stuart Marks" , "Xueming Shen" > , "core-libs-dev" > > 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
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
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
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
+ * @param codepoint a {@code codePoint}. does not match the formal parameter. javadoc -private should give an error on that.