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

Prefix for inner class of generic java classes is incorrect. #3943

Closed
scabug opened this issue Oct 20, 2010 · 14 comments
Closed

Prefix for inner class of generic java classes is incorrect. #3943

scabug opened this issue Oct 20, 2010 · 14 comments
Assignees
Milestone

Comments

@scabug
Copy link

scabug commented Oct 20, 2010

scalac - classpath javaPart.jar ./test/Child.scala

scalac.bat : .\test\Child.scala:5: error: class ActualClass needs to be abstract, since method f in class AChild of type (x$$1: test.TopLevelP
arent[A]#Inner2)Unit is not defined
class ActualClass[A] extends AnotherChild[A] {
^
.\test\Child.scala:6: error: method f overrides nothing
override def f(ic: Inner2) = ()
^
two errors found

@scabug
Copy link
Author

scabug commented Oct 20, 2010

Imported From: https://issues.scala-lang.org/browse/SI-3943?orig=1
Reporter: Alec Zorab (aleczorab)
See #6168
Attachments:

@scabug
Copy link
Author

scabug commented Oct 20, 2010

Alec Zorab (aleczorab) said:
source used to generate javaPart,jar

@scabug
Copy link
Author

scabug commented Oct 20, 2010

Alec Zorab (aleczorab) said:
scala source that does not compile

@scabug
Copy link
Author

scabug commented Oct 20, 2010

Alec Zorab (aleczorab) said:
jar compiled against

@scabug
Copy link
Author

scabug commented Oct 20, 2010

Alec Zorab (aleczorab) said:
I've tried all the obvious variations of ActualClass such as

class ActualClass[A] extends AnotherChild[A] {
override def f(ic: TopLevelParent#Inner2) = ()
}

class ActualClass[A] extends AnotherChild[A] {
override def f(ic: TopLevelParent[A]#Inner2) = ()
}

but it's possible that there's some syntactic finesse I could apply but don't know about

@scabug
Copy link
Author

scabug commented Nov 9, 2010

@adriaanm said:
this should work:

class ActualClass?[A] extends AnotherChild?[A] {

override def f(ic: TopLevelParent?[A]#Inner2) = ()

}

I'll have a look whether it does or not.

@scabug
Copy link
Author

scabug commented Nov 16, 2010

@adriaanm said:
this is indeed a bug -- using unique names for the type parameters makes the error message clearer:

package test

trait AnotherChild[A3] extends AChild[A3]

// error: class ActualClass needs to be abstract, since method f in class AChild of type 
// (x$$1: test.TopLevelParent[UpHere]#Inner2)Unit is not defined
class ActualClass[A] extends AnotherChild[A] {
  def f(ic: test.TopLevelParent[A]#Inner2): Unit = () //Inner2
}
package test;

public abstract class TopLevelParent<UpHere>{
    public class Inner1 {
    }

    public class Inner2 extends Inner1 {
    }
}

abstract class AChild<A1> extends TopLevelParent<A1> {
    abstract void f(Inner2 ic);
}

class child2<A2> extends AChild<A2> {
    void f(Inner2 ic){
    }
}

@scabug
Copy link
Author

scabug commented Jan 6, 2011

@adriaanm said:
things already go horribly wrong in sigToType:

here's some annotated debug output when parsing the class file of

package test;

public abstract class TopLevelParent<T> {
  public class Inner { }
}

abstract class Child<C> extends TopLevelParent<C> {
  abstract void f(Inner ic); // the implicit prefix for Inner is TopLevelParent<T> instead of TopLevelParent<C>
}
sig: class TopLevelParent : test.TopLevelParent[C]
methSig: constructor Child has params: List()
sig: class Inner : test.TopLevelParent[T]#Inner
methSig: method f has params: List(test.TopLevelParent[T]#Inner)

T is not in scope in Child, where method f is defined -- the culprit is probably on the line above (should be test.TopLevelParent[C]#Inner

@scabug
Copy link
Author

scabug commented Jan 6, 2011

@adriaanm said:
the fix I am testing now will compute the following type for f's ic param: test.Child[C]#Inner

@scabug
Copy link
Author

scabug commented Jan 7, 2011

@adriaanm said:
more poignant test case:

Outer.java:

public class Outer<E> {
  abstract class Inner {
    abstract public void foo(E e);
  }
}

class Child extends Outer<String> {
  // the implicit prefix for Inner is Outer<E> instead of Outer<String>
  public Inner getInner() {
    return new Inner() {
     public void foo(String e) { System.out.println("meh "+e); }
    };
  }
}
object Test extends Application {
  val x: Child = new Child
  x.getInner.foo("meh")
//                        ^
// error: type mismatch;
//  found   : java.lang.String("meh")
//  required: E
}

@scabug
Copy link
Author

scabug commented Jan 7, 2011

@adriaanm said:
I give up.

After figuring out the problem and the obvious fix, I've spent more than a day looking for a way that does not cause cyclesr1 during quick.lib.

The obvious fixr2 is to simply change the two occurrences of processClassType(processInner(???.tpe)) in sigToType
to processClassType(processInner(transformedTpe(???))), with:

// SI-3943
// cls.tpe.prefix is the thisType of the enclosing class of cls's definition
// later, we'll widen the this-type to the corresponding typeref (with type arguments that blindly refer
//   to the enclosing class's type parameters)
// since we're computing sym's signature, cls.tpe needs to be re-interpreted in the context of sym.enclClass.thisType
// otherwise, cls.tpe.widen may refer to type parameters of its enclosing class, which are not in scope where sym is defined
lazy val site = if(sym eq null) null else sym.enclClass.thisType
def transformedTpe(cls: Symbol) = if((site eq null) || cls.isStatic) cls.tpe else site.memberType(cls)

I've tried various approaches with LazyTypes, but the fact that these prefixes may occur in the super class type, method parameter types&result type,... makes it seem like an unlikely solution.

The only alternative I could think of was something like cooking raw types, but I can only hope there's a better way...

r1 assertion failed: illegal class file dependency between 'object ListItr' and 'class ListItr'

69.     assert(!busy.isDefined, {
          val (s1, s2) = (busy.get, root)
          if (s1 eq s2) "unsatisfiable cyclic dependency in '%s'".format(s1)
          else "illegal class file dependency between '%s' and '%s'".format(s1, s2)
        })

r2 adriaanm/scala@95bcc3f

@scabug
Copy link
Author

scabug commented May 12, 2013

@paulp said:
This has started working sometime since 2.10.

@scabug
Copy link
Author

scabug commented May 12, 2013

@retronym said:
#6168 / scala/scala@edee27f59f1787, perchance?

@scabug
Copy link
Author

scabug commented May 15, 2013

@retronym said (edited on May 15, 2013 8:42:57 AM UTC):
Yep, that was the one.

ticket/t3943 /code/scala javac -d sandbox test/files/pos/t3943/Outer_1.java && RUNNER=scalac scala-hash edee27f59f1787^ -classpath sandbox test/files/pos/t3943/Client_2.scala
[info] downloading http://scala-webapps.epfl.ch/artifacts/1187c9896c097e6e591e5655b35f52c06b3c900a/pack.tgz ...done.
[info] scala revision from 2013-03-23 12:20:41 -0700 downloaded to /Users/jason/usr/scala-v2.11.0-M2-33-g1187c98
[info] edee27f59f => /Users/jason/usr/scala-v2.11.0-M2-33-g1187c98
test/files/pos/t3943/Client_2.scala:3: error: type mismatch;
 found   : String("meh")
 required: E
  x.getInner.foo("meh")
                 ^
one error found
ticket/t3943 /code/scala javac -d sandbox test/files/pos/t3943/Outer_1.java && RUNNER=scalac scala-hash edee27f59f1787 -classpath sandbox test/files/pos/t3943/Client_2.scala
[info] edee27f59f => /Users/jason/usr/scala-v2.11.0-M2-34-gedee27f

Test case: scala/scala#2534

@scabug scabug closed this as completed May 16, 2013
@scabug scabug added this to the 2.11.0-M3 milestone Apr 7, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants