initarg caches are not invalidated when new methods are defined

Hello from Clasp. I noticed that a portion of clos/fixup.lsp still shared between ECL and Clasp concerns invalidation of class initarg caches when new methods are defined on initialize-instance et al. This code was wrong on Clasp and I believe it is wrong on ECL also.

In more detail, ECL (and Clasp, though it will be changing soon) has each class maintain a list of initarg keywords defined by methods (rather than class slot initargs) so that make-instance doesn't have to recompute the keywords from the methods each time. It's a bit arcane, but implements the second point of CLHS 7.1.2.

The cache has to be invalidated whenever the method-defined set of initarg keywords changes, which is whenever a method is added or removed on initialize-instance etc. ECL/Clasp therefore define a dummy initargs-updater object that is used with the dependent maintenance protocol. Whenever one of initialize-instance etc. changes, i.e. is reinitialized or has a method added or removed, CLOS calls update-dependent. On ECL the system method then calls recursively-update-classes with the class class as an argument. The recursion invalidates the cache and then recursively calls itself with all direct subclasses of its argument.

The problem is that most classes are instances of subclasses of class, not subclasses of class themselves, and so their caches are never invalidated.

So on Clasp we got the following buggy behavior:

(defclass foo () ())
(make-instance 'foo) ; this will set up the cache
(defmethod initialize-instance :after ((obj foo) &key test)
  ;; the content of this method is irrelevant; the point is that the method definition
  ;; adds :test as a valid keyword
  ;; to (make-instance 'foo ...) calls.
  (print test))
;;; Now:
(make-instance 'foo :test "hi")

This is valid code and should make and return a FOO instance, in the process of which printing "hi". Clasp instead signals an error that :test is not a valid initialization keyword. This is because the first make-instance call computed a cache of valid initialization keywords, and this cache was not invalidated when a new method with a new valid initialization keyword was defined, because foo is not a subclass of class.

Unfortunately I have not been able to build ECL due to some OS X chicanery I don't understand; but I think the logic is the same in both implementations and that if you test in ECL you'll find the same problem. I'm really sorry if it does not reproduce, and I would be interested to know why it does not if it does not.

My fix for Clasp was to have the method on update-dependent find which class is actually relevant, and recurse on that. I believe that would be something like the following for ECL:

(defmethod update-dependent ((object generic-function) (dep initargs-updater)
			     &rest initargs
                             &key ((add-method added-method) nil am-p)
                               ((remove-method removed-method) nil rm-p)
                             &allow-other-keys)
  (declare (ignore initargs))
  (let ((method (cond (am-p added-method) (rm-p removed-method))))
    ;; update-dependent is also called when the gf itself is reinitialized,
    ;; so make sure we actually have a method that's added or removed
    (when method
      (let ((spec (first (method-specializers method)))) ; the class being initialized or allocated
        (when (classp spec) ; sanity check against eql specialization
          (recursively-update-classes spec))))))

Basically it gets the method, and then the class the method is defined on (which is the first parameter for all the make-instance functions), and then recursively invalidates that and its subclasses. Since none of initialize-instance etc. should be reinitialized (being standard functions) the (when method might be a bit redundant. The keyword parameters are as specified in the MOP for update-dependent; I think ECL already implements that.