default-component-class slot normally gets clobbered

Hello,

In parse-component-form, the default-component-class slot of a module
pretty much always gets clobbered.  Take a look:

(defmethod (setf asdf::module-default-component-class) :around 
    (value (m module))
  (let ((before (asdf::module-default-component-class m)))
    (call-next-method)
    (format *trace-output* "module: ~A~%~
~8Tbefore: ~A~%~
~8Tafter: ~A~%"
	  m 
	  before
	  (asdf::module-default-component-class m))))

Add that method and find-system on a system whose .asd file doesn't
use any :default-component-class keywords (are there any .asds that do
use this?), and all the modules will have that slot set to nil.

The code that does this is straightforward and easy to find, so I
won't explain what it's doing.  As a result of this code, neither
initforms for this slot nor setting the slot :after one of the
standard initilizing functions is of any use.

Here's an example defsystem whose intended implied semantics are
fairly straightforward, but that can't work at present:

(defsystem my-texinfo-document
  :class texinfo-module
  :components ((:file "top")
               <more :files>
               (:image-module ((:file "diag1")
                               <more :files>))))

Instead, you would have to write the defsystem as something like the
following, which is a little redundant:

(defsystem some-texinfo-document
  :class texinfo-module
  :default-component-class texinfo-source-file
  :components ((:file "top")
               <more :files>
               (:image-module
                :default-component-class image-file
                ((:file "diag1")
                 <more :files>))))

I take the fact that the defclass for module has an initform as a sign
that initforms for this slot were supposed to work.

So I'd like to propose the following changes: only set the slot in
parse-component-form in case the slot is unbound or if the keyword was
supplied, and remove the :initform from the defclass for module.  This
way, :initforms in subclasses of module will work, but instances of
other subclasses of module (including direct instances of module) will
continue to obtain the value of this slot from their component-parent.

Aesthetically, if it's desired to preserve an :initform of
cl-source-file somewhere, it could be added to the system class
without harming anything, I think.

Unfortunately, there's a small way in which the changes I propose
breaks upward compatibility with possible existing .asd files.
Although no existing defsystem can be using the initform of a subclass
of module, it's possible that some defsystem happens to work only
because that initform is effectively ignored.  Consider an .asd that
contained the following:

(defclass some-module (module)
  ((asdf::default-component-class :initform 'static-file)))

(defsystem foo
  :component ((:file "f1")
              (:some-module
               :components ((:file "f2")))))

Correct performance of some operations on this system might depend on
file f2 being an instance of cl-source-file, which it would cease to
be after this change.

While I don't find any uses of default-component-class by grepping
around the .asds I have, that's not conclusive, inasmuch as people
might have undistributed customized asds that would get broken.  I
don't know what the attitude of asdf maintainers is about making
changes that expose hidden bugs in extensions and defsystems.

Thanks,
RmK


diff -u /home/kreuter/lsp/pkg/cclan/asdf/asdf.lisp.\~1.98.\~ /home/kreuter/lsp/pkg/cclan/asdf/asdf.lisp.dcc1
--- /home/kreuter/lsp/pkg/cclan/asdf/asdf.lisp.~1.98.~	2006-05-30 14:14:40.000000000 -0400
+++ /home/kreuter/lsp/pkg/cclan/asdf/asdf.lisp.dcc1	2006-06-15 00:49:49.000000000 -0400
@@ -242,7 +242,7 @@
 			   :accessor module-if-component-dep-fails
 			   :initarg :if-component-dep-fails)
    (default-component-class :accessor module-default-component-class
-     :initform 'cl-source-file :initarg :default-component-class)))
+     :initarg :default-component-class)))
 
 (defgeneric component-pathname (component)
   (:documentation "Extracts the pathname applicable for a particular component."))
@@ -285,7 +285,8 @@
     :accessor system-long-description :initarg :long-description)
    (author :accessor system-author :initarg :author)
    (maintainer :accessor system-maintainer :initarg :maintainer)
-   (licence :accessor system-licence :initarg :licence)))
+   (licence :accessor system-licence :initarg :licence)
+   (default-component-class :initform 'cl-source-file)))
 
 ;;; version-satisfies
 
@@ -994,10 +995,11 @@
 	     :parent parent
 	     other-args)
       (when (typep ret 'module)
-	(setf (module-default-component-class ret)
-	      (or default-component-class
-		  (and (typep parent 'module)
-		       (module-default-component-class parent))))
+	(when (or (not (slot-boundp ret 'default-component-class)) default-component-class)
+	  (setf (module-default-component-class ret)
+		(or default-component-class
+		    (and (typep parent 'module)
+			 (module-default-component-class parent)))))
 	(let ((*serial-depends-on* nil))
 	  (setf (module-components ret)
 		(loop for c-form in components

Diff finished.  Thu Jun 15 00:51:11 2006