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

SI-6169 Refine java wildcard bounds using corresponding tparam #3471

Merged
merged 3 commits into from Feb 10, 2014
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
51 changes: 49 additions & 2 deletions src/reflect/scala/reflect/internal/Types.scala
Expand Up @@ -899,7 +899,7 @@ trait Types
-1
}

/** If this is a poly- or methodtype, a copy with cloned type / value parameters
/** If this is a ExistentialType, PolyType or MethodType, a copy with cloned type / value parameters
* owned by `owner`. Identity for all other types.
*/
def cloneInfo(owner: Symbol) = this
Expand Down Expand Up @@ -2656,8 +2656,55 @@ trait Types
override def baseTypeSeq = underlying.baseTypeSeq map maybeRewrap
override def isHigherKinded = false

override def skolemizeExistential(owner: Symbol, origin: AnyRef) =
/** [SI-6169, SI-8197 -- companion to SI-1786]
*
* Approximation to improve the bounds of a Java-defined existential type,
* based on the bounds of the type parameters of the quantified type
* In Scala syntax, given a java-defined class C[T <: String], the existential type C[_]
* is improved to C[_ <: String] before skolemization, which captures (get it?) what Java does:
* enter the type paramers' bounds into the context when checking subtyping/type equality of existential types
*
* (Also tried doing this once during class file parsing or when creating the existential type,
* but that causes cyclic errors because it happens too early.)
*/
private def sharpenQuantifierBounds(): Unit = {
/* Check that we're looking at rawToExistential's handiwork
* (`existentialAbstraction(eparams, typeRef(apply(pre), sym, eparams map (_.tpe)))`).
* We can't do this sharpening there because we'll run into cycles.
*/
def rawToExistentialCreatedMe = (quantified corresponds underlying.typeArgs){ (q, a) => q.tpe =:= a }

if (underlying.typeSymbol.isJavaDefined && rawToExistentialCreatedMe) {
val tpars = underlying.typeSymbol.initialize.typeParams // TODO: is initialize needed?
debuglog(s"sharpen bounds: $this | ${underlying.typeArgs.map(_.typeSymbol)} <-- ${tpars.map(_.info)}")

foreach2(quantified, tpars) { (quant, tparam) =>
// TODO: check `tparam.info.substSym(tpars, quantified) <:< quant.info` instead (for some weird reason not working for test/t6169/ExistF)
// for now, crude approximation for the common case
if (quant.info.bounds.isEmptyBounds && !tparam.info.bounds.isEmptyBounds) {
// avoid creating cycles [pos/t2940] that consist of an existential quantifier's
// bounded by an existential type that unhygienically has that quantifier as its own quantifier
// (TODO: clone latter existential with fresh quantifiers -- not covering this case for now)
if ((existentialsInType(tparam.info) intersect quantified).isEmpty)
quant setInfo tparam.info.substSym(tpars, quantified)
}
}
}

_sharpenQuantifierBounds = false
}
private[this] var _sharpenQuantifierBounds = true

override def skolemizeExistential(owner: Symbol, origin: AnyRef) = {
// do this here because it's quite close to what Java does:
// when checking subtyping/type equality, enter constraints
// derived from the existentially quantified type into the typing environment
// (aka \Gamma, which tracks types for variables and constraints/kinds for types)
// as a nice bonus, delaying this until we need it avoids cyclic errors
if (_sharpenQuantifierBounds) sharpenQuantifierBounds

deriveType(quantified, tparam => (owner orElse tparam.owner).newExistentialSkolem(tparam, origin))(underlying)
}

private def wildcardArgsString(qset: Set[Symbol], args: List[Type]): List[String] = args map {
case TypeRef(_, sym, _) if (qset contains sym) =>
Expand Down
4 changes: 4 additions & 0 deletions test/files/pos/t6169/Exist.java
@@ -0,0 +1,4 @@
public class Exist<T extends String> {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about a variation with:

public class Exist<T extends String, U extends T>

And at the same time, show you can selectively provide bounds for some existential type args, leaving others unbounded to pick up bounds from the corresponding type param.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suspect this might need a little more substitution to get right. Will be interesting to see what Brian Boitano^W^W Java would do...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added test case -- looks like it passes :)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

public class Exist<T extends String, U extends T>
type X = Exist[String with Foo, _] // what bounds do you end up with for the existential here?  `_ <: String with Foo`? Or `_ <: T`? The latter will be ill-scoped.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

scala> type T = ExistIndir[String with Foo, _]
defined type alias T

scala> typeOf[T].normalize
res2: $r.intp.global.Type = ExistIndir[String with Foo, _ <: String with Foo]

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You win, sir.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Umm, let's make some more busy work first.

Is this symmetrical? What happens with class X<? super Man>?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(wildcards get super, but this inference only looks at type params)

// java helpfully re-interprets Exist<?> as Exist<? extends String>
public Exist<?> foo() { throw new RuntimeException(); }
}
4 changes: 4 additions & 0 deletions test/files/pos/t6169/ExistF.java
@@ -0,0 +1,4 @@
public class ExistF<T extends ExistF<T>> {
// java helpfully re-interprets ExistF<?> as ExistF<?0 extends ExistF<?0>>
public ExistF<?> foo() { throw new RuntimeException(); }
}
4 changes: 4 additions & 0 deletions test/files/pos/t6169/ExistIndir.java
@@ -0,0 +1,4 @@
public class ExistIndir<T extends String, U extends T> {
// java helpfully re-interprets ExistIndir<?> as ExistIndir<? extends String>
public ExistIndir<?, ?> foo() { throw new RuntimeException(); }
}
1 change: 1 addition & 0 deletions test/files/pos/t6169/OP.java
@@ -0,0 +1 @@
public abstract class OP<T> { }
1 change: 1 addition & 0 deletions test/files/pos/t6169/Skin.java
@@ -0,0 +1 @@
public interface Skin<C extends Skinnable> { }
3 changes: 3 additions & 0 deletions test/files/pos/t6169/Skinnable.java
@@ -0,0 +1,3 @@
public interface Skinnable {
OP<Skin<?>> skinProperty();
}
14 changes: 14 additions & 0 deletions test/files/pos/t6169/skinnable.scala
@@ -0,0 +1,14 @@
object ObjectProperty {
implicit def jfxObjectProperty2sfx[T](p: OP[T]) = new ObjectProperty[T](p)
}

class ObjectProperty[T](val delegate: OP[T])

trait TestWildcardBoundInference {
def delegate: Skinnable
def skin: ObjectProperty[Skin[_ /* inferred: <: Skinnable */]] = ObjectProperty.jfxObjectProperty2sfx(delegate.skinProperty)
skin: ObjectProperty[Skin[_ <: Skinnable]]

def skinCheckInference = delegate.skinProperty
skinCheckInference: ObjectProperty[Skin[_ <: Skinnable]]
}
7 changes: 7 additions & 0 deletions test/files/pos/t6169/t6169.scala
@@ -0,0 +1,7 @@
class Test {
class MyExist extends ExistF[MyExist]
// SI-8197, SI-6169: java infers the bounds of existentials, so we have to as well now that SI-1786 is fixed...
def stringy: Exist[_ <: String] = (new Exist[String]).foo
def fbounded: (ExistF[t] forSome {type t <: ExistF[t] }) = (new MyExist).foo
def indir: ExistIndir[_ <: String, _ <: String] = (new ExistIndir[String, String]).foo
}