GNU bug report logs - #13741
guile-2.0: optimize, and eq-ness of literals (test-suite)

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 13741 in the body.
You can then email your comments to 13741 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Daniel Hartwig <mandyke <at> gmail.com>
To: bug-guile <at> gnu.org
Subject: guile-2.0: optimize, and eq-ness of literals (test-suite)
Date: Mon, 18 Feb 2013 11:27:39 +0800

Version: 2.0.7
# stable-2.0, around commit: 3d2b267
# ./configure (no arguments)

hash.test has a failing case:

  FAIL: tests/hash.test: hash-set and hash-ref:
    ;; 1/2 and 2/4 are equal? and eqv? but not eq?
    (pass-if (equal? #f
                     (let ((table (make-hash-table)))
                       (hashq-set! table 1/2 (quote foo))
                       (hashq-ref table 1/2))))

which may be due to the optimizer or other component working on
literals:

  scheme@(guile-user)> (eqv? 1/2 2/4)
  $1 = #t
  scheme@(guile-user)> (eq? 1/2 2/4)
  $2 = #f
  scheme@(guile-user)> ,optimize (eq? 1/2 2/4)
  $3 = #f
  scheme@(guile-user)> (hashq 1/2 31)
  $4 = 6
  scheme@(guile-user)> (hashq 2/4 31)
  $5 = 20
  scheme@(guile-user)> ,optimize (hashq 2/4 31)
  $6 = (hashq 1/2 31)
  scheme@(guile-user)> ;; uh oh

The ramifications reach beyond hash functions:

  scheme@(guile-user)> (define x 1/2)
  scheme@(guile-user)> (eq? x 2/4)
  $7 = #f
  scheme@(guile-user)> ,optimize (eq? x 2/4)
  $8 = (eq? x 1/2)
  scheme@(guile-user)> (define y 2/4)
  scheme@(guile-user)> (eq? x y)
  $9 = #f
  scheme@(guile-user)> ,optimize (define y 2/4)
  $10 = (define y 1/2)

I recall some discussion where it was made clear that literals can not
be assumed eq?, and that *at least* eqv? should be used to compare
numeric values unless they are known to be the same value (i.e. ‘(define
y x) (eq? x y)’).

Is that right?  This particular test and some others properly should
fade away then, or at least drop the eq-case.

Message #8 received at 13741 <at> debbugs.gnu.org (full text, mbox):

From: ludo <at> gnu.org (Ludovic Courtès)
To: Daniel Hartwig <mandyke <at> gmail.com>
Cc: 13741 <at> debbugs.gnu.org
Subject: Re: bug#13741: guile-2.0: optimize,
	and eq-ness of literals (test-suite)
Date: Mon, 18 Feb 2013 10:16:57 +0100

Daniel Hartwig <mandyke <at> gmail.com> skribis:

>   scheme@(guile-user)> (define x 1/2)
>   scheme@(guile-user)> (eq? x 2/4)
>   $7 = #f
>   scheme@(guile-user)> ,optimize (eq? x 2/4)
>   $8 = (eq? x 1/2)
>   scheme@(guile-user)> (define y 2/4)
>   scheme@(guile-user)> (eq? x y)
>   $9 = #f
>   scheme@(guile-user)> ,optimize (define y 2/4)
>   $10 = (define y 1/2)

Quoth R5RS:

     `Eq?''s behavior on numbers and characters is
     implementation-dependent, but it will always return either true or
     false, and will return true only when `eqv?' would also return
     true.  `Eq?' may also behave differently from `eqv?' on empty
     vectors and empty strings.

What we may get wrong is that it looks as if it doesn’t always return
either true or false, because the behavior depends on whether one of the
operands is a literal.

However, it’s fundamentally wrong to rely on eq? to compare numbers.  So
the test case you mention seems buggy, to start with.

WDYT?

Ludo’.

Message #11 received at 13741 <at> debbugs.gnu.org (full text, mbox):

From: Daniel Hartwig <mandyke <at> gmail.com>
To: Ludovic Courtès <ludo <at> gnu.org>
Cc: 13741 <at> debbugs.gnu.org
Subject: Re: bug#13741: guile-2.0: optimize,
	and eq-ness of literals (test-suite)
Date: Mon, 18 Feb 2013 18:02:15 +0800

On 18 February 2013 17:16, Ludovic Courtès <ludo <at> gnu.org> wrote:
> Daniel Hartwig <mandyke <at> gmail.com> skribis:
>
>>   scheme@(guile-user)> (define x 1/2)
>>   scheme@(guile-user)> (eq? x 2/4)
>>   $7 = #f
>>   scheme@(guile-user)> ,optimize (eq? x 2/4)
>>   $8 = (eq? x 1/2)
>>   scheme@(guile-user)> (define y 2/4)
>>   scheme@(guile-user)> (eq? x y)
>>   $9 = #f
>>   scheme@(guile-user)> ,optimize (define y 2/4)
>>   $10 = (define y 1/2)
>
> Quoth R5RS:
>
>      `Eq?''s behavior on numbers and characters is
>      implementation-dependent, but it will always return either true or
>      false, and will return true only when `eqv?' would also return
>      true.  `Eq?' may also behave differently from `eqv?' on empty
>      vectors and empty strings.
>
> What we may get wrong is that it looks as if it doesn’t always return
> either true or false, because the behavior depends on whether one of the
> operands is a literal.

I took that to mean only that eq? always returns a boolean, rather
than requiring it to return the same boolean given the same numeric
arguments.  It would be fine to simplify some rationals and not
others, as this action does not affect the outcome of eqv?.

> However, it’s fundamentally wrong to rely on eq? to compare numbers.  So
> the test case you mention seems buggy, to start with.
>
> WDYT?
>
> Ludo’.

Right, the test cases involving eq-ness of numbers are broken, should
be removed.  Patch to follow.

Message #16 received at 13741-done <at> debbugs.gnu.org (full text, mbox):

From: Andy Wingo <wingo <at> pobox.com>
To: ludo <at> gnu.org (Ludovic Courtès)
Cc: 13741-done <at> debbugs.gnu.org, Daniel Hartwig <mandyke <at> gmail.com>
Subject: Re: bug#13741: guile-2.0: optimize, and eq-ness of literals
	(test-suite)
Date: Mon, 18 Feb 2013 18:19:23 +0100

On Mon 18 Feb 2013 10:16, ludo <at> gnu.org (Ludovic Courtès) writes:

> Daniel Hartwig <mandyke <at> gmail.com> skribis:
>
>>   scheme@(guile-user)> (define x 1/2)
>>   scheme@(guile-user)> (eq? x 2/4)
>>   $7 = #f
>>   scheme@(guile-user)> ,optimize (eq? x 2/4)
>>   $8 = (eq? x 1/2)
>>   scheme@(guile-user)> (define y 2/4)
>>   scheme@(guile-user)> (eq? x y)
>>   $9 = #f
>>   scheme@(guile-user)> ,optimize (define y 2/4)
>>   $10 = (define y 1/2)
>
> Quoth R5RS:
>
>      `Eq?''s behavior on numbers and characters is
>      implementation-dependent, but it will always return either true or
>      false, and will return true only when `eqv?' would also return
>      true.  `Eq?' may also behave differently from `eqv?' on empty
>      vectors and empty strings.
>
> What we may get wrong is that it looks as if it doesn’t always return
> either true or false, because the behavior depends on whether one of the
> operands is a literal.
>
> However, it’s fundamentally wrong to rely on eq? to compare numbers.  So
> the test case you mention seems buggy, to start with.

Agreed, FWIW.  Fractions are allocated on the heap, and eq? picks out
the difference between heap objects.  Sometimes two values are the same
object, and sometimes not.

Closing the bug, but please follow up with any questions.

Cheers,

Andy
-- 
http://wingolog.org/

Message #19 received at 13741 <at> debbugs.gnu.org (full text, mbox):

From: Mark H Weaver <mhw <at> netris.org>
To: ludo <at> gnu.org (Ludovic Courtès)
Cc: 13741 <at> debbugs.gnu.org, Daniel Hartwig <mandyke <at> gmail.com>
Subject: Re: bug#13741: guile-2.0: optimize,
	and eq-ness of literals (test-suite)
Date: Mon, 18 Feb 2013 18:48:42 -0500

ludo <at> gnu.org (Ludovic Courtès) writes:
> However, it’s fundamentally wrong to rely on eq? to compare numbers.  So
> the test case you mention seems buggy, to start with.

Agreed.  I removed the buggy test.

    Thanks,
      Mark

Message #22 received at 13741 <at> debbugs.gnu.org (full text, mbox):

From: Daniel Hartwig <mandyke <at> gmail.com>
To: 13741 <at> debbugs.gnu.org
Subject: [PATCH] test-suite: eq-ness of numbers, characters is unspecified
Date: Tue, 19 Feb 2013 09:55:14 +0800

* test-suite/tests/00-socket.test:
* test-suite/tests/alist.test:
* test-suite/tests/elisp.test:
* test-suite/tests/encoding-iso88591.test:
* test-suite/tests/encoding-iso88597.test:
* test-suite/tests/encoding-utf8.test:
* test-suite/tests/hash.test:
* test-suite/tests/i18n.test:
* test-suite/tests/modules.test:
* test-suite/tests/ports.test:
* test-suite/tests/srfi-35.test: Make tests use eqv? instead of eq? when
  comparing numbers, characters.  Checked also for similar uses of
  assq[-ref].

* test-suite/tests/vlist.test ("vhash-delete honors HASH"): Change test
  to use eqv-ness, not eq-ness, which should not impact its purpose as
  these two are equivalent for strings.
---
 test-suite/tests/00-socket.test         |    6 +++---
 test-suite/tests/alist.test             |    4 ++--
 test-suite/tests/elisp.test             |    8 ++++----
 test-suite/tests/encoding-iso88591.test |   10 +++++-----
 test-suite/tests/encoding-iso88597.test |   10 +++++-----
 test-suite/tests/encoding-utf8.test     |   10 +++++-----
 test-suite/tests/hash.test              |    6 +-----
 test-suite/tests/i18n.test              |   20 ++++++++++----------
 test-suite/tests/modules.test           |    2 +-
 test-suite/tests/ports.test             |    8 ++++----
 test-suite/tests/srfi-35.test           |   16 ++++++++--------
 test-suite/tests/vlist.test             |    8 ++++----
 12 files changed, 52 insertions(+), 56 deletions(-)

diff --git a/test-suite/tests/00-socket.test b/test-suite/tests/00-socket.test
index 6deb285..8079cf5 100644
--- a/test-suite/tests/00-socket.test
+++ b/test-suite/tests/00-socket.test
@@ -336,7 +336,7 @@
 	  (if (not server-pid)
 	      (throw 'unresolved)
 	      (let ((status (cdr (waitpid server-pid))))
-		(eq? 0 (status:exit-val status)))))
+		(eqv? 0 (status:exit-val status)))))
 
 	(false-if-exception (delete-file path))
 
@@ -409,7 +409,7 @@
           (if (not server-pid)
               (throw 'unresolved)
               (let ((status (cdr (waitpid server-pid))))
-                (eq? 0 (status:exit-val status)))))
+                (eqv? 0 (status:exit-val status)))))
 
         (false-if-exception (delete-file path))
 
@@ -505,7 +505,7 @@
 	  (if (not server-pid)
 	      (throw 'unresolved)
 	      (let ((status (cdr (waitpid server-pid))))
-		(eq? 0 (status:exit-val status)))))
+		(eqv? 0 (status:exit-val status)))))
 
 	#t)))
 
diff --git a/test-suite/tests/alist.test b/test-suite/tests/alist.test
index 699c10e..0ed5d22 100644
--- a/test-suite/tests/alist.test
+++ b/test-suite/tests/alist.test
@@ -124,8 +124,8 @@
   (pass-if "assoc-ref"
 	   (let ((x (assoc-ref b "one")))
 	     (and (list? x)
-		  (eq? (car x) 2)
-		  (eq? (cadr x) 3))))
+		  (eqv? (car x) 2)
+		  (eqv? (cadr x) 3))))
 
 
   (pass-if-not "assoc-ref not" (assoc-ref a 'testing))
diff --git a/test-suite/tests/elisp.test b/test-suite/tests/elisp.test
index 41800fd..baf8546 100644
--- a/test-suite/tests/elisp.test
+++ b/test-suite/tests/elisp.test
@@ -124,8 +124,8 @@
         (with-fluids* (cons f (cons g #nil))
                       '(3 4)
                       (lambda ()
-                        (and (eq? (fluid-ref f) 3)
-                             (eq? (fluid-ref g) 4))))))
+                        (and (eqv? (fluid-ref f) 3)
+                             (eqv? (fluid-ref g) 4))))))
 
     (pass-if "append!"
       (let ((a (copy-tree '(1 2 3)))
@@ -150,11 +150,11 @@
               '(5 4 3 2 1)))            ; Ditto.
 
     (pass-if "list-ref"
-      (eq? (list-ref '(0 1 2 3 4 . #nil) 4) 4))
+      (eqv? (list-ref '(0 1 2 3 4 . #nil) 4) 4))
 
     (pass-if-exception "list-ref"
                        exception:out-of-range
-                       (eq? (list-ref '(0 1 2 3 4 . #nil) 6) 6))
+                       (eqv? (list-ref '(0 1 2 3 4 . #nil) 6) 6))
 
     (pass-if "list-set!"
       (let ((l (copy-tree '(0 1 2 3 4 . #nil))))
diff --git a/test-suite/tests/encoding-iso88591.test b/test-suite/tests/encoding-iso88591.test
index f7bec5e..8265ff1 100644
--- a/test-suite/tests/encoding-iso88591.test
+++ b/test-suite/tests/encoding-iso88591.test
@@ -106,16 +106,16 @@
 (with-test-prefix "string length"
 
   (pass-if "�ltima"
-	   (eq? (string-length s1) 6))
+	   (eqv? (string-length s1) 6))
     
   (pass-if "c�dula"
-	   (eq? (string-length s2) 6))
+	   (eqv? (string-length s2) 6))
 
   (pass-if "a�os"
-	   (eq? (string-length s3) 4))
+	   (eqv? (string-length s3) 4))
 
   (pass-if "�C�mo?"
-	   (eq? (string-length s4) 6)))
+	   (eqv? (string-length s4) 6)))
 
 (with-test-prefix "internal encoding"
 
@@ -168,7 +168,7 @@
   (pass-if "1"
 	   (let ((� 1)
 		 (� 2))
-	     (eq? (+ � �) 3))))
+	     (eqv? (+ � �) 3))))
 
 (with-test-prefix "output errors"
 
diff --git a/test-suite/tests/encoding-iso88597.test b/test-suite/tests/encoding-iso88597.test
index f116194..a577b2a 100644
--- a/test-suite/tests/encoding-iso88597.test
+++ b/test-suite/tests/encoding-iso88597.test
@@ -95,16 +95,16 @@
 (with-test-prefix "string length"
 
   (pass-if "s1"
-	   (eq? (string-length s1) 4))
+	   (eqv? (string-length s1) 4))
   
   (pass-if "s2"
-	   (eq? (string-length s2) 3))
+	   (eqv? (string-length s2) 3))
   
   (pass-if "s3"
-	   (eq? (string-length s3) 8))
+	   (eqv? (string-length s3) 8))
   
   (pass-if "s4" 
-	   (eq? (string-length s4) 3)))
+	   (eqv? (string-length s4) 3)))
 
 (with-test-prefix "internal encoding"
 
@@ -157,7 +157,7 @@
   (pass-if "1"
 	   (let ((� 1)
 		 (� 2))
-	     (eq? (+ � �) 3))))
+	     (eqv? (+ � �) 3))))
 
 (with-test-prefix "output errors"
 
diff --git a/test-suite/tests/encoding-utf8.test b/test-suite/tests/encoding-utf8.test
index 966a04d..1de3fa7 100644
--- a/test-suite/tests/encoding-utf8.test
+++ b/test-suite/tests/encoding-utf8.test
@@ -126,16 +126,16 @@
 (with-test-prefix "string length"
 
   (pass-if "última"
-	   (eq? (string-length s1) 6))
+	   (eqv? (string-length s1) 6))
     
   (pass-if "cédula"
-	   (eq? (string-length s2) 6))
+	   (eqv? (string-length s2) 6))
 
   (pass-if "años"
-	   (eq? (string-length s3) 4))
+	   (eqv? (string-length s3) 4))
 
   (pass-if "羅生門"
-	   (eq? (string-length s4) 3)))
+	   (eqv? (string-length s4) 3)))
 
 (with-test-prefix "internal encoding"
 
@@ -188,7 +188,7 @@
   (pass-if "1"
 	   (let ((芥川龍之介  1)
 		 (ñ 2))
-	     (eq? (+  芥川龍之介 ñ) 3))))
+	     (eqv? (+  芥川龍之介 ñ) 3))))
 
 (if (defined? 'setlocale)
     (setlocale LC_ALL oldlocale))
diff --git a/test-suite/tests/hash.test b/test-suite/tests/hash.test
index cb6b5cc..3bd4004 100644
--- a/test-suite/tests/hash.test
+++ b/test-suite/tests/hash.test
@@ -134,7 +134,7 @@
                                  (with-output-to-string
                                    (lambda () (write table)))))))
 
- ;; 1 and 1 are equal? and eqv? and eq?
+ ;; 1 and 1 are equal? and eqv? (but not necessarily eq?)
  (pass-if (equal? 'foo
 		  (let ((table (make-hash-table)))
 		    (hash-set! table 1 'foo)
@@ -143,10 +143,6 @@
 		  (let ((table (make-hash-table)))
 		    (hashv-set! table 1 'foo)
 		    (hashv-ref table 1))))
- (pass-if (equal? 'foo
-		  (let ((table (make-hash-table)))
-		    (hashq-set! table 1 'foo)
-		    (hashq-ref table 1))))
 
  ;; 1/2 and 2/4 are equal? and eqv? (but not necessarily eq?)
  (pass-if (equal? 'foo
diff --git a/test-suite/tests/i18n.test b/test-suite/tests/i18n.test
index ef08dd4..ad65b73 100644
--- a/test-suite/tests/i18n.test
+++ b/test-suite/tests/i18n.test
@@ -255,30 +255,30 @@
 (with-test-prefix "character mapping"
 
   (pass-if "char-locale-downcase"
-    (and (eq? #\a (char-locale-downcase #\A))
-         (eq? #\a (char-locale-downcase #\A (make-locale LC_ALL "C")))))
+    (and (eqv? #\a (char-locale-downcase #\A))
+         (eqv? #\a (char-locale-downcase #\A (make-locale LC_ALL "C")))))
 
   (pass-if "char-locale-upcase"
-    (and (eq? #\Z (char-locale-upcase #\z))
-         (eq? #\Z (char-locale-upcase #\z (make-locale LC_ALL "C")))))
+    (and (eqv? #\Z (char-locale-upcase #\z))
+         (eqv? #\Z (char-locale-upcase #\z (make-locale LC_ALL "C")))))
 
   (pass-if "char-locale-titlecase"
-    (and (eq? #\T (char-locale-titlecase #\t))
-	 (eq? #\T (char-locale-titlecase #\t (make-locale LC_ALL "C")))))
+    (and (eqv? #\T (char-locale-titlecase #\t))
+	 (eqv? #\T (char-locale-titlecase #\t (make-locale LC_ALL "C")))))
 
   (pass-if "char-locale-titlecase Dž"
-    (and (eq? #\762 (char-locale-titlecase #\763))
-	 (eq? #\762 (char-locale-titlecase #\763 (make-locale LC_ALL "C")))))
+    (and (eqv? #\762 (char-locale-titlecase #\763))
+	 (eqv? #\762 (char-locale-titlecase #\763 (make-locale LC_ALL "C")))))
 
   (pass-if "char-locale-upcase Turkish"
     (under-turkish-utf8-locale-or-unresolved
      (lambda ()
-       (eq? #\İ (char-locale-upcase #\i %turkish-utf8-locale)))))
+       (eqv? #\İ (char-locale-upcase #\i %turkish-utf8-locale)))))
 
   (pass-if "char-locale-downcase Turkish"
     (under-turkish-utf8-locale-or-unresolved
      (lambda ()
-       (eq? #\i (char-locale-downcase #\İ %turkish-utf8-locale))))))
+       (eqv? #\i (char-locale-downcase #\İ %turkish-utf8-locale))))))
 
 
 (with-test-prefix "string mapping"
diff --git a/test-suite/tests/modules.test b/test-suite/tests/modules.test
index 79e3c98..fb54061 100644
--- a/test-suite/tests/modules.test
+++ b/test-suite/tests/modules.test
@@ -345,7 +345,7 @@
       (set-module-binder! m (lambda args (set! invoked? #t) #f))
       (module-define! m 'something 2)
       (and invoked?
-           (eq? (module-ref m 'something) 2))))
+           (eqv? (module-ref m 'something) 2))))
 
   (pass-if "honored (ref)"
     (let ((m (make-module))
diff --git a/test-suite/tests/ports.test b/test-suite/tests/ports.test
index 613d269..3729930 100644
--- a/test-suite/tests/ports.test
+++ b/test-suite/tests/ports.test
@@ -482,7 +482,7 @@
                 (display str))))
           #f)                            ; so the test really fails here
         (lambda (key subr message errno port chr)
-          (and (eq? chr #\ĉ)
+          (and (eqv? chr #\ĉ)
                (string? (strerror errno)))))))
 
   (pass-if "wrong encoding, substitute"
@@ -548,12 +548,12 @@
                      ((_ port (proc -> error))
                       (if (eq? 'substitute
                                (port-conversion-strategy port))
-                          (eq? (proc port) #\?)
+                          (eqv? (proc port) #\?)
                           (decoding-error? port (proc port))))
                      ((_ port (proc -> eof))
                       (eof-object? (proc port)))
                      ((_ port (proc -> char))
-                      (eq? (proc port) char))))
+                      (eqv? (proc port) char))))
                   (make-checks
                    (syntax-rules ()
                      ((_ port check ...)
@@ -1136,7 +1136,7 @@
         (display "This is GNU Guile.\nWelcome." p)))
     (call-with-input-file (test-file)
       (lambda (p)
-        (and (eq? #\T (read-char p))
+        (and (eqv? #\T (read-char p))
              (let ((line (port-line p))
                    (col  (port-column p)))
                (and (= line 0) (= col 1)
diff --git a/test-suite/tests/srfi-35.test b/test-suite/tests/srfi-35.test
index 6d725dc..5e4cb27 100644
--- a/test-suite/tests/srfi-35.test
+++ b/test-suite/tests/srfi-35.test
@@ -65,17 +65,17 @@
   (pass-if "condition-ref"
     (let* ((ct (make-condition-type 'chbouib &condition '(a b)))
            (c  (make-condition ct 'b 1 'a 0)))
-      (and (eq? (condition-ref c 'a) 0)
-           (eq? (condition-ref c 'b) 1))))
+      (and (eqv? (condition-ref c 'a) 0)
+           (eqv? (condition-ref c 'b) 1))))
 
   (pass-if "condition-ref with inheritance"
     (let* ((top (make-condition-type 'foo &condition '(a b)))
            (ct  (make-condition-type 'bar top '(c d)))
            (c   (make-condition ct 'b 1 'a 0 'd 3 'c 2)))
-      (and (eq? (condition-ref c 'a) 0)
-           (eq? (condition-ref c 'b) 1)
-           (eq? (condition-ref c 'c) 2)
-           (eq? (condition-ref c 'd) 3))))
+      (and (eqv? (condition-ref c 'a) 0)
+           (eqv? (condition-ref c 'b) 1)
+           (eqv? (condition-ref c 'c) 2)
+           (eqv? (condition-ref c 'd) 3))))
 
   (pass-if "extract-condition"
     (let* ((ct (make-condition-type 'chbouib &condition '(a b)))
@@ -149,8 +149,8 @@
                   (let ((c (make-condition &chbouib 'one 1 'two 2)))
                     (and (condition? c)
                          (chbouib? c)
-                         (eq? (chbouib-one c) 1)
-                         (eq? (chbouib-two c) 2))))
+                         (eqv? (chbouib-one c) 1)
+                         (eqv? (chbouib-two c) 2))))
             m)))
 
   (pass-if "condition"
diff --git a/test-suite/tests/vlist.test b/test-suite/tests/vlist.test
index d939284..a37be5e 100644
--- a/test-suite/tests/vlist.test
+++ b/test-suite/tests/vlist.test
@@ -287,12 +287,12 @@
     ;; using the supplied hash procedure, which could lead to
     ;; inconsistencies.
     (let* ((s  "hello")
-           (vh (fold vhash-consq
-                     (vhash-consq s "world" vlist-null)
+           (vh (fold vhash-consv
+                     (vhash-consv s "world" vlist-null)
                      (iota 300)
                      (iota 300))))
-      (and (vhash-assq s vh)
-           (pair? (vhash-assq s (vhash-delete 123 vh eq? hashq))))))
+      (and (vhash-assv s vh)
+           (pair? (vhash-assv s (vhash-delete 123 vh eqv? hashv))))))
 
   (pass-if "vhash-fold"
     (let* ((keys   '(a b c d e f g d h i))
-- 
1.7.10.4

Message #25 received at 13741 <at> debbugs.gnu.org (full text, mbox):

From: Daniel Hartwig <mandyke <at> gmail.com>
To: 13741 <at> debbugs.gnu.org
Subject: Re: bug#13741: [PATCH] test-suite: eq-ness of numbers,
	characters is unspecified
Date: Tue, 19 Feb 2013 12:29:23 +0800

On 19 February 2013 09:55, Daniel Hartwig <mandyke <at> gmail.com> wrote:
> * test-suite/tests/00-socket.test:
> * test-suite/tests/alist.test:
> * test-suite/tests/elisp.test:
> * test-suite/tests/encoding-iso88591.test:
> * test-suite/tests/encoding-iso88597.test:
> * test-suite/tests/encoding-utf8.test:
> * test-suite/tests/hash.test:
> * test-suite/tests/i18n.test:
> * test-suite/tests/modules.test:
> * test-suite/tests/ports.test:
> * test-suite/tests/srfi-35.test: Make tests use eqv? instead of eq? when
>   comparing numbers, characters.  Checked also for similar uses of
>   assq[-ref].
>
> * test-suite/tests/vlist.test ("vhash-delete honors HASH"): Change test
>   to use eqv-ness, not eq-ness, which should not impact its purpose as
>   these two are equivalent for strings.

Located using grep and inspecting each occurance of eq?, assq, consq.
The tests weren't necessarily failing, just incorrect for their
reliance on unspecified results.  Most checks involving numbers were
already using eqv?, equal?, or =.

The following were spotted but have been left alone for now:

* list.test (diff-unrolled): Uses eq? internally, called on lists
  with numbers.

* tree-il.test:

* elisp.test ("assq"): Is that correct for Guile's elisp?  _(elisp)
  Comparison of Numbers_ says that each integer does have a unique
  object in Emacs Lisp, and using ‘eq’ is valid though not
  recommended.

Regards

Message #28 received at 13741 <at> debbugs.gnu.org (full text, mbox):

From: Mark H Weaver <mhw <at> netris.org>
To: Daniel Hartwig <mandyke <at> gmail.com>
Cc: 13741 <at> debbugs.gnu.org
Subject: Re: bug#13741: [PATCH] test-suite: eq-ness of numbers,
	characters is unspecified
Date: Tue, 19 Feb 2013 00:19:12 -0500

Daniel Hartwig <mandyke <at> gmail.com> writes:
> * test-suite/tests/00-socket.test:
> * test-suite/tests/alist.test:
> * test-suite/tests/elisp.test:
> * test-suite/tests/encoding-iso88591.test:
> * test-suite/tests/encoding-iso88597.test:
> * test-suite/tests/encoding-utf8.test:
> * test-suite/tests/hash.test:
> * test-suite/tests/i18n.test:
> * test-suite/tests/modules.test:
> * test-suite/tests/ports.test:
> * test-suite/tests/srfi-35.test: Make tests use eqv? instead of eq? when
>   comparing numbers, characters.  Checked also for similar uses of
>   assq[-ref].
>
> * test-suite/tests/vlist.test ("vhash-delete honors HASH"): Change test
>   to use eqv-ness, not eq-ness, which should not impact its purpose as
>   these two are equivalent for strings.

I think we should apply this patch.  Although we can currently rely on
'eq?' working properly for fixnums and characters in Guile, misuse of
'eq?' is widespread, and our misuse of it in our own code contributes to
the confusion.  IMO we should set a better example.

What do other people think?

      Mark

Message #31 received at 13741 <at> debbugs.gnu.org (full text, mbox):

From: Mark H Weaver <mhw <at> netris.org>
To: Daniel Hartwig <mandyke <at> gmail.com>
Cc: 13741 <at> debbugs.gnu.org
Subject: Re: bug#13741: [PATCH] test-suite: eq-ness of numbers,
	characters is unspecified
Date: Fri, 01 Mar 2013 11:10:03 -0500

I wrote:

> Daniel Hartwig <mandyke <at> gmail.com> writes:
>> * test-suite/tests/00-socket.test:
>> * test-suite/tests/alist.test:
>> * test-suite/tests/elisp.test:
>> * test-suite/tests/encoding-iso88591.test:
>> * test-suite/tests/encoding-iso88597.test:
>> * test-suite/tests/encoding-utf8.test:
>> * test-suite/tests/hash.test:
>> * test-suite/tests/i18n.test:
>> * test-suite/tests/modules.test:
>> * test-suite/tests/ports.test:
>> * test-suite/tests/srfi-35.test: Make tests use eqv? instead of eq? when
>>   comparing numbers, characters.  Checked also for similar uses of
>>   assq[-ref].
>>
>> * test-suite/tests/vlist.test ("vhash-delete honors HASH"): Change test
>>   to use eqv-ness, not eq-ness, which should not impact its purpose as
>>   these two are equivalent for strings.
>
> I think we should apply this patch.  Although we can currently rely on
> 'eq?' working properly for fixnums and characters in Guile, misuse of
> 'eq?' is widespread, and our misuse of it in our own code contributes to
> the confusion.  IMO we should set a better example.
>
> What do other people think?

Having heard no objections over 10 days, I went ahead and pushed this.

    Thanks!
      Mark

Message #34 received at 13741 <at> debbugs.gnu.org (full text, mbox):

From: Mark H Weaver <mhw <at> netris.org>
To: Daniel Hartwig <mandyke <at> gmail.com>
Cc: Ludovic Courtès <ludo <at> gnu.org>, 13741 <at> debbugs.gnu.org
Subject: Re: bug#13741: guile-2.0: optimize,
	and eq-ness of literals (test-suite)
Date: Fri, 01 Mar 2013 11:13:17 -0500

Daniel Hartwig <mandyke <at> gmail.com> writes:
> On 18 February 2013 17:16, Ludovic Courtès <ludo <at> gnu.org> wrote:
>> Quoth R5RS:
>>
>>      `Eq?''s behavior on numbers and characters is
>>      implementation-dependent, but it will always return either true or
>>      false, and will return true only when `eqv?' would also return
>>      true.  `Eq?' may also behave differently from `eqv?' on empty
>>      vectors and empty strings.
>>
>> What we may get wrong is that it looks as if it doesn’t always return
>> either true or false, because the behavior depends on whether one of the
>> operands is a literal.
>
> I took that to mean only that eq? always returns a boolean, rather
> than requiring it to return the same boolean given the same numeric
> arguments.  It would be fine to simplify some rationals and not
> others, as this action does not affect the outcome of eqv?.

Yes, this is my understanding as well.

      Mark

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.