Bug (and fix): modulo

[Dirk Herrmann <dirk at ida dot ing dot tu-bs dot de>]

Hi!

I thought you might be interested about the following:  The misuse of SCM
values in numbers.c _really_ has lead to some bugs, which surprisingly
have not got caught up to now.  Look at the following test case:

(pass-if "modulo: 0 % (-2^32 + 1)"
  (eqv? 0 (modulo 0 -4294967295)))

The result should be 0.  However, it fails.  The reason is due to the
following code in numbers.c:

  /* precondition:  x is an immediate number, y is a bignum */
  return (SCM_BIGSIGN (y) ? (x > 0) : (x < 0)) ? scm_sum (x, y) : x;

x is the _encoded_ form of an immediate number.  Thus, even if the numeric
value of x equals zero, the predicate (x > 0) is true, since instead of
zero the variable x holds the number 2.

The bug fix decodes the value of x before it is used for numeric
comparison. I also decided to restructure the corresponding routines to
handle the case of immediate number parameters first, instead of at the
end of the code.  Otherwise the handling of the different cases for
conditional compilation would have been too messy.  Moreover, I have the
impression that the factorization of the resulting code with goops should
be a little more straightforward.

2000-04-11  Dirk Herrmann  <D.Herrmann@tu-bs.de>

	* numbers.c (scm_quotient, scm_modulo):  Reordered to handle the
	case of immediate numbers parameters first.  Also, only use
	decoded numbers for numerical comparison.

I have enclosed a (not exhaustive) set of testcases for quotient and
modulo.  There are still a lot more suspicious mixups between SCM and int
values, thus there are likely to appear more bugs like these.

Best regards
Dirk Herrmann
;;;; numbers.scm --- tests guile's numbers     -*- scheme -*-
;;;; Copyright (C) 2000 Free Software Foundation, Inc.
;;;;
;;;; This program is free software; you can redistribute it and/or modify
;;;; it under the terms of the GNU General Public License as published by
;;;; the Free Software Foundation; either version 2, or (at your option)
;;;; any later version.
;;;;
;;;; This program is distributed in the hope that it will be useful,
;;;; but WITHOUT ANY WARRANTY; without even the implied warranty of
;;;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
;;;; GNU General Public License for more details.
;;;;
;;;; You should have received a copy of the GNU General Public License
;;;; along with this software; see the file COPYING.  If not, write to
;;;; the Free Software Foundation, Inc., 59 Temple Place, Suite 330,
;;;; Boston, MA 02111-1307 USA
;;;;
;;;; As a special exception, the Free Software Foundation gives permission
;;;; for additional uses of the text contained in its release of GUILE.
;;;;
;;;; The exception is that, if you link the GUILE library with other files
;;;; to produce an executable, this does not by itself cause the
;;;; resulting executable to be covered by the GNU General Public License.
;;;; Your use of that executable is in no way restricted on account of
;;;; linking the GUILE library code into it.
;;;;
;;;; This exception does not however invalidate any other reasons why
;;;; the executable file might be covered by the GNU General Public License.
;;;;
;;;; This exception applies only to the code released by the
;;;; Free Software Foundation under the name GUILE.  If you copy
;;;; code from other Free Software Foundation releases into a copy of
;;;; GUILE, as the General Public License permits, the exception does
;;;; not apply to the code that you add in this way.  To avoid misleading
;;;; anyone as to the status of such modified files, you must delete
;;;; this exception notice from them.
;;;;
;;;; If you write modifications of your own for GUILE, it is your choice
;;;; whether to permit this exception to apply to your modifications.
;;;; If you do not wish that, delete this exception notice.

(use-modules (ice-9 doc))


;;;
;;; exact?
;;;


;;;
;;; odd?
;;;


;;;
;;; even?
;;;


;;;
;;; abs
;;;


;;;
;;; quotient
;;;


;; Is documentation available?

(greg-expect-fail "quotient: documented?"
  (let ((documented #f))
    (with-output-to-string
      (lambda ()
	(set! documented (documentation 'quotient))))
    documented))

;; Special case:  0 / n

(greg-expect-pass "quotient: 0 / 1"
  (eqv? 0 (quotient 0 1)))

(greg-expect-pass "quotient: 0 / -1"
  (eqv? 0 (quotient 0 -1)))

(for-each 
 (lambda (x)
   (let* ((x-string (number->string x))
	  (x-expt (expt 2 x))
	  (x-expt-1 (- x-expt 1)))
     (greg-expect-pass (string-append "quotient: 0 / (2^" x-string " - 1)")
       (eqv? 0 (quotient 0 x-expt-1)))
     (greg-expect-pass (string-append "quotient: 0 / (-2^" x-string " + 1)")
       (eqv? 0 (quotient 0 (- x-expt-1))))
     (greg-expect-pass (string-append "quotient: 0 / 2^" x-string)
       (eqv? 0 (quotient 0 x-expt)))
     (greg-expect-pass (string-append "quotient: 0 / -2^" x-string)
       (eqv? 0 (quotient 0 (- x-expt))))))
 '(8 16 32 64 128))

;; Special case:  n / 1

(greg-expect-pass "quotient: 1 / 1"
  (eqv? 1 (quotient 1 1)))

(greg-expect-pass "quotient: -1 / 1"
  (eqv? -1 (quotient -1 1)))

(for-each 
 (lambda (x)
   (let* ((x-string (number->string x))
	  (x-expt (expt 2 x))
	  (x-expt-1 (- x-expt 1)))
     (greg-expect-pass (string-append "quotient: (2^" x-string " - 1) / 1")
       (eqv? x-expt-1 (quotient x-expt-1 1)))
     (greg-expect-pass (string-append "quotient: (-2^" x-string " + 1) / 1")
       (eqv? (- x-expt-1) (quotient (- x-expt-1) 1)))
     (greg-expect-pass (string-append "quotient: 2^" x-string " / 1")
       (eqv? x-expt (quotient x-expt 1)))
     (greg-expect-pass (string-append "quotient: -2^" x-string " / 1")
       (eqv? (- x-expt) (quotient (- x-expt) 1)))))
 '(8 16 32 64 128))

;; Special case:  n / -1

(greg-expect-pass "quotient: 1 / -1"
  (eqv? -1 (quotient 1 -1)))

(greg-expect-pass "quotient: -1 / -1"
  (eqv? 1 (quotient -1 -1)))

(for-each
 (lambda (x)
   (let* ((x-string (number->string x))
	  (x-expt (expt 2 x))
	  (x-expt-1 (- x-expt 1)))
     (greg-expect-pass (string-append "quotient: (2^" x-string " - 1) / -1")
       (eqv? (- x-expt-1) (quotient x-expt-1 -1)))
     (greg-expect-pass (string-append "quotient: (-2^" x-string " + 1) / -1")
       (eqv? x-expt-1 (quotient (- x-expt-1) -1)))
     (greg-expect-pass (string-append "quotient: 2^" x-string " / -1")
       (eqv? (- x-expt) (quotient x-expt -1)))
     (greg-expect-pass (string-append "quotient: -2^" x-string " / -1")
       (eqv? x-expt (quotient (- x-expt) -1)))))
 '(8 16 32 64 128))

;; Special case:  n / n

(for-each
 (lambda (x)
   (let* ((x-string (number->string x))
	  (x-expt-1-string (string-append "(2^" x-string " - 1)"))
	  (x-expt-1-neg-string (string-append "(-2^" x-string " + 1)"))
	  (x-expt-string (string-append "2^" x-string))
	  (x-expt-neg-string (string-append "-2^" x-string))
	  (test-name (lambda (x) (string-append "quotient: " x " / " x)))
	  (x-expt (expt 2 x))
	  (x-expt-1 (- x-expt 1)))
     (greg-expect-pass (test-name x-expt-1-string)
       (eqv? 1 (quotient x-expt-1 x-expt-1)))
     (greg-expect-pass (test-name x-expt-1-neg-string)
       (eqv? 1 (quotient (- x-expt-1) (- x-expt-1))))
     (greg-expect-pass (test-name x-expt-string)
       (eqv? 1 (quotient x-expt x-expt)))
     (greg-expect-pass (test-name x-expt-neg-string)
       (eqv? 1 (quotient (- x-expt) (- x-expt))))))
 '(8 16 32 64 128))

;; Positive dividend and divisor

(greg-expect-pass "quotient: 35 / 7"
  (eqv? 5 (quotient 35 7)))

;; Negative dividend, positive divisor

(greg-expect-pass "quotient: -35 / 7"
  (eqv? -5 (quotient -35 7)))

;; Positive dividend, negative divisor

(greg-expect-pass "quotient: 35 / -7"
  (eqv? -5 (quotient 35 -7)))

;; Negative dividend and divisor

(greg-expect-pass "quotient: -35 / -7"
  (eqv? 5 (quotient -35 -7)))

;; Are numerical overflows detected correctly?

;; Are wrong type arguments detected correctly?


;;; remainder


;;;
;;; modulo
;;;


;; Is documentation available?

(greg-expect-fail "modulo: documented?"
  (let ((documented #f))
    (with-output-to-string
      (lambda ()
	(set! documented (documentation 'modulo))))
    documented))

;; Special case:  0 % n

(greg-expect-pass "modulo: 0 % 1"
  (eqv? 0 (modulo 0 1)))

(greg-expect-pass "modulo: 0 % -1"
  (eqv? 0 (modulo 0 -1)))

(for-each 
 (lambda (x)
   (let* ((x-string (number->string x))
	  (x-expt (expt 2 x))
	  (x-expt-1 (- x-expt 1)))
     (greg-expect-pass (string-append "modulo: 0 % (2^" x-string " - 1)")
       (eqv? 0 (modulo 0 x-expt-1)))
     (greg-expect-pass (string-append "modulo: 0 % (-2^" x-string " + 1)")
       (eqv? 0 (modulo 0 (- x-expt-1))))
     (greg-expect-pass (string-append "modulo: 0 % 2^" x-string)
       (eqv? 0 (modulo 0 x-expt)))
     (greg-expect-pass (string-append "modulo: 0 % -2^" x-string)
       (eqv? 0 (modulo 0 (- x-expt))))))
 '(8 16 32 64 128))

;; Special case:  n % 1

(greg-expect-pass "modulo: 1 % 1"
  (eqv? 0 (modulo 1 1)))

(greg-expect-pass "modulo: -1 % 1"
  (eqv? 0 (modulo -1 1)))

(for-each
 (lambda (x)
   (let* ((x-string (number->string x))
	  (x-expt (expt 2 x))
	  (x-expt-1 (- x-expt 1)))
     (greg-expect-pass (string-append "modulo: (2^" x-string " - 1) % 1")
       (eqv? 0 (modulo x-expt-1 1)))
     (greg-expect-pass (string-append "modulo: (-2^" x-string " + 1) % 1")
       (eqv? 0 (modulo (- x-expt-1) 1)))
     (greg-expect-pass (string-append "modulo: 2^" x-string " % 1")
       (eqv? 0 (modulo x-expt 1)))
     (greg-expect-pass (string-append "modulo: -2^" x-string " % 1")
       (eqv? 0 (modulo (- x-expt) 1)))))
 '(8 16 32 64 128))

;; Special case:  n % -1

(greg-expect-pass "modulo: 1 % -1"
  (eqv? 0 (modulo 1 -1)))

(greg-expect-pass "modulo: -1 % -1"
  (eqv? 0 (modulo -1 -1)))

(for-each
 (lambda (x)
   (let* ((x-string (number->string x))
	  (x-expt (expt 2 x))
	  (x-expt-1 (- x-expt 1)))
     (greg-expect-pass (string-append "modulo: (2^" x-string " - 1) % -1")
       (eqv? 0 (modulo x-expt-1 -1)))
     (greg-expect-pass (string-append "modulo: (-2^" x-string " + 1) % -1")
       (eqv? 0 (modulo (- x-expt-1) -1)))
     (greg-expect-pass (string-append "modulo: 2^" x-string " % -1")
       (eqv? 0 (modulo x-expt -1)))
     (greg-expect-pass (string-append "modulo: -2^" x-string " % -1")
       (eqv? 0 (modulo (- x-expt) -1)))))
 '(8 16 32 64 128))

;; Special case:  n % n

(for-each
 (lambda (x)
   (let* ((x-string (number->string x))
	  (x-expt-1-string (string-append "(2^" x-string " - 1)"))
	  (x-expt-1-neg-string (string-append "(-2^" x-string " + 1)"))
	  (x-expt-string (string-append "2^" x-string))
	  (x-expt-neg-string (string-append "-2^" x-string))
	  (test-name (lambda (x) (string-append "modulo: " x " % " x)))
	  (x-expt (expt 2 x))
	  (x-expt-1 (- x-expt 1)))
     (greg-expect-pass (test-name x-expt-1-string)
       (eqv? 0 (modulo x-expt-1 x-expt-1)))
     (greg-expect-pass (test-name x-expt-1-neg-string)
       (eqv? 0 (modulo (- x-expt-1) (- x-expt-1))))
     (greg-expect-pass (test-name x-expt-string)
       (eqv? 0 (modulo x-expt x-expt)))
     (greg-expect-pass (test-name x-expt-neg-string)
       (eqv? 0 (modulo (- x-expt) (- x-expt))))))
 '(8 16 32 64 128))

;; Positive dividend and divisor

(greg-expect-pass "modulo: 13 % 4"
  (eqv? 1 (modulo 13 4)))

(greg-expect-pass "modulo: 2177452800 % 86400"
  (eqv? 0 (modulo 2177452800 86400)))

;; Negative dividend, positive divisor

(greg-expect-pass "modulo: -13 % 4"
  (eqv? 3 (modulo -13 4)))

(greg-expect-pass "modulo: -2177452800 % 86400"
  (eqv? 0 (modulo -2177452800 86400)))

;; Positive dividend, negative divisor

(greg-expect-pass "modulo: 13 % -4"
  (eqv? -3 (modulo 13 -4)))

(greg-expect-pass "modulo: 2177452800 % -86400"
  (eqv? 0 (modulo 2177452800 -86400)))

;; Negative dividend and divisor

(greg-expect-pass "modulo: -13 % -4"
  (eqv? -1 (modulo -13 -4)))

(greg-expect-pass "modulo: -2177452800 % -86400"
  (eqv? 0 (modulo -2177452800 -86400)))

;; Are numerical overflows detected correctly?

;; Are wrong type arguments detected correctly?


;;;
;;; gcd
;;;