From c06926f30220cdc4dff93b490118e57f2973a02d Mon Sep 17 00:00:00 2001
From: Neil Jerram <address@hidden>
Date: Sat, 24 May 2008 12:36:58 +0100
Subject: [PATCH] Comments on srfi-18.scm

---
 srfi/srfi-18.scm |   48 +++++++++++++++++++++++++++++++++++++++---------
 1 files changed, 39 insertions(+), 9 deletions(-)

diff --git a/srfi/srfi-18.scm b/srfi/srfi-18.scm
index 0593f4e..85547f0 100644
--- a/srfi/srfi-18.scm
+++ b/srfi/srfi-18.scm
@@ -32,12 +32,21 @@
 
 (define-module (srfi srfi-18)
   :use-module (srfi srfi-34)
+
+;; NJ: Do we need this use-module, given that the following code
+;; accesses everything from SRFI-34 using (@ ...)?
+
   :export (
 
 ;;; Threads
  ;; current-thread			<= in the core
  ;; thread?				<= in the core
  make-thread
+
+;; NJ: Is it commented/documented somewhere that someone should
+;; normally either use (srfi srfi-18), or (ice-9 threads), but not
+;; both?
+
  thread-name
  thread-specific
  thread-specific-set!
@@ -50,6 +59,9 @@
 ;;; Mutexes
  ;; mutex?				<= in the core
  make-mutex
+
+;; NJ: make-mutex is is the core too.  So should this be a #:replace ?
+
  mutex-name
  mutex-specific
  mutex-specific-set!
@@ -60,6 +72,9 @@
 ;;; Condition variables
  ;; condition-variable?			<= in the core
  make-condition-variable
+
+;; NJ: as above; make-condition-variable is in the core.
+
  condition-variable-name
  condition-variable-specific
  condition-variable-specific-set!
@@ -69,6 +84,9 @@
 
 ;;; Time
  current-time
+
+;; NJ: as above; current-time is in the core.
+
  time?
  time->seconds
  seconds->time
@@ -83,6 +101,9 @@
  uncaught-exception-reason
  )
   :re-export (thread? mutex? condition-variable?)
+
+;; NJ: do things from the core need to be re-exported?
+
   :replace (current-time 
 	    make-thread 
 	    make-mutex 
@@ -103,10 +124,13 @@
 (define uncaught-exception (list 'uncaught-exception))
 
 (define mutex-owners (make-weak-key-hash-table))
+;; NJ: appears to be unused!
 (define object-names (make-weak-key-hash-table))
 (define object-specifics (make-weak-key-hash-table))
 (define thread-start-conds (make-weak-key-hash-table))
 (define thread-exception-handlers (make-weak-key-hash-table))
+;; NJ: suggest using make-object-property for all of these.  Note that
+;; hashq-remove! can be implemented as setting to #f.
 
 ;; EXCEPTIONS
 
@@ -136,6 +160,7 @@
   (let ((ct (current-thread)))
     (or (hashq-ref thread-exception-handlers ct)
 	(hashq-set! thread-exception-handlers ct (list initial-handler)))))
+;; NJ: does hashq-set! return the value that it has just set?
 
 (define (with-exception-handler handler thunk)
   (let ((ct (current-thread))
@@ -143,13 +168,14 @@
     (check-arg-type procedure? handler "with-exception-handler") 
     (check-arg-type thunk? thunk "with-exception-handler")
     (hashq-set! thread-exception-handlers ct (cons handler hl))
-    (apply (@ (srfi srfi-34) with-exception-handler) 
-           (list (lambda (obj)
-                   (hashq-set! thread-exception-handlers ct hl) 
-                   (handler obj))
-                 (lambda () 
-                   (let ((r (thunk)))
-                     (hashq-set! thread-exception-handlers ct hl) r))))))
+;; NJ: rewrite without apply:
+    ((@ (srfi srfi-34) with-exception-handler) 
+     (lambda (obj)
+       (hashq-set! thread-exception-handlers ct hl) 
+       (handler obj))
+     (lambda () 
+       (let ((r (thunk)))
+	 (hashq-set! thread-exception-handlers ct hl) r)))))
 
 (define (current-exception-handler)
   (car (current-handler-stack)))
@@ -246,13 +272,17 @@
 (define (wrap thunk)
   (lambda (continuation)
     (with-exception-handler (lambda (obj)
-			      (apply (current-exception-handler) (list obj))
-			      (apply continuation (list)))
+;; NJ: without apply: 
+			      ((current-exception-handler) obj)
+			      (continuation))
 			    thunk)))
 
 ;; A pass-thru to cancel-thread that first installs a handler that throws
 ;; terminated-thread exception, as per SRFI-18, 
 
+;; NJ: do similar semantics apply if a SRFI-18 thread terminates under
+;; its own stream?
+
 (define (thread-terminate! thread)
   (define (thread-terminate-inner!)
     (let ((current-handler (thread-cleanup thread)))
-- 
1.5.4.2