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

[Julien Fischer]

On Thu, 14 Jun 2018, Zoltan Somogyi wrote:

>
> 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.

Done.

>> 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?

Yes.

>> @@ -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.

Done.

>> 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

...

> 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.)

Yes, I was intending do so after I'd committed this.

Julien.