Skip to content

Generic signature for wildcards changed in 2.13.7 #12488

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

Open
lrytz opened this issue Nov 10, 2021 · 19 comments
Open

Generic signature for wildcards changed in 2.13.7 #12488

lrytz opened this issue Nov 10, 2021 · 19 comments
Assignees
Milestone

Comments

@lrytz
Copy link
Member

lrytz commented Nov 10, 2021

trait T[+X] { def x: X }
class C {
  type TT[+X] = T[X]
  def it(): TT[_] = new TT[String] { def x = "" }
}
diff --git a/C.class.asm b/C.class.asm
index c25e4e0..eeced80 100644
--- a/C.class.asm
+++ b/C.class.asm
@@ -14,8 +14,8 @@
     MAXLOCALS = 1

   // access flags 0x1
-  // signature ()LT<*>;
-  // declaration: T<?> it()
+  // signature ()LT<Ljava/lang/Object;>;
+  // declaration: T<java.lang.Object> it()
   public it()LT;
     NEW C$$anon$1
     DUP

The regression shows only when using the type alias. Discovered by @octonato and MiMa on Akka.

Not sure if that has any impact on Java clients. The following Java code works, no matter if the Scala code was compiled with 2.12.6 or 2.12.7:

public class A {
  public static void main(String[] args) {
    C c = new C();
    T<?> t = c.it();
    System.out.println(t.x());
  }
}
@lrytz lrytz added this to the 2.13.8 milestone Nov 10, 2021
@dwijnand
Copy link
Member

The regression shows only when using the type alias.

I almost felt like Object is right, seeing as TT is covariant and declaration-side variance isn't representable in Java. But it's T<?> and ()LT<*>; when not using the type alias, so it's at least inconsistent. But then Object is the top type of any type, so you're not gaining anything over ?.

Which lead me to compare what it looks like with another upper bound:

class Foo
trait T[+X <: Foo] { def x: X }
class C {
  type TT[+X <: Foo] = T[X]
  def it(): TT[_] = new TT[Foo] { def x = new Foo }
}

And that also emits Object over ?. So it's not doing anything more useful.

That is because I assumed TT[_] <:< TT[Foo], but it looks like it's not?

$ bat -p A.scala
object A {
  def main(args: Array[String]): Unit = {
    val c: C = new C()
    val t: T[Foo] = c.it()
    println(t.x)
  }
}
$ scalac -2.13.7 -cp . A.scala
A.scala:4: error: type mismatch;
 found   : T[Any]
 required: T[Foo]
    val t: T[Foo] = c.it()
                        ^
1 error

Confused... 🤯

@lrytz lrytz added the blocker label Nov 17, 2021
@lrytz
Copy link
Member Author

lrytz commented Nov 18, 2021

Bisected to scala/scala#9693

@lrytz
Copy link
Member Author

lrytz commented Nov 18, 2021

Uncurry's expandAlias calls normalize, which calls ExistentialType.rewrap, which calls existentialAbstraction, which eliminates simple covariant existentials (see scala/scala#9693 (comment)). I guess this is because T[_] and T[Any] are equivalent for a covariant T. Is that also true when looking at T<?> vs T<Object> from Java's type system? cc @joroKr21.

@joroKr21
Copy link
Member

Yes I already suspected as much but didn't have time to look at it yet. It's unfortunate that Scala doesn't dealias when computing the erasure (and bytecode signature). It leads to this kind of problems all the time 😬

@lrytz
Copy link
Member Author

lrytz commented Nov 18, 2021

unfortunate that Scala doesn't dealias when computing the erasure (and bytecode signature)

can you elaborate? uncurry should remove type aliases, and it's before erasure. there are no type aliases in the bytecode involved here, not in the method signatures, nor in their generic signatures.

@joroKr21
Copy link
Member

Oh sorry I think I confused an old PR of mine but it was specific to higher-kinded types: scala/scala#8720

@joroKr21
Copy link
Member

I guess this is because T[_] and T[Any] are equivalent for a covariant T.

That's right

Is that also true when looking at T<?> vs T from Java's type system?

I don't know to be honest

@dwijnand
Copy link
Member

That is because I assumed TT[_] <:< TT[Foo], but it looks like it's not?

Given type TT[+X <: Foo], and ideas why TT[_] <:< TT[Foo] fails?

@joroKr21
Copy link
Member

joroKr21 commented Nov 19, 2021

In TT[_] the _ (which is an existential quantifier) does not inherit the bounds of the underlying type parameter.
That's one of the reasons why we skip existential skolems in ref checks. If that could be fixed, we wouldn't have to.
typeOf[TT[_ <: Foo]] <:< typeOf[TT[Foo]] is true for example.

I already tried to add those bounds to wildcards but I think it breaks down for F-bounded type parameters and for inferred existential types. Perhaps worth another try 🤔 - but if that subtype relationship were true, it doesn't fix the OP does it?

@lrytz
Copy link
Member Author

lrytz commented Nov 19, 2021

Given type TT[+X <: Foo], and ideas why TT[_] <:< TT[Foo] fails?

If I understand the spec correctly, TT[_] is TT[t] forSome { t >: Nothing <: Any} no matter how TT is defined.

@lrytz
Copy link
Member Author

lrytz commented Nov 19, 2021

Scala 3 only has wildcards, no existentials, so it handles this differently it seems:

scala> trait T[+X <: String]
scala> val t: T[String] = (null: T[_])
val t: T[String] = null

@joroKr21
Copy link
Member

The equivalent question in Scala 3 is if _ in T[_] is a bounded wildcard or not?

@joroKr21
Copy link
Member

joroKr21 commented Nov 19, 2021

Also from that part of the spec:

Example
Assume a covariant type
class List[+T]
The type
List[T] forSome { type T <: java.lang.Number }
is equivalent (by simplification rule 4 above) to
List[java.lang.Number] forSome { type T <: java.lang.Number }
which is in turn equivalent (by simplification rules 2 and 3 above) to List[java.lang.Number].

@lrytz
Copy link
Member Author

lrytz commented Nov 19, 2021

I looked a bit at Java; T<?> in Java is an "Unbounded Wildcard".

The wildcard ? extends Object is equivalent to the unbounded wildcard ?

javac 11 tells me T<Object> <: T<? extends Object> but not the other way around:

public class A {
  public static class T<X> { }
  public static void main(String[] args) {
    T<? extends Object> t1a = null;
    T<Object> t1b = null;
    T t1c = null;
    T<? super Object> t1d = null;

    t1a = t1b;
    t1a = t1c;    // warning: [unchecked] unchecked conversion
    t1a = t1d;

    // t1b = t1a; // error: T<CAP#1> cannot be converted to T<Object>
    t1b = t1c;    // warning: [unchecked] unchecked conversion
    t1b = t1d;

    t1c = t1a;
    t1c = t1b;
    t1c = t1d;

    // t1d = t1a; // error: T<CAP#1> cannot be converted to T<? super Object>
    t1d = t1b;
    t1d = t1c;    // warning: [unchecked] unchecked conversion
  }
}

So this would cause a problem if a generic signature goes from (LT<*>;)I to (LT<Ljava/lang/Object;>;)I, passing in a value of type T<?> would no longer be possible.

Here are the rules (I didn't study them): https://docs.oracle.com/javase/specs/jls/se17/html/jls-4.html#jls-4.5.1

@dwijnand
Copy link
Member

but if that subtype relationship were true, it doesn't fix the OP does it?

No, I was just exploring with another upper bound, but then found an issue in my motivation. I'm somewhat convinced that the fact it compiles in Scala 3 means it's wrong in Scala 2. Anyways, thanks for the answers.

@joroKr21
Copy link
Member

So I think where things break down is that the spec states "for a covariant T[+X], T[_] is equivalent to T[Any]" but the signatures generated from those two types differ - how can we resolve this?

@lrytz
Copy link
Member Author

lrytz commented Nov 19, 2021

Maybe by booking it under "Java interop is best-effort"?

@lrytz
Copy link
Member Author

lrytz commented Nov 30, 2021

The attempt at scala/scala#9812 looks promising but not entirely trivial. I think we don't need to block 2.13.8 over this issue because

  • it's a corner case in Java interop
  • we didn't get any reports so far (it was discovered through MiMa / bytecode comparison)
  • the failure mode is benign

@SethTisue SethTisue modified the milestones: 2.13.10, Backlog Sep 23, 2022
@som-snytt
Copy link

Bookmarking #11480

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

No branches or pull requests

5 participants