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

Broken signature for inner case object surrounded by a parametrized class

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: None
    • Component/s: Misc Compiler
    • Labels:

      Description

      Consider following code:

      
      package test
      
      abstract class Test[A] {
        
        case object Y
        
      }
      

      Before r25326, badsigs.sh would report following errors:

      
      mac-grek:badsigs grek$ rm -rf classes/* && scalac -d classes/ Test.scala && ~/scala/badsigs/badsigs.sh `pwd`/classes
      Cleaning up /Users/grek/scala/badsigs/badsigs_working_dir
      Checking /Users/grek/tmp/badsigs/classes:
      Running Main app (will generate Java files and run ecj)
      ----------
      1. ERROR in /Users/grek/scala/badsigs/badsigs_working_dir/src/C0.java (at line 0)
      	import test.Test$Y$;
      	^
      Inconsistent classfile encountered: The undefined type parameter A is referenced from within Test$Y$
      ----------
      1 problem (1 error)
      
      ----------
      1. ERROR in /Users/grek/scala/badsigs/badsigs_working_dir/src/C1.java (at line 0)
      	import test.Test;
      	^
      The class file Test<A> contains a signature '()Ltest/Test<TA;>.Y;' ill-formed at position 18
      ----------
      1 problem (1 error)
      
      Found 2 errors
      

      By updating compiler to r25326, we get a bit different signatures but still broken:

      
      mac-grek:badsigs grek$ rm -rf classes/* && scalac -d classes/ Test.scala && ~/scala/badsigs/badsigs.sh `pwd`/classes
      Cleaning up /Users/grek/scala/badsigs/badsigs_working_dir
      Checking /Users/grek/tmp/badsigs/classes:
      Running Main app (will generate Java files and run ecj)
      ----------
      1. ERROR in /Users/grek/scala/badsigs/badsigs_working_dir/src/C0.java (at line 0)
      	import test.Test$Y$;
      	^
      Inconsistent classfile encountered: The undefined type parameter A is referenced from within Test$Y$
      ----------
      1 problem (1 error)
      
      ----------
      1. ERROR in /Users/grek/scala/badsigs/badsigs_working_dir/src/C1.java (at line 0)
      	import test.Test;
      	^
      The class file Test<A> contains a signature '()Ltest/Test<TA;>.Y$;' ill-formed at position 18
      ----------
      1 problem (1 error)
      
      Found 2 errors
      

        Issue Links

          Activity

          Hide
          Grzegorz Kossakowski added a comment -

          More interesting findings here. It turns out that it's broken for object (not case object) too. Thus the minimal test-case would be:

          package test
          abstract class Test[A] {
            object Y  
          }
          

          You'll get following errors:

          
          mac-grek:badsigs grek$ rm -rf classes/* && scalac -d classes/ Test.scala && ~/scala/badsigs/badsigs.sh `pwd`/classes
          Cleaning up /Users/grek/scala/badsigs/badsigs_working_dir
          Checking /Users/grek/tmp/badsigs/classes:
          Running Main app (will generate Java files and run ecj)
          ----------
          1. ERROR in /Users/grek/scala/badsigs/badsigs_working_dir/src/C0.java (at line 0)
          	import test.Test$Y$;
          	^
          Inconsistent classfile encountered: The undefined type parameter A is referenced from within Test$Y$
          ----------
          1 problem (1 error)
          
          ----------
          1. ERROR in /Users/grek/scala/badsigs/badsigs_working_dir/src/C1.java (at line 0)
          	import test.Test;
          	^
          The class file Test<A> contains a signature '()Ltest/Test<TA;>.Y;' ill-formed at position 18
          ----------
          1 problem (1 error)
          
          Found 2 errors
          

          However, if I use lazy val and inner class:

          package test
          abstract class Test[A] {  
            class Y
            lazy val Y = new Y
          }
          

          badsigs.sh doesn't report any problems. It turns out that signatures of accessor method Y are in both cases the same (and correct). It's InnerClasses attribute that's broken in first case (object Y is not reported as an inner class).

          Diff of javap for both cases proves the point:

          mac-grek:badsigs grek$ diff object_sig lazyval_sig 
          [...]
          >   InnerClass: 
          >    public #3= #17 of #13; //Y=class test/Test$Y of class test/Test
          [...]
          
          Show
          Grzegorz Kossakowski added a comment - More interesting findings here. It turns out that it's broken for object (not case object) too. Thus the minimal test-case would be: package test abstract class Test[A] { object Y } You'll get following errors: mac-grek:badsigs grek$ rm -rf classes/* && scalac -d classes/ Test.scala && ~/scala/badsigs/badsigs.sh `pwd`/classes Cleaning up /Users/grek/scala/badsigs/badsigs_working_dir Checking /Users/grek/tmp/badsigs/classes: Running Main app (will generate Java files and run ecj) ---------- 1. ERROR in /Users/grek/scala/badsigs/badsigs_working_dir/src/C0.java (at line 0) import test.Test$Y$; ^ Inconsistent classfile encountered: The undefined type parameter A is referenced from within Test$Y$ ---------- 1 problem (1 error) ---------- 1. ERROR in /Users/grek/scala/badsigs/badsigs_working_dir/src/C1.java (at line 0) import test.Test; ^ The class file Test<A> contains a signature '()Ltest/Test<TA;>.Y;' ill-formed at position 18 ---------- 1 problem (1 error) Found 2 errors However, if I use lazy val and inner class: package test abstract class Test[A] { class Y lazy val Y = new Y } badsigs.sh doesn't report any problems. It turns out that signatures of accessor method Y are in both cases the same (and correct). It's InnerClasses attribute that's broken in first case (object Y is not reported as an inner class). Diff of javap for both cases proves the point: mac-grek:badsigs grek$ diff object_sig lazyval_sig [...] > InnerClass: > public #3= #17 of #13; //Y=class test/Test$Y of class test/Test [...]
          Hide
          Paul Phillips added a comment -

          Trivia for 4820 fans: this ticket number is also the number of iulian's thesis.

          http://library.epfl.ch/en/theses/?nr=4820

          It's a sign from the heavens!

          Show
          Paul Phillips added a comment - Trivia for 4820 fans: this ticket number is also the number of iulian's thesis. http://library.epfl.ch/en/theses/?nr=4820 It's a sign from the heavens!
          Hide
          Grzegorz Kossakowski added a comment -

          Do we have any progress on it?

          Show
          Grzegorz Kossakowski added a comment - Do we have any progress on it?
          Hide
          Paul Phillips added a comment -

          Yeah, I burned the entirety of wednesday on it. It is the name mangling chicken coming home to roost. I knew he was coming someday. I should have the situation summarized by our 8AM meeting.

          Show
          Paul Phillips added a comment - Yeah, I burned the entirety of wednesday on it. It is the name mangling chicken coming home to roost. I knew he was coming someday. I should have the situation summarized by our 8AM meeting.
          Hide
          Simon Ochsenreither added a comment -

          > "object Y is not reported as an inner class"

          This sounds pretty much like the problem previously uncovered with java reflection and objects.

          Show
          Simon Ochsenreither added a comment - > "object Y is not reported as an inner class" This sounds pretty much like the problem previously uncovered with java reflection and objects.
          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.
          Hide
          Simon Ochsenreither added a comment -

          Judging from the last commit message it seems like this one should be closed, but it isn't.

          I ran the badsigs tool and it doesn't report any errors:

          Root is /home/soc/Entwicklung/badsigs
          Working dir is /tmp/badsigs.VG0Ed
          Checking /home/soc/Entwicklung/badsigs/./classes/:
          Running Main app (will generate Java files and run ecj)
          

          I guess this can be closed?

          Show
          Simon Ochsenreither added a comment - Judging from the last commit message it seems like this one should be closed, but it isn't. I ran the badsigs tool and it doesn't report any errors: Root is /home/soc/Entwicklung/badsigs Working dir is /tmp/badsigs.VG0Ed Checking /home/soc/Entwicklung/badsigs/./classes/: Running Main app (will generate Java files and run ecj) I guess this can be closed?
          Hide
          Heather Miller added a comment -

          Judging by the conversation, the resulting commit, and the fact that all seems to build and run:

          Heathers-iMac:scala-heather hmiller$ ../badsigs/badsigs.sh ../scala-heather/classes
          Root is /Users/hmiller/Dropbox/git-shared/badsigs
          Working dir is /tmp/badsigs.vsQMC
          Badsigs JAR is /Users/hmiller/Dropbox/git-shared/badsigs/target/badsigs-assembly-0.1-SNAPSHOT.jar
          Checking /Users/hmiller/Dropbox/git-shared/badsigs/../scala-heather/classes:
          Running Main app (will generate Java files and run ecj)
          Heathers-iMac:scala-heather hmiller$ 
          

          All seems resolved, so I'll be closing this.

          Show
          Heather Miller added a comment - Judging by the conversation, the resulting commit, and the fact that all seems to build and run: Heathers-iMac:scala-heather hmiller$ ../badsigs/badsigs.sh ../scala-heather/classes Root is /Users/hmiller/Dropbox/git-shared/badsigs Working dir is /tmp/badsigs.vsQMC Badsigs JAR is /Users/hmiller/Dropbox/git-shared/badsigs/target/badsigs-assembly-0.1-SNAPSHOT.jar Checking /Users/hmiller/Dropbox/git-shared/badsigs/../scala-heather/classes: Running Main app (will generate Java files and run ecj) Heathers-iMac:scala-heather hmiller$ All seems resolved, so I'll be closing this.
          Hide
          Grzegorz Kossakowski added a comment -

          Yes, it's fixed. I forgot to close it. Thanks Heather!

          Show
          Grzegorz Kossakowski added a comment - Yes, it's fixed. I forgot to close it. Thanks Heather!

            People

            • Assignee:
              Paul Phillips
              Reporter:
              Grzegorz Kossakowski
            • Votes:
              0 Vote for this issue
              Watchers:
              5 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development