regression: hu.dwim.computed-class breaks somewhere between 1.0.46.2 and 1.0.46.24

[Attila Lendvai]

CL-USER> (lisp-implementation-version)
"1.0.46"
CL-USER> (asdf:load-system :hu.dwim.computed-class.test)
CL-USER> (hu.dwim.computed-class.test::compute4)
...
T
#<test-run: 1 test, 3 assertions, 0 failures in 0.0 sec>
CL-USER>

===========================

CL-USER> (lisp-implementation-version)
"1.0.47"
CL-USER> (asdf:load-system :hu.dwim.computed-class.test)
CL-USER> (hu.dwim.computed-class.test::compute4)

ends up in this error:

Argument X is not a NUMBER:
  #<SBCL-CLASS-CACHE-COMPUTED-TEST
    {1002A36751}> / <#SLOT-A :pulse -1 :value NIL :kind OBJECT-SLOT>

what you see there is the underlying 'cell' object stored in the slot. it should never be returned by a computed-class accessor, so i suspect the real problem is that our svuc customization doesn't get called.

i'll try to come up with a reduced test case, but don't hold your breadth.

it's bisected by Tomas Hlavaty (many thanks!) to some extent: up to 1.0.46.2 it works, but 1.0.46.24 fails.

[Attila Lendvai (attila-lendvai)]

fyi, this test was added to our library way back in 2006:

http://dwim.hu/darcsweb/darcsweb.cgi?r=HEAD%20hu.dwim.computed-class;a=filediff;h=20061026170522-e3083-595df2a80698f8d7c703707889885e856d224d18.gz;f=test.lisp

judging from the test code, class redefinition plays a key role in this. the erasure of the comments you can find in the original version inlined below suggests that the bug has been fixed in sbcl at some point.

+;; TODO: send a bug report to SBCL's list
+(test computed-class/compute/3
+ (setf (find-class 'sbcl-class-cache-computed-test) nil)
+ (defclass sbcl-class-cache-computed-test ()
+ ((slot-a :accessor slot-a-of :initarg :slot-a)
+ (slot-b :accessor slot-b-of :initarg :slot-b))
+ (:metaclass computed-class))
+ (let ((object (make-instance 'sbcl-class-cache-computed-test :slot-a (compute-as 1) :slot-b 1)))
+ (slot-a-of object)
+ (slot-b-of object))
+ (defclass sbcl-class-cache-computed-test ()
+ ((slot-a :accessor slot-a-of :initarg :slot-a :computed #t)
+ (slot-b :accessor slot-b-of :initarg :slot-b :computed #t))
+ (:metaclass computed-class))
+ (let ((object (make-instance 'sbcl-class-cache-computed-test
+ :slot-a (compute-as 1)
+ :slot-b (compute-as (1+ (slot-a-of self))))))
+ (is (= 1 (slot-a-of object)))
+ ;; the next call does not call slot-value-using-class probably because of some accessor method cache?
+ (is (= 2 (slot-b-of object)))
+ (setf (slot-a-of object) 2)
+ ;; the next call does not call slot-value-using-class probably because of some accessor method cache?
+ ;; even slot-value does not call svuc?!
+ (is (= 3 (slot-b-of object)))))

[Tomas Hlavaty (tomas-hlavaty)]

The commit that broke it is 0223f43d5f199914ebceff12b6f4c60448369edd
1.0.46.11: faster slot-accesses in the presence of SLOT-VALUE-USING-CLASS &co

[Tomas Hlavaty (tomas-hlavaty)]

Checked that reverting that commit fixed the issue.

[Nikodemus Siivola (nikodemus)]

In 1.0.47.25.

[Tomas Hlavaty (tomas-hlavaty)]

Tried with sbcl 1.0.47.30: redefinition seems fixed. However, the first definition still leaks the slot thingy.

Slightly modified test:

(def test compute4 ()
  (setf (find-class 'sbcl-class-cache-computed-test) nil)
  (defclass sbcl-class-cache-computed-test ()
    ((slot-a :accessor slot-a-of :initarg :slot-a)
     (slot-b :accessor slot-b-of :initarg :slot-b))
    (:metaclass computed-class*))
  (let ((object (make-instance 'sbcl-class-cache-computed-test
                               :slot-a (compute-as 1)
                               :slot-b 1)))
    (is (eql 1 (slot-a-of object)))
    (is (eql 1 (slot-b-of object))))
  (defclass sbcl-class-cache-computed-test ()
    ((slot-a :accessor slot-a-of :initarg :slot-a :computed-in test-universe)
     (slot-b :accessor slot-b-of :initarg :slot-b :computed-in test-universe))
    (:metaclass computed-class*))
  (let ((object (make-instance 'sbcl-class-cache-computed-test
                               :slot-a (compute-as 1)
                               :slot-b (compute-as (1+ (slot-a-of -self-))))))
    (is (= 1 (slot-a-of object)))
    (is (= 2 (slot-b-of object)))
    (setf (slot-a-of object) 2)
    (is (= 3 (slot-b-of object)))))

The first assertion (is (eql 1 (slot-a-of object))) gives

Test assertion failed:

Binary predicate (EQL X Y) failed.
x: 1 => 1
y: (HU.DWIM.COMPUTED-CLASS.TEST::SLOT-A-OF
    HU.DWIM.COMPUTED-CLASS.TEST::OBJECT) => NIL / <#NIL :pulse -1 :value NIL :kind OBJECT-SLOT>
   [Condition of type HU.DWIM.STEFIL::ASSERTION-FAILED]

[Attila Lendvai (attila-lendvai)]

Tomas, i can't reproduce:

CL-USER> (lisp-implementation-version)
"1.0.47.30"
CL-USER> (hu.dwim.asdf:develop-system :hu.dwim.computed-class)
TEST> (test)
................................................................................
...............................................
*** Reader, no computation, standard accessor:

Evaluation took:
  0.107 seconds of real time
  0.090000 seconds of total run time (0.090000 user, 0.000000 system)
  84.11% CPU
  241,081,411 processor cycles
  0 bytes consed

*** Reader, no computation, computed accessor:

Evaluation took:
  1.479 seconds of real time
  1.480000 seconds of total run time (1.480000 user, 0.000000 system)
  100.07% CPU
  3,344,623,682 processor cycles
  0 bytes consed

*** Reader, writer, no computation, standard accessor:

Evaluation took:
  0.061 seconds of real time
  0.060000 seconds of total run time (0.060000 user, 0.000000 system)
  98.36% CPU
  138,176,790 processor cycles
  0 bytes consed

*** Reader, writer, recomputation, computed accessor:

Evaluation took:
  1.676 seconds of real time
  1.680000 seconds of total run time (1.670000 user, 0.010000 system)
  [ Run times consist of 0.030 seconds GC time, and 1.650 seconds non-GC time. ]
  100.24% CPU
  3,787,943,094 processor cycles
  63,995,744 bytes consed

#<test-run: 24 tests, 127 assertions, 0 failures in 4.01 sec>
TEST>

[Attila Lendvai (attila-lendvai)]

sorry, i was slow and didn't notice the test was changed.

but what Tomas sees is expected behavior: a slot only gets computed if at class definition it's requested

1) with an explicit :computed-in slot argument
2) implicitly with a (compute-as ...) initform