wanted: better type errors reporting in PRINT

Bug #586940 reported by Roman Marynchak
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
SBCL
Triaged
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

Tags: printer
Revision history for this message
Roman Marynchak (roman-marynchak) wrote :

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)
Revision history for this message
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.

Revision history for this message
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  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.