Incorrect "fallback" optimized constructors for compiled MAKE-INSTANCE calls

[Jan Moringen]

What I do (from the test case in the attached patch):
;; Define a class and force the "fallback" constructor generator to be
;; used by having a HAIRY-AROUND-OR-NONSTANDARD-PRIMARY-METHOD-P on
;; SHARED-INITIALIZE.
(defclass bug-? () ((foo :initarg :foo :reader bug-?-foo))
  (:default-initargs :foo (error "Should not be evaluated")))
(defmethod shared-initialize :around ((instance bug-?) (slot-names t) &key)
  (call-next-method))

;; Now compile a lambda containing MAKE-INSTANCE to exercise the
;; fallback constructor generator. Call the resulting compiled
;; function to trigger the bug.
(funcall (compile nil (lambda () (make-instance 'bug-? :foo t)))))
=| ERROR "Should not be evaluated"

What did I expect to happen:
An instance of BUG-? should be returned in which the FOO slot is bound to T. The code associated to the default initarg should not be evaluated.

SBCL Version: ~ 1.1.7

$ uname -a
Linux ferberit 3.5.0-28-lowlatency #32-Ubuntu SMP PREEMPT Fri Apr 26 11:05:36 UTC 2013 i686 i686 i686 GNU/Linux

*FEATURES*:
(:ALIEN-CALLBACKS :ANSI-CL :C-STACK-IS-CONTROL-STACK :COMMON-LISP
 :COMPARE-AND-SWAP-VOPS :CYCLE-COUNTER :ELF :GENCGC :IEEE-FLOATING-POINT
 :INLINE-CONSTANTS :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 :SB-DOC :SB-EVAL :SB-FUTEX
 :SB-LDB :SB-PACKAGE-LOCKS :SB-SOURCE-LOCATIONS :SB-TEST :SB-THREAD :SB-UNICODE
 :SBCL :STACK-ALLOCATABLE-CLOSURES :STACK-ALLOCATABLE-FIXED-OBJECTS
 :STACK-ALLOCATABLE-LISTS :STACK-ALLOCATABLE-VECTORS
 :STACK-GROWS-DOWNWARD-NOT-UPWARD :UNIX :UNWIND-TO-FRAME-AND-CALL-VOP :X86)

[Jan Moringen (scymtym)]

The attached patch fixes the problem and does not make any other tests fail (as far as I could test).

However, it includes a small change in behavior the correctness of which I cannot completely determine: previously, default initargs did not go through QUOTE-PLIST-KEYS which they would now do.

[Christophe Rhodes (csr21-cantab)]

 status fixcommitted
 tag -review
 done

Jan Moringen <email address hidden> writes:

> The attached patch fixes the problem and does not make any other tests
> fail (as far as I could test).
>
> However, it includes a small change in behavior the correctness of which
> I cannot completely determine: previously, default initargs did not go
> through QUOTE-PLIST-KEYS which they would now do.

I think this change of behaviour is more than correct, it is necessary;
I've included a test case demonstrating that previously, given
non-keyword default initargs, SBCL's behaviour was wrong, leading to
errors of the form
  keyword argument not a symbol: #<unbound marker {51}>
which is pretty good as errors go.

commit 6c296da561efd25c22e051a1e55080d9689f3ecc
Author: Christophe Rhodes <email address hidden>
Date: Mon Sep 9 12:42:30 2013 +0100

    better ctor fallback-generators

    The logic surrounding default-initargs in the presence of "hairy"
    methods on make-instance and friends was not quite right, leading
    to evaluation of the wrong things at the wrong times. Patch by
    Jan Moringen with extra test cases (lp#1179858).

Thank you,

Christophe