Scala Programming Language
  1. Scala Programming Language
  2. SI-4789

Broken class Signature in GenTraversableFactory

    Details

      Description

      Commit r25230 reveals some problem in scalac that results in broken class signature emitted for GenTraversableFactory. As for now, scala library from trunk is completely unusable for Eclipse users.

      In order to see the problem it's enough to create a following file in Eclipse:

      import scala.collection.immutable.List;
      import scala.collection.immutable.List$;
      
      public class Test {
        void foo() {
          Object x = List$.MODULE$;
        }
      }
      

      Eclipse will complain about the first import with following error message:
      Multiple markers at this line

      • The class file GenTraversableFactory<CC> contains a signature '()Lscala/collection/generic/GenTraversableFactory<TCC;>.ReusableCBF;' ill-formed at
        position 56
      • The class file GenTraversableFactory<CC> contains a signature '()Lscala/collection/generic/GenTraversableFactory<TCC;>.ReusableCBF;' ill-formed at
        position 56

      A bit more involved example exposes this issue with javac:

      mac-grek:src grek$ cat scala/collection/Test.java
      package scala.collection;
      
      import scala.collection.generic.GenTraversableFactory;
      import scala.collection.immutable.List;
      
      public class Test {
      
        public static void main(String[] args) {
          GenTraversableFactory<List<Object>> x = null;
          x.ReusableCBF();
        }
      
      }
      
      mac-grek:src grek$ javac -classpath ~/tmp/badsigs/scala-library-2.10.0-20110711.015803-87.jar scala/collection/Test.java
      scala/collection/Test.java:10: cannot access scala.collection.generic.GenTraversableFactory$ReusableCBF
      class file for scala.collection.generic.GenTraversableFactory$ReusableCBF not found
          x.ReusableCBF();
                       ^
      1 error
      

      so javac doesn't crash but fails to find a method that is defined in class file:

      mac-grek:generic grek$ javap GenTraversableFactory | grep Reusable
          public volatile scala.collection.generic.GenTraversableFactory$ReusableCBF$ ReusableCBF$module;
          public final scala.collection.generic.GenTraversableFactory$ReusableCBF$ ReusableCBF();
      

      Iulian proposed following work-around:

      mac-grek:scalagwt-scala grek$ git diff
      diff --git a/src/library/scala/collection/generic/GenTraversableFactory.scala b/src/library/scala/collection/generic/GenTraversableFactory.scala
      index 4695f7b..c5c9525 100644
      --- a/src/library/scala/collection/generic/GenTraversableFactory.scala
      +++ b/src/library/scala/collection/generic/GenTraversableFactory.scala
      @@ -38,9 +38,11 @@ abstract class GenTraversableFactory[CC[X] <: GenTraversable[X] with GenericTrav
         
         // A default implementation of GenericCanBuildFrom which can be cast
         // to whatever is desired.
      -  private[collection] object ReusableCBF extends GenericCanBuildFrom[Nothing] {
      +  private[collection] class reusableCBF extends GenericCanBuildFrom[Nothing] {
           override def apply() = newBuilder[Nothing]
         }
      +  
      +  lazy val ReusableCBF = new reusableCBF
       
         /** A generic implementation of the `CanBuildFrom` trait, which forwards
          *  all calls to `apply(from)` to the `genericBuilder` method of
      

      I can confirm that it works.

        Activity

        Hide
        Grzegorz Kossakowski added a comment -

        Paul, I'm currently trying to setup jenkis job for that, here:

        https://scala-webapps.epfl.ch/jenkins/view/Misc/job/scala-signatures/

        Will post update specific to this issue and will keep general discussion on internals mailing list.

        Show
        Grzegorz Kossakowski added a comment - Paul, I'm currently trying to setup jenkis job for that, here: https://scala-webapps.epfl.ch/jenkins/view/Misc/job/scala-signatures/ Will post update specific to this issue and will keep general discussion on internals mailing list.
        Hide
        Paul Phillips added a comment -

        I find that "badsigs" still reports the same number of errors, seems no happier, but now I have less idea what the problem is. I'm pretty sure that fix is right regardless of whether we have yet made ecj happy.

        Show
        Paul Phillips added a comment - I find that "badsigs" still reports the same number of errors, seems no happier, but now I have less idea what the problem is. I'm pretty sure that fix is right regardless of whether we have yet made ecj happy.
        Hide
        Simon Ochsenreither added a comment -

        Maybe it is a mistake on ecj's side?

        Show
        Simon Ochsenreither added a comment - Maybe it is a mistake on ecj's side?
        Hide
        Paul Phillips added a comment -

        I am open to this possibility, and it would certainly be convenient, but for now evidence and intuition both suggest it is us.

        Show
        Paul Phillips added a comment - I am open to this possibility, and it would certainly be convenient, but for now evidence and intuition both suggest it is us.
        Hide
        Commit Message Bot added a comment -

        (grek in r25639) Fix various InnerClasses bugs.

        This commit fixes two major problems:

        1. InnerClasses table missed entries
        that would close the chain between
        nested and top-level class.

        2. In some situations, classes
        corresponding to objects would be
        not be reported in the InnerClasses
        table.

        For details it's the best to check SI-4819, SI-4820 and
        SI-4983.

        First problem mentioned above was straightforward to
        fix so I won't be going into details.

        The second one deserves more attention. From now, classes
        corresponding to objects are properly reported as inner
        classes. Also, members (classes, objects) of objects are
        reported as inner classes of classes corresponding to
        objects.

        There's one caveat though: top level objects get two
        classes (regular and mirror). Members of top-level
        objects are declared as inner classes of mirror class
        and not regular one. The reason for that is to allow
        importing them from Java. For example:

        object A

        { class B }

        will be compiled into following classes: A, A$, A$B.
        If we declared A$B as inner class of A$ (regular class
        for objects) then it would be impossible to import
        B using "import A.B" or "import A$.B" constructs. The
        reason for that is that Java compiler seems to blindly
        put dollars instead of looking at InnerClasses attribute.

        Since non-top-level objects don't have a mirror class
        it's impossible to use the same solution. Thus, in case
        like this:

        object A { object B

        { class C }

        }

        it's impossible to import C from Java. That's the tradeoff
        for fixing other (more serious) problems. It's never been
        possible to do that in a clean way so we are not making
        situation worse.

        As a nice consequence of this change, we get better way to
        refer to inner members of top-level objects. It's been
        reflected in one of test-cases that is updated by this
        change.

        Fixes SI-4789 SI-4819 SI-4820 SI-4983 and possibly some
        other tickets related to reflection.

        Review by extempore, dragos.

        Show
        Commit Message Bot added a comment - (grek in r25639 ) Fix various InnerClasses bugs. This commit fixes two major problems: 1. InnerClasses table missed entries that would close the chain between nested and top-level class. 2. In some situations, classes corresponding to objects would be not be reported in the InnerClasses table. For details it's the best to check SI-4819 , SI-4820 and SI-4983 . First problem mentioned above was straightforward to fix so I won't be going into details. The second one deserves more attention. From now, classes corresponding to objects are properly reported as inner classes. Also, members (classes, objects) of objects are reported as inner classes of classes corresponding to objects. There's one caveat though: top level objects get two classes (regular and mirror). Members of top-level objects are declared as inner classes of mirror class and not regular one. The reason for that is to allow importing them from Java. For example: object A { class B } will be compiled into following classes: A, A$, A$B. If we declared A$B as inner class of A$ (regular class for objects) then it would be impossible to import B using "import A.B" or "import A$.B" constructs. The reason for that is that Java compiler seems to blindly put dollars instead of looking at InnerClasses attribute. Since non-top-level objects don't have a mirror class it's impossible to use the same solution. Thus, in case like this: object A { object B { class C } } it's impossible to import C from Java. That's the tradeoff for fixing other (more serious) problems. It's never been possible to do that in a clean way so we are not making situation worse. As a nice consequence of this change, we get better way to refer to inner members of top-level objects. It's been reflected in one of test-cases that is updated by this change. Fixes SI-4789 SI-4819 SI-4820 SI-4983 and possibly some other tickets related to reflection. Review by extempore, dragos.

          People

          • Assignee:
            Hubert Plociniczak
            Reporter:
            Grzegorz Kossakowski
          • Votes:
            0 Vote for this issue
            Watchers:
            10 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development