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

Bug #1818142 reported by Paul F. Dietz on 2019-02-28
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
SBCL
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) on 2019-02-28
Changed in sbcl:
status: New → Confirmed
importance: Undecided → Wishlist
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))

Stas Boukarev (stassats) wrote :

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

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

Paul F. Dietz (paul-f-dietz) wrote :

Just saw this happen again with someone. Bump.

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.

Paul F. Dietz (paul-f-dietz) wrote :

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

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.

A patch, for discussion.

Stas Boukarev (stassats) wrote :

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

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.

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.

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?

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.

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  Edit
Everyone can see this information.

Other bug subscribers