Restore (under user control) the warnings for defmethod on functions without defgenerics

Bug #1818142 reported by Paul F. Dietz
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
SBCL
Confirmed
Wishlist
Unassigned

Bug Description

In e0a9a73d891344cb188549770a40562fc58aae69 SBCL stopped warning if a defmethod referred to a function for which defgeneric had not been seen. This issue is to bring this back, under user control. Add a special variable or a declaration that reenables this behavior.

(I ask this because of real users cases involving typos in the names of methods, and also when packages are not set up properly and the symbol isn't the one the user is thinking of.)

Stas Boukarev (stassats)
Changed in sbcl:
status: New → Confirmed
importance: Undecided → Wishlist
Revision history for this message
Paul F. Dietz (paul-f-dietz) wrote :

What's the best way to do something like this? An issue is making code using it play well with earlier versions of SBCL. So if there's a symbol SB-EXT:*FOO* that controls the behavior, one might write (in the .sbclrc file, say):

(let ((sym (find-symbol "*FOO*" "SB-EXT")))
  (when sym (setf (symbol-value sym) t)))

With a declaration, one could use the DECLARATION declaration, but the problem there would be the package could be locked.

(declaim (declaration sb-ext:defmethod-warn))
(declaim (sb-ext:defmethod-warn))

Revision history for this message
Stas Boukarev (stassats) wrote :

There can be more non-standard warnings issued, so it should be a more general solution.

Revision history for this message
Paul F. Dietz (paul-f-dietz) wrote :

Note the following comment in src/pcl/boot.lisp:

    ;; Note: We could DECLAIM the ftype of the generic function here,
    ;; since ANSI specifies that we create it if it does not
    ;; exist. However, I chose not to, because I think it's more
    ;; useful to support a style of programming where every generic
    ;; function has an explicit DEFGENERIC and any typos in DEFMETHODs
    ;; are warned about. Otherwise
    ;;
    ;; (DEFGENERIC FOO-BAR-BLETCH (X))
    ;; (DEFMETHOD FOO-BAR-BLETCH ((X HASH-TABLE)) ..)
    ;; (DEFMETHOD FOO-BRA-BLETCH ((X SIMPLE-VECTOR)) ..)
    ;; (DEFMETHOD FOO-BAR-BLETCH ((X VECTOR)) ..)
    ;; (DEFMETHOD FOO-BAR-BLETCH ((X ARRAY)) ..)
    ;; (DEFMETHOD FOO-BAR-BLETCH ((X LIST)) ..)
    ;;
    ;; compiles without raising an error and runs without raising an
    ;; error (since SIMPLE-VECTOR cases fall through to VECTOR) but
    ;; still doesn't do what was intended. I hate that kind of bug
    ;; (code which silently gives the wrong answer), so we don't do a
    ;; DECLAIM here. -- WHN 20000229

Revision history for this message
Paul F. Dietz (paul-f-dietz) wrote :

Just saw this happen again with someone. Bump.

Revision history for this message
Michał "phoe" Herda (phoe-krk) wrote :

That someone was me. In particular, on SBCL 2.0.10 Linux amd64:

* (progn
    (defclass foo () ())
    (defmethod intialize-instance :after ((o foo) &key) (break))
    (make-instance 'foo))
#<FOO {1002F223B3}>

It took me a while to realize why BREAK is not called and the instance is silently returned.

Revision history for this message
Paul F. Dietz (paul-f-dietz) wrote :

It was someone else, actually. But you too. :)

Revision history for this message
Steve Losh (stevelosh) wrote :

+1 from me. Today SBCL was complaining about an unset slot in a funcallable class when I was trying to use the MOP dependent maintenance protocol. Turns out this was the culprit:

    (defmethod update-dependent ((obj some-metaclass) (dep some-dependent) &rest initargs)
      (do-something))

It's particularly annoying because there's no obvious typo, I just forgot the c2mop: package prefix.

Revision history for this message
Christophe Rhodes (csr21-cantab) wrote :

A patch, for discussion.

Revision history for this message
Stas Boukarev (stassats) wrote :

I would've liked it to be signaled only when the user requests it. Possibly via a proclamation.

Revision history for this message
Paul F. Dietz (paul-f-dietz) wrote :

My desires: something that can turn it on, and something that can turn it off. I'd like to be able to put something in the .sbclrc file, or something that could wrap a call to the compiler in asdf to enable it just for a particular system. Default being turned off is fine, but I want to be able to enable it for the software of others without having to modify their files.

Revision history for this message
Stas Boukarev (stassats) wrote :

I like defining just a method when there's a default, so I just have

(defmethod method (no-specialization))

instead of duplicating it with a defgeneric.
But if the protocol calls for a specialized method to be defined then I use a defgeneric.

Revision history for this message
Christophe Rhodes (csr21-cantab) wrote :

So, with this patch,

* (proclaim '(sb-ext:muffle-conditions sb-pcl::defmethod-for-unknown-defgeneric))

turns the compile-time warning off (so compile-and-load of

    (defmethod foo (x) 'x)

issues no warnings). It doesn't turn the load-source warning off.

It could be the other way around by default, I suppose: though I would prefer the default to be most helpful to newcomers, and allow people to turn it off once they are in a position to know how rather than have to go looking for it. So I suppose two questions:

1. can the default be this way around?

2. is the load-source behaviour a deal-breaking issue?

Revision history for this message
Stas Boukarev (stassats) wrote :

I envision a general solution for enabling or disabling non-standard but useful warnings. So that they can be all enabled at once. Something like an OPTIMIZE declaration.

Revision history for this message
Michał "phoe" Herda (phoe-krk) wrote :

Something like (declaim (sb-ext:please-be-nitpicky)) in .sbclrc sounds good to me, and could certainly be advertised for newcomers to SBCL as a source of more useful information.

I think it would also be worth to create a new subtype of all such non-standard style-warnings that SBCL signals, so they can be handled or muffled as appropriate.

To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.