clojure.core.read-line broken

[Perry Trolard]

On the most recent svn (r1293), read-line throws a ClassCastException
when called:

user=> (read-line)
java.lang.ClassCastException: clojure.lang.LineNumberingPushbackReader
cannot be cast to java.io.BufferedReader (NO_SOURCE_FILE:0)

Removing the type hint solves the issue:

user=> (source read-line)
(defn read-line
"Reads the next line from stream that is the current value of
*in* ."
[] (. #^java.io.BufferedReader *in* (readLine)))
nil
user=> (.readLine *in*)
this is a line
"this is a line"


Perry

[Stephen C. Gilardi]

This is from issue 47, svn 1229. It's unfortunate that a class can't
be both a BufferedReader and a PushbackReader simultaneously. Those
classes were implemented using class inheritance rather than via
interfaces. In this case, the default *in* is not a BufferedReader,
but does provide readLine. It looks to me like removing the hint is
the correct fix.

--Steve

[Perry Trolard]

On Feb 18, 12:27 pm, "Stephen C. Gilardi" <squee...@mac.com> wrote:

> This is from issue 47, svn 1229. It's unfortunate that a class can't  
> be both a BufferedReader and a PushbackReader simultaneously. Those  
> classes were implemented using class inheritance rather than via  
> interfaces. In this case, the default *in* is not a BufferedReader,  
> but does provide readLine. It looks to me like removing the hint is  
> the correct fix.

Thanks, Steve -- I wasn't able to track down which revision introduced
the breakage. Now I understand the motivation (avoiding reflection):

user=> (set! *warn-on-reflection* true)
true
user=> (.readLine *in*)
Reflection warning, line: 2 - reference to field readLine can't be
resolved.
hello
"hello"
user=> (.readLine #^clojure.lang.LineNumberingPushbackReader *in*)
hello
"hello"

&, if I follow, the latter isn't robust because we need to allow for
*in* to be dynamically rebound, & wouldn't want to require that it's
always a LineNumberingPR.

Some microbenchmarking (details below) shows that doing an instance?
check combined with type hinting is almost twice as fast as relying on
reflection, & hardly slower than the type-hinted code w/o an instance?
check. How does the following implementation strike you?

(defn r-l []
(if (instance? clojure.lang.LineNumberingPushbackReader *in*)
(.readLine #^clojure.lang.LineNumberingPushbackReader *in*)
(.readLine #^java.io.BufferedReader *in*)))


Best,
Perry

--- 10 runs w/o type hinting: 0.119 msecs (avg) --

user=> (time (.readLine *in*)) "hello"
"Elapsed time: 0.105 msecs"
" \"hello\""
user=> (time (.readLine *in*)) "hello"
"Elapsed time: 0.116 msecs"
" \"hello\""
user=> (time (.readLine *in*)) "hello"
"Elapsed time: 0.113 msecs"
" \"hello\""
user=> (time (.readLine *in*)) "hello"
"Elapsed time: 0.122 msecs"
" \"hello\""
user=> (time (.readLine *in*)) "hello"
"Elapsed time: 0.153 msecs"
" \"hello\""
user=> (time (.readLine *in*)) "hello"
"Elapsed time: 0.108 msecs"
" \"hello\""
user=> (time (.readLine *in*)) "hello"
"Elapsed time: 0.11 msecs"
" \"hello\""
user=> (time (.readLine *in*)) "hello"
"Elapsed time: 0.115 msecs"
" \"hello\""
user=> (time (.readLine *in*)) "hello"
"Elapsed time: 0.125 msecs"
" \"hello\""
user=> (time (.readLine *in*)) "hello"
"Elapsed time: 0.124 msecs"
" \"hello\""

--- 10 runs with (r-l) func: 0.058 msecs (avg) ---

user=> (time (r-l)) "hello"
"Elapsed time: 0.055 msecs"
" \"hello\""
user=> (time (r-l)) "hello"
"Elapsed time: 0.055 msecs"
" \"hello\""
user=> (time (r-l)) "hello"
"Elapsed time: 0.064 msecs"
" \"hello\""
user=> (time (r-l)) "hello"
"Elapsed time: 0.061 msecs"
" \"hello\""
user=> (time (r-l)) "hello"
"Elapsed time: 0.057 msecs"
" \"hello\""
user=> (time (r-l)) "hello"
"Elapsed time: 0.056 msecs"
" \"hello\""
user=> (time (r-l)) "hello"
"Elapsed time: 0.058 msecs"
" \"hello\""
user=> (time (r-l)) "hello"
"Elapsed time: 0.062 msecs"
" \"hello\""
user=> (time (r-l)) "hello"
"Elapsed time: 0.054 msecs"
" \"hello\""
user=> (time (r-l)) "hello"
"Elapsed time: 0.061 msecs"
" \"hello\""

[Perry Trolard]

Hope I didn't imply by the above that I was suggesting a name change.

I know my proposed version is ugly, but since BufferedReader &
LineNumberingPR can't be unified, special-case-ing for the latter
strikes me as an acceptable fix. But removing the type hint so that
the function works out of the box is the next best option.

(I know the above microbenchmarking is dubious & smacks of premature
optimization, especially for a function whose use case, at least for
me, is command-line interaction -- which is not performance-critical.
But then I thought about reading strings from stdin, which could be;
but perhaps a (line-seq) on a wrapped System/in would be better
anyway...)

Perry

[Stephen C. Gilardi]

Hi Perry,

I think clojure.core/read-line should accept the default value of *in*
as a suitable provider of lines via "readLine" calls.

It should work. It used to work. It doesn't work now. I consider that
a bug.

I like the fix you've proposed that specifically allows
LineNumberingPushbackReader as a type for the argument. I think
removing the hint is also a good fix, but runs against an apparent
desire to remove reflection from clojure.core as much as possible.

Good luck,

--Steve

[Rayne]

I filed it as an issue a few days ago, until then I wrote my own read-
line from the old read-line.

>  smime.p7s
> 3KViewDownload

[Perry Trolard]

Just for the record, this problem with read-line was fixed at SVN
r1321.

(Thanks for packaging up the patch, Chouser.)

Perry