singleton manifest, serialization

On Mon, Nov 29, 2010 at 11:24 PM, Paul Phillips <paulp@improving.org> wrote:

Is there anyone who knows a little something about serialization who can
explain what's happening here and what should be happening? I had to
disable a couple lines from this test:

https://lampsvn.epfl.ch/trac/scala/browser/scala/trunk/test/files/jvm/manifests.scala?rev=23622

because after the change, object Test and object List caused "not
serializable" exceptions.  Seeing as the objects didn't implement the
Serializable interface before or after the change, I'm not really sure
why it worked before or whether it's really supposed to.  

object List is not serializable, but :: and Nil are (like all case classes).
 

I looked at
the bytecode and post-change there is a reference to an additional
manifest: but all the manifests are marked serializable, so I don't know
why that would be breaking it.

While I agree with the change, I think a bit more context would be helpful. Martin closed the original ticket twice with a 'wont fix' and an invitation to a SIP, so maybe he could be at least a reviewer on your commit. Or, in the more likely situation he agreed to your fix through other means of communication, it would be nice to have the reasoning as documentation, either in the commit message or in an email.
iulian 

--
Paul Phillips      | Giving every man a vote has no more made men wise
Caged Spirit       | and free than Christianity has made them good.
Empiricist         |     -- H. L. Mencken
i pull his palp!   |----------* http://www.improving.org/paulp/ *----------

--
« Je déteste la montagne, ça cache le paysage »
Alphonse Allais

On Tue, Nov 30, 2010 at 10:36 AM, iulian dragos <jaguarul@gmail.com> wrote:

On Mon, Nov 29, 2010 at 11:24 PM, Paul Phillips <paulp@improving.org> wrote:

Is there anyone who knows a little something about serialization who can
explain what's happening here and what should be happening? I had to
disable a couple lines from this test:

https://lampsvn.epfl.ch/trac/scala/browser/scala/trunk/test/files/jvm/manifests.scala?rev=23622

because after the change, object Test and object List caused "not
serializable" exceptions.  Seeing as the objects didn't implement the
Serializable interface before or after the change, I'm not really sure
why it worked before or whether it's really supposed to.  

object List is not serializable, but :: and Nil are (like all case classes).

Actually, I don't know about case classes, but List is for sure serializable..

 

I looked at
the bytecode and post-change there is a reference to an additional
manifest: but all the manifests are marked serializable, so I don't know
why that would be breaking it.

While I agree with the change, I think a bit more context would be helpful. Martin closed the original ticket twice with a 'wont fix' and an invitation to a SIP, so maybe he could be at least a reviewer on your commit. Or, in the more likely situation he agreed to your fix through other means of communication, it would be nice to have the reasoning as documentation, either in the commit message or in an email.
iulian 

--
Paul Phillips      | Giving every man a vote has no more made men wise
Caged Spirit       | and free than Christianity has made them good.
Empiricist         |     -- H. L. Mencken
i pull his palp!   |----------* http://www.improving.org/paulp/ *----------

--
« Je déteste la montagne, ça cache le paysage »
Alphonse Allais

--
« Je déteste la montagne, ça cache le paysage »
Alphonse Allais

On Tue, Nov 30, 2010 at 13:38, iulian dragos <jaguarul@gmail.com> wrote:

On Tue, Nov 30, 2010 at 10:36 AM, iulian dragos <jaguarul@gmail.com> wrote:

On Mon, Nov 29, 2010 at 11:24 PM, Paul Phillips <paulp@improving.org> wrote:

Is there anyone who knows a little something about serialization who can
explain what's happening here and what should be happening? I had to
disable a couple lines from this test:

https://lampsvn.epfl.ch/trac/scala/browser/scala/trunk/test/files/jvm/manifests.scala?rev=23622

because after the change, object Test and object List caused "not
serializable" exceptions.  Seeing as the objects didn't implement the
Serializable interface before or after the change, I'm not really sure
why it worked before or whether it's really supposed to.  

object List is not serializable, but :: and Nil are (like all case classes).

Actually, I don't know about case classes, but List is for sure serializable..

It's true, all CCss are serializable. 

 

I looked at
the bytecode and post-change there is a reference to an additional
manifest: but all the manifests are marked serializable, so I don't know
why that would be breaking it.

While I agree with the change, I think a bit more context would be helpful. Martin closed the original ticket twice with a 'wont fix' and an invitation to a SIP, so maybe he could be at least a reviewer on your commit. Or, in the more likely situation he agreed to your fix through other means of communication, it would be nice to have the reasoning as documentation, either in the commit message or in an email.
iulian 

--
Paul Phillips      | Giving every man a vote has no more made men wise
Caged Spirit       | and free than Christianity has made them good.
Empiricist         |     -- H. L. Mencken
i pull his palp!   |----------* http://www.improving.org/paulp/ *----------

--
« Je déteste la montagne, ça cache le paysage »
Alphonse Allais

--
« Je déteste la montagne, ça cache le paysage »
Alphonse Allais

On Tue, Nov 30, 2010 at 10:36:36AM +0100, iulian dragos wrote:
> object List is not serializable, but :: and Nil are (like all case
> classes).

And then in followup:

> Actually, I don't know about case classes, but List is for sure
> serializable..

By which you mean instances of List, right? Everything in that test
serialized except for singletons "object List" and "object Test", so if
those are not supposed to be serializable then it's all clear (but I
think there is likely still something wrong with the singleton type
manifest wrt serialization.)

> Or, in the more likely situation he agreed to your fix through other
> means of communication, it would be nice to have the reasoning as
> documentation, either in the commit message or in an email.

Yeah, that's a pretty reasonable observation. All I can say for sure is
that when I showed him the change, explained why it was desirable, and
demonstrated that it didn't break anything, he agreed. I believe he
misunderstood what was being pursued in the original ticket, but only
martin can say for sure.