Re: nulls and WrappedArray

I've thought about this a bit more, and I think this is a Scala API problem.

The Tree[A](root:A, getChildren: A => Seq[A]) constructor is a general
structure; there's no reason why it's API should take into account some
semantic foible of an old Java class. Using Option[Array[File]] would be
good advice for the creator of the java.io.File class, but that is obviously
impossible.

The sequence of events is:
1) Tree expects a function that yields a Seq[A].
2) The caller provides a function returning Array[File], which can be
expected to implicitly convert into a WrappedArray
3) When the internals of Tree get around to calling the function, the
implicit converter is called.
4) If the result is null, the implicit method returns a new
WrappedArray(null).
5) Tree uses the WrappedArray in good faith, yet a NPE emanates somewhere
from the guts of WrappedArray.

To me, 4) and 5) are broken. For a start, the WrappedArray(null) object is
inherently busted; it should never happen. If WrappedArray's behaviour is
undefined for a null value, then it should prohibit it by immediately
throwing an exception on creation. If it is, then its methods should behave
in a sensible manner and not fall over.

Secondly, if the implicit method is allowed to receive a null, it should
return a valid result. If not, then it should throw an exception.

Bottom line is, the Tree is entitled to think it's getting valid Seqs; and
the caller is entitled to think that its array will be implicitly converted
to a valid Seq. Currently, it's getting a broken half-way twilight-zone
value.

It seems to me the easiest thing would be for the implicit cast to simply
pass on null values -- nobody's contract gets broken.
-Ken

On Thu, Feb 18, 2010 at 11:38 PM, Ken Scambler <ken.scambler@gmail.com> wrote:

4) If the result is null, the implicit method returns a new
WrappedArray(null).
5) Tree uses the WrappedArray in good faith, yet a NPE emanates somewhere
from the guts of WrappedArray.

To me, 4) and 5) are broken.

To me too.  This just isn't a valid way for a library to behave in a language that has null running around _unless_ the library is designed for the most blisteringly fast performance possible (and mostly the libraries are not designed for performance trump all other concerns).
 

It seems to me the easiest thing would be for the implicit cast to simply
pass on null values -- nobody's contract gets broken.

Agreed.  The null pointer check on the WrappedArray implicit will hardly add any runtime cost, and will produce comprehensible behavior.

  --Rex

In my opinion the only right thing to do in this context is not to wrap null at all. This has the implication that an implicit is unable to wrap null under any circumstances as there isn't really any method which can be called on null.
Throwing a NPE early is not a valid use case as there might be apis for which it is perfectly legal to pass on null.
The only thing I'm not sure is if this is still type safe but considering that null is the bottom type of everything it should work shouldn't it?

-Stefan

2010/2/19 Ken Scambler <ken.scambler@gmail.com>

I've thought about this a bit more, and I think this is a Scala API problem.

The Tree[A](root:A, getChildren: A => Seq[A]) constructor is a general
structure; there's no reason why it's API should take into account some
semantic foible of an old Java class.  Using Option[Array[File]] would be
good advice for the creator of the java.io.File class, but that is obviously
impossible.

The sequence of events is:
1) Tree expects a function that yields a Seq[A].
2) The caller provides a function returning Array[File], which can be
expected to implicitly convert into a WrappedArray
3) When the internals of Tree get around to calling the function, the
implicit converter is called.
4) If the result is null, the implicit method returns a new
WrappedArray(null).
5) Tree uses the WrappedArray in good faith, yet a NPE emanates somewhere
from the guts of WrappedArray.

To me, 4) and 5) are broken.   For a start, the WrappedArray(null) object is
inherently busted; it should never happen. If WrappedArray's behaviour is
undefined for a null value, then it should prohibit it by immediately
throwing an exception on creation. If it is, then its methods should behave
in a sensible manner and not fall over.

Secondly, if the implicit method is allowed to receive a null, it should
return a valid result.  If not, then it should throw an exception.

Bottom line is, the Tree is entitled to think it's getting valid Seqs; and
the caller is entitled to think that its array will be implicitly converted
to a valid Seq.  Currently, it's getting a broken half-way twilight-zone
value.

It seems to me the easiest thing would be for the implicit cast to simply
pass on null values -- nobody's contract gets broken.
-Ken
--
View this message in context: http://old.nabble.com/nulls-and-WrappedArray-tp27637395p27649671.html
Sent from the Scala - User mailing list archive at Nabble.com.

Thanks Rex. Guess I'll file a TRAC issue. Any dissenters?

Rex Kerr-2 wrote:
>
> On Thu, Feb 18, 2010 at 11:38 PM, Ken Scambler
> wrote:
>
>>
>> 4) If the result is null, the implicit method returns a new
>> WrappedArray(null).
>> 5) Tree uses the WrappedArray in good faith, yet a NPE emanates somewhere
>> from the guts of WrappedArray.
>>
>> To me, 4) and 5) are broken.
>
>
> To me too. This just isn't a valid way for a library to behave in a
> language that has null running around _unless_ the library is designed for
> the most blisteringly fast performance possible (and mostly the libraries
> are not designed for performance trump all other concerns).
>
>
>> It seems to me the easiest thing would be for the implicit cast to simply
>> pass on null values -- nobody's contract gets broken.
>>
>
> Agreed. The null pointer check on the WrappedArray implicit will hardly
> add
> any runtime cost, and will produce comprehensible behavior.
>
> --Rex
>
>

>>>>> "Stefan" == Stefan Langer writes:

Stefan> In my opinion the only right thing to do in this context is not
Stefan> to wrap null at all. This has the implication that an implicit
Stefan> is unable to wrap null under any circumstances as there isn't
Stefan> really any method which can be called on null. Throwing a NPE
Stefan> early is not a valid use case as there might be apis for which
Stefan> it is perfectly legal to pass on null. The only thing I'm not
Stefan> sure is if this is still type safe but considering that null is
Stefan> the bottom type of everything it should work shouldn't it?

We just had a whole thread about this a week ago. Anyone interested
may want to review
http://old.nabble.com/Scala-null-and-implicit-conversion-%28bug-%29-td27...

but it didn't really result in any conclusion. If somebody opens a ticket could they please post the ticket number

2010/2/19 Seth Tisue <seth@tisue.net>

>>>>> "Stefan" == Stefan Langer <mailtolanger@googlemail.com> writes:

 Stefan> In my opinion the only right thing to do in this context is not
 Stefan> to wrap null at all. This has the implication that an implicit
 Stefan> is unable to wrap null under any circumstances as there isn't
 Stefan> really any method which can be called on null.  Throwing a NPE
 Stefan> early is not a valid use case as there might be apis for which
 Stefan> it is perfectly legal to pass on null.  The only thing I'm not
 Stefan> sure is if this is still type safe but considering that null is
 Stefan> the bottom type of everything it should work shouldn't it?

We just had a whole thread about this a week ago.  Anyone interested
may want to review
http://old.nabble.com/Scala-null-and-implicit-conversion-%28bug-%29-td27546874.html#a27546874

--
Seth Tisue @ Northwestern University | http://tisue.net
lead developer, NetLogo: http://ccl.northwestern.edu/netlogo/

On Fri, Feb 19, 2010 at 4:14 AM, Stefan Langer <mailtolanger@googlemail.com> wrote:

In my opinion the only right thing to do in this context is not to wrap null at all. This has the implication that an implicit is unable to wrap null under any circumstances as there isn't really any method which can be called on null.
Throwing a NPE early is not a valid use case as there might be apis for which it is perfectly legal to pass on null.
The only thing I'm not sure is if this is still type safe but considering that null is the bottom type of everything it should work shouldn't it?

-Stefan

A common implicit conversion I use is:implicit def toOption[T](in: T): Option[T] = if (in == null) None else Some(in)
If this were to instead fail and pass on null instead of None that would break some programs I have made. And I think this is a perfectly acceptable implicit conversion to use, especially when working with java libs that are expected to return null at some point.
My opinion (and the way I have been handling this) is that if I am working with a lib that might return null, I need to handle that myself before doing anything else. For myself this usually means converting the value to an Option.  --
Derek

On Fri, Feb 19, 2010 at 10:51 AM, Derek Williams <derek@nebvin.ca> wrote:

On Fri, Feb 19, 2010 at 4:14 AM, Stefan Langer <mailtolanger@googlemail.com> wrote:

In my opinion the only right thing to do in this context is not to wrap null at all. This has the implication that an implicit is unable to wrap null under any circumstances as there isn't really any method which can be called on null.
Throwing a NPE early is not a valid use case as there might be apis for which it is perfectly legal to pass on null.
The only thing I'm not sure is if this is still type safe but considering that null is the bottom type of everything it should work shouldn't it?

-Stefan

A common implicit conversion I use is:implicit def toOption[T](in: T): Option[T] = if (in == null) None else Some(in)
If this were to instead fail and pass on null instead of None that would break some programs I have made. And I think this is a perfectly acceptable implicit conversion to use, especially when working with java libs that are expected to return null at some point.
My opinion (and the way I have been handling this) is that if I am working with a lib that might return null, I need to handle that myself before doing anything else. For myself this usually means converting the value to an Option.

I agree that the compiler shouldn't pass through null *automatically*.  However, implicit conversions in the library should not result in weird breakage at some later point if a null goes through.  So the _library implicit conversions should be rewritten (by hand)_ for those classes that break if they're initialized with null.  Some classes, like Option, are perfectly happy if you put null into them--then it's up to you whether you want to stick null into them, whether you want to catch the null and do something else sensible (e.g. null -> None), or whether you want to pass the null through (often handy when interfacing heavily with Java).

  --Rex
 

I agree. To me, they should plain return null.asInstanceOf[X] and be done with it.

On Fri, Feb 19, 2010 at 2:38 AM, Ken Scambler <ken.scambler@gmail.com> wrote:

I've thought about this a bit more, and I think this is a Scala API problem.

The Tree[A](root:A, getChildren: A => Seq[A]) constructor is a general
structure; there's no reason why it's API should take into account some
semantic foible of an old Java class.  Using Option[Array[File]] would be
good advice for the creator of the java.io.File class, but that is obviously
impossible.

The sequence of events is:
1) Tree expects a function that yields a Seq[A].
2) The caller provides a function returning Array[File], which can be
expected to implicitly convert into a WrappedArray
3) When the internals of Tree get around to calling the function, the
implicit converter is called.
4) If the result is null, the implicit method returns a new
WrappedArray(null).
5) Tree uses the WrappedArray in good faith, yet a NPE emanates somewhere
from the guts of WrappedArray.

To me, 4) and 5) are broken.   For a start, the WrappedArray(null) object is
inherently busted; it should never happen. If WrappedArray's behaviour is
undefined for a null value, then it should prohibit it by immediately
throwing an exception on creation. If it is, then its methods should behave
in a sensible manner and not fall over.

Secondly, if the implicit method is allowed to receive a null, it should
return a valid result.  If not, then it should throw an exception.

Bottom line is, the Tree is entitled to think it's getting valid Seqs; and
the caller is entitled to think that its array will be implicitly converted
to a valid Seq.  Currently, it's getting a broken half-way twilight-zone
value.

It seems to me the easiest thing would be for the implicit cast to simply
pass on null values -- nobody's contract gets broken.
-Ken
--
View this message in context: http://old.nabble.com/nulls-and-WrappedArray-tp27637395p27649671.html
Sent from the Scala - User mailing list archive at Nabble.com.

--
Daniel C. Sobral

I travel to the future all the time.

The 'right' behaviour varies from java function to java function. Any one-size-fits-all approach will be unsuitable a significant portion of the time. However, each approach will always be correct for a given java function. However, I'm not aware of any scala language features that would make it easy to implement per-function implicits.

Matthew

It's almost like we need implicits to be injected at pointcuts, rather than based upon type...

On 19 February 2010 17:34, Matthew Pocock <turingatemyhamster@googlemail.com> wrote:

The 'right' behaviour varies from java function to java function. Any one-size-fits-all approach will be unsuitable a significant portion of the time. However, each approach will always be correct for a given java function. However, I'm not aware of any scala language features that would make it easy to implement per-function implicits.

Matthew

The problem here is pretty easy:   scala> val a: Seq[String] = null
a: Seq[String] = null   scala> val b: Array[String] = null
b: Array[String] = null   scala> val c: Seq[String] = b
java.lang.NullPointerException   The implicit conversion from Array to Seq returns an exception if passed "null". The solution is easy: if null is received, return null.

On Fri, Feb 19, 2010 at 3:34 PM, Matthew Pocock <turingatemyhamster@googlemail.com> wrote:

The 'right' behaviour varies from java function to java function. Any one-size-fits-all approach will be unsuitable a significant portion of the time. However, each approach will always be correct for a given java function. However, I'm not aware of any scala language features that would make it easy to implement per-function implicits.

Matthew

--
Daniel C. Sobral

I travel to the future all the time.

On Fri, Feb 19, 2010 at 05:35:29PM +0000, Matthew Pocock wrote:
> pointcuts

You! Back of the line!

I agree - this is the obvious thing to do in many/most cases, but consider java.util.Map<String[]>.get() (where we want option, not null) or File.listFiles() (where we want an Either explaining why the files could not be listed).

Matthew

The implicit conversion from Array to Seq returns an exception if passed "null". The solution is easy: if null is received, return null.

No, we don't want that. If File.listFiles() returned "null", then a "null" it must remain. Unless I explicitly transform it into something else.   I'm not discussing how an interface should be constructed. I'm discussing how wrappers are supposed to deal with "null", and the way they should do that is to _leave_ them "null". Heck, I might need to pass this back to a Java API that expects the "null".

On Fri, Feb 19, 2010 at 3:58 PM, Matthew Pocock <turingatemyhamster@googlemail.com> wrote:

I agree - this is the obvious thing to do in many/most cases, but consider java.util.Map<String[]>.get() (where we want option, not null) or File.listFiles() (where we want an Either explaining why the files could not be listed).

Matthew

The implicit conversion from Array to Seq returns an exception if passed "null". The solution is easy: if null is received, return null.

--
Daniel C. Sobral

I travel to the future all the time.

I think the possibilities are now on the table:
 (a) Discuss each and every library implicit case by case what mapping is appropriate for a null      (e.g. to empty whatever) Tough, and will take some time  (b) Or treat null analogously to a NaN in floating point operations: any operation on a NaN yields a NaN,        so any implicit conversion of a null yields a null. Simple concept, easy to understand  (c) Leave it as it is.
In no case a compiler hack should be appropriate / needed. My personal preference is (b) mostly since AFAIK the NonNull-trait has to be considered as half baked and I believe (w/o proof) that for (a) you need exactly that: A thorough understanding of NonNull-types. For the same reason throwing an NPE at conversion time is no option.  
I think (c) is nod good solution either, since implicits and the underlying wrapper types are not an every-day concept. I for my case don not want to know about them, they just shall happen. But nulls are an every day concept (as long there is Java ;-) and (b) is probably the best approximation to keep nulls (which remain in the programmers duty) separate from implicits (which shall remain just a magic the programmer need not know about)
Could somebody file a ticket? 
Burkhard

PS.: (c) is only an option if (b) severely breaks some performance expectations. This out of my scope.

2010/2/19 Matthew Pocock <turingatemyhamster@googlemail.com>

I agree - this is the obvious thing to do in many/most cases, but consider java.util.Map<String[]>.get() (where we want option, not null) or File.listFiles() (where we want an Either explaining why the files could not be listed).

Matthew

The implicit conversion from Array to Seq returns an exception if passed "null". The solution is easy: if null is received, return null.

That would seriously break semantic compatibility with Java and would slow down every operation with a compiler generated null check.

Otherwise it's a fine idea.

Or did you mean just the compiler generated implicit conversions?

On Fri, Feb 19, 2010 at 11:58 AM, Burkhard Ludwig <ludwig.burkhard@googlemail.com> wrote:

 (b) Or treat null analogously to a NaN in floating point operations: any operation on a NaN yields a NaN,        so any implicit conversion of a null yields a null. Simple concept, easy to understand

On Fri, Feb 19, 2010 at 2:58 PM, Burkhard Ludwig <ludwig.burkhard@googlemail.com> wrote:

I think the possibilities are now on the table:
 (a) Discuss each and every library implicit case by case what mapping is appropriate for a null (e.g. to empty whatever) Tough, and will take some time

I think this is the responsibility of a library designer: make your libraries handle exceptional cases in sensible ways.  While you're deciding whether to return an Option or Either or throw an exception, while you're deciding what to compose in with traits and what to provide from an ancestor class and so on, and while you're deciding what to implicitly convert, you should also consider--especially if you might interact with Java--what happens if a null comes in.

There are lots of things to consider when building libraries, and there's rarely time to perfect them completely, but I would consider these things bugs to be fixed on a case-by-case basis just like all the other bugs in libraries that get fixed on a case-by-case basis.

  --Rex

Burkhard Ludwig
Halskestr. 26
D 81379 München
Telefon: (089) 78 69 64
e-mail: ludwig.burkhard@googlemail.com


2010/2/19 James Iry <jamesiry@gmail.com>

That would seriously break semantic compatibility with Java and would slow down every operation with a compiler generated null check.

Otherwise it's a fine idea.

Or did you mean just the compiler generated implicit conversions?

Exactly, and the compiler is not involved in this change. It is a entirely a library issue. Approximately like this
   implicit def wrapSomething(x: Something) = if (something == null) null else new WrappedSomething(x)
This part of my point, the compiler should be left alone. And it could also be a case by case decision or a Something to Something decision so to say, in case there is a Something which has a natural interpretation for null. But I doubt that there are many cases of the latter. The string pimp-up-classes may be. But I would prefer fewer magic here. I suspect that a wrapper and its implicit are a bad spot to try a wholesale fix for vintage APIs. I suspect that any wholesale solution will require a solution of the NonNull problem which we all are waiting for.
But there is of course a performance concern: The whole implicit-magic is based on the assumption that the creation of the wrapper object is cheap or, even better, optimized away by some JVM magic. And if that magic gets spoiled by the additional f-then-else then it might be a bad idea.

On Fri, Feb 19, 2010 at 11:58 AM, Burkhard Ludwig <ludwig.burkhard@googlemail.com> wrote:

 (b) Or treat null analogously to a NaN in floating point operations: any operation on a NaN yields a NaN,        so any implicit conversion of a null yields a null. Simple concept, easy to understand

Having read
http://old.nabble.com/Scala-null-and-implicit-conversion-%28bug-%29-td27...
last week's thread , I think we can say that WrappedString and WrappedArray
share this same problem, that is not apparent for other compulsory implicit
casts.

Some of the talk on both threads around AOP, compiler hacks and annotations
is starting to sound like the http://www.fensende.com/~mcuddy/toaster.html
parable of the toaster to me.

AFAICT the only real argument made against passing on the null value in
these two implicit conversions (through normal written code) as per Burkhard
and Rex's posts, is performance. Against this, we can say that
performance-minded coders are more than welcome to perform the conversion
explicitly. Either way, their code will not break.

The status quo is handing out time-bombs from a hidden method call. The
other suggestions are over-engineered IMHO and will further violate the
principle of least surprise. Using Burkhard and Rex's solution, no code,
assumptions nor contracts will be broken.
-Ken

Burkhard Ludwig wrote:
>
> Burkhard Ludwig
> Halskestr. 26
> D 81379 München
> Telefon: (089) 78 69 64
> e-mail: ludwig.burkhard@googlemail.com
>
>
> 2010/2/19 James Iry
>
>> That would seriously break semantic compatibility with Java and would
>> slow
>> down every operation with a compiler generated null check.
>>
>> Otherwise it's a fine idea.
>>
>> Or did you mean just the compiler generated implicit conversions?
>
>
> Exactly, and the compiler is not involved in this change. It is a entirely
> a
> library issue. Approximately like this
>
> implicit def wrapSomething(x: Something) = if (something == null) null
> else new WrappedSomething(x)
>
> This part of my point, the compiler should be left alone. And it could
> also
> be a case by case decision or a Something to Something decision so to say,
> in case there is a Something which has a natural interpretation for null.
> But I doubt that there are many cases of the latter. The string
> pimp-up-classes may be. But I would prefer fewer magic here. I suspect
> that
> a wrapper and its implicit are a bad spot to try a wholesale fix for
> vintage
> APIs. I suspect that any wholesale solution will require a solution of the
> NonNull problem which we all are waiting for.
>
> But there is of course a performance concern: The whole implicit-magic is
> based on the assumption that the creation of the wrapper object is cheap
> or,
> even better, optimized away by some JVM magic. And if that magic gets
> spoiled by the additional f-then-else then it might be a bad idea.
>
>
>> On Fri, Feb 19, 2010 at 11:58 AM, Burkhard Ludwig <
>> ludwig.burkhard@googlemail.com> wrote:
>>
>>>
>>> (b) Or treat null analogously to a NaN in floating point operations:
>>> any
>>> operation on a NaN yields a NaN,
>>> so any implicit conversion of a null yields a null. Simple
>>> concept,
>>> easy to understand
>>>
>>>
>>
>
>

Raised ticket http://lampsvn.epfl.ch/trac/scala/ticket/3086 #3086 .