[m-rev.] for review: adjust bounds checking behaviour of array.fetch_items/4

Julien Fischer jfischer at opturion.com
Fri Aug 16 12:03:31 AEST 2019


For review by Zoltan.

--------------------

Adjust bounds checking behaviour of array.fetch_items/4.

Change array.fetch_items/4 to return an empty list for an empty range in the
case where the endpoints of that range are not within the array bounds.

library/array.m:
    As above.

deep_profiler/autopar_types.m:
    Undo Zoltan's recent workaround for the above behaviour.

tests/hard_coded/array_fetch_items.{m,ex}:
    Add tests for this behaviour.

Julien.

diff --git a/deep_profiler/autopar_types.m b/deep_profiler/autopar_types.m
index b67fbed..1f595f8 100644
--- a/deep_profiler/autopar_types.m
+++ b/deep_profiler/autopar_types.m
@@ -366,22 +366,14 @@ ip_get_goals_before(Parallelisation) = GoalsBefore :-
      Goals = Parallelisation ^ ip_goals,
      FirstParGoalIndex = Parallelisation ^ ip_first_par_goal,
      LastGoalBefore = FirstParGoalIndex - 1,
-    ( if LastGoalBefore < 0 then
-        GoalsBefore = []
-    else
-        array.fetch_items(Goals, 0, LastGoalBefore, GoalsBefore)
-    ).
+    array.fetch_items(Goals, 0, LastGoalBefore, GoalsBefore).

  ip_get_goals_after(Parallelisation) = GoalsAfter :-
      Goals = Parallelisation ^ ip_goals,
      LastParGoalIndex = Parallelisation ^ ip_last_par_goal,
      NumGoals = array.size(Goals),
      FirstGoalAfter = LastParGoalIndex + 1,
-    ( if FirstGoalAfter >= NumGoals then
-        GoalsAfter = []
-    else
-        array.fetch_items(Goals, FirstGoalAfter, NumGoals - 1, GoalsAfter)
-    ).
+    array.fetch_items(Goals, FirstGoalAfter, NumGoals - 1, GoalsAfter).

  ip_get_par_conjs(Incomplete) = ParConjs :-
      Goals = Incomplete ^ ip_goals,
diff --git a/library/array.m b/library/array.m
index c21191d..b7ce4b6 100644
--- a/library/array.m
+++ b/library/array.m
@@ -433,7 +433,7 @@
      % Returns a list containing the items in the array with index in the range
      % Lo..Hi (both inclusive) in the same order that they occurred in the
      % array. Returns an empty list if Hi < Lo. Throws an index_out_of_bounds/0
-    % exception if Lo or Hi is out of bounds.
+    % exception if Lo or Hi is out of bounds and Hi >= Lo.
      %
  :- pred fetch_items(array(T), int, int, list(T)).
  :- mode fetch_items(in, in, in, out) is det.
@@ -2347,16 +2347,16 @@ fetch_items(Array, Low, High) = List :-
      fetch_items(Array, Low, High, List).

  fetch_items(Array, Low, High, List) :-
-    ( if not in_bounds(Array, Low) then
-        arg_out_of_bounds_error(Array, "second", "fetch_items", Low)
-    else if not in_bounds(Array, High) then
-        arg_out_of_bounds_error(Array, "third", "fetch_items", High)
-    else if High < Low then
+    ( if High < Low then
          % If High is less than Low, then there cannot be any array indexes
          % within the range Low -> High (inclusive). This can happen when
          % calling to_list/2 on the empty array. Testing for this condition
          % here rather than in to_list/2 is more general.
          List = []
+    else if not in_bounds(Array, Low) then
+        arg_out_of_bounds_error(Array, "second", "fetch_items", Low)
+    else if not in_bounds(Array, High) then
+        arg_out_of_bounds_error(Array, "third", "fetch_items", High)
      else
          List = do_foldr_func(func(X, Xs) = [X | Xs], Array, [], Low, High)
      ).
diff --git a/tests/hard_coded/array_fetch_items.exp b/tests/hard_coded/array_fetch_items.exp
index d692d8e..f7ff888 100644
--- a/tests/hard_coded/array_fetch_items.exp
+++ b/tests/hard_coded/array_fetch_items.exp
@@ -28,3 +28,13 @@ Array = array([1, 2, 3, 4, 5])
  Lo = 4
  Hi = 2
  List = []
+================
+Array = array([1, 2, 3, 4, 5])
+Lo = 561
+Hi = -1
+List = []
+================
+Array = array([1, 2, 3, 4, 5])
+Lo = 561
+Hi = 561
+EXCEPTION: "second argument of fetch_items: index 561 not in range [0, 4]"
diff --git a/tests/hard_coded/array_fetch_items.m b/tests/hard_coded/array_fetch_items.m
index 43648e4..167e993 100644
--- a/tests/hard_coded/array_fetch_items.m
+++ b/tests/hard_coded/array_fetch_items.m
@@ -27,7 +27,9 @@ main(!IO) :-
      test_fetch_items([1, 2, 3, 4, 5], 0, 0, !IO),
      test_fetch_items([1, 2, 3, 4, 5], -1, 0, !IO),
      test_fetch_items([1, 2, 3, 4, 5], 0, 6, !IO),
-    test_fetch_items([1, 2, 3, 4, 5], 4, 2, !IO).
+    test_fetch_items([1, 2, 3, 4, 5], 4, 2, !IO),
+    test_fetch_items([1, 2, 3, 4, 5], 561, -1, !IO),
+    test_fetch_items([1, 2, 3, 4, 5], 561, 561, !IO).

  :- pred test_fetch_items(list(T)::in, int::in, int::in, io::di, io::uo)
      is cc_multi.


More information about the reviews mailing list