[m-rev.] for review: fixes for deconstruct.functor and io.write

[Zoltan Somogyi]

On Wed, 13 Jun 2018 22:10:51 -0400 (EDT), Julien Fischer <jfischer at opturion.com> wrote:
>   quote_special_escape_char('\\', "'\\\\'").
> -quote_special_escape_char('\'', "'\\'").
> +quote_special_escape_char('\'', "'\\''").
>   quote_special_escape_char('\a', "'\\a'").
>   quote_special_escape_char('\b', "'\\b'").
>   quote_special_escape_char('\r', "'\\r'").
> diff --git a/library/term_io.m b/library/term_io.m
> index 3fee715..a818b7a 100644
> --- a/library/term_io.m
> +++ b/library/term_io.m
> @@ -1005,12 +1005,16 @@ encode_escaped_char(Char::out, Str::in) :-
>   :- mode mercury_escape_special_char(in, out) is semidet.
>   :- mode mercury_escape_special_char(out, in) is semidet.
> 
> +mercury_escape_special_char('\a', 'a').
> +mercury_escape_special_char('\b', 'b').
> +mercury_escape_special_char('\r', 'r').
> +mercury_escape_special_char('\f', 'f').
> +mercury_escape_special_char('\t', 't').
> +mercury_escape_special_char('\n', 'n').
> +mercury_escape_special_char('\v', 'v').
> +mercury_escape_special_char('\\', '\\').
>   mercury_escape_special_char('''', '''').
>   mercury_escape_special_char('"', '"').
> -mercury_escape_special_char('\\', '\\').
> -mercury_escape_special_char('\n', 'n').
> -mercury_escape_special_char('\t', 't').
> -mercury_escape_special_char('\b', 'b').

These two places should have comments referring maintainers to the other,
if they don't already, to reduce the probability of anyone modifying one
without also modifying the other.

> diff --git a/runtime/mercury_ml_expand_body.h b/runtime/mercury_ml_expand_body.h
> index 6c1bf54..9e1bfee 100644
> --- a/runtime/mercury_ml_expand_body.h
> +++ b/runtime/mercury_ml_expand_body.h
> @@ -880,10 +880,9 @@ EXPAND_FUNCTION_NAME(MR_TypeInfo type_info, MR_Word *data_word_ptr,
>                   char        *str;
> 
>                   data_word = *data_word_ptr;
> -                // XXX what should we do with other non-printable characters.
>                   switch (data_word) {
>                       case '\\': str_ptr = "'\\\\'"; break;
> -                    case '\'': str_ptr = "'\\''"; break;
> +                    case '\'': str_ptr = "'\\''";  break;
>                       case '\a': str_ptr = "'\\a'";  break;

What changed here? Just white space?

> @@ -892,7 +891,28 @@ EXPAND_FUNCTION_NAME(MR_TypeInfo type_info, MR_Word *data_word_ptr,
>                       case '\n': str_ptr = "'\\n'";  break;
>                       case '\v': str_ptr = "'\\v'";  break;
>                       default:
> -                        sprintf(buf, "\'%c\'", (char) data_word);
> +                        // Print C0 control characters and Delete in
> +                        // octal.
> +                        if (data_word <= 0x1f || data_word == 0x7f) {
> +                            sprintf(buf, "\'\\%03o\\\'", data_word);
> +                        } else if (MR_is_ascii(data_word)) {
> +                            sprintf(buf, "\'%c\'", (char) data_word);
> +                        } else if (MR_is_surrogate(data_word)) {
> +                            // XXX Should throw an exception.
> +                            MR_fatal_error(MR_STRINGIFY(EXPAND_FUNCTION_NAME)
> +                                ": attempt to deconstruct surrogate code point");
> +                        } else {
> +                            size_t n = MR_utf8_encode(buf + 1, (MR_Char)data_word);
> +                            // XXX Should throw an exception.
> +                            if (n == 0) {
> +                                MR_fatal_error(MR_STRINGIFY(EXPAND_FUNCTION_NAME)
> +                                    ": attempt to deconstruct illegal code point");
> +                            }
> +                            buf[0] = '\'';
> +                            buf[n + 1] = '\'';
> +                            buf[n + 2] = '\0';
> +
> +                        }

Likewise, I think this code should have a comment pointing to its Mercury equivalent,
and vice versa.

> diff --git a/tests/hard_coded/write.m b/tests/hard_coded/write.m
> index 54d5b1b..3e9c9f0 100644
> --- a/tests/hard_coded/write.m
> +++ b/tests/hard_coded/write.m
> @@ -12,6 +12,7 @@
>   :- implementation.
> 
>   :- import_module array.
> +:- import_module float.
>   :- import_module int.
>   :- import_module list.
>   :- import_module map.
> @@ -125,15 +126,29 @@ test_builtins(!IO) :-
>       io.write("Hello, world\n", !IO), newline(!IO),
>       io.write("Foo%sFoo", !IO), newline(!IO),
>       io.write("""", !IO), newline(!IO),    % interesting - prints """ of course
> +    io.write("\a\b\f\t\n\r\v\"\\", !IO), newline(!IO),
> 
>       % Test characters.
>       io.write('a', !IO), newline(!IO),
>       io.write('&', !IO), newline(!IO),
> +    io.write('\a', !IO), newline(!IO),
> +    io.write('\b', !IO), newline(!IO),
> +    io.write('\f', !IO), newline(!IO),
> +    io.write('\t', !IO), newline(!IO),
> +    io.write('\n', !IO), newline(!IO),
> +    io.write('\r', !IO), newline(!IO),
> +    io.write('\v', !IO), newline(!IO),
> +    io.write('\'', !IO), newline(!IO),
> +    io.write(('\\') : character, !IO), newline(!IO),
> +    io.write('\"', !IO), newline(!IO),
> 
>       % Test floats.
>       io.write(3.14159, !IO), newline(!IO),
>       io.write(11.28324983E-22, !IO), newline(!IO),
>       io.write(22.3954899E22, !IO), newline(!IO),
> +    NegInf : float = -float.infinity,
> +    io.write(NegInf, !IO), newline(!IO),
> +    io.write(float.infinity, !IO), newline(!IO),

These would be simpler if they called io.write_line, not io.write.
(Though io.write_line did not yet exist when this test was first written.)

Other than the above, the diff seems fine.

Zoltan.