From 5ad47c2264f00ce4abb22bb78e7b8780dc65b884 Mon Sep 17 00:00:00 2001 From: Dale Wijnand Date: Thu, 19 Jan 2023 21:56:53 +0000 Subject: [PATCH 1/4] Don't capture wildcards if in closure or by-name --- .../src/dotty/tools/dotc/core/Types.scala | 2 ++ .../dotty/tools/dotc/typer/ProtoTypes.scala | 10 ++++++++- .../src/dotty/tools/dotc/typer/Typer.scala | 4 ++++ tests/neg/t9419.scala | 22 +++++++++++++++++++ 4 files changed, 37 insertions(+), 1 deletion(-) create mode 100644 tests/neg/t9419.scala diff --git a/compiler/src/dotty/tools/dotc/core/Types.scala b/compiler/src/dotty/tools/dotc/core/Types.scala index 15b0b00ed0f3..3fd58a567277 100644 --- a/compiler/src/dotty/tools/dotc/core/Types.scala +++ b/compiler/src/dotty/tools/dotc/core/Types.scala @@ -4474,6 +4474,8 @@ object Types { def hasWildcardArg(using Context): Boolean = args.exists(isBounds) + def hasCaptureConversionArg(using Context): Boolean = args.exists(_.typeSymbol == defn.TypeBox_CAP) + def derivedAppliedType(tycon: Type, args: List[Type])(using Context): Type = if ((tycon eq this.tycon) && (args eq this.args)) this else tycon.appliedTo(args) diff --git a/compiler/src/dotty/tools/dotc/typer/ProtoTypes.scala b/compiler/src/dotty/tools/dotc/typer/ProtoTypes.scala index 8ba842ad695f..53c4022417a3 100644 --- a/compiler/src/dotty/tools/dotc/typer/ProtoTypes.scala +++ b/compiler/src/dotty/tools/dotc/typer/ProtoTypes.scala @@ -13,6 +13,7 @@ import Decorators._ import Uniques._ import inlines.Inlines import config.Printers.typr +import ErrorReporting.* import util.SourceFile import TypeComparer.necessarySubType @@ -492,7 +493,14 @@ object ProtoTypes { val targ = cacheTypedArg(arg, typer.typedUnadapted(_, wideFormal, locked)(using argCtx), force = true) - typer.adapt(targ, wideFormal, locked) + val targ1 = typer.adapt(targ, wideFormal, locked) + if wideFormal eq formal then targ1 + else targ1.tpe match + case tp: AppliedType if tp.hasCaptureConversionArg => + errorTree(targ1, + em"""argument for by-name parameter contains capture conversion skolem types: + |$tp""") + case _ => targ1 } /** The type of the argument `arg`, or `NoType` if `arg` has not been typed before diff --git a/compiler/src/dotty/tools/dotc/typer/Typer.scala b/compiler/src/dotty/tools/dotc/typer/Typer.scala index 1a24a94e527e..782b9a3af10c 100644 --- a/compiler/src/dotty/tools/dotc/typer/Typer.scala +++ b/compiler/src/dotty/tools/dotc/typer/Typer.scala @@ -1570,6 +1570,10 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer else if ((tree.tpt `eq` untpd.ContextualEmptyTree) && mt.paramNames.isEmpty) // Note implicitness of function in target type since there are no method parameters that indicate it. TypeTree(defn.FunctionOf(Nil, mt.resType, isContextual = true, isErased = false)) + else if mt.resType.match { case tp: AppliedType => tp.hasCaptureConversionArg case _ => false } then + errorTree(tree, + em"""cannot turn method type $mt into closure + |because it has capture conversion skolem types""") else EmptyTree } diff --git a/tests/neg/t9419.scala b/tests/neg/t9419.scala new file mode 100644 index 000000000000..75534e8311c6 --- /dev/null +++ b/tests/neg/t9419.scala @@ -0,0 +1,22 @@ +trait Magic[S]: + def init: S + def step(s: S): String + +object IntMagic extends Magic[Int]: + def init = 0 + def step(s: Int): String = (s - 1).toString + +object StrMagic extends Magic[String]: + def init = "hi" + def step(s: String): String = s.reverse + +object Main: + def onestep[T](m: () => Magic[T]): String = m().step(m().init) + def unostep[T](m: => Magic[T]): String = m.step(m.init) + + val iter: Iterator[Magic[?]] = Iterator(IntMagic, StrMagic) + + // was: class java.lang.String cannot be cast to class java.lang.Integer + def main(args: Array[String]): Unit = + onestep(() => iter.next()) // error + unostep(iter.next()) // error From c0f997b6de1d9da2c9ecc44474ae0efc6c2abe2c Mon Sep 17 00:00:00 2001 From: Dale Wijnand Date: Fri, 20 Jan 2023 10:05:03 +0000 Subject: [PATCH 2/4] Allow wildcard capture of by-name values --- .../dotty/tools/dotc/typer/ProtoTypes.scala | 12 ++++++++--- tests/neg/t9419.scala | 4 +++- tests/pos/t9419.jackson.scala | 20 +++++++++++++++++++ 3 files changed, 32 insertions(+), 4 deletions(-) create mode 100644 tests/pos/t9419.jackson.scala diff --git a/compiler/src/dotty/tools/dotc/typer/ProtoTypes.scala b/compiler/src/dotty/tools/dotc/typer/ProtoTypes.scala index 53c4022417a3..170d12b998de 100644 --- a/compiler/src/dotty/tools/dotc/typer/ProtoTypes.scala +++ b/compiler/src/dotty/tools/dotc/typer/ProtoTypes.scala @@ -497,9 +497,15 @@ object ProtoTypes { if wideFormal eq formal then targ1 else targ1.tpe match case tp: AppliedType if tp.hasCaptureConversionArg => - errorTree(targ1, - em"""argument for by-name parameter contains capture conversion skolem types: - |$tp""") + stripCast(targ1).tpe match + case tp: AppliedType if tp.hasWildcardArg => + errorTree(targ1, + em"""argument for by-name parameter is not a value + |and contains wildcard arguments: $tp + | + |Assign it to a val and pass that instead. + |""") + case _ => targ1 case _ => targ1 } diff --git a/tests/neg/t9419.scala b/tests/neg/t9419.scala index 75534e8311c6..e9358c0ba641 100644 --- a/tests/neg/t9419.scala +++ b/tests/neg/t9419.scala @@ -14,9 +14,11 @@ object Main: def onestep[T](m: () => Magic[T]): String = m().step(m().init) def unostep[T](m: => Magic[T]): String = m.step(m.init) - val iter: Iterator[Magic[?]] = Iterator(IntMagic, StrMagic) + val iter: Iterator[Magic[?]] = Iterator.tabulate(Int.MaxValue)(i => if i % 2 == 0 then IntMagic else StrMagic) // was: class java.lang.String cannot be cast to class java.lang.Integer def main(args: Array[String]): Unit = onestep(() => iter.next()) // error unostep(iter.next()) // error + val m = iter.next() + unostep(m) // ok, because m is a value diff --git a/tests/pos/t9419.jackson.scala b/tests/pos/t9419.jackson.scala new file mode 100644 index 000000000000..bf26c7e4c672 --- /dev/null +++ b/tests/pos/t9419.jackson.scala @@ -0,0 +1,20 @@ +// from failure in the community project +// jackson-module-scala +// in ScalaAnnotationIntrospectorModule.scala:139:12 + +import scala.language.implicitConversions + +trait EnrichedType[X]: + def value: X + +trait ClassW extends EnrichedType[Class[_]]: + def extendsScalaClass = false + +class Test: + implicit def mkClassW(c: => Class[_]): ClassW = new ClassW: + lazy val value = c + + def test1(c1: Class[_]) = c1.extendsScalaClass // ok: c1 is a value + def test2(c2: Class[_]) = mkClassW(c2).extendsScalaClass // ok: c2 is a value + // c1 in test1 goes throw adapting to find the extension method and gains the wildcard capture cast then + // c2 in test2 goes straight to typedArg, as it's already an arg, so it never gets wildcard captured From 3e498557bf81820186adb04ead4332d77b006aba Mon Sep 17 00:00:00 2001 From: Dale Wijnand Date: Mon, 23 Jan 2023 16:31:28 +0000 Subject: [PATCH 3/4] Redefine hasCaptureConversionArg --- .../src/dotty/tools/dotc/core/Types.scala | 2 -- .../dotty/tools/dotc/typer/Inferencing.scala | 4 +++ .../dotty/tools/dotc/typer/ProtoTypes.scala | 28 +++++++++++-------- .../src/dotty/tools/dotc/typer/Typer.scala | 2 +- 4 files changed, 21 insertions(+), 15 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/core/Types.scala b/compiler/src/dotty/tools/dotc/core/Types.scala index 3fd58a567277..15b0b00ed0f3 100644 --- a/compiler/src/dotty/tools/dotc/core/Types.scala +++ b/compiler/src/dotty/tools/dotc/core/Types.scala @@ -4474,8 +4474,6 @@ object Types { def hasWildcardArg(using Context): Boolean = args.exists(isBounds) - def hasCaptureConversionArg(using Context): Boolean = args.exists(_.typeSymbol == defn.TypeBox_CAP) - def derivedAppliedType(tycon: Type, args: List[Type])(using Context): Type = if ((tycon eq this.tycon) && (args eq this.args)) this else tycon.appliedTo(args) diff --git a/compiler/src/dotty/tools/dotc/typer/Inferencing.scala b/compiler/src/dotty/tools/dotc/typer/Inferencing.scala index 2aef3433228b..bd2f365cbba3 100644 --- a/compiler/src/dotty/tools/dotc/typer/Inferencing.scala +++ b/compiler/src/dotty/tools/dotc/typer/Inferencing.scala @@ -542,6 +542,10 @@ object Inferencing { case tp: AnnotatedType => tp.derivedAnnotatedType(captureWildcards(tp.parent), tp.annot) case _ => tp } + + def hasCaptureConversionArg(tp: Type)(using Context): Boolean = tp match + case tp: AppliedType => tp.args.exists(_.typeSymbol == defn.TypeBox_CAP) + case _ => false } trait Inferencing { this: Typer => diff --git a/compiler/src/dotty/tools/dotc/typer/ProtoTypes.scala b/compiler/src/dotty/tools/dotc/typer/ProtoTypes.scala index 170d12b998de..31e862ecd569 100644 --- a/compiler/src/dotty/tools/dotc/typer/ProtoTypes.scala +++ b/compiler/src/dotty/tools/dotc/typer/ProtoTypes.scala @@ -13,6 +13,7 @@ import Decorators._ import Uniques._ import inlines.Inlines import config.Printers.typr +import Inferencing.* import ErrorReporting.* import util.SourceFile import TypeComparer.necessarySubType @@ -495,18 +496,21 @@ object ProtoTypes { force = true) val targ1 = typer.adapt(targ, wideFormal, locked) if wideFormal eq formal then targ1 - else targ1.tpe match - case tp: AppliedType if tp.hasCaptureConversionArg => - stripCast(targ1).tpe match - case tp: AppliedType if tp.hasWildcardArg => - errorTree(targ1, - em"""argument for by-name parameter is not a value - |and contains wildcard arguments: $tp - | - |Assign it to a val and pass that instead. - |""") - case _ => targ1 - case _ => targ1 + else checkNoWildcardCaptureForCBN(targ1) + } + + def checkNoWildcardCaptureForCBN(targ1: Tree)(using Context): Tree = { + if hasCaptureConversionArg(targ1.tpe) then + stripCast(targ1).tpe match + case tp: AppliedType if tp.hasWildcardArg => + errorTree(targ1, + em"""argument for by-name parameter is not a value + |and contains wildcard arguments: $tp + | + |Assign it to a val and pass that instead. + |""") + case _ => targ1 + else targ1 } /** The type of the argument `arg`, or `NoType` if `arg` has not been typed before diff --git a/compiler/src/dotty/tools/dotc/typer/Typer.scala b/compiler/src/dotty/tools/dotc/typer/Typer.scala index 782b9a3af10c..3c4146f7ca2d 100644 --- a/compiler/src/dotty/tools/dotc/typer/Typer.scala +++ b/compiler/src/dotty/tools/dotc/typer/Typer.scala @@ -1570,7 +1570,7 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer else if ((tree.tpt `eq` untpd.ContextualEmptyTree) && mt.paramNames.isEmpty) // Note implicitness of function in target type since there are no method parameters that indicate it. TypeTree(defn.FunctionOf(Nil, mt.resType, isContextual = true, isErased = false)) - else if mt.resType.match { case tp: AppliedType => tp.hasCaptureConversionArg case _ => false } then + else if hasCaptureConversionArg(mt.resType) then errorTree(tree, em"""cannot turn method type $mt into closure |because it has capture conversion skolem types""") From 479c0f6acd07a05301cd8a0429fc9b9a202bd6b8 Mon Sep 17 00:00:00 2001 From: Dale Wijnand Date: Wed, 1 Feb 2023 14:48:29 +0000 Subject: [PATCH 4/4] Restrict captureWildcards to only be used if needed Rather than blindly using the newly wildcard-captured type, check that it's compatible with the proto/formal type. That way values that have wildcard types can be passed, uncast, to extension methods that don't require the capture. For instance in specs2, a value of type `Class[? <: Foo]` needn't become `Class[?1.CAP]` just so it can be applied to `def theValue[T](t: => T)`. For the zio-http case, despite knowing that JFuture is morally covariant, we don't have any way to knowing that - so we must be safe and error. --- .../dotty/tools/dotc/typer/Applications.scala | 9 +++++++-- .../dotty/tools/dotc/typer/ProtoTypes.scala | 16 +++++++--------- .../src/dotty/tools/dotc/typer/Typer.scala | 2 +- tests/neg/t9419.zio-http.scala | 18 ++++++++++++++++++ tests/pos/t9419.specs2.scala | 13 +++++++++++++ 5 files changed, 46 insertions(+), 12 deletions(-) create mode 100644 tests/neg/t9419.zio-http.scala create mode 100644 tests/pos/t9419.specs2.scala diff --git a/compiler/src/dotty/tools/dotc/typer/Applications.scala b/compiler/src/dotty/tools/dotc/typer/Applications.scala index cd33fe9cef24..224359faaf3b 100644 --- a/compiler/src/dotty/tools/dotc/typer/Applications.scala +++ b/compiler/src/dotty/tools/dotc/typer/Applications.scala @@ -714,8 +714,8 @@ trait Applications extends Compatibility { || argMatch == ArgMatch.CompatibleCAP && { val argtpe1 = argtpe.widen - val captured = captureWildcards(argtpe1) - (captured ne argtpe1) && isCompatible(captured, formal.widenExpr) + val captured = captureWildcardsCompat(argtpe1, formal.widenExpr) + captured ne argtpe1 } /** The type of the given argument */ @@ -2412,4 +2412,9 @@ trait Applications extends Compatibility { def isApplicableExtensionMethod(methodRef: TermRef, receiverType: Type)(using Context): Boolean = methodRef.symbol.is(ExtensionMethod) && !receiverType.isBottomType && tryApplyingExtensionMethod(methodRef, nullLiteral.asInstance(receiverType)).nonEmpty + + def captureWildcardsCompat(tp: Type, pt: Type)(using Context): Type = + val captured = captureWildcards(tp) + if (captured ne tp) && isCompatible(captured, pt) then captured + else tp } diff --git a/compiler/src/dotty/tools/dotc/typer/ProtoTypes.scala b/compiler/src/dotty/tools/dotc/typer/ProtoTypes.scala index 31e862ecd569..ef9b083bef4f 100644 --- a/compiler/src/dotty/tools/dotc/typer/ProtoTypes.scala +++ b/compiler/src/dotty/tools/dotc/typer/ProtoTypes.scala @@ -501,15 +501,13 @@ object ProtoTypes { def checkNoWildcardCaptureForCBN(targ1: Tree)(using Context): Tree = { if hasCaptureConversionArg(targ1.tpe) then - stripCast(targ1).tpe match - case tp: AppliedType if tp.hasWildcardArg => - errorTree(targ1, - em"""argument for by-name parameter is not a value - |and contains wildcard arguments: $tp - | - |Assign it to a val and pass that instead. - |""") - case _ => targ1 + val tp = stripCast(targ1).tpe + errorTree(targ1, + em"""argument for by-name parameter is not a value + |and contains wildcard arguments: $tp + | + |Assign it to a val and pass that instead. + |""") else targ1 } diff --git a/compiler/src/dotty/tools/dotc/typer/Typer.scala b/compiler/src/dotty/tools/dotc/typer/Typer.scala index 3c4146f7ca2d..425b800a0e72 100644 --- a/compiler/src/dotty/tools/dotc/typer/Typer.scala +++ b/compiler/src/dotty/tools/dotc/typer/Typer.scala @@ -3944,7 +3944,7 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer return adaptConstant(tree, ConstantType(converted)) case _ => - val captured = captureWildcards(wtp) + val captured = captureWildcardsCompat(wtp, pt) if (captured `ne` wtp) return readapt(tree.cast(captured)) diff --git a/tests/neg/t9419.zio-http.scala b/tests/neg/t9419.zio-http.scala new file mode 100644 index 000000000000..cff9ec51e6f9 --- /dev/null +++ b/tests/neg/t9419.zio-http.scala @@ -0,0 +1,18 @@ +// Minimisation of how the fix for t9419 affected zio-http +import java.util.concurrent.Future as JFuture + +trait Test: + def shutdownGracefully(): JFuture[_] + + def executedWildcard(jFuture: => JFuture[_]): Unit + def executedGeneric[A](jFuture: => JFuture[A]): Unit + def executedWildGen[A](jFuture: => JFuture[? <: A]): Unit + + // Even though JFuture is morally covariant, at least currently, + // there's no definition-side variance, so it's treated as invariant. + // So we have to be concerned that two different values of `JFuture[A]` + // with different types, blowing up together. So error in `fails`. + def works = executedWildcard(shutdownGracefully()) + def fails = executedGeneric(shutdownGracefully()) // error + def fixed = executedGeneric(shutdownGracefully().asInstanceOf[JFuture[Any]]) // fix + def best2 = executedWildGen(shutdownGracefully()) // even better, use use-site variance in the method diff --git a/tests/pos/t9419.specs2.scala b/tests/pos/t9419.specs2.scala new file mode 100644 index 000000000000..fe4a44312594 --- /dev/null +++ b/tests/pos/t9419.specs2.scala @@ -0,0 +1,13 @@ +// Minimisation of how the fix for t9419 affected specs2 +class MustExpectable[T](tm: () => T): + def must_==(other: => Any) = tm() == other + +class Foo + +object Main: + implicit def theValue[T](t: => T): MustExpectable[T] = new MustExpectable(() => t) + def main(args: Array[String]): Unit = + val cls = classOf[Foo] + val instance = new Foo() + val works = cls must_== cls + val fails = instance.getClass must_== cls