<div dir="ltr">This looks fine.</div> <div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Wed, Oct 16, 2019 at 5:04 PM Peter Wang <<a href="mailto:novalazy@gmail.com">novalazy@gmail.com</a>> wrote: </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">library/string.m: Make string.index_next and string.unsafe_index_next return either U+FFFD or an unpaired surrogate code point when an ill-formed code unit sequence is detected. tests/hard_coded/Mmakefile: tests/hard_coded/string_index_next_ilseq.exp: tests/hard_coded/string_index_next_ilseq.exp2: tests/hard_coded/string_index_next_ilseq.m: Add test case. --- library/string.m | 69 ++++++++++--------- tests/hard_coded/Mmakefile | 1 + tests/hard_coded/string_index_next_ilseq.exp | 5 ++ tests/hard_coded/string_index_next_ilseq.exp2 | 5 ++ tests/hard_coded/string_index_next_ilseq.m | 44 ++++++++++++ 5 files changed, 90 insertions(+), 34 deletions(-) create mode 100644 tests/hard_coded/string_index_next_ilseq.exp create mode 100644 tests/hard_coded/string_index_next_ilseq.exp2 create mode 100644 tests/hard_coded/string_index_next_ilseq.m diff --git a/library/string.m b/library/string.m index 09716dc49..b687cece3 100644 --- a/library/string.m +++ b/library/string.m @@ -274,17 +274,26 @@ % index_next(String, Index, NextIndex, Char): % - % Like `index'/3 but also returns the position of the code unit - % that follows the code point beginning at `Index', - % i.e. NextIndex = Index + num_code_units_to_encode(Char). + % If `Index' is the initial code unit offset of a well-formed code unit + % sequence in `String' then `Char' is the code point encoded by that + % sequence, and `NextIndex' is the offset immediately following that + % sequence. + % + % If the code unit in `String' at `Index' is part of an ill-formed sequence + % then `Char' is either U+FFFD REPLACEMENT CHARACTER (if strings use UTF-8 + % encoding) or the unpaired surrogate code point at `Index' (if strings use + % UTF-16 encoding), and `NextIndex' is Index + 1. + % + % Fails if `Index' is out of range (negative, or greater than or equal to + % the length of `String'). % :- pred index_next(string::in, int::in, int::out, char::uo) is semidet. % unsafe_index_next(String, Index, NextIndex, Char): % - % `Char' is the character (code point) in `String', beginning at the - % code unit `Index'. `NextIndex' is the offset following the encoding - % of `Char'. Fails if `Index' is equal to the length of `String'. + % Like index_next/4 but does not check that `Index' is in range. + % Fails if `Index' is equal to the length of `String'. + % % WARNING: behavior is UNDEFINED if `Index' is out of range % (negative, or greater than the length of `String'). % @@ -2166,9 +2175,7 @@ duplicate_char(Char, Count, String) :- % so that the compiler can do loop invariant hoisting on calls to them % that occur in loops. -% XXX ILSEQ The index_next family fails if an ill-formed sequence is detected. -% It needs to be updated to match the behaviour of the index family. -% +% XXX ILSEQ % We should allow the possibility of working with strings containing ill-formed % sequences. That would require predicates that can return either a code point % when possible, or else code units from ill-formed sequences. @@ -2251,11 +2258,6 @@ String ^ unsafe_elem(Index) = unsafe_index(String, Index). %---------------------% -% XXX ILSEQ index_next/4 and unsafe_index_next/4 fail when an ill-formed -% sequence is detected, unlike the index family. Most programs will probably -% work as intended if we just replace code units in ill-formed sequences with -% replacement char (for UTF-8) or return the unpaired surrogate code point -% (for UTF-16). index_next(Str, Index, NextIndex, Char) :- Len = length(Str), ( if index_check(Index, Len) then @@ -2264,10 +2266,6 @@ index_next(Str, Index, NextIndex, Char) :- fail ). -% XXX ILSEQ Behaviour of unsafe_index_next depends on target language. -% - c: fails at ill-formed sequence -% - java: fails at unpaired surrogate -% - csharp: System.Char.ConvertToUtf32 throws exception for unpaired surrogate :- pragma foreign_proc("C", unsafe_index_next(Str::in, Index::in, NextIndex::out, Ch::uo), [will_not_call_mercury, promise_pure, thread_safe, will_not_modify_trail, @@ -2280,7 +2278,11 @@ index_next(Str, Index, NextIndex, Char) :- } else { NextIndex = Index; Ch = MR_utf8_get_next_mb(Str, &NextIndex); - SUCCESS_INDICATOR = (Ch > 0); + if (Ch < 0) { + Ch = 0xfffd; + NextIndex = Index + 1; + } + SUCCESS_INDICATOR = MR_TRUE; } "). :- pragma foreign_proc("C#", @@ -2288,7 +2290,7 @@ index_next(Str, Index, NextIndex, Char) :- [will_not_call_mercury, promise_pure, thread_safe, will_not_modify_trail, does_not_affect_liveness, no_sharing], " - if (Index < Str.Length) { + try { Ch = System.Char.ConvertToUtf32(Str, Index); if (Ch <= 0xffff) { NextIndex = Index + 1; @@ -2296,10 +2298,15 @@ index_next(Str, Index, NextIndex, Char) :- NextIndex = Index + 2; } SUCCESS_INDICATOR = true; - } else { - Ch = -1; + } catch (System.ArgumentOutOfRangeException) { + Ch = 0; NextIndex = Index; SUCCESS_INDICATOR = false; + } catch (System.ArgumentException) { + // Return unpaired surrogate code point. + Ch = Str[Index]; + NextIndex = Index + 1; + SUCCESS_INDICATOR = true; } "). :- pragma foreign_proc("Java", @@ -2307,19 +2314,12 @@ index_next(Str, Index, NextIndex, Char) :- [will_not_call_mercury, promise_pure, thread_safe, will_not_modify_trail, does_not_affect_liveness, no_sharing], " - if (Index < Str.length()) { + try { Ch = Str.codePointAt(Index); - SUCCESS_INDICATOR = - !java.lang.Character.isHighSurrogate((char) Ch) && - !java.lang.Character.isLowSurrogate((char) Ch); - if (SUCCESS_INDICATOR) { - NextIndex = Index + java.lang.Character.charCount(Ch); - } else { - Ch = -1; - NextIndex = Index; - } - } else { - Ch = -1; + NextIndex = Index + java.lang.Character.charCount(Ch); + SUCCESS_INDICATOR = true; + } catch (IndexOutOfBoundsException e) { + Ch = 0; NextIndex = Index; SUCCESS_INDICATOR = false; } @@ -2329,6 +2329,7 @@ index_next(Str, Index, NextIndex, Char) :- [will_not_call_mercury, promise_pure, thread_safe, will_not_modify_trail, does_not_affect_liveness, no_sharing], " + % XXX does not handle ill-formed sequences as described case Str of << _:Index/binary, Ch/utf8, _/binary >> -> if diff --git a/tests/hard_coded/Mmakefile b/tests/hard_coded/Mmakefile index 631d0da19..8c317da86 100644 --- a/tests/hard_coded/Mmakefile +++ b/tests/hard_coded/Mmakefile @@ -359,6 +359,7 @@ ORDINARY_PROGS = \ string_codepoint \ string_first_char \ string_index_ilseq \ + string_index_next_ilseq \ string_loop \ string_presuffix \ string_set_char \ diff --git a/tests/hard_coded/string_index_next_ilseq.exp b/tests/hard_coded/string_index_next_ilseq.exp new file mode 100644 index 000000000..1ad4aba9a --- /dev/null +++ b/tests/hard_coded/string_index_next_ilseq.exp @@ -0,0 +1,5 @@ +string.index_next(S, 0, 4, 0x1f600) +string.index_next(S, 4, 5, 0xfffd) +string.index_next(S, 5, 6, 0xfffd) +string.index_next(S, 6, 10, 0x1f600) +string.index_next(S, 10, _, _) failed diff --git a/tests/hard_coded/string_index_next_ilseq.exp2 b/tests/hard_coded/string_index_next_ilseq.exp2 new file mode 100644 index 000000000..c889bf0d8 --- /dev/null +++ b/tests/hard_coded/string_index_next_ilseq.exp2 @@ -0,0 +1,5 @@ +string.index_next(S, 0, 2, 0x1f600) +string.index_next(S, 2, 3, 0xde00) +string.index_next(S, 3, 4, 0xd83d) +string.index_next(S, 4, 6, 0x1f600) +string.index_next(S, 6, _, _) failed diff --git a/tests/hard_coded/string_index_next_ilseq.m b/tests/hard_coded/string_index_next_ilseq.m new file mode 100644 index 000000000..848a6fbae --- /dev/null +++ b/tests/hard_coded/string_index_next_ilseq.m @@ -0,0 +1,44 @@ +%---------------------------------------------------------------------------% +% vim: ts=4 sw=4 et ft=mercury +%---------------------------------------------------------------------------% +% +% The .exp file is for backends using UTF-8 string encoding. +% The .exp2 file is for backends using UTF-16 string encoding. +% +%---------------------------------------------------------------------------% + +:- module string_index_next_ilseq. +:- interface. + +:- import_module io. + +:- pred main(io::di, io::uo) is det. + +%---------------------------------------------------------------------------% + +:- implementation. + +:- import_module char. +:- import_module list. +:- import_module string. + +%---------------------------------------------------------------------------% + +main(!IO) :- + S0 = "😀", + S1 = string.between(S0, 0, 1), + S2 = string.between(S0, 1, 2), + S = S0 ++ S2 ++ S1 ++ S0, + test_index_next(S, 0, !IO). + +:- pred test_index_next(string::in, int::in, io::di, io::uo) is det. + +test_index_next(S, Index, !IO) :- + ( if string.index_next(S, Index, NextIndex, Char) then + io.format("string.index_next(S, %d, %d, %#x)\n", + [i(Index), i(NextIndex), i(char.to_int(Char))], !IO), + test_index_next(S, NextIndex, !IO) + else + io.format("string.index_next(S, %d, _, _) failed\n", + [i(Index)], !IO) + ). -- 2.23.0 _______________________________________________ reviews mailing list <a href="mailto:reviews@lists.mercurylang.org" target="_blank">reviews@lists.mercurylang.org</a> <a href="https://lists.mercurylang.org/listinfo/reviews" rel="noreferrer" target="_blank">https://lists.mercurylang.org/listinfo/reviews</a> </blockquote></div>