wanted: FORMAT should warn on (FORMAT "FOO ~A" ...)

[Tobias C. Rittweiler]

I regularly forget to add T, and simply write (FORMAT "FOO ~A" ...).

Notice that a string is valid as first argument to FORMAT, that's the
reason why this isn't complained about by type inference.

Still a string literal may not be modified, so SBCL has all right to
signal a full warning in this case.

[Roman Marynchak (roman-marynchak)]

CLHS says that a string should have a fill pointer in order to be an acceptable first argument to FORMAT. So, in such a case SBCL should give a compilation error, not a warning. When a string has no fill pointer, the code will fail to execute anyway, due to SB-IMPL::MAKE-FILL-POINTER-OUTPUT-STREAM call failure.

So it seems to me that the right way is to modify the FORMAT function info in file /src/compiler/fndb.lisp to be like this:

(defknown format ((or (member nil t) stream
                        (and string (satisfies array-has-fill-pointer-p)))
                  (or string function) &rest t)
  (or string null)
  (explicit-check))

Here a string as a first argument should also pass array-has-fill-pointer-p predicate. Unfortunately, such a modification breaks the cross-compilation and SBCL goes to LDB. Is SATISFIES prohibited to use with DEFKNOWN, or I have used it in a wrong way? The LDB info is:

     0: Foreign fp = 0x7ffff517c080, ra = 0x410fbf
   1: Foreign fp = 0x7ffff517c0b0, ra = 0x41471d
   2: Foreign fp = 0x7ffff517c0f0, ra = 0x41215a
   3: Foreign fp = 0x7ffff517c5a0, ra = 0x7ffff79cd080
   4: (SB!C::TL-XEP COMMON-LISP::ERROR)
   5: COMMON-LISP::FDEFINITION
   6: SB!C::CONSTANT-FUNCTION-CALL-VALUE
   7: SB!C::CONSTANT-FUNCTION-CALL-VALUE
   8: SB!C::CONSTANT-FUNCTION-CALL-P
   9: SB!KERNEL::CTYPEP

Regards,
Roman

[Roman Marynchak (roman-marynchak)]

The attached patch provides the required functionality and detects a string without a fill pointer as a first FORMAT argument. The only thing I don't like in that code is the kind of the error. As for now it is a fatal error, because COMPILER-ERROR does not work in the optimizer body. Maybe I should bind *compiler-error-context* to make it work?

This is my first patch to SBCL, and in case I do something wrong, please let me know and I will try to improve it.

Regards,
Roman

[Christophe Rhodes (csr21-cantab)]

Roman Marynchak <email address hidden> writes:

> The attached patch provides the required functionality and detects a
> string without a fill pointer as a first FORMAT argument. The only thing
> I don't like in that code is the kind of the error. As for now it is a
> fatal error, because COMPILER-ERROR does not work in the optimizer body.
> Maybe I should bind *compiler-error-context* to make it work?

I would suggest modelling the diagnostic on the other format argument
checking (in the case of a constant control string): that is, to emit
warnings, not errors of any kind. A warning that is not a style-warning
is enough to treat a compilation as unsuccessful, while it can be
muffled by users who know what they are doing.

> This is my first patch to SBCL, and in case I do something wrong, please
> let me know and I will try to improve it.

At least here, the patch either has poor indentation or has tabs for
whitespace, neither of which is ideal :-)

Christophe

[Roman Marynchak (roman-marynchak)]

Christophe, thank you for the valuable feedback :) Both notes are counted in the new variant of the patch. Also, I moved the fix to FORMAT transform to avoid multiple warnings generation in the iterative optimizer calls. So, now SBCL warns about the wrong FORMAT argument:

* (defun f() (format "FOO~A" "bar"))
; in: LAMBDA NIL
; (FORMAT "FOO~A" "bar")
;
; caught WARNING:
; A simple string "FOO~A" is not a valid first argument to FORMAT.
;
; compilation unit finished
; caught 1 WARNING condition

F
*

Regards,
Roman

[Nikodemus Siivola (nikodemus)]

In 1.0.46.8.