* bug#13741: guile-2.0: optimize, and eq-ness of literals (test-suite) @ 2013-02-18 3:27 Daniel Hartwig 2013-02-18 9:16 ` Ludovic Courtès 0 siblings, 1 reply; 10+ messages in thread From: Daniel Hartwig @ 2013-02-18 3:27 UTC (permalink / raw) To: 13741 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. ^ permalink raw reply [flat|nested] 10+ messages in thread
* bug#13741: guile-2.0: optimize, and eq-ness of literals (test-suite) 2013-02-18 3:27 bug#13741: guile-2.0: optimize, and eq-ness of literals (test-suite) Daniel Hartwig @ 2013-02-18 9:16 ` Ludovic Courtès 2013-02-18 10:02 ` Daniel Hartwig ` (2 more replies) 0 siblings, 3 replies; 10+ messages in thread From: Ludovic Courtès @ 2013-02-18 9:16 UTC (permalink / raw) To: Daniel Hartwig; +Cc: 13741 Daniel Hartwig <mandyke@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’. ^ permalink raw reply [flat|nested] 10+ messages in thread
* bug#13741: guile-2.0: optimize, and eq-ness of literals (test-suite) 2013-02-18 9:16 ` Ludovic Courtès @ 2013-02-18 10:02 ` Daniel Hartwig 2013-03-01 16:13 ` Mark H Weaver 2013-02-18 17:19 ` Andy Wingo 2013-02-18 23:48 ` Mark H Weaver 2 siblings, 1 reply; 10+ messages in thread From: Daniel Hartwig @ 2013-02-18 10:02 UTC (permalink / raw) To: Ludovic Courtès; +Cc: 13741 On 18 February 2013 17:16, Ludovic Courtès <ludo@gnu.org> wrote: > Daniel Hartwig <mandyke@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. ^ permalink raw reply [flat|nested] 10+ messages in thread
* bug#13741: guile-2.0: optimize, and eq-ness of literals (test-suite) 2013-02-18 10:02 ` Daniel Hartwig @ 2013-03-01 16:13 ` Mark H Weaver 0 siblings, 0 replies; 10+ messages in thread From: Mark H Weaver @ 2013-03-01 16:13 UTC (permalink / raw) To: Daniel Hartwig; +Cc: Ludovic Courtès, 13741 Daniel Hartwig <mandyke@gmail.com> writes: > On 18 February 2013 17:16, Ludovic Courtès <ludo@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 ^ permalink raw reply [flat|nested] 10+ messages in thread
* bug#13741: guile-2.0: optimize, and eq-ness of literals (test-suite) 2013-02-18 9:16 ` Ludovic Courtès 2013-02-18 10:02 ` Daniel Hartwig @ 2013-02-18 17:19 ` Andy Wingo 2013-02-18 23:48 ` Mark H Weaver 2 siblings, 0 replies; 10+ messages in thread From: Andy Wingo @ 2013-02-18 17:19 UTC (permalink / raw) To: Ludovic Courtès; +Cc: 13741-done On Mon 18 Feb 2013 10:16, ludo@gnu.org (Ludovic Courtès) writes: > Daniel Hartwig <mandyke@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/ ^ permalink raw reply [flat|nested] 10+ messages in thread
* bug#13741: guile-2.0: optimize, and eq-ness of literals (test-suite) 2013-02-18 9:16 ` Ludovic Courtès 2013-02-18 10:02 ` Daniel Hartwig 2013-02-18 17:19 ` Andy Wingo @ 2013-02-18 23:48 ` Mark H Weaver 2013-02-19 1:55 ` bug#13741: [PATCH] test-suite: eq-ness of numbers, characters is unspecified Daniel Hartwig 2 siblings, 1 reply; 10+ messages in thread From: Mark H Weaver @ 2013-02-18 23:48 UTC (permalink / raw) To: Ludovic Courtès; +Cc: 13741 ludo@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 ^ permalink raw reply [flat|nested] 10+ messages in thread
* bug#13741: [PATCH] test-suite: eq-ness of numbers, characters is unspecified 2013-02-18 23:48 ` Mark H Weaver @ 2013-02-19 1:55 ` Daniel Hartwig 2013-02-19 4:29 ` Daniel Hartwig 2013-02-19 5:19 ` Mark H Weaver 0 siblings, 2 replies; 10+ messages in thread From: Daniel Hartwig @ 2013-02-19 1:55 UTC (permalink / raw) To: 13741 [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #1: Type: text/plain; charset=UTF-8, Size: 13629 bytes --] * 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)))))) \f (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 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* bug#13741: [PATCH] test-suite: eq-ness of numbers, characters is unspecified 2013-02-19 1:55 ` bug#13741: [PATCH] test-suite: eq-ness of numbers, characters is unspecified Daniel Hartwig @ 2013-02-19 4:29 ` Daniel Hartwig 2013-02-19 5:19 ` Mark H Weaver 1 sibling, 0 replies; 10+ messages in thread From: Daniel Hartwig @ 2013-02-19 4:29 UTC (permalink / raw) To: 13741 On 19 February 2013 09:55, Daniel Hartwig <mandyke@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 ^ permalink raw reply [flat|nested] 10+ messages in thread
* bug#13741: [PATCH] test-suite: eq-ness of numbers, characters is unspecified 2013-02-19 1:55 ` bug#13741: [PATCH] test-suite: eq-ness of numbers, characters is unspecified Daniel Hartwig 2013-02-19 4:29 ` Daniel Hartwig @ 2013-02-19 5:19 ` Mark H Weaver 2013-03-01 16:10 ` Mark H Weaver 1 sibling, 1 reply; 10+ messages in thread From: Mark H Weaver @ 2013-02-19 5:19 UTC (permalink / raw) To: Daniel Hartwig; +Cc: 13741 Daniel Hartwig <mandyke@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 ^ permalink raw reply [flat|nested] 10+ messages in thread
* bug#13741: [PATCH] test-suite: eq-ness of numbers, characters is unspecified 2013-02-19 5:19 ` Mark H Weaver @ 2013-03-01 16:10 ` Mark H Weaver 0 siblings, 0 replies; 10+ messages in thread From: Mark H Weaver @ 2013-03-01 16:10 UTC (permalink / raw) To: Daniel Hartwig; +Cc: 13741 I wrote: > Daniel Hartwig <mandyke@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 ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2013-03-01 16:13 UTC | newest] Thread overview: 10+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2013-02-18 3:27 bug#13741: guile-2.0: optimize, and eq-ness of literals (test-suite) Daniel Hartwig 2013-02-18 9:16 ` Ludovic Courtès 2013-02-18 10:02 ` Daniel Hartwig 2013-03-01 16:13 ` Mark H Weaver 2013-02-18 17:19 ` Andy Wingo 2013-02-18 23:48 ` Mark H Weaver 2013-02-19 1:55 ` bug#13741: [PATCH] test-suite: eq-ness of numbers, characters is unspecified Daniel Hartwig 2013-02-19 4:29 ` Daniel Hartwig 2013-02-19 5:19 ` Mark H Weaver 2013-03-01 16:10 ` Mark H Weaver
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).