Subject: bug#17474: Making *unspecified* equivalent to (values) would seem convenient

From: David Kastrup <[email-masked]>
Date: Mon, 12 May 2014 13:40:22 +0200

Guile has sort of a love/hate-relationship with SCM_UNSPECIFIED as part
of its API.  A comment in boot-9 states:

;;; {The Unspecified Value}
;;;
;;; Currently Guile represents unspecified values via one particular value,
;;; which may be obtained by evaluating (if #f #f). It would be nice in the
;;; future if we could replace this with a return of 0 values, though.

There are, of course, lots of compatibility issues involved here.  Those
can be minimized by actually making *unspecified* exactly equivalent to
(values).  Since *unspecified* is used in a lot of places as a
first-class value, can be compared and stored in variables, the cost of
this equivalence is to allow (values) in single-value contexts.

I _think_ that we are talking about behavior undefined by the Scheme
standard here.  *unspecified* has been a pure Guileism anyway in all its
aspects.  (values), however, is firmly a standard Scheme construct, and
Guile often takes the choice to map undefined behavior in those
constructs to an error, making it easier to check for code being
portable.  Conflating (values) with *unspecified* has the consequence
that single-value contexts (such as an accessor like (car x)) may
deliver zero values to a multi-value accepting continuation by producing
*unspecified* as its single value and vice versa.

Accepting this drawback leads to a reasonably nice integration of
*unspecified* with values, making SCM_UNSPECIFIED the C level
representation of (values).

The incompatibility of this patch with Guile's existing code base and
regression test, arguably a complex code base, is limited to a single
failing test written under the assumption that (if #f #f) returns
exactly one value.  The first patch rewrites that test to get along
without this assumption.  The second patch does the actual work, the
third patch documents the changed semantics.

>From 85917446d03e2562b978575b34c1f3dda105c026 Mon Sep 17 00:00:00 2001
From: David Kastrup <[email-masked]>
Date: Sat, 10 May 2014 16:05:12 +0200
Subject: [PATCH 1/3] Coverage test should not require (if #f #f) to return a
 value

* test-suite/tests/coverage.test ("instrumented/executed-lines"): Use
	let rather than let-values when there may not be more than one
	value.
---
 test-suite/tests/coverage.test | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/test-suite/tests/coverage.test b/test-suite/tests/coverage.test
index 1a63353..4829a6f 100644
--- a/test-suite/tests/coverage.test
+++ b/test-suite/tests/coverage.test
@@ -60,8 +60,7 @@
                                        (begin        ;; 2
                                          (display x) ;; 3
                                          (+ x y))))  ;; 4")))
-      (let-values (((data result)
-                    (with-code-coverage
+      (let ((data (with-code-coverage
                       (lambda () (proc 1 2)))))
         (and (coverage-data? data)
              (let-values (((instr exec)
-- 
1.9.1

>From 722b97a77421a2723a9edfa14b148d6d6b847430 Mon Sep 17 00:00:00 2001
From: David Kastrup <[email-masked]>
Date: Wed, 7 May 2014 18:43:23 +0200
Subject: [PATCH 2/3] Make (values) equivalent to *unspecified*

module/ice-9/boot-9.scm states in a comment:

;;; {The Unspecified Value}
;;;
;;; Currently Guile represents unspecified values via one particular value,
;;; which may be obtained by evaluating (if #f #f). It would be nice in the
;;; future if we could replace this with a return of 0 values, though.
;;;

(define-syntax *unspecified*
  (identifier-syntax (if #f #f)))

(define (unspecified? v) (eq? v *unspecified*))

This code remains actually valid while addressing the "it would be nice"
angle since (values) is now represented as SCM_UNSPECIFIED in the API
while being generally treated as a valid entity in implicit single-value
contexts.  Since the Scheme standard considers using zero-value
expressions in such contexts unspecified behavior, reverting to an
identifiable *unspecified* value does not appear to violate the
standard.

Factually, SCM_UNSPECIFIED has already been used in the API to signify
no usefully returned value, and the REPL already treated *unspecified*
identical to (values) as a return value in that it did not print
anything.  This interpretation is now pervasive into the Scheme/vm
level.  *unspecified* remains less ephemeral than multiple-valued
expressions since it can generally be used as a first-class value and
stored in data structures.  Consequently, data structure access may
deliver zero values to a multiple-value accepting context:

(call-with-values (lambda () (car (list *unspecified*))) list) => ()

Scheme or C calls of values will no longer create a values object when
called without argument just like they did not do so for single
arguments previously. Consequently SCM_VALUESP(SCM_UNSPECIFIED) remains
false since SCM_UNSPECIFIED, like single values, is not a values object.
---
 libguile/eval.c      | 13 +++----------
 libguile/values.c    |  6 ++++++
 libguile/vm-engine.c | 18 ++++++++++++------
 libguile/vm.c        |  8 --------
 4 files changed, 21 insertions(+), 24 deletions(-)

diff --git a/libguile/eval.c b/libguile/eval.c
index 2488ee2..46a74f1 100644
--- a/libguile/eval.c
+++ b/libguile/eval.c
@@ -231,16 +231,7 @@ truncate_values (SCM x)
       if (SCM_LIKELY (scm_is_pair (l)))
         return scm_car (l);
       else
-        {
-          scm_ithrow (scm_from_latin1_symbol ("vm-run"),
-                      scm_list_3 (scm_from_latin1_symbol ("vm-run"),
-                                  scm_from_locale_string
-                                  ("Too few values returned to continuation"),
-                                  SCM_EOL),
-                      1);
-          /* Not reached.  */
-          return SCM_BOOL_F;
-        }
+	return SCM_UNSPECIFIED;
     }
 }
 #define EVAL1(x, env) (truncate_values (eval ((x), (env))))
@@ -351,6 +342,8 @@ eval (SCM x, SCM env)
         v = scm_call_0 (producer);
         if (SCM_VALUESP (v))
           args = scm_struct_ref (v, SCM_INUM0);
+	else if (scm_is_eq (v, SCM_UNSPECIFIED))
+	  args = SCM_EOL;
         else
           args = scm_list_1 (v);
         goto apply_proc;
diff --git a/libguile/values.c b/libguile/values.c
index 670e222..12c79d5 100644
--- a/libguile/values.c
+++ b/libguile/values.c
@@ -72,6 +72,8 @@ scm_c_nvalues (SCM obj)
 {
   if (SCM_LIKELY (SCM_VALUESP (obj)))
     return scm_ilength (scm_struct_ref (obj, SCM_INUM0));
+  else if (scm_is_eq (obj, SCM_UNSPECIFIED))
+    return 0;
   else
     return 1;
 }
@@ -116,6 +118,8 @@ SCM_DEFINE (scm_values, "values", 0, 0, 1,
   SCM_VALIDATE_LIST_COPYLEN (1, args, n);
   if (n == 1)
     result = SCM_CAR (args);
+  else if (n == 0)
+    result = SCM_UNSPECIFIED;
   else
     result = scm_c_make_struct (scm_values_vtable, 0, 1, SCM_UNPACK (args));
 
@@ -130,6 +134,8 @@ scm_c_values (SCM *base, size_t nvalues)
 
   if (nvalues == 1)
     return *base;
+  else if (nvalues == 0)
+    return SCM_UNSPECIFIED;
 
   for (ret = SCM_EOL, walk = base + nvalues - 1; walk >= base; walk--)
     ret = scm_cons (*walk, ret);
diff --git a/libguile/vm-engine.c b/libguile/vm-engine.c
index c405b2b..5916cbb 100644
--- a/libguile/vm-engine.c
+++ b/libguile/vm-engine.c
@@ -266,8 +266,12 @@
     old_fp[-1] = SCM_BOOL_F;                            \
     old_fp[-2] = SCM_BOOL_F;                            \
     /* Leave proc. */                                   \
-    SCM_FRAME_LOCAL (old_fp, 1) = val;                  \
-    vp->sp = &SCM_FRAME_LOCAL (old_fp, 1);              \
+    if (SCM_UNLIKELY (scm_is_eq (val, SCM_UNSPECIFIED)))\
+      vp->sp = &SCM_FRAME_LOCAL (old_fp, 0);		\
+    else {						\
+      SCM_FRAME_LOCAL (old_fp, 1) = val;		\
+      vp->sp = &SCM_FRAME_LOCAL (old_fp, 1);		\
+    }							\
     POP_CONTINUATION_HOOK (old_fp);                     \
     NEXT (0);                                           \
   } while (0)
@@ -695,8 +699,8 @@ VM_NAME (scm_i_thread *thread, struct scm_vm *vp,
   /* receive dst:12 proc:12 _:8 nlocals:24
    *
    * Receive a single return value from a call whose procedure was in
-   * PROC, asserting that the call actually returned at least one
-   * value.  Afterwards, resets the frame to NLOCALS locals.
+   * PROC, using *unspecified* if the call did not actually return at
+   * least one value.  Afterwards, resets the frame to NLOCALS locals.
    */
   VM_DEFINE_OP (6, receive, "receive", OP2 (U8_U12_U12, X8_U24) | OP_DST)
     {
@@ -704,8 +708,10 @@ VM_NAME (scm_i_thread *thread, struct scm_vm *vp,
       scm_t_uint32 nlocals;
       UNPACK_12_12 (op, dst, proc);
       UNPACK_24 (ip[1], nlocals);
-      VM_ASSERT (FRAME_LOCALS_COUNT () > proc + 1, vm_error_no_values ());
-      LOCAL_SET (dst, LOCAL_REF (proc + 1));
+      if (SCM_UNLIKELY (FRAME_LOCALS_COUNT () <= proc + 1))
+	LOCAL_SET (dst, SCM_UNSPECIFIED);
+      else
+	LOCAL_SET (dst, LOCAL_REF (proc + 1));
       RESET_FRAME (nlocals);
       NEXT (2);
     }
diff --git a/libguile/vm.c b/libguile/vm.c
index 4516a68..ca6592b 100644
--- a/libguile/vm.c
+++ b/libguile/vm.c
@@ -479,7 +479,6 @@ static void vm_error_not_a_bytevector (const char *subr, SCM x) SCM_NORETURN SCM
 static void vm_error_not_a_struct (const char *subr, SCM x) SCM_NORETURN SCM_NOINLINE;
 static void vm_error_not_a_vector (const char *subr, SCM v) SCM_NORETURN SCM_NOINLINE;
 static void vm_error_out_of_range (const char *subr, SCM k) SCM_NORETURN SCM_NOINLINE;
-static void vm_error_no_values (void) SCM_NORETURN SCM_NOINLINE;
 static void vm_error_not_enough_values (void) SCM_NORETURN SCM_NOINLINE;
 static void vm_error_wrong_number_of_values (scm_t_uint32 expected) SCM_NORETURN SCM_NOINLINE;
 static void vm_error_continuation_not_rewindable (SCM cont) SCM_NORETURN SCM_NOINLINE;
@@ -617,13 +616,6 @@ vm_error_out_of_range (const char *subr, SCM k)
 }
 
 static void
-vm_error_no_values (void)
-{
-  vm_error ("Zero values returned to single-valued continuation",
-            SCM_UNDEFINED);
-}
-
-static void
 vm_error_not_enough_values (void)
 {
   vm_error ("Too few values returned to continuation", SCM_UNDEFINED);
-- 
1.9.1

>From 0cb77a1b15db2f88829879187bd28e42822704be Mon Sep 17 00:00:00 2001
From: David Kastrup <[email-masked]>
Date: Sun, 11 May 2014 14:21:13 +0200
Subject: [PATCH 3/3] Document semantics and API of (values)/*unspecified*

---
 doc/ref/api-compound.texi |  4 ++--
 doc/ref/api-control.texi  | 10 ++++++++--
 doc/ref/data-rep.texi     | 20 +++++++++++++++-----
 libguile/values.c         |  5 ++++-
 4 files changed, 29 insertions(+), 10 deletions(-)

diff --git a/doc/ref/api-compound.texi b/doc/ref/api-compound.texi
index 055de99..30d9691 100644
--- a/doc/ref/api-compound.texi
+++ b/doc/ref/api-compound.texi
@@ -1359,8 +1359,8 @@ array that uses a @code{f64vector} for storing its elements, and
 When @var{fill} is not the special @emph{unspecified} value, the new
 array is filled with @var{fill}.  Otherwise, the initial contents of
 the array is unspecified.  The special @emph{unspecified} value is
-stored in the variable @code{*unspecified*} so that for example
-@code{(make-typed-array 'u32 *unspecified* 4)} creates a uninitialized
+available as @code{*unspecified*} so that for example
+@code{(make-typed-array 'u32 *unspecified* 4)} creates an uninitialized
 @code{u32} vector of length 4.
 
 Each @var{bound} may be a positive non-zero integer @var{n}, in which
diff --git a/doc/ref/api-control.texi b/doc/ref/api-control.texi
index 4253a20..3612693 100644
--- a/doc/ref/api-control.texi
+++ b/doc/ref/api-control.texi
@@ -882,7 +882,13 @@ Delivers all of its arguments to its continuation.  Except for
 continuations created by the @code{call-with-values} procedure,
 all continuations take exactly one value.  The effect of
 passing no value or more than one value to continuations that
-were not created by @code{call-with-values} is unspecified.
+were not created by @code{call-with-values} is unspecified by the Scheme
+standard.
+
+Guile's behavior in that case is to drop all but the first value when
+given more than one value, and to use @code{*unspecified*}
+(@code{SCM_UNSPECIFIED} in@tie{}C) as an identifiable single-value
+representation of @code{(values)}.
 
 For @code{scm_values}, @var{args} is a list of arguments and the
 return is a multiple-values object which the caller can return.  In
@@ -902,7 +908,7 @@ representation.
 
 @deftypefn {C Function} size_t scm_c_nvalues (SCM obj)
 If @var{obj} is a multiple-values object, returns the number of values
-it contains.  Otherwise returns 1.
+it contains.  It returns 0 for @code{SCM_UNSPECIFIED}, and 1 otherwise.
 @end deftypefn
 
 @deftypefn {C Function} SCM scm_c_value_ref (SCM obj, size_t idx)
diff --git a/doc/ref/data-rep.texi b/doc/ref/data-rep.texi
index d0a76e9..5a6e055 100644
--- a/doc/ref/data-rep.texi
+++ b/doc/ref/data-rep.texi
@@ -444,12 +444,22 @@ representation, for obvious reasons.
 
 @deftypefn Macro SCM SCM_UNSPECIFIED
 The value returned by some (but not all) expressions that the Scheme
-standard says return an ``unspecified'' value.
+standard says return an ``unspecified'' value.  From Scheme, it is
+available as @code{*unspecified*} and is the representation of
+@code{(values)}, a `multi-valued' expression without value.
+
+As such, the standard read-eval-print loop prints nothing when some
+expression returns this value, so it's not a bad idea to return this
+when you can't think of anything else helpful.
+
+@code{*unspecified*} differs from calls of @code{values} with multiple
+arguments in that it can serve as a first-class value, be stored in
+variables, compared with @code{eq?} and used in other single-value
+contexts.
+
+The Scheme standard neither provides an @code{*unspecified*} value, nor
+does it define the behavior of @code{(values)} in single-value contexts.
 
-This is sort of a weirdly literal way to take things, but the standard
-read-eval-print loop prints nothing when the expression returns this
-value, so it's not a bad idea to return this when you can't think of
-anything else helpful.
 @end deftypefn
 
 @deftypefn Macro SCM SCM_UNDEFINED
diff --git a/libguile/values.c b/libguile/values.c
index 12c79d5..541f55f 100644
--- a/libguile/values.c
+++ b/libguile/values.c
@@ -109,7 +109,10 @@ SCM_DEFINE (scm_values, "values", 0, 0, 1,
 	    "continuations created by the @code{call-with-values} procedure,\n"
 	    "all continuations take exactly one value.  The effect of\n"
 	    "passing no value or more than one value to continuations that\n"
-	    "were not created by @code{call-with-values} is unspecified.")
+	    "were not created by @code{call-with-values} is unspecified by\n"
+	    "the Scheme standard.\n\n"
+	    "Guile in that case uses the first of multiple values,\n"
+	    "or @code{*unspecified*} when given no value.")
 #define FUNC_NAME s_scm_values
 {
   long n;
-- 
1.9.1

-- 
David Kastrup