2008/5/15 Julian Graham <[EMAIL PROTECTED]>:
> Hi Neil,
>
> > I haven't covered these yet. Will try to soon, but could you resubmit
> > anyway as a GIT commit patch, so that you end up being properly
> > credited for the commit?
>
> Yes -- find one attached.
OK, that's in now. I had a few minor comments, please see the attached.
Neil
From c06926f30220cdc4dff93b490118e57f2973a02d Mon Sep 17 00:00:00 2001
From: Neil Jerram <[EMAIL PROTECTED]>
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
