From: ludo@chbouib.org (Ludovic Courtès)
To: guile-devel@gnu.org
Subject: Re: slowness in guile 1.8
Date: Sat, 26 May 2007 16:45:17 +0200 [thread overview]
Message-ID: <87k5uviqyq.fsf@chbouib.org> (raw)
In-Reply-To: 87sl9jn2tl.fsf@chbouib.org
[-- Attachment #1: Type: text/plain, Size: 1368 bytes --]
Hi,
ludo@chbouib.org (Ludovic Courtès) writes:
> Actually, no: the test does a `define' _within_ the body of `begin', so
> I *think* this qualifies as an internal define, and internal defines are
> equivalent to `letrec' (Section 5.2.2).
I was wrong: `define' within `begin' does not qualify as an "internal
define", so the test "binding is created before expression is evaluated"
was incorrect (or over-specified compared to R5RS[*]).
After further discussion with Andy, I committed the patch below to HEAD.
It inverts the order of expression evaluation and `scm_sym2var' in
`scm_m_define ()' (which is concerned only with top-level defines),
thereby fixing the `(define round round)' case. At the same time, it
breaks the aforementioned test from `syntax.test', but there's nothing
wrong with that.
For 1.8, I'm pretty much inclined to commit a similar patch, i.e., where
`module-make-local-var!' and `scm_m_define' are copied from HEAD. This
would break code that does things like:
(define foo (begin (set! foo 1) (+ foo 1)))
but I think it's reasonable to break such code (which relies on
non-R5RS-compliant behavior anyway), especially given the performance
gain we get in return. What do you think?
Thanks,
Ludovic.
[*] FWIW, the wording for `define' in the newly-released R5.93RS
(Section 9.3.1) is the same as that of R5RS (Section 5.2.1).
[-- Attachment #2: The fix for `(define round round)' in HEAD --]
[-- Type: text/x-patch, Size: 3796 bytes --]
--- orig/libguile/ChangeLog
+++ mod/libguile/ChangeLog
@@ -1,3 +1,9 @@
+2007-05-26 Ludovic Courtès <ludo@chbouib.org>
+
+ * eval.c (scm_m_define): Updated comment. Changed order for value
+ evaluation and `scm_sym2var ()' call, which is perfectly valid per
+ R5RS. This reverts the change dated 2004-04-22 by Dirk Herrmann.
+
2007-05-05 Ludovic Courtès <ludo@chbouib.org>
Implemented lazy duplicate binding handling.
--- orig/libguile/eval.c
+++ mod/libguile/eval.c
@@ -1209,10 +1209,11 @@
return expr;
}
-/* According to section 5.2.1 of R5RS we first have to make sure that the
- * variable is bound, and then perform the (set! variable expression)
- * operation. This means, that within the expression we may already assign
- * values to variable: (define foo (begin (set! foo 1) (+ foo 1))) */
+/* According to Section 5.2.1 of R5RS we first have to make sure that the
+ variable is bound, and then perform the `(set! variable expression)'
+ operation. However, EXPRESSION _can_ be evaluated before VARIABLE is
+ bound. This means that EXPRESSION won't necessarily be able to assign
+ values to VARIABLE as in `(define foo (begin (set! foo 1) (+ foo 1)))'. */
SCM
scm_m_define (SCM expr, SCM env)
{
@@ -1222,9 +1223,9 @@
const SCM canonical_definition = canonicalize_define (expr);
const SCM cdr_canonical_definition = SCM_CDR (canonical_definition);
const SCM variable = SCM_CAR (cdr_canonical_definition);
+ const SCM value = scm_eval_car (SCM_CDR (cdr_canonical_definition), env);
const SCM location
= scm_sym2var (variable, scm_env_top_level (env), SCM_BOOL_T);
- const SCM value = scm_eval_car (SCM_CDR (cdr_canonical_definition), env);
if (SCM_REC_PROCNAMES_P)
{
--- orig/test-suite/ChangeLog
+++ mod/test-suite/ChangeLog
@@ -1,3 +1,11 @@
+2007-05-26 Ludovic Courtès <ludo@chbouib.org>
+
+ * tests/syntax.test (top-level define)[binding is created before
+ expression is evaluated]: Moved to "internal define", using `let'
+ instead of `begin'. The test was not necessarily valid for
+ top-level defines, according to Section 5.2.1 or R5RS.
+ [redefinition]: New.
+
2007-05-09 Ludovic Courtès <ludo@chbouib.org>
* tests/srfi-19.test ((current-time time-tai) works): Use `time?'.
--- orig/test-suite/tests/syntax.test
+++ mod/test-suite/tests/syntax.test
@@ -725,15 +725,16 @@
(with-test-prefix "top-level define"
- (pass-if "binding is created before expression is evaluated"
- (= (eval '(begin
- (define foo
- (begin
- (set! foo 1)
- (+ foo 1)))
- foo)
- (interaction-environment))
- 2))
+ (pass-if "redefinition"
+ (let ((m (make-module)))
+ (beautify-user-module! m)
+
+ ;; The previous value of `round' must still be visible at the time the
+ ;; new `round' is defined. According to R5RS (Section 5.2.1), `define'
+ ;; should behave like `set!' in this case (except that in the case of
+ ;; Guile, we respect module boundaries).
+ (eval '(define round round) m)
+ (eq? (module-ref m 'round) round)))
(with-test-prefix "currying"
@@ -780,6 +781,17 @@
(eq? 'c (a 2) (a 5))))
(interaction-environment)))
+ (pass-if "binding is created before expression is evaluated"
+ ;; Internal defines are equivalent to `letrec' (R5RS, Section 5.2.2).
+ (= (eval '(let ()
+ (define foo
+ (begin
+ (set! foo 1)
+ (+ foo 1)))
+ foo)
+ (interaction-environment))
+ 2))
+
(pass-if "internal defines with begin"
(false-if-exception
(eval '(let ((a identity) (b identity) (c identity))
[-- Attachment #3: Type: text/plain, Size: 143 bytes --]
_______________________________________________
Guile-devel mailing list
Guile-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/guile-devel
next prev parent reply other threads:[~2007-05-26 14:45 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2007-05-25 16:33 slowness in guile 1.8 Andy Wingo
2007-05-25 18:12 ` Ludovic Courtès
2007-05-26 10:49 ` Andy Wingo
2007-05-26 10:57 ` Andy Wingo
2007-05-26 13:15 ` Ludovic Courtès
2007-05-26 14:45 ` Ludovic Courtès [this message]
2007-05-26 15:39 ` Andy Wingo
2007-06-13 22:24 ` Ludovic Courtès
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: https://www.gnu.org/software/guile/
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87k5uviqyq.fsf@chbouib.org \
--to=ludo@chbouib.org \
--cc=guile-devel@gnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).