[m-rev.] for review: improve the documentation for array.fetch_items

[Julien Fischer]

On Mon, 24 Dec 2018, Zoltan Somogyi wrote:

> On Mon, 24 Dec 2018 04:25:16 +0000 (UTC), Julien Fischer <jfischer at opturion.com> wrote:
>> -    % fetch_items takes an array and a lower and upper index,
>> -    % and places those items in the array between these indices into a list.
>> -    % It is an error if either index is out of bounds.
>> +    % fetch_items(Array, Lo, Hi, List):
>> +    % Returns a list containing the items in the array with index in the range
>> +    % Lo..Hi (inclusive) in the same order that they occurred in the array.
>> +    % If Hi < Lo then the empty list is returned.
>
> I would reword as "Returns an empty list if Hi < Lo", to fit in better
> with neighbouring sentences. And I would replace "(inclusive)" with
> "(both inclusive)" for clarity.

Both done.

>> +    else if in_bounds(Array, Low) then
>> +        arg_out_of_bounds_error(Array, "second", "fetch_items", Low)
>> +    else if in_bounds(Array, High) then
>> +        arg_out_of_bounds_error(Array, "third", "fetch_items", High)
>
> ??? I think you are missing a "not" in each of those conditions ...

I am; fixed.

> Aren't there any tests that would have caught these?

Only the indirect one in tests/hard_coded/array_test2; I will add a more
thorough one.

> And another possible problem: when Hi < Lo but one or both are out of
> bounds, this code returns an empty list, instead of throwing an exception.

True, although the old version had that behaviour as well.  I've swapped
the order of the tests about.   Here's a diff to the NEWS file
announcing the change.

diff --git a/NEWS b/NEWS
index 60f3746..db0738a 100644
--- a/NEWS
+++ b/NEWS
@@ -123,6 +123,11 @@ Changes that may break compatibility:
  * We have changed the semantics of array.shrink/3 and array.resize/4 so that
    they throw an exception if their first argument is negative.

+* We have changed the semantics of array.fetch_times/4 so that it always
+  throws an exception if its second or third argument is out of bounds.
+  Previously, it would return an empty if list if its third argument was less
+  than the second even if one, or both, of these arguments was out of bounds.
+
  Changes to the Mercury language:

  * We have added a new primitive type, uint, which is an unsigned integer type

Julien.