wrong free declaration scope

[Nikodemus Siivola]

The following code must signal type error:

    (locally (declare (optimize (safety 3)))
      (flet ((foo (x &optional (y (car x)))
               (declare (optimize (safety 0)))
               (list x y)))
        (funcall (eval #'foo) 1)))

This also affects LET* (but not LET):

(defun foo (x)
  (declare (optimize safety))
  (let* ((z (the symbol x))
         (y z))
    (declare (optimize (safety 0)))
    (symbolp y)))

(foo 42) => T

[Nikodemus Siivola (nikodemus)]

Dropping to LOW: misread the meaning of the bug.

[Nikodemus Siivola (nikodemus)]

Attached patch fixes this.

It has potential for notable performance regressions in both our
own and user code originally developed with the original behaviour.

Currently (and historically) in SBCL OPTIMIZE declarations such as

 (lambda (...) (declare (optimize ...)) ...)

affect lambda-list processing, which is wrong -- they should apply
only to the body. This means that after this patch declarations which
used to eg. disable argument count checking no longer have that
effect. Etc.

Most insidiously this means that it's impossible to locally change the
compiler policy for:

 (compile nil '(lambda ...))

The patch also breaks several of our test-cases, which --among other
things-- rely on COMPILE forms such as above.

This is such a sour apple that I am inclined to call this a known bug
and declare it WONTFIX. Not the SBCL policy to take such a low road,
though. If this is fixed, then we need to either support :POLICY
hacking in WITH-COMPILATION-UNIT, or extend COMPILE (or have
SB-INT:COMPILE*, or something.)

Bah.

[Nikodemus Siivola (nikodemus)]

Upping to MEDIUM due to effect on LET*.

[Nikodemus Siivola (nikodemus)]

Updated patch. Some tests (eg. backtrace tests) still fail.

[Christophe Rhodes (csr21-cantab)]

This has been making me sad.

Firstly, updated patch is attached; it should apply, but it is hand-edited.

Secondly, some of the test cases fail, in surprising ways. In particular, one which fails is

(defun foo (x)
  (declare (optimize speed (safety 1)) (type (and simple-bit-vector (satisfies undefined)) x)
  (elt x 5))

(foo #*101010) ; see :bug-316078 in tests/compiler.pure.lisp

where what we assume happens is that the hairy type test is weakened by the speed-3/safety-1 policy, and this no longer happens.

The semantics of mixing type and policy declarations is not 100% clear, but the way ANSI explains type declarations as being equivalent to inserting THE forms in the body (including at the head of the body) suggests to me that the test is correct and the new semantics are wrong, for this case. I have not been able to adjust the compiler to implement what I think are the correct semantics, however.

[Christophe Rhodes (csr21-cantab)]

The attached patch updates all the test cases, as well as being an up-to-date fix for the code.

However, it really is horrible. One major problem with the patch as it stands is that it's impossible to compile multiple local functions with different policies: in the old world

(labels ((foo (x) (declare (optimize safety) (type fixnum x)) (foo/unsafe x))
       (foo/unsafe (x) (declare (optimize speed (safety 0))) (frob-unsafely x)))
  ...)

would do the "right" (really "wrong") thing, with FOO doing type checking and FOO/UNSAFE being as fast as possible. Now, however, the lambda bindings and typechecking depend on any outer policy, and (crucially) is necessarily the same for all the local functions. We can see the contortions that this causes even in simple cases in the fixups to the debug.impure.lisp tests; complex cases become impossible to express.

This could be mitigated somewhat by deciding that free optimize declarations at the head of a lambda cause the component as a whole to gain those optimize qualities (as they currently do), irrespective of the freeness of the declaration -- while still being careful not to have any arguments evaluated under that policy. This would help for insertion of debug catch tags and step conditions and the like, leaving arg-count checking as the major loser (which might be acceptable).

Argh. (still, at least this bug isn't tagged "easy")