Revised at http://cr.openjdk.java.net/~jlaskey/8223780/webrev-03/src/java.base/share/classes/java/lang/String.java.frames.html <http://cr.openjdk.java.net/~jlaskey/8223780/webrev-03/src/java.base/share/classes/java/lang/String.java.frames.html>
> On May 21, 2019, at 3:23 PM, Jim Laskey <[email protected]> wrote: > > > >> On May 21, 2019, at 2:57 PM, Ivan Gerasimov <[email protected]> >> wrote: >> >> Hi Jim! >> >> A few comments: >> >> 1) >> Probably, there's no need to update ch in these cases: >> case '\'': >> ch = '\''; >> break; >> case '\"': >> ch = '\"'; >> break; > > True > >> >> 2) >> Character.digit(ch, 8) will accept non-Latin1 digits. >> So, a sequence \\3\uFF17\uFF17 will be parsed as \\377. >> (Note, that the first digit still can only be from range '0'-'9'). > > Also true. Subtlety in UnicodeReader. > >> >> 3) >> It's not obvious how this condition can be triggered: >> if (0377 < code) { >> throw new MalformedEscapeException(from); >> } >> I might be missing something, but I cannot see how 'code' can become > 0377. > > I removed this check in round 2. > >> >> 4) >> throw new MalformedEscapeException(from); >> This will report the next index after the error. Was it intentional? >> > > I switched to using an IllegalArgumentError and displaying the actual > character. > > >> With kind regards, >> Ivan > > Much thanks. > > >> >> >> On 5/21/19 7:56 AM, Jim Laskey wrote: >>> Please do a code review of the new String:: translateEscapes instance >>> method. This instance method is being introduced to support JEP-355: Text >>> Blocks, by translating escape sequences in the text block content. >>> >>> Thank you. >>> >>> -- Jim >>> >>> webrev: http://cr.openjdk.java.net/~jlaskey/8223780/webrev-01 >>> <http://cr.openjdk.java.net/~jlaskey/8223780/webrev-01> >>> jbs: https://bugs.openjdk.java.net/browse/JDK-8223780 >>> <https://bugs.openjdk.java.net/browse/JDK-8223780> >>> csr: https://bugs.openjdk.java.net/browse/JDK-8223781 >>> <https://bugs.openjdk.java.net/browse/JDK-8223781> >>> jep: https://bugs.openjdk.java.net/browse/JDK-8222530 >>> <https://bugs.openjdk.java.net/browse/JDK-8222530> >>> >>> >> >> -- >> With kind regards, >> Ivan Gerasimov >> >
