From 52379b66f96bf7e0ca22f7835ddb06c58d94d4a0 Mon Sep 17 00:00:00 2001 From: Jason Zaugg Date: Wed, 22 Jan 2014 16:05:29 +0100 Subject: SI-8170 Fix regression in TypeRef#transform w. PolyTypes Regressed in SI-8046 / edc9edb7, by my hand. At the time, I noticed the problem: transform wasn't accounting for the potential Poly-Type-ness of its argument, and this would lead to under-substituted types. The commit comment of edc9edb7 shows an example. But the remedy wasn't the right one. The root problem is that a TypeMap over a PolyType can return one with cloned type parameter symbols, which means we've lose the ability to substitute the type arguments into the result. This commit detects up front whether the type-under-transform is a PolyType with the current TypeRef's type parameters, and just runs the `asSeenFrom` over its result type. --- src/reflect/scala/reflect/internal/Types.scala | 21 +++++++++++++++----- test/files/pos/t8170.scala | 27 ++++++++++++++++++++++++++ test/files/pos/t8170b.scala | 25 ++++++++++++++++++++++++ 3 files changed, 68 insertions(+), 5 deletions(-) create mode 100644 test/files/pos/t8170.scala create mode 100644 test/files/pos/t8170b.scala diff --git a/src/reflect/scala/reflect/internal/Types.scala b/src/reflect/scala/reflect/internal/Types.scala index 9ddaea4c62..209d31c5af 100644 --- a/src/reflect/scala/reflect/internal/Types.scala +++ b/src/reflect/scala/reflect/internal/Types.scala @@ -1988,12 +1988,23 @@ trait Types // too little information is known to determine its kind, and // it later turns out not to have kind *. See SI-4070. Only // logging it for now. - if (sym.typeParams.size != args.size) + val tparams = sym.typeParams + if (tparams.size != args.size) devWarning(s"$this.transform($tp), but tparams.isEmpty and args=$args") - - val GenPolyType(tparams, result) = asSeenFromOwner(tp) - assert((tparams eq Nil) || tparams == sym.typeParams, (tparams, sym.typeParams)) - result.instantiateTypeParams(sym.typeParams, args) + def asSeenFromInstantiated(tp: Type) = + asSeenFromOwner(tp).instantiateTypeParams(tparams, args) + // If we're called with a poly type, and we were to run the `asSeenFrom`, over the entire + // type, we can end up with new symbols for the type parameters (clones from TypeMap). + // The subsequent substitution of type arguments would fail. This problem showed up during + // the fix for SI-8046, however the solution taken there wasn't quite right, and led to + // SI-8170. + // + // Now, we detect the PolyType before both the ASF *and* the substitution, and just operate + // on the result type. + tp match { + case PolyType(`tparams`, result) => PolyType(tparams, asSeenFromInstantiated(result)) + case _ => asSeenFromInstantiated(tp) + } } // note: does not go through typeRef. There's no need to because diff --git a/test/files/pos/t8170.scala b/test/files/pos/t8170.scala new file mode 100644 index 0000000000..b65f4b8572 --- /dev/null +++ b/test/files/pos/t8170.scala @@ -0,0 +1,27 @@ +object O { + trait X + trait B extends A { + override type T[F1 <: X] = F1 + } + trait A { + type T[F <: X] + } +} + +object Test { + import O._ + val a: B = ??? + val b: a.T[X] = ??? + b.ensuring(x => true) // trigger an implicit search +} + + +/* +this = {AliasArgsTypeRef@3004}"Test#7680.a#14899.T#14823[O#7702.X#7793]" + sym = type T#14823 + info = namer: [F#14824 <: O#7703.X#7793]F#14824 +result = {AbstractNoArgsTypeRef@3237}"F#24451" +tp = {PolyType@3235}"[F#14824 <: O#7703.X#7793]F#14824" +tparams = + (0) = {AbstractTypeSymbol@3247}"type F#24451" +*/ \ No newline at end of file diff --git a/test/files/pos/t8170b.scala b/test/files/pos/t8170b.scala new file mode 100644 index 0000000000..53036f6c8a --- /dev/null +++ b/test/files/pos/t8170b.scala @@ -0,0 +1,25 @@ +import language._ + +object ScalaZeee { + trait HFold[M[_], U] { + type Apply[E, A <: U] <: U + } + trait GenericCons[M[_], H, +T <: GenericList[M]] extends GenericList[M] { + val tail: T + override type Folded[N[X] >: M[X], U, F <: HFold[N, U]] = F#Apply[H, tail.Folded[N, U, F]] + } + val KNil: GenericList[Nothing] = ??? + sealed trait GenericList[+M[_]] { + type Folded[N[X] >: M[X], U, F <: HFold[N, U]] <: U + } +} + +object TypelevelUsage { + import ScalaZeee._ + type T = GenericCons[Some, String, KNil.type] + val klist1: T = ??? + type T2 = klist1.Folded[Option, Int, HFold[Option, Int]] + val count2: T2 = ??? + + count2.ensuring(x => true).toChar // trigger an implicit search +} -- cgit v1.2.3 From a02fae978bbf3a4296e7ab0baffdbe0b80d6c126 Mon Sep 17 00:00:00 2001 From: Jason Zaugg Date: Wed, 5 Feb 2014 12:01:38 +0100 Subject: SI-8170 Posing outstanding questions as TODOs We'll get to the bottom of this as soon as we get one of those Round Tuits. --- src/reflect/scala/reflect/internal/Types.scala | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/src/reflect/scala/reflect/internal/Types.scala b/src/reflect/scala/reflect/internal/Types.scala index 209d31c5af..6dac20459e 100644 --- a/src/reflect/scala/reflect/internal/Types.scala +++ b/src/reflect/scala/reflect/internal/Types.scala @@ -2001,6 +2001,22 @@ trait Types // // Now, we detect the PolyType before both the ASF *and* the substitution, and just operate // on the result type. + // + // TODO: Revisit this and explore the questions raised: + // + // AM: I like this better than the old code, but is there any way the tparams would need the ASF treatment as well? + // JZ: I think its largely irrelevant, as they are no longer referred to in the result type. + // In fact, you can get away with returning a type of kind * here and the sky doesn't fall: + // `case PolyType(`tparams`, result) => asSeenFromInstantiated(result)` + // But I thought it was better to retain the kind. + // AM: I've been experimenting with apply-type-args-then-ASF, but running into cycles. + // In general, it seems iffy the tparams can never occur in the result + // then we might as well represent the type as a no-arg typeref. + // AM: I've also been trying to track down uses of transform (pretty generic name for something that + // does not seem that widely applicable). + // It's kind of a helper for computing baseType (since it tries to propagate our type args to some + // other type, which has to be related to this type for that to make sense). + // tp match { case PolyType(`tparams`, result) => PolyType(tparams, asSeenFromInstantiated(result)) case _ => asSeenFromInstantiated(tp) -- cgit v1.2.3