search

seanjensengrey / kiama

Automatically exported from code.google.com/p/kiama
GNU Lesser General Public License v3.0
0 stars 0 forks source link

deepclone fails on Seq[] for Kiama 2? #70

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago 
The attached file - self-contained, except for the package declaration - throws 
an ArrayIndexOutOfBoundsException, as in:

Exception in thread "main" 
com.google.common.util.concurrent.UncheckedExecutionException: 
java.lang.ArrayIndexOutOfBoundsException: 0
    at com.google.common.cache.LocalCache$Segment.get(LocalCache.java:2201)
    at com.google.common.cache.LocalCache.get(LocalCache.java:3934)
    at com.google.common.cache.LocalCache.getOrLoad(LocalCache.java:3938)
    at com.google.common.cache.LocalCache$LocalLoadingCache.get(LocalCache.java:4821)
    at org.kiama.rewriting.RewriterCore$Duplicator$.apply(RewriterCore.scala:431)
    at org.kiama.rewriting.RewriterCore$class.copy(RewriterCore.scala:455)
    at org.kiama.rewriting.Rewriter$.copy(Rewriter.scala:705)

I've isolated the problem to deepcloning nodes that have Seq[...]. All other 
nodes seem to clone fine.

I'm using Kiama 2.0.0-SNAPSHOT.

Any idea whether the issue is on my side - perhaps misusing/misunderstanding 
Kiama's changes for 2.x?

Original issue reported on code.google.com by msbra...@gmail.com on 2 Feb 2015 at 11:46

Attachments:

GoogleCodeExporter commented 9 years ago 
Thanks for reporting. Yes, this is a bug in the new Tree support in 2.0.0. deep 
cloning uses a Tree capability to detect leaves but this capability is not 
correctly set up for sequences (and maps too) so it claims that these are 
leaves. The crash occurs when the rewriter tries to copy the (not really) leaf 
node.

 I think the fix should be pretty easy. I should have time to try it and test in the next few days.

Original comment by inkytonik on 5 Feb 2015 at 11:16

GoogleCodeExporter commented 9 years ago

Original comment by inkytonik on 5 Feb 2015 at 11:17

GoogleCodeExporter commented 9 years ago 
Actually the cause of this bug turns out not to be the Tree support as
indicated above.

The real cause is a problem in the way that some nodes are duplicated (which
is used by the generic rewriting and the deepcloner).

The specific case of the bug report is where a Nil value is duplicated. It
crashes because the duplicator goes looking for a constructor to use but there
is no public one.

In fact, this circumstance suggests that duplicating the Nil is not the right
thing to do anyway, since that value is intended to be a singleton and it
doesn't really make sense to have more than one instance of Nil.

Extending this to user-land, we can also argue that case objects should not be
duplicated or cloned either since they are supposed to be singletons too.

All of this means that a fix will take loner to arrive at since these cases
have to be supported properly and it's non-trivial to detect these situations
dynamically.

Original comment by inkytonik on 11 Feb 2015 at 2:32

GoogleCodeExporter commented 9 years ago 
As part of a recently pushed batch of fixes, revision d49539e22a6a fixes this 
problem.The crash due to singletons being cloned (or duplicated in general) has 
been removed. In these cases the same instance is returned, applying the logic 
that it doesn't make sense to clone a singleton.

A new snapshot with these changes has been published on the Sonatype OSS repo. 
It would be good if you could try it and let us know if the problem is fixed 
from your perspective also. (I have run your test case here and it is fine now.)

Original comment by inkytonik on 13 Feb 2015 at 2:13

GoogleCodeExporter commented 9 years ago 
The new snapshop looks good to me. We had 3 scenarios where the problem 
occurred and all now work as expected.

Original comment by msbra...@gmail.com on 13 Feb 2015 at 12:46

GoogleCodeExporter commented 9 years ago 
Great. Thanks for the feedback.

Original comment by inkytonik on 13 Feb 2015 at 10:49