wanted: better type errors reporting in PRINT

Bug #586940 reported by Roman Marynchak on 2010-05-28
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
SBCL
Low
Roman Marynchak
Declined for Trunk by Nikodemus Siivola

Bug Description

A simple issue, discovered by accident. SBCL 1.0.38

Try entering (print 1 2) in REPL. The next error raises:

debugger invoked on a SIMPLE-ERROR in thread #<THREAD "initial thread" RUNNING
                                               {1002ABEEF1}>:
  There is no applicable method for the generic function
    #<STANDARD-GENERIC-FUNCTION STREAM-TERPRI (1)>
  when called with arguments
    (2).

It will be better to signal the error in PRINT and to claim that the second argument is not a valid output stream. Both CLISP and Lispworks go this way.

The same issue exists for similar functions, for example PRINC.

Regards,
Roman

The patch, as per discussion on sbcl-devel.

Now (print 1 2) yields:

(print 1 2)

debugger invoked on a TYPE-ERROR in thread #<THREAD "initial thread" RUNNING
                                             {A9F5941}>:
  The value 2 is not of type STREAM.

PRINC, PPRINT and PRIN1 now give the same error as the above when given non-output-stream second argument.

Regards,
Roman

Changed in sbcl:
status: New → In Progress
assignee: nobody → Roman Marynchak (roman-marynchak)
tags: added: review
Changed in sbcl:
assignee: Roman Marynchak (roman-marynchak) → Nikodemus Siivola (nikodemus)
Nikodemus Siivola (nikodemus) wrote :

This patch causes a notable drop in output performance, since OUT-SYNONYM-OF is called quite a lot -- often needlessly -- and OUTPUT-STREAM-P is a generic function.

While I agree that this patch is in principle a good thing, other code needs to change as well for this to go in.

As an example:

(defun rush (stream n object)
  (declare (fixnum n))
  (loop repeat n
        do (print object stream)))

(with-open-file (f "/tmp/rush.tmp" :if-exists :supersede :direction :output)
  (time (rush f 10000000 42)))

Without the patch this runs in under 3.9 seconds run-time and 4.2 real-time on my system. With the patch it takes 6.4 seconds run-time and 6.6 seconds real-time.

Adding

(defun %write-char (character stream)
  (with-out-stream/no-synonym stream
    (ansi-stream-out character)
    (stream-write-char character)))

-- like WRITE-CHAR, but doesn't deal with output stream designators -- and changing PRINT to

(defun print (object &optional stream)
  #!+sb-doc
  "Output a newline, the mostly READable printed representation of OBJECT, and
  space to the specified STREAM."
  (let ((stream (out-synonym-of stream))
        (*print-escape* t))
    (%write-char #\newline stream)
    (output-object object stream)
    (%write-char #\space stream)
    object))

and similarly using %WRITE-CHAR instead of WRITE-CHAR in %OUTPUT-REASONABLE-INTEGER-IN-BASE makes the test-case run in 4.0 run-time and 4.3 real-time -- which is an acceptable cost, IMO.

So, all output functions underneath the user-callable layer need to be audited so that OUT-SYNONYM-OF is not called uselessly, instead of using WRITE-CHAR/STRING/WHATEVER using corresponding %WRITE-FOO functions which need to be written.

Nikodemus Siivola (nikodemus) wrote :

Um, and even if the performance loss under control (small, but not catastrophic), the #!high-security feature is perhaps better kept (or rather resurrected and renamed) so that people running systems where IO speed is critical don't have to pay the price.

tags: added: printer
removed: review
Changed in sbcl:
assignee: Nikodemus Siivola (nikodemus) → nobody
status: In Progress → Triaged
Changed in sbcl:
assignee: nobody → Roman Marynchak (roman-marynchak)
Changed in sbcl:
importance: Undecided → Low
To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers