From c32adc2e66ab74b7eaafa1e6f54fed9b975269c6 Mon Sep 17 00:00:00 2001 From: Dale Wijnand Date: Fri, 2 Jun 2023 11:33:24 +0100 Subject: [PATCH 1/3] Avoid embedding SelectionProtos in Conversions --- .../dotty/tools/dotc/typer/Implicits.scala | 24 ++++++++++++++----- .../test/dotc/pos-test-pickling.blacklist | 1 + tests/pos/i15867.scala | 19 +++++++++++++++ 3 files changed, 38 insertions(+), 6 deletions(-) create mode 100644 tests/pos/i15867.scala diff --git a/compiler/src/dotty/tools/dotc/typer/Implicits.scala b/compiler/src/dotty/tools/dotc/typer/Implicits.scala index c6795ed25a0e..9c044207d4b3 100644 --- a/compiler/src/dotty/tools/dotc/typer/Implicits.scala +++ b/compiler/src/dotty/tools/dotc/typer/Implicits.scala @@ -1122,17 +1122,29 @@ trait Implicits: adapt(generated, pt.widenExpr, locked) else { def untpdGenerated = untpd.TypedSplice(generated) - def producesConversion(info: Type): Boolean = info match - case info: PolyType => producesConversion(info.resType) - case info: MethodType if info.isImplicitMethod => producesConversion(info.resType) - case _ => info.derivesFrom(defn.ConversionClass) + def conversionResultType(info: Type): Type = info match + case info: PolyType => conversionResultType(info.resType) + case info: MethodType if info.isImplicitMethod => conversionResultType(info.resType) + case _ if info.derivesFrom(defn.ConversionClass) => pt match + case selProto: SelectionProto => + info.baseType(defn.ConversionClass) match + case AppliedType(_, List(_, restpe)) if selProto.isMatchedBy(restpe) => + // if we embed the SelectionProto as the Conversion result type + // it might end up within a GADT cast type + // so instead replace it with the targeted conversion type, if it matches + // see tests/pos/i15867.scala. + restpe + case _ => pt + case _ => pt + case _ => NoType def tryConversion(using Context) = { + val restpeConv = if ref.symbol.is(Given) then conversionResultType(ref.symbol.info) else NoType val untpdConv = - if ref.symbol.is(Given) && producesConversion(ref.symbol.info) then + if restpeConv.exists then untpd.Select( untpd.TypedSplice( adapt(generated, - defn.ConversionClass.typeRef.appliedTo(argument.tpe, pt), + defn.ConversionClass.typeRef.appliedTo(argument.tpe, restpeConv), locked)), nme.apply) else untpdGenerated diff --git a/compiler/test/dotc/pos-test-pickling.blacklist b/compiler/test/dotc/pos-test-pickling.blacklist index 81c0d3e35d3a..2ea2b045448f 100644 --- a/compiler/test/dotc/pos-test-pickling.blacklist +++ b/compiler/test/dotc/pos-test-pickling.blacklist @@ -94,6 +94,7 @@ i4176-gadt.scala # GADT difference i13974a.scala +i15867.scala java-inherited-type1 diff --git a/tests/pos/i15867.scala b/tests/pos/i15867.scala new file mode 100644 index 000000000000..2e62177ba590 --- /dev/null +++ b/tests/pos/i15867.scala @@ -0,0 +1,19 @@ +enum SUB[-A, +B]: + case Refl[S]() extends SUB[S, S] + +class Pow(self: Int): + def **(other: Int): Int = math.pow(self, other).toInt + +given fromList[T]: Conversion[List[T], Pow] = ??? + +given fromInt: Conversion[Int, Pow] = Pow(_) + +def foo[T](t1: T, ev: T SUB List[Int]) = + ev match { case SUB.Refl() => + t1 ** 2 // error + } + +def baz[T](t2: T, ev: T SUB Int) = + ev match { case SUB.Refl() => + t2 ** 2 // works + } From 2517f386fa123087f71b7508f74b5e4c86d728cd Mon Sep 17 00:00:00 2001 From: Dale Wijnand Date: Fri, 2 Jun 2023 16:48:32 +0100 Subject: [PATCH 2/3] Fix implicit type used for conversions, as seen from usage Using the minimisation of the specs2 failure that occurred in CI, the conversion desired is `Conversion[String, Bar.this.Data]`. But if we use the given's symbol info, we end up with the type `Conversion[String, Foo.this.Data]` which the generated tree will fail to adapt to. Using the widening of the implicit candidate's TermRef will yield a method type, as seen from the right prefix - `Bar.this.Data`. --- compiler/src/dotty/tools/dotc/typer/Implicits.scala | 2 +- tests/pos/i15867.specs2.scala | 9 +++++++++ 2 files changed, 10 insertions(+), 1 deletion(-) create mode 100644 tests/pos/i15867.specs2.scala diff --git a/compiler/src/dotty/tools/dotc/typer/Implicits.scala b/compiler/src/dotty/tools/dotc/typer/Implicits.scala index 9c044207d4b3..29337165272a 100644 --- a/compiler/src/dotty/tools/dotc/typer/Implicits.scala +++ b/compiler/src/dotty/tools/dotc/typer/Implicits.scala @@ -1138,7 +1138,7 @@ trait Implicits: case _ => pt case _ => NoType def tryConversion(using Context) = { - val restpeConv = if ref.symbol.is(Given) then conversionResultType(ref.symbol.info) else NoType + val restpeConv = if ref.symbol.is(Given) then conversionResultType(ref.widenTermRefExpr) else NoType val untpdConv = if restpeConv.exists then untpd.Select( diff --git a/tests/pos/i15867.specs2.scala b/tests/pos/i15867.specs2.scala new file mode 100644 index 000000000000..da89b2cba9f0 --- /dev/null +++ b/tests/pos/i15867.specs2.scala @@ -0,0 +1,9 @@ +class Foo: + given Conversion[String, Data] with + def apply(str: String): Data = new Data(str) + + class Data(str: String): + def |(str: String) = new Data(this.str + str) + +class Bar extends Foo: + "str" | "ing" From 701254f9a9104c7206241e498f7e9911558b1c26 Mon Sep 17 00:00:00 2001 From: Dale Wijnand Date: Mon, 5 Jun 2023 15:12:44 +0100 Subject: [PATCH 3/3] Fixup, skip adapting to non-conversions, and all prototypes --- .../dotty/tools/dotc/typer/Implicits.scala | 28 +++++++++++-------- 1 file changed, 16 insertions(+), 12 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/typer/Implicits.scala b/compiler/src/dotty/tools/dotc/typer/Implicits.scala index 29337165272a..0d40f818fd86 100644 --- a/compiler/src/dotty/tools/dotc/typer/Implicits.scala +++ b/compiler/src/dotty/tools/dotc/typer/Implicits.scala @@ -1125,18 +1125,22 @@ trait Implicits: def conversionResultType(info: Type): Type = info match case info: PolyType => conversionResultType(info.resType) case info: MethodType if info.isImplicitMethod => conversionResultType(info.resType) - case _ if info.derivesFrom(defn.ConversionClass) => pt match - case selProto: SelectionProto => - info.baseType(defn.ConversionClass) match - case AppliedType(_, List(_, restpe)) if selProto.isMatchedBy(restpe) => - // if we embed the SelectionProto as the Conversion result type - // it might end up within a GADT cast type - // so instead replace it with the targeted conversion type, if it matches - // see tests/pos/i15867.scala. - restpe - case _ => pt - case _ => pt - case _ => NoType + case _ => + if info.derivesFrom(defn.ConversionClass) then + pt match + case selProto: SelectionProto => + // we want to avoid embedding a SelectionProto in a Conversion, as the result type + // as it might end up within a GADT cast type, e.g. tests/pos/i15867.scala + // so, if we can find the target result type - as in, + // if it matches the selection prototype, then let's adapt to that instead + // otherwise just skip adapting with a prototype (by returning NoType) + info.baseType(defn.ConversionClass) match + case AppliedType(_, List(_, restpe)) if selProto.isMatchedBy(restpe) => + restpe + case _ => NoType // can't find conversion result type, avoid adapting with SelectionProto + case _: ProtoType => NoType // avoid adapting with ProtoType + case _ => pt // not a ProtoType, so use it for adapting + else NoType // not a Conversion, don't adapt def tryConversion(using Context) = { val restpeConv = if ref.symbol.is(Given) then conversionResultType(ref.widenTermRefExpr) else NoType val untpdConv =