Re: [Sbcl-devel] pcl patch

[William H. N. <wil...@ai...>]

On Wed, May 30, 2001 at 10:38:06AM +0200, Martin Atzmueller wrote:
> Attached is a patch for pcl that fixes two bugs reported on cmucl-imp
> (based on work by pierre mai), and removes some dead code from pcl.
> Also, I wanted to get rid of some bogus "redefinition warnings" when
> defining a class, and so I reviewed the "inform the type system" stuff,
> and deleted, what was unnecessary.
> Since in some cases some functions were spread in different files, and
> I thought they'd belong together I moved them around, too.
> A detailed descriptions is found below.

Thank you. I've merged this in sbcl-0.6.12.25, except for the one 
point noted below.

> - in file pcl/defclass.lisp:
>   - removed a FIXME for some special variables, because that is already
>   fixed.
>   - commented out code to inform the type system about a class, such
>   that a CONSTANTLY-NIL returning type-predicate was created.
>   This caused a redefinition warning when defining classes at the
>   prompt.
>   I'm not really sure about this, but it seems unnecessary,
>   and commenting it out seems to work.
>   Could someone please review this (Bill?)?

That's this part of the patch, I think:
@@ -125,10 +121,14 @@
 		`(progn	      ; ..the defstruct can be compiled.
 		   ,(class-defstruct-form (find-class name))
 		   ,defclass-form))
-	      (progn
-		(when (eq *boot-state* 'complete)
-		  (inform-type-system-about-std-class name))
-		defclass-form)))))))
+            #| MNA: REMOVEME??
+            is it really necessary to put class-predicates to always
+            return CONSTANTLY-NIL in the compile-time environment?
+            (progn
+              (when (eq *boot-state* 'complete)
+                (inform-type-system-about-std-class name))
+            |#
+            defclass-form))))))

I don't completely understand what's going on here, but it looks to
me, just based on local inspection of the code (sometimes dangerous
with PCL, but worth a try..) that just doing nothing here (i.e.
commenting out the PROGN and not replacing it with anything) isn't
right. As I understand it, the commented-out code tries to notify the
type system when a class other than a DEFSTRUCT has been defined. That
seems like an appropriate thing to do, since otherwise I'd expect code
like this
  (DEFCLASS FOO ..)
  ..
  (DEFMETHOD BAR ((X FOO) (Y FIXNUM))
    ..)
  ..
  (DEFUN BARF (X)
    (TYPECASE X
      (FOO ..)
      (LIST ..)
      ..))
to generate bogus "undefined type FOO" warnings when it's compiled.

So, I'd guess that the proper fix would be not to comment out the call
to INFORM-TYPE-SYSTEM-ABOUT-STD-CLASS, but instead to make
INFORM-TYPE-SYSTEM-ABOUT-STD-CLASS do the right thing.

On the strength of this guess, I undid this part of your patch (and
replaced it with a FIXME note trying to paraphrase your description of
the problem). If I'm misunderstanding things, let me know.
 
-- 
William Harold Newman <wil...@ai...>
pending patches from sbcl-devel:
	(none)
PGP key fingerprint 85 CE 1C BA 79 8D 51 8C  B9 25 FB EE E0 C3 E5 7C