Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Broken signature for inner case class of parametrized outer class (reference to non-existing type parameter) #4819

Closed
scabug opened this issue Jul 20, 2011 · 4 comments
Assignees

Comments

@scabug
Copy link

@scabug scabug commented Jul 20, 2011

Came up with minimal test-case for this issue. My script badsigs.sh might turn out to be handy to test a possible fix for this:

mac-grek:badsigs grek$ cat Test.scala
package test

abstract class Test[A] {
  
  case class Y(x: Int)
  
}
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)

Found 1 errors
@scabug

This comment has been minimized.

Copy link
Author

@scabug scabug commented Jul 20, 2011

Imported From: https://issues.scala-lang.org/browse/SI-4819?orig=1
Reporter: @gkossakowski
Affected Versions: 2.9.1

@scabug

This comment has been minimized.

Copy link
Author

@scabug scabug commented Jul 20, 2011

@gkossakowski said:
I gave it another look and it seems like the root cause is the same as #4820. To keep story short, constructor of test.Test$Y$ refers to A parameter because it takes Test instance as it's argument <init>(Ltest/Test;)V.

Now, test.Test$Y$ is not declared as inner class in InnerClasses attribute so from ecj's point of view A is not in scope. I guess that fix for #4820 will fix this issue too.

@scabug

This comment has been minimized.

Copy link
Author

@scabug scabug commented Jul 20, 2011

@paulp said:
There are several bugs, I'm finding, but with your tests to draw the lines for me the resolutions are making themselves apparent. The major remaining unknown is whether everything will reconcile nicely once we throw in the inner class issues of yesteryear.

@scabug

This comment has been minimized.

Copy link
Author

@scabug scabug commented Sep 9, 2011

Commit Message Bot (anonymous) said:
(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 #4819, #4820 and
#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 #4789 #4819 #4820 #4983 and possibly some
other tickets related to reflection.

Review by extempore, dragos.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
2 participants
You can’t perform that action at this time.