[m-rev.] for review: Add string indexing predicates that indicate a code unit was replaced.

Julien Fischer jfischer at opturion.com
Fri Nov 15 20:42:47 AEDT 2019



On Fri, 15 Nov 2019, Peter Wang wrote:

> On Fri, 15 Nov 2019 17:21:59 +1100 (AEDT), Julien Fischer <jfischer at opturion.com> wrote:
>>> diff --git a/library/string.m b/library/string.m
>>> index a9fbfd693..a28c33792 100644
>>> --- a/library/string.m
>>> +++ b/library/string.m
>>> @@ -262,6 +262,10 @@
>>> % Reading characters from strings.
>>> %
>>>
>>> +:- type maybe_replaced
>>> +    --->    not_replaced
>>> +    ;       replaced_code_unit(uint8).
>>> +
>>
>> Document that type.
>>
>> Is there a reason you didn't have the predicates below return a single
>> value?  (i.e. a value that's either a character in the string of the
>> invalid code unit)
>
> As in
>
>    :- type char_or_replaced
> 	--->	char(char)
> 	;	replaced_code_unit(uint8).
>
>    :- pred index_next_repl(string::in, int::in, int::in,
> 	char_or_replaced::out) is semidet.

Yes.

> I forgot to consider it.
>
> Using two return values has an advantage in that it avoids memory
> allocation in the normal case. That advantage should go away once the
> necessary argument packing optimisations are turned on.
>
> The argument packing wouldn't be used for Java/C#.

At least as they are now ...

> The Java backend needs to allocate an array to return two values,
> so maybe there is no difference there.
> But the C# backend will not need to allocate an object to hold the char.
> (I didn't measure anything; maybe none of this matters.)

It doesn't particularly matter, I was just curious.

Julien.


More information about the reviews mailing list