Mismatched symbols in scala.collection.parallel.package.CompositeThrowable

Yeah, case class defined in package object. Only one in trunk.

On 7/27/11 10:28 AM, Geoff Reedy wrote:
> I'm seeing a rather peculiar mismatch of symbols in the ICode generated
> for the equals method in CompositeThrowable.

CompositeThrowable, eh? Interesting...

https://github.com/scala/scala/commit/1f6e7ec462

On Wed, Jul 27, 2011 at 10:33:50AM -0700, Paul Phillips said
> Yeah, case class defined in package object. Only one in trunk.

That's what I was thinking too, but I can't reproduce it like that. The
code below which is exactly the definition of CompositeThrowable
compiles fine.

package scala

package object foo {
final case class CompositeThrowable(val throwables: Set[Throwable])
extends Throwable("Multiple exceptions thrown during a parallel
computation: " + throwables.map(
t => t + "\n" +
t.getStackTrace.take(10).++("...").mkString("\n")
).mkString("\n\n"))
}

Hi,
> final case class CompositeThrowable(val throwables: Set[Throwable])
> extends Throwable("Multiple exceptions thrown during a parallel
> computation: " + throwables.map( t => t + "\n" +
> t.getStackTrace.take(10).++("...").mkString("\n") ).mkString("\n\n"))

btw, is there a reason not using the mechanism provided by Throwable?
I could imagine that would be a lot faster.

Thanks and bye,

Simon

On Wed, Jul 27, 2011 at 07:59:31PM +0200, Simon Ochsenreither said
> Hi,
>> final case class CompositeThrowable(val throwables: Set[Throwable])
>> extends Throwable("Multiple exceptions thrown during a parallel
>> computation: " + throwables.map( t => t + "\n" +
>> t.getStackTrace.take(10).++("...").mkString("\n") ).mkString("\n\n"))
>
> btw, is there a reason not using the mechanism provided by Throwable?
> I could imagine that would be a lot faster.

Not sure what mechanism you're referring to but my best guess is the
cause support. The trouble is Throwable only allows one cause but
CompositeThrowable is for packaging up all the exceptions that occurred
during some parallel operation on a collection none of which necessarily
caused the others.

Hi, Geoff

> Not sure what mechanism you're referring to but my best guess is the
> cause support. The trouble is Throwable only allows one cause but
> CompositeThrowable is for packaging up all the exceptions that occurred
> during some parallel operation on a collection none of which necessarily
> caused the others.
I thought about addSuppressed [1] ...

Thanks and bye!

Simon

[1]
http://download.java.net/jdk7/docs/api/java/lang/Throwable.html#addSuppr...

Am 7/27/2011 8:28 PM, schrieb Simon Ochsenreither:
> Hi, Geoff
>
>> Not sure what mechanism you're referring to but my best guess is the
>> cause support. The trouble is Throwable only allows one cause but
>> CompositeThrowable is for packaging up all the exceptions that occurred
>> during some parallel operation on a collection none of which necessarily
>> caused the others.
> I thought about addSuppressed [1] ...
>
:-) Great to see that Java is not quite stone dead yet. It might be a
while before support for 1.6 is dropped, though ...

Regards,

Roland

> :-) Great to see that Java is not quite stone dead yet. It might be a
> while before support for 1.6 is dropped, though ...

Sure, but I wonder if it would be possible to maybe add the
functionality in a slightly more "compatible way" e. g. more or less
"backporting" the appropriate fields and as soon as we target Java 7,
we might just add an @override to it.

Thanks and bye!

Simon

I've been plagued by this recently: 
[error] {file:/home/jsuereth/projects/scala/scalac-plugins/scala/}locker-library/*:compile: java.lang.AssertionError: assertion failed: List(object package$CompositeThrowable, object package$CompositeThrowable)

Funny enough, things build fine in Ant and I still can't figure out which incantation makes it do so....
On Wed, Jul 27, 2011 at 1:31 PM, Paul Phillips <paulp [at] improving [dot] org> wrote:

On 7/27/11 10:28 AM, Geoff Reedy wrote:

I'm seeing a rather peculiar mismatch of symbols in the ICode generated
for the equals method in CompositeThrowable.

CompositeThrowable, eh? Interesting...

https://github.com/scala/scala/commit/1f6e7ec462

On 7/28/11 10:16 AM, Josh Suereth wrote:
> I've been plagued by this recently:
>
> [error]
> {file:/home/jsuereth/projects/scala/scalac-plugins/scala/}locker-library/*:compile:
> java.lang.AssertionError: assertion failed: List(object
> package$CompositeThrowable, object package$CompositeThrowable)

Yeah, that's the assertion being suppressed in the linked commit.

On 7/27/11 10:54 AM, Geoff Reedy wrote:
>> Yeah, case class defined in package object. Only one in trunk.
>
> That's what I was thinking too, but I can't reproduce it like that. The
> code below which is exactly the definition of CompositeThrowable
> compiles fine.

Oh yeah, now we get to another major open wtf with package objects. Try
compiling it a second time.

// first time
% scalac3 ./a.scala
// second time
% scalac3 ./a.scala
Suppressing failed assert: java.lang.AssertionError: assertion failed:
List(object package$CompositeThrowable, object package$CompositeThrowable)
// remove the classfiles, it's fine again
% rm -rf ./scala
% scalac ./a.scala
%

The coup de grace is that those classfiles under the current directory
will fail the compile *even if the current directory is not on the
classpath*.

I opened the ticket a little while back:

https://issues.scala-lang.org/browse/SI-4695

At the rate josh is going, I might even get to work on fixing this sort
of thing before I die.

On Thu, Jul 28, 2011 at 11:31:02AM -0700, Paul Phillips said
> On 7/27/11 10:54 AM, Geoff Reedy wrote:
>>> Yeah, case class defined in package object. Only one in trunk.
>>
>> That's what I was thinking too, but I can't reproduce it like that. The
>> code below which is exactly the definition of CompositeThrowable
>> compiles fine.
>
> Oh yeah, now we get to another major open wtf with package objects. Try
> compiling it a second time.

Yeah, with two compiles I can get the problem with just:

package object foo { case class X(z: Int) }

> At the rate josh is going, I might even get to work on fixing this sort
> of thing before I die.

I hope I can figure at least this one out. If we're lucky all the
package object anomalies will have the same root cause:

Here's where I'm at in tracking this down now. It definitely seems to
stem from the use of Ident(clazz.name.toTermName) in equalsClassMethod
of SyntheticMethods.scala. It seemed like a better idea to change it to
ID(clazz.companionModule) so it doesn't go through name -> symbol
resolution again. This again works fine the first compile. On the second
compile clazz.companionModule is NoSymbol! This at least gives a real
compiler error instead of a (suppressed) assertion failure. Sure enough,
on the second compile clazz.owner.decls doesn't contain the module for
the case class. That's all for now, though I will keep digging.

So there's this chunk of code in PackageLoader.doComplete

// if there's a $member object, enter its members as well.
val pkgModule = root.info.decl(nme.PACKAGEkw)
if (pkgModule.isModule && !pkgModule.rawInfo.isInstanceOf[SourcefileLoader]) {
printf("open %s rawInfo = %s\n", pkgModule, pkgModule.rawInfo.getClass.getName)//DEBUG
openPackageModule(pkgModule)()
}

which looks like it's intended to prevent this problem from occuring.
However pkgModule.rawInfo is not an instance of SourcefileLoader even if
there is a source file for the package object. It makes sense since the
package symbol is entered before the source file is even parsed. Anyone
here have an idea on how to make this test work right?

On 8/3/11 3:21 PM, Geoff Reedy wrote:
> which looks like it's intended to prevent this problem from occuring.
> However pkgModule.rawInfo is not an instance of SourcefileLoader even if
> there is a source file for the package object. It makes sense since the
> package symbol is entered before the source file is even parsed. Anyone
> here have an idea on how to make this test work right?

I have attempted to go one better. I hacked namers to lift class and module definitions out of package objects and into the package, and all these complications (and lots of other ones) vanish. The only thing which isn't working yet is classes nested in other classes which are themselves nested in package objects. Until I figure that out, I lifted those out by hand.

Try this branch.

https://github.com/paulp/scala-dev/tree/lift-classes-from-package-objects

Let me know if it helps (or doesn't.)

Among the good things which emerges from that approach is all these name changes: all those classes called package$Foo were unlikely to be correctness enhancers.

< ./scala/collection/immutable/package$RangeUtils$class.class
< ./scala/collection/immutable/package$RangeUtils.class
< ./scala/collection/parallel/immutable/package$Repetition$$anon$1.class
< ./scala/collection/parallel/immutable/package$Repetition$ParIterator$$anon$2.class
< ./scala/collection/parallel/immutable/package$Repetition$ParIterator$$anonfun$1.class
< ./scala/collection/parallel/immutable/package$Repetition$ParIterator$$anonfun$psplit$1.class
< ./scala/collection/parallel/immutable/package$Repetition$ParIterator$$anonfun$psplit$2$$anon$3.class
< ./scala/collection/parallel/immutable/package$Repetition$ParIterator$$anonfun$psplit$2.class
< ./scala/collection/parallel/immutable/package$Repetition$ParIterator$.class
< ./scala/collection/parallel/immutable/package$Repetition$ParIterator.class
< ./scala/collection/parallel/immutable/package$Repetition.class
< ./scala/collection/parallel/mutable/package$ExposedArrayBuffer.class
< ./scala/collection/parallel/mutable/package$ExposedArraySeq.class
< ./scala/collection/parallel/mutable/package$SizeMapUtils$class.class
< ./scala/collection/parallel/mutable/package$SizeMapUtils.class
< ./scala/collection/parallel/package$BucketCombiner.class
< ./scala/collection/parallel/package$BufferSplitter$$anonfun$debugInformation$1.class
< ./scala/collection/parallel/package$BufferSplitter.class
< ./scala/collection/parallel/package$CompositeThrowable$$anonfun$$init$$1.class
< ./scala/collection/parallel/package$CompositeThrowable$.class
< ./scala/collection/parallel/package$CompositeThrowable.class
< ./scala/collection/parallel/package$FactoryOps$Otherwise.class
< ./scala/collection/parallel/package$FactoryOps$class.class
< ./scala/collection/parallel/package$FactoryOps.class
< ./scala/collection/parallel/package$ThrowableOps.class
< ./scala/collection/parallel/package$TraversableOps$Otherwise.class
< ./scala/collection/parallel/package$TraversableOps$class.class
< ./scala/collection/parallel/package$TraversableOps.class

> ./scala/collection/immutable/RangeUtils$class.class
> ./scala/collection/immutable/RangeUtils.class
> ./scala/collection/parallel/BucketCombiner.class
> ./scala/collection/parallel/BufferSplitter$$anonfun$debugInformation$1.class
> ./scala/collection/parallel/BufferSplitter.class
> ./scala/collection/parallel/CompositeThrowable$$anonfun$$init$$1.class
> ./scala/collection/parallel/CompositeThrowable$.class
> ./scala/collection/parallel/CompositeThrowable.class
> ./scala/collection/parallel/FactoryOps$Otherwise.class
> ./scala/collection/parallel/FactoryOps$class.class
> ./scala/collection/parallel/FactoryOps.class
> ./scala/collection/parallel/ThrowableOps.class
> ./scala/collection/parallel/TraversableOps$Otherwise.class
> ./scala/collection/parallel/TraversableOps$class.class
> ./scala/collection/parallel/TraversableOps.class
> ./scala/collection/parallel/immutable/Repetition$$anon$1.class
> ./scala/collection/parallel/immutable/Repetition$ParIterator$$anon$2.class
> ./scala/collection/parallel/immutable/Repetition$ParIterator$$anonfun$1.class
> ./scala/collection/parallel/immutable/Repetition$ParIterator$$anonfun$psplit$1.class
> ./scala/collection/parallel/immutable/Repetition$ParIterator$$anonfun$psplit$2$$anon$3.class
> ./scala/collection/parallel/immutable/Repetition$ParIterator$$anonfun$psplit$2.class
> ./scala/collection/parallel/immutable/Repetition$ParIterator$.class
> ./scala/collection/parallel/immutable/Repetition$ParIterator.class
> ./scala/collection/parallel/immutable/Repetition.class
> ./scala/collection/parallel/mutable/ExposedArrayBuffer.class
> ./scala/collection/parallel/mutable/ExposedArraySeq.class
> ./scala/collection/parallel/mutable/SizeMapUtils$class.class
> ./scala/collection/parallel/mutable/SizeMapUtils.class

Somehow I think this breaks binary compatibility :(
Still, this is *awesome*.  I'd love to have this in 2.10!

On Thu, Aug 4, 2011 at 3:21 AM, Paul Phillips <paulp [at] improving [dot] org> wrote:

Among the good things which emerges from that approach is all these name changes: all those classes called package$Foo were unlikely to be correctness enhancers.


< ./scala/collection/immutable/package$RangeUtils$class.class
< ./scala/collection/immutable/package$RangeUtils.class
< ./scala/collection/parallel/immutable/package$Repetition$$anon$1.class
< ./scala/collection/parallel/immutable/package$Repetition$ParIterator$$anon$2.class
< ./scala/collection/parallel/immutable/package$Repetition$ParIterator$$anonfun$1.class
< ./scala/collection/parallel/immutable/package$Repetition$ParIterator$$anonfun$psplit$1.class
< ./scala/collection/parallel/immutable/package$Repetition$ParIterator$$anonfun$psplit$2$$anon$3.class
< ./scala/collection/parallel/immutable/package$Repetition$ParIterator$$anonfun$psplit$2.class
< ./scala/collection/parallel/immutable/package$Repetition$ParIterator$.class
< ./scala/collection/parallel/immutable/package$Repetition$ParIterator.class
< ./scala/collection/parallel/immutable/package$Repetition.class
< ./scala/collection/parallel/mutable/package$ExposedArrayBuffer.class
< ./scala/collection/parallel/mutable/package$ExposedArraySeq.class
< ./scala/collection/parallel/mutable/package$SizeMapUtils$class.class
< ./scala/collection/parallel/mutable/package$SizeMapUtils.class
< ./scala/collection/parallel/package$BucketCombiner.class
< ./scala/collection/parallel/package$BufferSplitter$$anonfun$debugInformation$1.class
< ./scala/collection/parallel/package$BufferSplitter.class
< ./scala/collection/parallel/package$CompositeThrowable$$anonfun$$init$$1.class
< ./scala/collection/parallel/package$CompositeThrowable$.class
< ./scala/collection/parallel/package$CompositeThrowable.class
< ./scala/collection/parallel/package$FactoryOps$Otherwise.class
< ./scala/collection/parallel/package$FactoryOps$class.class
< ./scala/collection/parallel/package$FactoryOps.class
< ./scala/collection/parallel/package$ThrowableOps.class
< ./scala/collection/parallel/package$TraversableOps$Otherwise.class
< ./scala/collection/parallel/package$TraversableOps$class.class
< ./scala/collection/parallel/package$TraversableOps.class


> ./scala/collection/immutable/RangeUtils$class.class
> ./scala/collection/immutable/RangeUtils.class
> ./scala/collection/parallel/BucketCombiner.class
> ./scala/collection/parallel/BufferSplitter$$anonfun$debugInformation$1.class
> ./scala/collection/parallel/BufferSplitter.class
> ./scala/collection/parallel/CompositeThrowable$$anonfun$$init$$1.class
> ./scala/collection/parallel/CompositeThrowable$.class
> ./scala/collection/parallel/CompositeThrowable.class
> ./scala/collection/parallel/FactoryOps$Otherwise.class
> ./scala/collection/parallel/FactoryOps$class.class
> ./scala/collection/parallel/FactoryOps.class
> ./scala/collection/parallel/ThrowableOps.class
> ./scala/collection/parallel/TraversableOps$Otherwise.class
> ./scala/collection/parallel/TraversableOps$class.class
> ./scala/collection/parallel/TraversableOps.class
> ./scala/collection/parallel/immutable/Repetition$$anon$1.class
> ./scala/collection/parallel/immutable/Repetition$ParIterator$$anon$2.class
> ./scala/collection/parallel/immutable/Repetition$ParIterator$$anonfun$1.class
> ./scala/collection/parallel/immutable/Repetition$ParIterator$$anonfun$psplit$1.class
> ./scala/collection/parallel/immutable/Repetition$ParIterator$$anonfun$psplit$2$$anon$3.class
> ./scala/collection/parallel/immutable/Repetition$ParIterator$$anonfun$psplit$2.class
> ./scala/collection/parallel/immutable/Repetition$ParIterator$.class
> ./scala/collection/parallel/immutable/Repetition$ParIterator.class
> ./scala/collection/parallel/immutable/Repetition.class
> ./scala/collection/parallel/mutable/ExposedArrayBuffer.class
> ./scala/collection/parallel/mutable/ExposedArraySeq.class
> ./scala/collection/parallel/mutable/SizeMapUtils$class.class
> ./scala/collection/parallel/mutable/SizeMapUtils.class

On Thu, Aug 04, 2011 at 12:18:35AM -0700, Paul Phillips said
> On 8/3/11 3:21 PM, Geoff Reedy wrote:
> I have attempted to go one better. I hacked namers to lift class and
> module definitions out of package objects and into the package, and
> all these complications (and lots of other ones) vanish. The only
> thing which isn't working yet is classes nested in other classes which
> are themselves nested in package objects. Until I figure that out, I
> lifted those out by hand.
>
> Try this branch.
>
> https://github.com/paulp/scala-dev/tree/lift-classes-from-package-objects
>
> Let me know if it helps (or doesn't.)

This is much better, thanks. With these patches I can assemble all of
the library without errors.