Fix #4297: handle type param reference #4298
Conversation
| case tp: TypeRef if tp.underlying.isInstanceOf[TypeBounds] => | ||
| val lo = this(tp.info.loBound) | ||
| val hi = this(tp.info.hiBound) | ||
| if (variance == 0) range(lo, hi) |
There was a problem hiding this comment.
The range method already does variance checks like this.
|
test performance with #fast please |
|
performance test scheduled: 1 job(s) in queue, 0 running. |
|
Performance test finished successfully: Visit http://dotty-bench.epfl.ch/4298/ to see the changes. Benchmarks is based on merging with master (7b91742) |
| def apply(tp: Type): Type = tp match { | ||
| case tp: TypeRef if tp.underlying.isInstanceOf[TypeBounds] => | ||
| val lo = this(tp.info.loBound) | ||
| val hi = this(tp.info.hiBound) |
There was a problem hiding this comment.
Replace this by apply? It makes it clearer that this is a recursive call
| case tpe: AppliedType => isClassDetermined(X, tpe)(ctx.fresh.setNewTyperState()) | ||
| case tpe: AppliedType => | ||
| isClassDetermined(X, tpe)(ctx.fresh.setNewTyperState()) || | ||
| isClassDetermined(stripTypeParam.apply(X), tpe)(ctx.fresh.setNewTyperState()) |
There was a problem hiding this comment.
Call it as stripTypeParam(X) instead of stripTypeParam.apply(X)?
There was a problem hiding this comment.
It seems scalac has problem with this syntax, however, I've changed the method so we don't need to call apply anymore.
| @@ -116,7 +130,9 @@ object Checkable { | |||
| case defn.ArrayOf(tpE) => recur(tpE, tpT) | |||
| case _ => recur(defn.AnyType, tpT) | |||
| } | |||
| case tpe: AppliedType => isClassDetermined(X, tpe)(ctx.fresh.setNewTyperState()) | |||
| case tpe: AppliedType => | |||
| isClassDetermined(X, tpe)(ctx.fresh.setNewTyperState()) || | |||
There was a problem hiding this comment.
Here, you first try without stripping type parameters for performance reasons? Maybe add a comment
| @@ -88,19 +88,33 @@ object Checkable { | |||
| } | |||
| } | |||
|
|
|||
| def stripTypeParam(implicit ctx: Context) = new ApproximatingTypeMap { | |||
| def apply(tp: Type): Type = tp match { | |||
| case tp: TypeRef if tp.underlying.isInstanceOf[TypeBounds] => | |||
There was a problem hiding this comment.
Probably equivalent to case tp: TypeRef if tp.symbol.isTypeParam. Maybe clearer?
There was a problem hiding this comment.
tp.symbol.isTypeParam doesn't cover the case of type members, so I think it's better to keep the original version here.
| @@ -88,19 +88,33 @@ object Checkable { | |||
| } | |||
| } | |||
|
|
|||
| def stripTypeParam(implicit ctx: Context) = new ApproximatingTypeMap { | |||
There was a problem hiding this comment.
Maybe add a comment: /** Approximate type parameters depending on variance */
Fix #4297: handle type param reference