[m-rev.] diff: add a test case for list.split_upto/4 and list.take_upto/3

[Julien Fischer]

On Thu, 9 Jun 2016, Mark Brown wrote:

> On Wed, Jun 8, 2016 at 4:21 PM, Julien Fischer <jfischer at opturion.com> wrote:
>> I am inclined to change the behaviour of both of these predicates so
>> that they throw an exception if their first argument is negative.
>> Opinions?
>
> I agree with this change.

Here's a diff that making that change.

Julien.

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

Make list.split_upto and list.take_upto abort for N < 0.

library/list.m:
    Make split_upto/4 and take_upto/{2,3} abort for N < 0.

tests/hard_coded/take_split_upto.{m,exp}:
    Update this test case to test the for new behaviour.

tests/hard_coded/Mmakefile:
    Do not run the take_split_upto test case in deep profiling
    grades since it now requires catching exceptions to be supported.

NEWS:
 	Announce the above change.

diff --git a/NEWS b/NEWS
index 42ac2da..5147773 100644
--- a/NEWS
+++ b/NEWS
@@ -312,6 +312,10 @@ Changes to the Mercury standard library:
  * The take/3 and drop/3 predicates in the list module have been modified
    so that they fail if their first argument is less than zero.

+* The split_upto/4 and take_upto/3 predicates and take_upto/2 function
+  in the list module have been modified so that they throw an exception
+  if their first argument is negative.
+
  * The following predicate and function in the builtin module have been
    deprecated and will be removed in a future release:

diff --git a/library/list.m b/library/list.m
index bf567b8..60e0a36 100644
--- a/library/list.m
+++ b/library/list.m
@@ -226,7 +226,8 @@
      % split_upto(N, List, Start, End):
      %
      % splits `List' into a prefix `Start' of length `min(N, length(List))',
-    % and a remainder `End'. See also: split_list, take, drop.
+    % and a remainder `End'.   Throws an exception if `N' < 0.
+    % See also: split_list, take, drop.
      %
  :- pred split_upto(int::in, list(T)::in, list(T)::out, list(T)::out)
      is det.
@@ -247,7 +248,7 @@
      % take_upto(Len, List, Start):
      %
      % `Start' is the first `Len' elements of `List'. If `List' has less than
-    % `Len' elements, return the entire list.
+    % `Len' elements, return the entire list.  Throws an exception if `N' < 0.
      %
  :- pred take_upto(int::in, list(T)::in, list(T)::out) is det.
  :- func take_upto(int, list(T)) = list(T).
@@ -2326,12 +2327,23 @@ det_split_list(N, List, Start, End) :-
          unexpected($module, $pred, "index out of range")
      ).

+
  split_upto(N, List, Start, End) :-
+    ( if N < 0 then
+        unexpected($file, $pred, "index is negative")
+    else
+        do_split_upto(N, List, Start, End)
+    ).
+
+:- pred do_split_upto(int::in, list(T)::in, list(T)::out, list(T)::out)
+    is det.
+
+do_split_upto(N, List, Start, End) :-
      ( if
          N > 0,
          List = [Head | Tail]
      then
-        split_upto(N - 1, Tail, StartTail, End),
+        do_split_upto(N - 1, Tail, StartTail, End),
          Start = [Head | StartTail]
      else
          Start = [],
@@ -2361,6 +2373,15 @@ take_upto(N, Xs) = InitialXs :-
      list.take_upto(N, Xs, InitialXs).

  take_upto(N, Xs, InitialXs) :-
+    ( if N < 0 then
+        unexpected($file, $pred, "index is negative")
+    else
+        do_take_upto(N, Xs, InitialXs)
+    ).
+
+:- pred do_take_upto(int::in, list(T)::in, list(T)::out) is det.
+
+do_take_upto(N, Xs, InitialXs) :-
      ( if list.take(N, Xs, InitialXsPrime) then
          InitialXs = InitialXsPrime
      else
diff --git a/tests/hard_coded/Mmakefile b/tests/hard_coded/Mmakefile
index 2d36bb4..d6cc299 100644
--- a/tests/hard_coded/Mmakefile
+++ b/tests/hard_coded/Mmakefile
@@ -312,7 +312,6 @@ ORDINARY_PROGS =	\
  	switch_detect \
  	system_sort \
  	tag_switch_dup_label \
-	take_split_upto \
  	term_io_test \
  	term_to_univ_test \
  	test234_sorted_insert \
@@ -620,6 +619,7 @@ ifeq "$(findstring profdeep,$(GRADE))" ""
  		map_merge_test \
  		mutable_excp \
  		null_char \
+		take_split_upto \
  		test_array2d \
  		test_injection \
  		try_syntax_1 \
diff --git a/tests/hard_coded/take_split_upto.exp b/tests/hard_coded/take_split_upto.exp
index 10d15c0..f11804b 100644
--- a/tests/hard_coded/take_split_upto.exp
+++ b/tests/hard_coded/take_split_upto.exp
@@ -1,9 +1,9 @@
-take_upto(int.min_int, []) ===> []
-take_upto(int.min_int, [111]) ===> [111]
-take_upto(int.min_int, [111, 222]) ===> [111, 222]
-take_upto(-1, []) ===> []
-take_upto(-1, [111]) ===> [111]
-take_upto(-1, [111, 222]) ===> [111, 222]
+take_upto(int.min_int, []) ===> <<exception>>
+take_upto(int.min_int, [111]) ===> <<exception>>
+take_upto(int.min_int, [111, 222]) ===> <<exception>>
+take_upto(-1, []) ===> <<exception>>
+take_upto(-1, [111]) ===> <<exception>>
+take_upto(-1, [111, 222]) ===> <<exception>>
  take_upto(0, []) ===> []
  take_upto(0, [111]) ===> []
  take_upto(0, [111, 222]) ===> []
@@ -18,12 +18,12 @@ take_upto(int.max_int, []) ===> []
  take_upto(int.max_int, [111]) ===> [111]
  take_upto(int.max_int, [111, 222]) ===> [111, 222]

-split_upto(int.min_int, []) ===> ([], [])
-split_upto(int.min_int, [111]) ===> ([], [111])
-split_upto(int.min_int, [111, 222]) ===> ([], [111, 222])
-split_upto(-1, []) ===> ([], [])
-split_upto(-1, [111]) ===> ([], [111])
-split_upto(-1, [111, 222]) ===> ([], [111, 222])
+split_upto(int.min_int, []) ===> <<exception>>
+split_upto(int.min_int, [111]) ===> <<exception>>
+split_upto(int.min_int, [111, 222]) ===> <<exception>>
+split_upto(-1, []) ===> <<exception>>
+split_upto(-1, [111]) ===> <<exception>>
+split_upto(-1, [111, 222]) ===> <<exception>>
  split_upto(0, []) ===> ([], [])
  split_upto(0, [111]) ===> ([], [111])
  split_upto(0, [111, 222]) ===> ([], [111, 222])
diff --git a/tests/hard_coded/take_split_upto.m b/tests/hard_coded/take_split_upto.m
index b7703b5..0adc7e5 100644
--- a/tests/hard_coded/take_split_upto.m
+++ b/tests/hard_coded/take_split_upto.m
@@ -9,7 +9,7 @@

  :- import_module io.

-:- pred main(io::di, io::uo) is det.
+:- pred main(io::di, io::uo) is cc_multi.

  %---------------------------------------------------------------------------%
  %---------------------------------------------------------------------------%
@@ -17,6 +17,7 @@
  :- implementation.

  :- import_module assoc_list.
+:- import_module exception.
  :- import_module int.
  :- import_module list.
  :- import_module maybe.
@@ -30,21 +31,37 @@ main(!IO) :-
      io.nl(!IO),
      list.foldl(run_split_upto_test, tests, !IO).

-:- pred run_take_upto_test(pair(int, list(int))::in, io::di, io::uo) is det.
+:- pred run_take_upto_test(pair(int, list(int))::in, io::di, io::uo)
+    is cc_multi.

  run_take_upto_test(N - List, !IO) :-
      NStr = describe_int(N),
      io.format("take_upto(%s, %s) ===> ", [s(NStr), s(string(List))], !IO),
-    list.take_upto(N, List, Start),
-    io.format("%s\n", [s(string(Start))], !IO).
+    (
+        try [] (
+            list.take_upto(N, List, Start)
+        )
+    then
+        io.format("%s\n", [s(string(Start))], !IO)
+    catch_any _ ->
+        io.write_string("<<exception>>\n", !IO)
+    ).

-:- pred run_split_upto_test(pair(int, list(int))::in, io::di, io::uo) is det.
+:- pred run_split_upto_test(pair(int, list(int))::in, io::di, io::uo)
+    is cc_multi.

  run_split_upto_test(N - List, !IO) :-
      NStr = describe_int(N),
      io.format("split_upto(%s, %s) ===> ", [s(NStr), s(string(List))], !IO),
-    list.split_upto(N, List, Start, End),
-    io.format("(%s, %s)\n", [s(string(Start)), s(string(End))], !IO).
+    (
+        try [] (
+            list.split_upto(N, List, Start, End)
+        )
+    then
+        io.format("(%s, %s)\n", [s(string(Start)), s(string(End))], !IO)
+    catch_any _ ->
+        io.write_string("<<exception>>\n", !IO)
+    ).

  %---------------------------------------------------------------------------%