CAREFUL-SPECIFIER-TYPE isn't careful enough

[Douglas Katzman]

Because CAREFUL-blah can end up calling the non-careful version of same, we end up with totally opaque error messages on a mis-stated type declaration. (Even though it certainly looks like the CAREFUL wrapper should trap any nested errors)

Here I meant (simple-vector 3) but I accidentally wrote (simple-array 3)

* (defun foo (a) (declare ((simple-array 3) a)) (foo a))

debugger invoked on a TYPE-ERROR:
  The value 3 is not of type (OR CONS SYMBOL SB-KERNEL:INSTANCE).

restarts (invokable by number or by possibly-abbreviated name):
  0: [ABORT] Exit debugger, returning to top level.

(TYPEXPAND 3 NIL) [tl,external]

[Kanru Chen (kanru)]

The type-error is from the declaration of TYPEXPAND

(defun typexpand (type-specifier &optional env)
  (declare (type type-specifier type-specifier))
  ...

This patch make VALUES-SPECIFIER-TYPE check the type before calling TYPEXPAND.

About CAREFUL-SPECIFIER-TYPE.. do you think it needs to catch all nested errors?

[Kanru Chen (kanru)]

Now the error messages is

* (defun foo (a) (declare ((simple-array 3) a)) (foo a))
; in: DEFUN FOO
; ((SIMPLE-ARRAY 3) A)
;
; caught ERROR:
; The object 3 is not valid as a type specifier.

debugger invoked on a SB-INT:SIMPLE-PROGRAM-ERROR in thread
#<THREAD "main thread" RUNNING {1003E06973}>:
  The object 3 is not valid as a type specifier.

Type HELP for debugger help, or (SB-EXT:EXIT) to exit from SBCL.

restarts (invokable by number or by possibly-abbreviated name):
  0: [ABORT] Exit debugger, returning to top level.

((LAMBDA NIL :IN SB-C::ACTUALLY-COMPILE))
0]

[Douglas Katzman (dougk)]

that certainly works. But since we expose SB-EXT:TYPEXPAND to users, it might make sense to catch it there.

* (sb-ext:typexpand 3)
debugger invoked on a TYPE-ERROR:
  The value 3 is not of type (OR CONS SYMBOL SB-KERNEL:INSTANCE).

I'd prefer to see your nice error message.

[Douglas Katzman (dougk)]

I see, typexpand does have a type-declaration on its input, but the 'careful' wrapper won't catch an error of type type-error but will catch (error "whatever"). This seems weird.

Plus I found a more interesting failure mode - somewhere we've got *really* unsafe parsing code:

* (SB-C::CAREFUL-SPECIFIER-TYPE '(cons . 3))
CORRUPTION WARNING in SBCL pid 74962(tid 140735206552336):
Memory fault at 0x7 (pc=0x10005869db, sp=0x11ff5b0)
The integrity of this image is possibly compromised.
Continuing with fingers crossed.

[Kanru Chen (kanru)]

Check if the input is valid type-specifier. I'm not sure if I should wrap the check in a inline function.

[Douglas Katzman (dougk)]

It seems to me that careful-specifier-type is really supposed to catch more than just 'simple-error'.
So while it's certainly nice to report the condition in slightly different words, perhaps the simplest change is this:

--- a/src/compiler/ir1util.lisp
+++ b/src/compiler/ir1util.lisp
@@ -2223,7 +2223,9 @@ is :ANY, the function name is not checked."
                 (values nil (list (format nil "~A" condition))))
               (simple-error (condition)
                 (values nil (list* (simple-condition-format-control condition)
- (simple-condition-format-arguments condition))))))
+ (simple-condition-format-arguments condition))))
+ (type-error (condition)
+ (values nil (list (write-to-string condition :escape nil))))))

But I don't like the return convention. It should be just (values nil condition) in either case.
And as far as I can tell, nothing uses the 2nd value. It can be returned from type derivers (some in 'knownfun' and in 'array-tran') but the only places that call a 'fun-info-derive-type' function ('srctran' and 'ir1opt') do not use the second value.
Probably we should do all these things: robustify the 'careful' function, alter its return convention, and make the message for user-visible APIs the one in your patch.