<div dir="ltr">This looks fine.</div> <div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Tue, Oct 29, 2019 at 12:02 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 from_char_list, from_rev_char_list, to_char_list throw an Â Â exception if the list of chars includes a surrogate code point that Â Â cannot be encoded in a UTF-8 string. Â Â Make semidet_from_char_list, semidet_from_rev_char_list, Â Â to_char_list fail if the list of chars includes a surrogate code Â Â point that cannot be encoded in a UTF-8 string. runtime/mercury_string.h: Â Â Document return value of MR_utf8_width. tests/hard_coded/Mmakefile: tests/hard_coded/string_from_char_list_ilseq.exp: tests/hard_coded/string_from_char_list_ilseq.exp2: tests/hard_coded/string_from_char_list_ilseq.m: Â Â Add test case. tests/hard_coded/null_char.exp: Â Â Expect new message in exceptions thrown by from_char_list, Â Â from_rev_char_list. tests/hard_coded/string_hash.m: Â Â Don't generate surrogate code points in random strings. --- Â library/string.mÂ Â Â Â Â Â Â Â Â Â Â Â Â Â Â | 162 +++++++++--------- Â runtime/mercury_string.hÂ Â Â Â Â Â Â Â Â Â Â |Â Â 1 + Â tests/hard_coded/MmakefileÂ Â Â Â Â Â Â Â Â Â |Â Â 1 + Â tests/hard_coded/null_char.expÂ Â Â Â Â Â Â Â |Â Â 4 +- Â .../string_from_char_list_ilseq.expÂ Â Â Â Â Â |Â 15 ++ Â .../string_from_char_list_ilseq.exp2Â Â Â Â Â |Â 15 ++ Â .../hard_coded/string_from_char_list_ilseq.mÂ |Â 88 ++++++++++ Â tests/hard_coded/string_hash.mÂ Â Â Â Â Â Â Â |Â Â 7 +- Â 8 files changed, 211 insertions(+), 82 deletions(-) Â create mode 100644 tests/hard_coded/string_from_char_list_ilseq.exp Â create mode 100644 tests/hard_coded/string_from_char_list_ilseq.exp2 Â create mode 100644 tests/hard_coded/string_from_char_list_ilseq.m diff --git a/library/string.m b/library/string.m index ab19974d2..cbdbcd128 100644 --- a/library/string.m +++ b/library/string.m @@ -131,10 +131,10 @@ Â Â Â % If strings use UTF-16 encoding then each unpaired surrogate code point Â Â Â % is returned as a separate code point in the list. Â Â Â % -Â Â % The reverse mode of the predicate throws an exception if -Â Â % the list of characters contains a null character. -Â Â % NOTE: In the future we may also throw an exception if the list contains -Â Â % a surrogate code point. +Â Â % The reverse mode of the predicate throws an exception if the list +Â Â % contains a null character or code point that cannot be encoded in a +Â Â % string (namely, surrogate code points cannot be encoded in UTF-8 +Â Â % strings). Â Â Â % Â Â Â % The reverse mode of to_char_list/2 is deprecated because the implied Â Â Â % ability to round trip convert a string to a list then back to the same @@ -155,10 +155,10 @@ Â Â Â % If strings use UTF-16 encoding then each unpaired surrogate code point Â Â Â % is returned as a separate code point in the list. Â Â Â % -Â Â % The reverse mode of the predicate throws an exception if -Â Â % the list of characters contains a null character. -Â Â % NOTE: In the future we may also throw an exception if the list contains -Â Â % a surrogate code point. +Â Â % The reverse mode of the predicate throws an exception if the list +Â Â % contains a null character or code point that cannot be encoded in a +Â Â % string (namely, surrogate code points cannot be encoded in UTF-8 +Â Â % strings). Â Â Â % Â Â Â % The reverse mode of to_rev_char_list/2 is deprecated because the implied Â Â Â % ability to round trip convert a string to a list then back to the same @@ -171,10 +171,9 @@ Â :- mode to_rev_char_list(uo, in) is det. Â Â Â % Convert a list of characters (code points) to a string. -Â Â % Throws an exception if the list of characters contains a null character. -Â Â % -Â Â % NOTE: In the future we may also throw an exception if the list contains -Â Â % a surrogate code point. +Â Â % Throws an exception if the list contains a null character or code point +Â Â % that cannot be encoded in a string (namely, surrogate code points cannot +Â Â % be encoded in UTF-8 strings). Â Â Â % Â Â Â % The forward mode of from_char_list/2 is deprecated because the implied Â Â Â % ability to round trip convert a string to a list then back to the same @@ -187,28 +186,21 @@ Â :- mode from_char_list(out, in) is det. Â Â Â % As above, but fail instead of throwing an exception if the list contains -Â Â % a null character. -Â Â % -Â Â % NOTE: In the future we may also throw an exception if the list contains -Â Â % a surrogate code point. +Â Â % a null character or code point that cannot be encoded in a string. Â Â Â % Â :- pred semidet_from_char_list(list(char)::in, string::uo) is semidet. Â Â Â % Same as from_char_list, except that it reverses the order Â Â Â % of the characters. -Â Â % Throws an exception if the list of characters contains a null character. -Â Â % -Â Â % NOTE: In the future we may also throw an exception if the list contains -Â Â % a surrogate code point. +Â Â % Throws an exception if the list contains a null character or code point +Â Â % that cannot be encoded in a string (namely, surrogate code points cannot +Â Â % be encoded in UTF-8 strings). Â Â Â % Â :- func from_rev_char_list(list(char)::in) = (string::uo) is det. Â :- pred from_rev_char_list(list(char)::in, string::uo) is det. Â Â Â % As above, but fail instead of throwing an exception if the list contains -Â Â % a null character. -Â Â % -Â Â % NOTE: In the future we may also throw an exception if the list contains -Â Â % a surrogate code point. +Â Â % a null character or code point that cannot be encoded in a string. Â Â Â % Â :- pred semidet_from_rev_char_list(list(char)::in, string::uo) is semidet. @@ -1696,7 +1688,7 @@ from_char_list(Chars::in, Str::uo) :- Â Â Â ( if semidet_from_char_list(Chars, Str0) then Â Â Â Â Â Str = Str0 Â Â Â else -Â Â Â Â unexpected($pred, "null character in list") +Â Â Â Â unexpected($pred, "null character or surrogate code point in list") Â Â Â ). Â :- pragma foreign_proc("C", @@ -1704,51 +1696,55 @@ from_char_list(Chars::in, Str::uo) :- Â Â Â [will_not_call_mercury, promise_pure, thread_safe, will_not_modify_trail, Â Â Â Â Â does_not_affect_liveness, may_not_duplicate, no_sharing], Â "{ -Â Â // mode (uo, in) is det Â Â Â MR_Word char_list_ptr; Â Â Â size_t size; -Â Â MR_Char c; Â Â Â // Loop to calculate list length + sizeof(MR_Word) in `size' Â Â Â // using list in `char_list_ptr'. -Â Â size = 0; -Â Â char_list_ptr = CharList; -Â Â while (! MR_list_is_empty(char_list_ptr)) { -Â Â Â Â c = (MR_Char) MR_list_head(char_list_ptr); -Â Â Â Â if (MR_is_ascii(c)) { -Â Â Â Â Â Â size++; -Â Â Â Â } else { -Â Â Â Â Â Â // XXX ILSEQ Do something if c is a surrogate code point. -Â Â Â Â Â Â size += MR_utf8_width(c); -Â Â Â Â } -Â Â Â Â char_list_ptr = MR_list_tail(char_list_ptr); -Â Â } - -Â Â // Allocate heap space for string. -Â Â MR_allocate_aligned_string_msg(Str, size, MR_ALLOC_ID); - -Â Â // Loop to copy the characters from the char_list to the string. Â Â Â SUCCESS_INDICATOR = MR_TRUE; Â Â Â size = 0; Â Â Â char_list_ptr = CharList; Â Â Â while (! MR_list_is_empty(char_list_ptr)) { -Â Â Â Â c = (MR_Char) MR_list_head(char_list_ptr); -Â Â Â Â // It is an error to put a null character in a string -Â Â Â Â // (see the comments at the top of this file). +Â Â Â Â MR_Char c = (MR_Char) MR_list_head(char_list_ptr); Â Â Â Â Â if (c == '\\0') { Â Â Â Â Â Â Â SUCCESS_INDICATOR = MR_FALSE; Â Â Â Â Â Â Â break; Â Â Â Â Â } Â Â Â Â Â if (MR_is_ascii(c)) { -Â Â Â Â Â Â Str[size] = c; Â Â Â Â Â Â Â size++; Â Â Â Â Â } else { -Â Â Â Â Â Â size += MR_utf8_encode(Str + size, c); +Â Â Â Â Â Â size_t csize = MR_utf8_width(c); +Â Â Â Â Â Â if (csize == 0) { +Â Â Â Â Â Â Â Â // c is a surrogate code point (or even out of range, +Â Â Â Â Â Â Â Â // but that is not supposed to happen). +Â Â Â Â Â Â Â Â SUCCESS_INDICATOR = MR_FALSE; +Â Â Â Â Â Â Â Â break; +Â Â Â Â Â Â } +Â Â Â Â Â Â size += csize; Â Â Â Â Â } Â Â Â Â Â char_list_ptr = MR_list_tail(char_list_ptr); Â Â Â } -Â Â Str[size] = '\\0'; +Â Â if (SUCCESS_INDICATOR) { +Â Â Â Â // Allocate heap space for string. +Â Â Â Â MR_allocate_aligned_string_msg(Str, size, MR_ALLOC_ID); + +Â Â Â Â // Loop to copy the characters from the char_list to the string. +Â Â Â Â size = 0; +Â Â Â Â char_list_ptr = CharList; +Â Â Â Â while (! MR_list_is_empty(char_list_ptr)) { +Â Â Â Â Â Â MR_Char c = (MR_Char) MR_list_head(char_list_ptr); +Â Â Â Â Â Â if (MR_is_ascii(c)) { +Â Â Â Â Â Â Â Â Str[size] = c; +Â Â Â Â Â Â Â Â size++; +Â Â Â Â Â Â } else { +Â Â Â Â Â Â Â Â size += MR_utf8_encode(Str + size, c); +Â Â Â Â Â Â } +Â Â Â Â Â Â char_list_ptr = MR_list_tail(char_list_ptr); +Â Â Â Â } + +Â Â Â Â Str[size] = '\\0'; +Â Â } Â }"). Â :- pragma foreign_proc("C#", Â Â Â semidet_from_char_list(CharList::in, Str::uo), @@ -1808,6 +1804,7 @@ semidet_from_char_list(CharList, Str) :- Â Â Â ; Â Â Â Â Â CharList = [C | Cs], Â Â Â Â Â not char.to_int(C, 0), +Â Â Â Â internal_encoding_is_utf8 => not char.is_surrogate(C), Â Â Â Â Â semidet_from_char_list(Cs, Str0), Â Â Â Â Â first_char(Str, C, Str0) Â Â Â ). @@ -1827,7 +1824,7 @@ from_rev_char_list(Chars, Str) :- Â Â Â ( if semidet_from_rev_char_list(Chars, Str0) then Â Â Â Â Â Str = Str0 Â Â Â else -Â Â Â Â unexpected($pred, "null character in list") +Â Â Â Â unexpected($pred, "null character or surrogate code point in list") Â Â Â ). Â :- pragma foreign_proc("C", @@ -1837,48 +1834,55 @@ from_rev_char_list(Chars, Str) :- Â "{ Â Â Â MR_Word list_ptr; Â Â Â MR_Word size; -Â Â MR_Char c; Â Â Â // Loop to calculate list length in `size' using list in `list_ptr'. +Â Â SUCCESS_INDICATOR = MR_TRUE; Â Â Â size = 0; Â Â Â list_ptr = Chars; Â Â Â while (!MR_list_is_empty(list_ptr)) { -Â Â Â Â c = (MR_Char) MR_list_head(list_ptr); -Â Â Â Â if (MR_is_ascii(c)) { -Â Â Â Â Â Â size++; -Â Â Â Â } else { -Â Â Â Â Â Â // XXX ILSEQ Do something if c is a surrogate code point. -Â Â Â Â Â Â size += MR_utf8_width(c); -Â Â Â Â } -Â Â Â Â list_ptr = MR_list_tail(list_ptr); -Â Â } - -Â Â // Allocate heap space for string. -Â Â MR_allocate_aligned_string_msg(Str, size, MR_ALLOC_ID); - -Â Â // Set size to be the offset of the end of the string -Â Â // (ie the \\0) and null terminate the string. -Â Â Str[size] = '\\0'; - -Â Â // Loop to copy the characters from the list_ptr to the string -Â Â // in reverse order. -Â Â list_ptr = Chars; -Â Â SUCCESS_INDICATOR = MR_TRUE; -Â Â while (!MR_list_is_empty(list_ptr)) { -Â Â Â Â c = (MR_Char) MR_list_head(list_ptr); +Â Â Â Â MR_Char c = (MR_Char) MR_list_head(list_ptr); Â Â Â Â Â if (c == '\\0') { Â Â Â Â Â Â Â SUCCESS_INDICATOR = MR_FALSE; Â Â Â Â Â Â Â break; Â Â Â Â Â } Â Â Â Â Â if (MR_is_ascii(c)) { -Â Â Â Â Â Â size--; -Â Â Â Â Â Â Str[size] = c; +Â Â Â Â Â Â size++; Â Â Â Â Â } else { -Â Â Â Â Â Â size -= MR_utf8_width(c); -Â Â Â Â Â Â MR_utf8_encode(Str + size, c); +Â Â Â Â Â Â size_t csize = MR_utf8_width(c); +Â Â Â Â Â Â if (csize == 0) { +Â Â Â Â Â Â Â Â // c is a surrogate code point (or even out of range, +Â Â Â Â Â Â Â Â // but that is not supposed to happen). +Â Â Â Â Â Â Â Â SUCCESS_INDICATOR = MR_FALSE; +Â Â Â Â Â Â Â Â break; +Â Â Â Â Â Â } +Â Â Â Â Â Â size += csize; Â Â Â Â Â } Â Â Â Â Â list_ptr = MR_list_tail(list_ptr); Â Â Â } + +Â Â if (SUCCESS_INDICATOR) { +Â Â Â Â // Allocate heap space for string. +Â Â Â Â MR_allocate_aligned_string_msg(Str, size, MR_ALLOC_ID); + +Â Â Â Â // Set size to be the offset of the end of the string +Â Â Â Â // (ie the \\0) and null terminate the string. +Â Â Â Â Str[size] = '\\0'; + +Â Â Â Â // Loop to copy the characters from the list_ptr to the string +Â Â Â Â // in reverse order. +Â Â Â Â list_ptr = Chars; +Â Â Â Â while (! MR_list_is_empty(list_ptr)) { +Â Â Â Â Â Â MR_Char c = (MR_Char) MR_list_head(list_ptr); +Â Â Â Â Â Â if (MR_is_ascii(c)) { +Â Â Â Â Â Â Â Â size--; +Â Â Â Â Â Â Â Â Str[size] = c; +Â Â Â Â Â Â } else { +Â Â Â Â Â Â Â Â size -= MR_utf8_width(c); +Â Â Â Â Â Â Â Â MR_utf8_encode(Str + size, c); +Â Â Â Â Â Â } +Â Â Â Â Â Â list_ptr = MR_list_tail(list_ptr); +Â Â Â Â } +Â Â } Â }"). Â :- pragma foreign_proc("C#", Â Â Â semidet_from_rev_char_list(Chars::in, Str::uo), diff --git a/runtime/mercury_string.h b/runtime/mercury_string.h index d3cb00262..a4dcf7c3c 100644 --- a/runtime/mercury_string.h +++ b/runtime/mercury_string.h @@ -485,6 +485,7 @@ extern MR_int_least32_t MR_utf8_get_next_mb(const MR_String s, Â extern MR_int_least32_t MR_utf8_prev_get(const MR_String s, MR_Integer *pos); Â // Return the number of bytes required to encode the code point `c' in UTF-8. +// Returns 0 for surrogate code points or illegal values. Â extern size_tÂ Â Â Â Â Â MR_utf8_width(MR_Char c); diff --git a/tests/hard_coded/Mmakefile b/tests/hard_coded/Mmakefile index ee9f51179..47e8ec5a8 100644 --- a/tests/hard_coded/Mmakefile +++ b/tests/hard_coded/Mmakefile @@ -365,6 +365,7 @@ ORDINARY_PROGS = \ Â Â Â Â string_count_codepoints_ilseq \ Â Â Â Â string_first_char \ Â Â Â Â string_fold_ilseq \ +Â Â Â Â string_from_char_list_ilseq \ Â Â Â Â string_index_ilseq \ Â Â Â Â string_index_next_ilseq \ Â Â Â Â string_loop \ diff --git a/tests/hard_coded/null_char.exp b/tests/hard_coded/null_char.exp index 7e03f6907..842f8aa9e 100644 --- a/tests/hard_coded/null_char.exp +++ b/tests/hard_coded/null_char.exp @@ -1,6 +1,6 @@ Â All tests should result in exceptions being thrown. -exception(univ_cons(software_error("predicate `string.from_char_list\'/2: Unexpected: null character in list"))) -exception(univ_cons(software_error("predicate `string.from_rev_char_list\'/2: Unexpected: null character in list"))) +exception(univ_cons(software_error("predicate `string.from_char_list\'/2: Unexpected: null character or surrogate code point in list"))) +exception(univ_cons(software_error("predicate `string.from_rev_char_list\'/2: Unexpected: null character or surrogate code point in list"))) Â exception(univ_cons(software_error("predicate `string.set_char\'/4: Unexpected: null character"))) Â exception(univ_cons(software_error("predicate `string.unsafe_set_char\'/4: Unexpected: null character"))) Â error("", io_error("null character in input")) diff --git a/tests/hard_coded/string_from_char_list_ilseq.exp b/tests/hard_coded/string_from_char_list_ilseq.exp new file mode 100644 index 000000000..73f24d867 --- /dev/null +++ b/tests/hard_coded/string_from_char_list_ilseq.exp @@ -0,0 +1,15 @@ +semidet_from_char_list: +a b +semidet_from_char_list failed +semidet_from_char_list failed +semidet_from_char_list failed +semidet_from_char_list failed +semidet_from_char_list failed + +semidet_from_rev_char_list: +b a +semidet_from_rev_char_list failed +semidet_from_rev_char_list failed +semidet_from_rev_char_list failed +semidet_from_rev_char_list failed +semidet_from_rev_char_list failed diff --git a/tests/hard_coded/string_from_char_list_ilseq.exp2 b/tests/hard_coded/string_from_char_list_ilseq.exp2 new file mode 100644 index 000000000..c77d3d069 --- /dev/null +++ b/tests/hard_coded/string_from_char_list_ilseq.exp2 @@ -0,0 +1,15 @@ +semidet_from_char_list: +a b +semidet_from_char_list failed +a 0xd83d b +a 0xde00 b +a ðŸ˜€ b +a 0xde00 0xd83d b + +semidet_from_rev_char_list: +b a +semidet_from_rev_char_list failed +b 0xd83d a +b 0xde00 a +b 0xde00 0xd83d a +b ðŸ˜€ a diff --git a/tests/hard_coded/string_from_char_list_ilseq.m b/tests/hard_coded/string_from_char_list_ilseq.m new file mode 100644 index 000000000..740f256b3 --- /dev/null +++ b/tests/hard_coded/string_from_char_list_ilseq.m @@ -0,0 +1,88 @@ +%---------------------------------------------------------------------------% +% 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_from_char_list_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) :- +Â Â io.write_string("semidet_from_char_list:\n", !IO), +Â Â list.foldl(test_from_char_list, test_cases, !IO), + +Â Â io.write_string("\nsemidet_from_rev_char_list:\n", !IO), +Â Â list.foldl(test_from_rev_char_list, test_cases, !IO). + +:- func test_cases = list(list(char)). + +test_cases = [ +Â Â ['a', 'b'], +Â Â ['a', char.det_from_int(0), 'b'], +Â Â ['a', char.det_from_int(0xd83d), 'b'], +Â Â ['a', char.det_from_int(0xde00), 'b'], +Â Â ['a', char.det_from_int(0xd83d), char.det_from_int(0xde00), 'b'], +Â Â ['a', char.det_from_int(0xde00), char.det_from_int(0xd83d), 'b'] +]. + +:- pred test_from_char_list(list(char)::in, io::di, io::uo) is det. + +test_from_char_list(Chars, !IO) :- +Â Â ( if semidet_from_char_list(Chars, Str) then +Â Â Â Â write_string_debug(Str, !IO), +Â Â Â Â <a href="http://io.nl" rel="noreferrer" target="_blank">io.nl</a>(!IO) +Â Â else +Â Â Â Â io.write_string("semidet_from_char_list failed\n", !IO) +Â Â ). + +:- pred test_from_rev_char_list(list(char)::in, io::di, io::uo) is det. + +test_from_rev_char_list(Chars, !IO) :- +Â Â ( if semidet_from_rev_char_list(Chars, Str) then +Â Â Â Â write_string_debug(Str, !IO), +Â Â Â Â <a href="http://io.nl" rel="noreferrer" target="_blank">io.nl</a>(!IO) +Â Â else +Â Â Â Â io.write_string("semidet_from_rev_char_list failed\n", !IO) +Â Â ). + +:- pred write_string_debug(string::in, io::di, io::uo) is det. + +write_string_debug(S, !IO) :- +Â Â write_string_debug_loop(S, 0, !IO). + +:- pred write_string_debug_loop(string::in, int::in, io::di, io::uo) is det. + +write_string_debug_loop(S, Index, !IO) :- +Â Â ( if string.index_next(S, Index, NextIndex, Char) then +Â Â Â Â ( if is_surrogate(Char) then +Â Â Â Â Â Â write_hex(char.to_int(Char), !IO) +Â Â Â Â else +Â Â Â Â Â Â io.write_char(Char, !IO) +Â Â Â Â ), +Â Â Â Â io.write_char(' ', !IO), +Â Â Â Â write_string_debug_loop(S, NextIndex, !IO) +Â Â else +Â Â Â Â true +Â Â ). + +:- pred write_hex(int::in, io::di, io::uo) is det. + +write_hex(I, !IO) :- +Â Â io.format("%#x", [i(I)], !IO). diff --git a/tests/hard_coded/string_hash.m b/tests/hard_coded/string_hash.m index 1e8afe68b..5d534935b 100644 --- a/tests/hard_coded/string_hash.m +++ b/tests/hard_coded/string_hash.m @@ -93,7 +93,12 @@ rand_char(Char, !RS) :- Â Â Â random.random(Rand, !RS), Â Â Â % U+0001..U+10ffff (avoid null character). Â Â Â Int = 1 + (Rand `mod` char.max_char_value), -Â Â char.det_from_int(Int, Char). +Â Â char.det_from_int(Int, Char0), +Â Â ( if is_surrogate(Char0) then +Â Â Â Â rand_char(Char, !RS) +Â Â else +Â Â Â Â Char = Char0 +Â Â ). Â :- pred test_hash(string::in, int::in, int::in, string::in, Â Â Â bool::in, bool::out, io::di, io::uo) is det. -- 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>