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

propagate bounds from class type params to existential quantifiers #6169

Closed
scabug opened this issue Aug 2, 2012 · 26 comments

Comments

Projects
None yet
2 participants
@scabug
Copy link

commented Aug 2, 2012

see attachments

In java you can write

Circle circle = CircleBuilder.create().centerX(100).centerY(115).radius(100).build();

This is a builder pattern which make use of existential f-bounds
The problem is that in scala the skolems get erased after two method calls and so the next method doesn't exist.

if I javap the java code

const #33 = NameAndType #51:#52;//  create:()Ljavafx/scene/shape/CircleBuilder;
const #34 = NameAndType #53:#54;//  centerX:(D)Ljavafx/scene/shape/CircleBuilder
;
const #35 = NameAndType #55:#54;//  centerY:(D)Ljavafx/scene/shape/CircleBuilder
;
const #36 = NameAndType #56:#54;//  radius:(D)Ljavafx/scene/shape/CircleBuilder;

const #37 = NameAndType #57:#58;//  build:()Ljavafx/scene/shape/Circle;

then I can see that the names and types are stored in bytecode (so a sort of runtime type info / refied generics)

In scala:

val circle = CircleBuilder.create().centerX(100).centerY(115).radius(100).build()

causes

C:\scala-2.10.0-M6\myexamples>scalac javafxbuilder.scala
javafxbuilder.scala:19: error: value centerY is not a member of ?0
      val circle = CircleBuilder.create().centerX(100).centerY(115).radius(100).
build()
                                                       ^
one error found

The only way to solve it in scala is to do a downcast after two methods (.asInstanceOf[CircleBuilder[_]]) or implicit conversion or to generate reified skolems _$x beforehand.

val circlebuilder = CircleBuilder.create().asInstanceOf[CircleBuilder[_ <: CircleBuilder[_ <: CircleBuilder[_ <: CircleBuilder[_]]]]]

which generates $_1, $_2 and $_3 for calling 6 methods (2 per skolem)

This looks a bit hackish to me and very unscala.

associated thread:
https://groups.google.com/forum/?hl=en&fromgroups#!topic/scala-user/Op79HcAoj2M

@scabug

This comment has been minimized.

Copy link
Author

commented Aug 2, 2012

Imported From: https://issues.scala-lang.org/browse/SI-6169?orig=1
Reporter: DaveScala (davescala)
Affected Versions: 2.10.0-M6
See #8198, #1786, #4745
Duplicates #2385
Attachments:

@scabug

This comment has been minimized.

Copy link
Author

commented Aug 2, 2012

DaveScala (davescala) said:
Test files

@scabug

This comment has been minimized.

Copy link
Author

commented Mar 30, 2013

@retronym said (edited on Mar 30, 2013 9:29:03 PM UTC):
Actually, it's not a Java-interop problem, the equivalent Scala code also fails.

object Main {
  def foo(b: CircleBuilderScala[_]) =
    b.foo.foo  // error: value foo is not a member of _$1
}

class CircleBuilderScala[B <: CircleBuilderScala[B]] {
  def foo: B = ???
}

See also #4745 and other tickets about f-bounded type parameters + type inference.

@scabug

This comment has been minimized.

Copy link
Author

commented Mar 30, 2013

@retronym said:
Here's a way you can use the techniques touched on in #4745 to centralize your casting pain.

scala> type CircleBuilderF = ({type CB <: CircleBuilder[CB]; type CB1 = CB with CircleBuilder[CB]})#CB1
defined type alias CircleBuilderF

scala> def newCircleBuilder: CircleBuilderF = CircleBuilder.create.asInstanceOf[CircleBuilderF]
newCircleBuilder: CircleBuilderF

scala> newCircleBuilder.centerX(0).centerX(0).build()
res28: javafx.scene.shape.Circle = Circle@38b9a306

Now, if we can just figure out how to roll this into the compiler...

@scabug

This comment has been minimized.

Copy link
Author

commented Mar 31, 2013

@retronym said (edited on Mar 31, 2013 10:40:42 AM UTC):
You can actually centralize this hack as follows:

scala> implicit class RichFBoundBuilder[B[X <: B[X]]](val _builder: B[_]) {
  type B1 = X forSome { type X <: B[X] }
  def asScala: B1 = _builder.asInstanceOf[B1]
}
warning: there were 2 feature warning(s); re-run with -feature for details
defined class RichFBoundBuilder

scala> import javafx.scene.shape._
import javafx.scene.shape._

scala> CircleBuilder.create.asScala.centerX(0).centerX(0).build()
res0: javafx.scene.shape.Circle = Circle@6c60efe7
@scabug

This comment has been minimized.

Copy link
Author

commented Mar 31, 2013

Stephen Compall (s11001001) said:
RichFBoundBuilder doesn't require the proof it looks like it requires, though:

Welcome to Scala version 2.10.1 (OpenJDK Client VM, Java 1.6.0_27).
Type in expressions to have them evaluated.
Type :help for more information.

scala> :paste
// Entering paste mode (ctrl-D to finish)

implicit class RichFBoundBuilder[B[X <: B[X]]](val _builder: B[_]) {
  type B1 = X forSome { type X <: B[X] }
  def asScala: B1 = _builder.asInstanceOf[B1]
}

// Exiting paste mode, now interpreting.

defined class RichFBoundBuilder

scala> (x: List[Int]) => x.asScala
res0: List[Int] => X forSome { type X <: List[X] } = <function1>
@scabug

This comment has been minimized.

Copy link
Author

commented Apr 1, 2013

@retronym said:
I can't figure out a way to tighten this up, unfortunately. You are probably best making a library to wrap all the builder creation calls to localize the use of this hack.

@scabug

This comment has been minimized.

Copy link
Author

commented Apr 1, 2013

Stephen Compall (s11001001) said (edited on Apr 2, 2013 7:03:48 PM UTC):
Yes; I've written casts for just the builders we use in Scala.

I don't see how 6169 can be fixed, interop-wise, if #7318 isn't a bug, though.

@scabug

This comment has been minimized.

Copy link
Author

commented Apr 1, 2013

@retronym said:
I would suggest to go for this to tighten up the availability of asScala:

scala> import javafx.scene._, shape._import javafx.scene._
import shape._

scala> implicit class RichJavaFXBuilder[B[X <: B[X]] <: NodeBuilder[_]](val _builder: B[_]) {
     |   type B1 = X forSome { type X <: B[X] }
     |   def asScala: B1 = _builder.asInstanceOf[B1]
     | }
warning: there were 2 feature warning(s); re-run with -feature for details
defined class RichJavaFXBuilder

scala> CircleBuilder.create().asScala.centerX(0).centerX(0).build()
res3: javafx.scene.shape.Circle = Circle@2becc08c

scala> List(1).asScala
<console>:20: error: value asScala is not a member of List[Int]
              List(1).asScala
                      ^
@scabug

This comment has been minimized.

Copy link
Author

commented May 30, 2013

Stephen Compall (s11001001) said:
#1786 almost seems that it should have fixed this, but the F-bound case in particular doesn't seem to have benefited from the inferred existential bound introduced there.

(in 2.11.0-M3, a variant of 1786's sample code):

abstract class SomeClass2[T] {def tValue: T}
class MyClass2[T <: SomeClass2[T]](val myValue:T) 

def myMethod(i:MyClass2[_]) {
   i.myValue.tValue      // << error: value tValue is not a member of _$1
}

def myMethod[T <: SomeClass2[T]](i:MyClass2[T]) {
   i.myValue.tValue      // << works
}
@scabug

This comment has been minimized.

Copy link
Author

commented Jun 28, 2013

Stephen Compall (s11001001) said:
...but with better justification.

@scabug

This comment has been minimized.

Copy link
Author

commented Jul 10, 2013

@adriaanm said:
Unassigning and rescheduling to M6 as previous deadline was missed.

@scabug

This comment has been minimized.

Copy link
Author

commented Jan 29, 2014

Stephen Compall (s11001001) said:
An F-bound isn't necessary to exercise this. In 2.11.0-M8:

public final class It<T extends String>
{
  private final T _it;
  public It(T it){_it = it;}

  public static It<?> instance() {
    return new It<String>("42");
  }

  public static It<? extends String> instance2() {
    return instance();
  }

  public T it(){return _it;}
}
scala> It.instance.it
res1: Any = 42

scala> It.instance2.it
res2: String = 42
@scabug

This comment has been minimized.

Copy link
Author

commented Jan 29, 2014

@adriaanm said:
I don't see what's wrong here. It<?> corresponds to the existential type It[_], so the instance method packages a string into a box that hides its contents, and when you open up the box you get Any.

@scabug

This comment has been minimized.

Copy link
Author

commented Jan 29, 2014

@adriaanm said:
I hadn't considered that It's type parameter is bounded by String, so the It<?> in instance's return type is implicitly It[_ <: String].

@scabug

This comment has been minimized.

Copy link
Author

commented Jan 29, 2014

Stephen Compall (s11001001) said:
And whatever strategy lets that automatic existential bound be added in #1786 doesn't work in this case.

@scabug

This comment has been minimized.

Copy link
Author

commented Jan 29, 2014

@adriaanm said (edited on Jan 29, 2014 6:36:39 PM UTC):
Right, so we have to apply the same magic when loading Java classes, as the bound is not in the bytecode:

  public static It<?> instance();
    descriptor: ()LIt;
    flags: ACC_PUBLIC, ACC_STATIC
    Code:
      stack=3, locals=0, args_size=0
         0: new           #3                  // class It
         3: dup           
         4: ldc           #4                  // String 42
         6: invokespecial #5                  // Method "<init>":(Ljava/lang/String;)V
         9: areturn       
      LineNumberTable:
        line 7: 0
    Signature: #19                          // ()LIt<*>;
@scabug

This comment has been minimized.

Copy link
Author

commented Jan 29, 2014

@adriaanm said:
Just to verify, the instance2 method does have it:

  public static It<? extends java.lang.String> instance2();
    descriptor: ()LIt;
    flags: ACC_PUBLIC, ACC_STATIC
    Code:
      stack=1, locals=0, args_size=0
         0: invokestatic  #6                  // Method instance:()LIt;
         3: areturn       
      LineNumberTable:
        line 11: 0
    Signature: #21                          // ()LIt<+Ljava/lang/String;>;
@scabug

This comment has been minimized.

Copy link
Author

commented Jan 30, 2014

@adriaanm said (edited on Feb 4, 2014 8:35:43 AM UTC):
hacky but working wip: https://github.com/adriaanm/scala/compare/t6169
I should probably read the java spec instead of winging it (or spend some more time with http://www.cs.cornell.edu/~ross/publications/tamewild/tamewild-tate-pldi11-tr.pdf)

The idea is to do a bit more work during classfile parsing (while trying to avoid cycles) to propagate bounds declared on a class's type params to the corresponding existential quantifiers that abstract over them.

@scabug

This comment has been minimized.

Copy link
Author

commented Jan 31, 2014

Stephen Compall (s11001001) said:
Well, that's just neat!

@scabug

This comment has been minimized.

Copy link
Author

commented Feb 4, 2014

@adriaanm said:
As always with this kind of stuff, trickier than hoped. Running test suite on latest incarnation: https://scala-webapps.epfl.ch/jenkins/job/scala-checkin-manual/1194/console

@scabug

This comment has been minimized.

Copy link
Author

commented Feb 10, 2014

@scabug

This comment has been minimized.

Copy link
Author

commented Feb 10, 2014

@adriaanm said:
Let's see if this subsumes https://github.com/scala/scala/pull/2518/files?w=1#diff-4eab1aad4533a31c10565971e90f73eaR4910
(so canEnhanceIdent == false because we no longer need it)

@scabug scabug closed this Feb 10, 2014

@scabug

This comment has been minimized.

Copy link
Author

commented Feb 11, 2014

@retronym said:
I've just been building Slick with 2.11.

https://github.com/retronym/slick/compare/tmp;2.11-compat-2?expand=1

#1786 actually seems to be a step backwards. Before, slick left off a lot of existential bounds, but because it did that across the boards we it actually got away with it.

Now, depending on compilation order, some bounds are inferred, and others aren't. The inferred bounds are infectious; type errors spring up in other places.

[error] /Users/jason/code/slick/slick-testkit/src/main/scala/com/typesafe/slick/testkit/tests/JdbcMapperTest.scala:194: class PairShape needs to be abstract, since method copy in class ProductNodeShape of type (shapes: Seq[scala.slick.lifted.Shape[_ <: scala.slick.lifted.ShapeLevel, _, _, _]])scala.slick.lifted.Shape[Level, _, _, _] is not defined
[error] (Note that Seq[scala.slick.lifted.Shape[_ <: scala.slick.lifted.ShapeLevel, _, _, _]] does not match Seq[JdbcMapperTest.this.tdb.profile.simple.Shape[_, _, _, _]]: their type parameters differ)
[error]     final class PairShape[Level <: ShapeLevel, M <: Pair[_,_], U <: Pair[_,_], P <: Pair[_,_]](val shapes: Seq[Shape[_, _, _, _]]) extends MappedScalaProductShape[Level, Pair[_,_], M, U, P] {
[error]

So I can see the attraction of deferring the bounds sharpening until subtype checks.

@scabug

This comment has been minimized.

Copy link
Author

commented Feb 11, 2014

@adriaanm said:
Ok, you have me convinced. I'll revert the fix to #1786 and reopen it to receive a fuller fix based on #6169 in 2.12

@scabug

This comment has been minimized.

Copy link
Author

commented Feb 11, 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.