* [PATCH] Fix primitive-eval to return #<unspecified> for definitions
2012-01-29 22:38 Inconsistent 'return value' for definitions Mark H Weaver
@ 2012-01-29 23:14 ` Mark H Weaver
2012-01-30 10:58 ` Andy Wingo
0 siblings, 1 reply; 3+ messages in thread
From: Mark H Weaver @ 2012-01-29 23:14 UTC (permalink / raw)
To: guile-devel
[-- Attachment #1: Type: text/plain, Size: 1064 bytes --]
I wrote:
> Hello all,
>
> I think the following behavior is inconsistent and undesirable:
>
> scheme@(guile-user)> (list (compile '(define foo 3)))
> $1 = (#<unspecified>)
> scheme@(guile-user)> (list (primitive-eval '(define foo 3)))
> $2 = (#<variable 103b9c80 value: 3>)
>
> It doesn't really make sense for 'define' to return a value, because it
> cannot be used in expression context. AFAICT, the only way to get this
> 'return value' is to call 'eval' or 'primitive-eval'. This weirdness
> has bitten Paul Smith in his efforts to port his code to Guile 2.
>
> I think we should change 'define' to always "return" SCM_UNSPECIFIED, at
> least in 2.2. We might also consider nipping this in the bud and making
> the same change in 2.0.4.
The more I think about it, the more convinced I am that this is simply a
bug in our Scheme evaluator. Both our compiler and our bootstrap C
evaluator returns SCM_UNSPECIFIED for definitions. Therefore, I would
like to nip this in the bud and push the following fix for 2.0.4.
What do you think?
Mark
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: [PATCH] Fix primitive-eval to return #<unspecified> for definitions --]
[-- Type: text/x-patch, Size: 2073 bytes --]
From 3509a7f224bc6d7fa4e7eb629a416720eb7256fa Mon Sep 17 00:00:00 2001
From: Mark H Weaver <mhw@netris.org>
Date: Sun, 29 Jan 2012 17:43:13 -0500
Subject: [PATCH] Fix primitive-eval to return #<unspecified> for definitions
* module/ice-9/eval.scm (primitive-eval): Return #<unspecified> for
definitions. Previously the variable object was returned.
* test-suite/tests/eval.test (evaluator): Add test.
* NEWS: Add news entry.
---
NEWS | 1 +
module/ice-9/eval.scm | 3 ++-
test-suite/tests/eval.test | 4 ++++
3 files changed, 7 insertions(+), 1 deletions(-)
diff --git a/NEWS b/NEWS
index ce47686..02b824d 100644
--- a/NEWS
+++ b/NEWS
@@ -180,6 +180,7 @@ Search the manual for these identifiers and modules, for more.
** Fix erroneous check in `set-procedure-properties!'.
** Fix generalized-vector-{ref,set!} for slices.
** Fix error messages involving definition forms.
+** Fix primitive-eval to return #<unspecified> for definitions.
** HTTP: Extend handling of "Cache-Control" header.
** HTTP: Fix qstring writing of cache-extension values
** HTTP: Fix validators for various list-style headers.
diff --git a/module/ice-9/eval.scm b/module/ice-9/eval.scm
index c0fa64c..74b8532 100644
--- a/module/ice-9/eval.scm
+++ b/module/ice-9/eval.scm
@@ -428,7 +428,8 @@
(let ((x (eval x env)))
(if (and (procedure? x) (not (procedure-property x 'name)))
(set-procedure-property! x 'name name))
- (define! name x)))
+ (define! name x)
+ (if #f #f)))
(('toplevel-set! (var-or-sym . x))
(variable-set!
diff --git a/test-suite/tests/eval.test b/test-suite/tests/eval.test
index f532059..c3121c5 100644
--- a/test-suite/tests/eval.test
+++ b/test-suite/tests/eval.test
@@ -75,6 +75,10 @@
(with-test-prefix "evaluator"
+ (pass-if "definitions return #<unspecified>"
+ (eq? (primitive-eval '(define test-var 'foo))
+ (if #f #f)))
+
(with-test-prefix "symbol lookup"
(with-test-prefix "top level"
--
1.7.5.4
^ permalink raw reply related [flat|nested] 3+ messages in thread