sb-pcl::constructor-function-form calls optimizing-generator even though there are custom methods

[Christophe]

Depending on whether make-instance is specialized on a class object or a class name, the CTOR optimization might or might not be applied. The potential bug could be that the code relies on compute-applicable-methods with the class object and does not check the applicable methods for the class-name.

This case was described at https://stackoverflow.com/q/56617695/124319, in an question that shows different behaviors depending on how an :around method specializes make-instance.

First of all, I am not sure the linked example is a conforming program, because I think make-instance is supposed to return an instance of the class, and not an arbitrary value (the glossary entry for direct-instance in the HyperSpec even says that "The function make-instance always returns a direct instance of the class which is (or is named by) its first argument.", but this is just an example and examples are not part of the standard AFAIK).

That being said, there could be conforming program where the return value is an instance but where the around method is not called.

Here is a way to spot the difference:

(defpackage :launchpad-ctor (:use :cl))
(in-package :launchpad-ctor)

(defclass foo () ())
(defclass bar () ())

(defmethod make-instance :around ((o (eql (find-class 'foo))) &key &allow-other-keys) :foo)
(defmethod make-instance :around ((o (eql 'bar)) &key &allow-other-keys) :bar)

(let ((x (make-instance 'foo))
      (y (make-instance 'bar)))
  (list x y))

;; (:FOO #<BAR {1006DD7403}>)

A little bit of investigation gives:

(defparameter *foo* (SB-PCL::ENSURE-CTOR '(SB-PCL::CTOR FOO T) 'FOO 'NIL 'T))
(defparameter *bar* (SB-PCL::ENSURE-CTOR '(SB-PCL::CTOR BAR T) 'BAR 'NIL 'T))

(setf (sb-pcl::ctor-class *foo*)
      (sb-pcl::ensure-class-finalized (find-class 'foo)))

(setf (sb-pcl::ctor-class *bar*)
      (sb-pcl::ensure-class-finalized (find-class 'bar)))

(sb-pcl::constructor-function-form *foo*)
;; (LAMBDA ()
;; (DECLARE
;; (OPTIMIZE (SPEED 3) (SAFETY 0) (SB-EXT:INHIBIT-WARNINGS 3) (DEBUG 0)))
;; (MAKE-INSTANCE #<STANDARD-CLASS LAUNCHPAD-CTOR::FOO>))

(sb-pcl::constructor-function-form *bar*)
;; (LAMBDA ()
;; (DECLARE
;; (OPTIMIZE (SPEED 3) (SAFETY 0) (SB-EXT:INHIBIT-WARNINGS 3) (DEBUG 0)))
;; (BLOCK NIL
;; (WHEN (SB-KERNEL:LAYOUT-INVALID #<SB-KERNEL:LAYOUT for BAR {2062C803}>)
;; (SB-PCL::INSTALL-INITIAL-CONSTRUCTOR #<SB-PCL::CTOR BAR ()>)
;; (RETURN (FUNCALL #<SB-PCL::CTOR BAR ()>)))
;; (LET ((SB-PCL::.INSTANCE. (SB-PCL::%MAKE-STANDARD-INSTANCE NIL))
;; (SB-PCL::.SLOTS. (MAKE-ARRAY 0)))
;; (SETF (SB-KERNEL:%INSTANCE-LAYOUT SB-PCL::.INSTANCE.)
;; #<SB-KERNEL:LAYOUT for BAR {2062C803}>)
;; (SETF (SB-PCL::STD-INSTANCE-SLOTS SB-PCL::.INSTANCE.) SB-PCL::.SLOTS.)
;; (LET ()
;; (DECLARE (IGNORABLE))
;; (FLET ((SB-PCL::INITIALIZE-IT (SB-PCL::.II-ARGS. SB-PCL::.NEXT-METHODS.)
;; (DECLARE (IGNORE SB-PCL::.NEXT-METHODS.))
;; (LET* ((SB-PCL::.INSTANCE. (CAR SB-PCL::.II-ARGS.)))
;; SB-PCL::.INSTANCE.)))
;; (DECLARE (DYNAMIC-EXTENT (FUNCTION SB-PCL::INITIALIZE-IT)))
;; (LET ((SB-PCL::.II-ARGS. (LIST SB-PCL::.INSTANCE.)))
;; (SB-PCL::INITIALIZE-IT SB-PCL::.II-ARGS. NIL))))
;; SB-PCL::.INSTANCE.)))
;; NIL
;; NIL
;; T

Adding breaks in CONSTRUCTOR-FUNCTION-FORM shows that CUSTOM-MAKE-INSTANCE is NIL for BAR (and T for FOO). This variable is T when the list of applicable methods contains more than one element, but that list is computed for a class argument, not a symbol argument; in the example, we have:

(compute-applicable-methods #'make-instance (list (find-class 'bar)))
;; (#<STANDARD-METHOD COMMON-LISP:MAKE-INSTANCE (CLASS) {10005EEE53}>)

(compute-applicable-methods #'make-instance (list 'bar))
;; (#<STANDARD-METHOD COMMON-LISP:MAKE-INSTANCE :AROUND ((EQL BAR)) {100637D383}>
;; #<STANDARD-METHOD COMMON-LISP:MAKE-INSTANCE (SYMBOL) {10005EEE43}>)

By computing the applicable method for a class argument, the function does not see that the called being optimized, which uses a symbol, is going to call other methods, in particular the one that completely discard the instance and returns :BAR.

SBCL 1.4.0.75.release.1710-6a36da1

Linux XPS-13-9343 4.4.0-150-generic #176-Ubuntu SMP Wed May 29 18:56:26 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux

(:CL-OPENGL-CHECKS-ERRORS :FSBV :SDL2 :CL-PPCRE :OSICAT-FD-STREAMS
 :CL-JSON-CLOS :CL-JSON :BORDEAUX-THREADS :21BIT-CHARS :FLOAT-QUIET-NAN
 :FLOAT-INFINITY :FLEXI-STREAMS :CLOSER-MOP :THREAD-SUPPORT
 CFFI-FEATURES:FLAT-NAMESPACE CFFI-FEATURES:X86-64 CFFI-FEATURES:UNIX :CFFI
 CFFI-SYS::FLAT-NAMESPACE :SPLIT-SEQUENCE ALEXANDRIA.0.DEV::SEQUENCE-EMPTYP
 :SWANK :QUICKLISP :SB-BSD-SOCKETS-ADDRINFO :ASDF3.3 :ASDF3.2 :ASDF3.1 :ASDF3
 :ASDF2 :ASDF :OS-UNIX :NON-BASE-CHARS-EXIST-P :ASDF-UNICODE :64-BIT
 :64-BIT-REGISTERS :ALIEN-CALLBACKS :ANSI-CL :ASH-RIGHT-VOPS
 :C-STACK-IS-CONTROL-STACK :CALL-SYMBOL :COMMON-LISP :COMPACT-INSTANCE-HEADER
 :COMPARE-AND-SWAP-VOPS :COMPLEX-FLOAT-VOPS :CYCLE-COUNTER :ELF :FLOAT-EQL-VOPS
 :FP-AND-PC-STANDARD-SAVE :GCC-TLS :GENCGC :IEEE-FLOATING-POINT :IMMOBILE-CODE
 :IMMOBILE-SPACE :INLINE-CONSTANTS :INTEGER-EQL-VOP :LARGEFILE :LINKAGE-TABLE
 :LINUX :LITTLE-ENDIAN :MEMORY-BARRIER-VOPS :MULTIPLY-HIGH-VOPS
 :OS-PROVIDES-BLKSIZE-T :OS-PROVIDES-DLADDR :OS-PROVIDES-DLOPEN
 :OS-PROVIDES-GETPROTOBY-R :OS-PROVIDES-POLL :OS-PROVIDES-PUTWC
 :OS-PROVIDES-SUSECONDS-T :PACKAGE-LOCAL-NICKNAMES :RAW-INSTANCE-INIT-VOPS
 :RAW-SIGNED-WORD :RELOCATABLE-HEAP :SB-CORE-COMPRESSION :SB-DOC :SB-EVAL
 :SB-FUTEX :SB-LDB :SB-PACKAGE-LOCKS :SB-SIMD-PACK :SB-SOURCE-LOCATIONS
 :SB-THREAD :SB-UNICODE :SB-XREF-FOR-INTERNALS :SBCL
 :STACK-ALLOCATABLE-CLOSURES :STACK-ALLOCATABLE-FIXED-OBJECTS
 :STACK-ALLOCATABLE-LISTS :STACK-ALLOCATABLE-VECTORS
 :STACK-GROWS-DOWNWARD-NOT-UPWARD :SYMBOL-INFO-VOPS :UNBIND-N-VOP
 :UNDEFINED-FUN-RESTARTS :UNIX :UNWIND-TO-FRAME-AND-CALL-VOP :X86-64)

[Stas Boukarev (stassats)]

Technically, you're not allowed to do that, according to

http://www.lispworks.com/documentation/HyperSpec/Body/11_abab.htm

19. Defining a method for a standardized generic function which is applicable when all of the arguments are direct instances of standardized classes.

[Christophe Rhodes (csr21-cantab)]

A couple of things...

Christophe <email address hidden> writes:

> First of all, I am not sure the linked example is a conforming program,
> because I think make-instance is supposed to return an instance of the
> class, and not an arbitrary value (the glossary entry for direct-
> instance in the HyperSpec even says that "The function make-instance
> always returns a direct instance of the class which is (or is named by)
> its first argument.", but this is just an example and examples are not
> part of the standard AFAIK).

Glossary entries are, generally, normative.

> That being said, there could be conforming program where the return
> value is an instance but where the around method is not called.

You mean, the around method doesn't call the next method?

The MOP has some details about what user programs are allowed to do.
Specifically,

  Portable programs may define methods that extend specified methods
  unless the description of the specified method explicitly prohibits
  this. Unless there is a specific statement to the contrary, these
  extending methods must return whatever value was returned by the call
  to call-next-method.

which specifically prohibits not using the return value of the base
method.

Further,

  Any method defined by a portable program on a specified generic
  function must have at least one specializer that is neither a
  specified class nor an eql specializer whose associated value is an
  instance of a specified class.

would, in at least one reading, prohibit the use of both (eql 'bar) and
(eql (find-class 'foo)) as a specializer for make-instance (because
both #<STANDARD-CLASS FOO> and BAR are direct instances of a specified
class). In a way, this is reasonable: if you want special stuff to
happen for MAKE-INSTANCE, you should define a metaclass.

[Christophe (junke-christophe)]

Thanks for both quick replies and explanations.

> You mean, the around method doesn't call the next method?

As a matter of fact, no, I mean the around method itself is not called.
There is a call to (MAKE-INSTANCE 'FOO), so ignoring for a second that MAKE-INSTANCE is a standardized gf, the call would be expected to invoke methods specialized on symbols; here the symbol is already resolved, at compile-time, to a class; methods specializing on symbols, including the :around one, are not called because the compiler takes a shortcut.