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

subclassing a java annotation generates an invalid classfile #9400

Closed
scabug opened this issue Jul 15, 2015 · 7 comments · Fixed by scala/scala#6869
Closed

subclassing a java annotation generates an invalid classfile #9400

scabug opened this issue Jul 15, 2015 · 7 comments · Fixed by scala/scala#6869
Assignees
Labels
Milestone

Comments

@scabug
Copy link

scabug commented Jul 15, 2015

JavaAnnot.java:

public @interface JavaAnnot { }

Test.scala:

class C extends JavaAnnot {
  def annotationType: Class[_ <: java.lang.annotation.Annotation] = null
}

object Test extends App {
  new C
}

There is also a (mostly) spurious warning emitted:

➜  sandbox git:(t9393) ✗ javac JavaAnnot.java
➜  sandbox git:(t9393) ✗ scalac Test.scala
Test.scala:1: warning: Implementation restriction: subclassing Classfile does not
make your annotation visible at runtime.  If that is what
you want, you must write the annotation class in Java.
class C extends JavaAnnot {
      ^
one warning found
➜  sandbox git:(t9393) ✗ scala Test
java.lang.IncompatibleClassChangeError: class C has interface JavaAnnot as super class
	at java.lang.ClassLoader.defineClass1(Native Method)
@scabug
Copy link
Author

scabug commented Jul 15, 2015

Imported From: https://issues.scala-lang.org/browse/SI-9400?orig=1
Reporter: @lrytz
Affected Versions: 2.11.7
See #8778, #9393

@scabug
Copy link
Author

scabug commented Jul 15, 2015

@lrytz said:
the reason is that the classfile / java source parser make class symbols for java annotations look like classes, not interfaces (the INTERFACE / DEFERRED flags are deliberately not added).

@scabug
Copy link
Author

scabug commented Jul 15, 2015

@lrytz said:
fixing this problem (which I did locally) still produces an invalid classfile:

➜  sandbox git:(t9393) ✗ qs Test
java.lang.VerifyError: Bad <init> method call
Exception Details:
  Location:
    C.<init>()V @1: invokespecial
  Reason:
    Type 'JavaAnnot' is not assignable to 'C'
  Bytecode:
    0x0000000: 2ab7 0012 b1

	at Test$.<init>(Test.scala:6)
	at Test$.<clinit>(Test.scala)

@scabug
Copy link
Author

scabug commented Jul 15, 2015

@lrytz said (edited on Jul 15, 2015 7:26:40 PM UTC):
Yet another related issue: the classfile parser adds scala.annotation.Annotation as superclass for Java annotations, so the following compiles, but generates invalid bytecode:

object Test extends App {
  def foo(a: JavaAnnot): scala.annotation.Annotation = a
}
➜  sandbox git:(t9393) ✗ scalac Test.scala
➜  sandbox git:(t9393) ✗ scala Test
java.lang.VerifyError: Bad return type
Exception Details:
  Location:
    Test$.foo(LJavaAnnot;)Lscala/annotation/Annotation; @1: areturn
  Reason:
    Type 'JavaAnnot' (current frame, stack[0]) is not assignable to 'scala/annotation/Annotation' (from method signature)
  Current Frame:
    bci: @1
    flags: { }
    locals: { 'Test$', 'JavaAnnot' }
    stack: { 'JavaAnnot' }
  Bytecode:
    0x0000000: 2bb0

	at Test.main(Test.scala)
	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)

GenBCode catches the problem at bytecode generation time and crashes with an assertion error.

@scabug
Copy link
Author

scabug commented Jul 16, 2015

@lrytz said:
related to scala/scala#4566, which adds a workaround for a similar hack for ENUM classfiles

@scabug
Copy link
Author

scabug commented Feb 2, 2016

@lrytz said:
see also discussion on scala/scala#4638

@scabug
Copy link
Author

scabug commented Feb 2, 2016

@scabug scabug added the backend label Apr 7, 2017
@scabug scabug added this to the Backlog milestone Apr 7, 2017
hrhino added a commit to hrhino/scala that referenced this issue Jun 29, 2018
Previously, `JavaParsers` and `ClassfileParser` would "fix-up"
Java annotations to be non-abstract classes with `Annotation`
and `StaticAnnotation` as parents. This caused all manner of
strangeness and invalid classfiles (see linked tickets) and
was surprising, to say the least.

Now, the only special dispensation given to Java annotations
is that they get a public, no-args class constructor, because
that's what it takes to get `typedAnnotation` to be able to
look up the type. This means that `new Foo`, where `Foo` is
a Java annotation, still compiles (as it does right now,
actually). Part of the reason for this commit is to provoke
a discussion about how to fix that.

Fixes scala/bug#8778.
Fixes scala/bug#9400.
Fixes scala/bug#9644.
hrhino added a commit to hrhino/scala that referenced this issue Jul 1, 2018
Previously, `JavaParsers` and `ClassfileParser` would "fix-up"
Java annotations to be non-abstract classes with `Annotation`
and `StaticAnnotation` as parents. This caused all manner of
strangeness and invalid classfiles (see linked tickets) and
was surprising, to say the least.

Now, the only special dispensation given to Java annotations
is that they get a public, no-args class constructor, because
that's what it takes to get `typedAnnotation` to be able to
look up the type. This means that `new Foo`, where `Foo` is
a Java annotation, still compiles (as it does right now,
actually). Part of the reason for this commit is to provoke
a discussion about how to fix that.

Fixes scala/bug#8778.
Fixes scala/bug#9400.
Fixes scala/bug#9644.
hrhino added a commit to hrhino/scala that referenced this issue Jul 1, 2018
Previously, `JavaParsers` and `ClassfileParser` would "fix-up"
Java annotations to be non-abstract classes with `Annotation`
and `StaticAnnotation` as parents. This caused all manner of
strangeness and invalid classfiles (see linked tickets) and
was surprising, to say the least.

Now, the only special dispensation given to Java annotations
is that they get a public, no-args class constructor, because
that's what it takes to get `typedAnnotation` to be able to
look up the type. This means that `new Foo`, where `Foo` is
a Java annotation, still compiles (as it does right now,
actually). Part of the reason for this commit is to provoke
a discussion about how to fix that.

Fixes scala/bug#8778.
Fixes scala/bug#9400.
Fixes scala/bug#9644.
hrhino added a commit to hrhino/scala that referenced this issue Jul 2, 2018
Previously, `JavaParsers` and `ClassfileParser` would "fix-up"
Java annotations to be non-abstract classes with `Annotation`
and `StaticAnnotation` as parents. This caused all manner of
strangeness and invalid classfiles (see linked tickets) and
was surprising, to say the least.

Now, the only special dispensation given to Java annotations
is that they get a public, no-args class constructor, because
that's what it takes to get `typedAnnotation` to be able to
look up the type. This means that `new Foo`, where `Foo` is
a Java annotation, still compiles (as it does right now,
actually). Part of the reason for this commit is to provoke
a discussion about how to fix that.

Fixes scala/bug#8778.
Fixes scala/bug#9400.
Fixes scala/bug#9644.
hrhino added a commit to hrhino/scala that referenced this issue Jul 2, 2018
Previously, `JavaParsers` and `ClassfileParser` would "fix-up"
Java annotations to be non-abstract classes with `Annotation`
and `StaticAnnotation` as parents. This caused all manner of
strangeness and invalid classfiles (see linked tickets) and
was surprising, to say the least.

Now, the only special dispensation given to Java annotations
is that they get a public, no-args class constructor, because
that's what it takes to get `typedAnnotation` to be able to
look up the type. This means that `new Foo`, where `Foo` is
a Java annotation, still compiles (as it does right now,
actually). Part of the reason for this commit is to provoke
a discussion about how to fix that.

Fixes scala/bug#8778.
Fixes scala/bug#9400.
Fixes scala/bug#9644.
@SethTisue SethTisue modified the milestones: Backlog, 2.13.0-M5 Aug 7, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants